Bug 50128

Summary: [Qt][WK2] webkit2 does not compile on OS X
Product: WebKit Reporter: Jan Erik Hanssen <jhanssen>
Component: WebKit2Assignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, kenneth, webkit.review.bot
Priority: P2 Keywords: Qt, QtTriaged
Version: 528+ (Nightly build)   
Hardware: PC   
OS: OS X 10.6   
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

Jan Erik Hanssen
Reported 2010-11-28 03:02:54 PST
The Qt webkit2 port does not compile on OS X
Attachments
Patch (1.36 KB, patch)
2010-11-28 03:06 PST, Jan Erik Hanssen 		no flags
DetailsFormatted DiffDiff
Patch (3.80 KB, patch)
2010-12-06 16:14 PST, Jan Erik Hanssen 		no flags
DetailsFormatted DiffDiff
Patch (3.37 KB, patch)
2010-12-10 13:51 PST, Jan Erik Hanssen 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Jan Erik Hanssen
Comment 1 2010-11-28 03:06:15 PST
Created attachment 74969 [details] Patch prctl(2) is Linux specific, it's not part of POSIX unfortunately
Kenneth Rohde Christiansen
Comment 2 2010-11-28 10:17:25 PST
Comment on attachment 74969 [details] Patch unfortunately the above code was introduced to fix something, that now needs special handling on mac. i will r+ but you need to take this up with the person who introduced the above code.
Jan Erik Hanssen
Comment 3 2010-12-06 16:14:27 PST
Created attachment 75747 [details] Patch New patch with a generic implementation for killing all child processes when the parent dies. This will not be invoked if the parent is killed with SIGKILL though.
WebKit Review Bot
Comment 4 2010-12-06 16:16:37 PST
Attachment 75747 [details] did not pass style-queue: Failed to run "[u'git', u'reset', u'--hard', u'HEAD']" exit_code: 128 error: Could not write new index file. fatal: Could not reset index file to revision 'HEAD'. If any of these errors are false positives, please file a bug against check-webkit-style.
Eric Seidel (no email)
Comment 5 2010-12-10 00:07:42 PST
Comment on attachment 75747 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=75747&action=review > WebKit2/UIProcess/Launcher/qt/ProcessLauncherQt.cpp:91 > + : QProcess(parent), m_pid(0) webkit normally puts each of these on its own line.
Balazs Kelemen
Comment 6 2010-12-10 04:37:33 PST
Comment on attachment 75747 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=75747&action=review > WebKit2/UIProcess/Launcher/qt/ProcessLauncherQt.cpp:75 > +#if defined Q_OS_UNIX > +Q_GLOBAL_STATIC(QSet<Q_PID>, processes); Maybe using a WTF::HashSet would be more WebKitish. Furthermore, WTF::HashSet is stored in FastMalloc memory and I think the less we mixing the FastMalloc and the standard heap is the better. > WebKit2/UIProcess/Launcher/qt/ProcessLauncherQt.cpp:84 > +static void cleanupProcesses() > +{ > + QSet<Q_PID>::ConstIterator it = processes()->begin(); > + while (it != processes()->end()) { > + kill(*it, SIGINT); > + ++it; > + } > +} We can do it more platform independently by storing QProcess pointers and use QProcess::kill. > WebKit2/UIProcess/Launcher/qt/ProcessLauncherQt.cpp:110 > + processes()->remove(m_pid); > + m_pid = 0; m_pid = 0 seems to be useless here.
Jan Erik Hanssen
Comment 7 2010-12-10 13:51:28 PST
Created attachment 76251 [details] Patch
Eric Seidel (no email)
Comment 8 2010-12-13 00:28:14 PST
Comment on attachment 76251 [details] Patch Looks sane to me.
Andreas Kling
Comment 9 2010-12-15 00:39:08 PST
Comment on attachment 76251 [details] Patch Looks sane indeed, r=me
WebKit Commit Bot
Comment 10 2010-12-15 04:34:23 PST
Comment on attachment 76251 [details] Patch Clearing flags on attachment: 76251 Committed r74108: <http://trac.webkit.org/changeset/74108>
WebKit Commit Bot
Comment 11 2010-12-15 04:34:29 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.