[Qt][WK2] webkit2 does not compile on OS X

[Jan Erik Hanssen]

The Qt webkit2 port does not compile on OS X

[Jan Erik Hanssen]

Created attachment 74969 [details]
Patch

prctl(2) is Linux specific, it's not part of POSIX unfortunately

[Kenneth Rohde Christiansen]

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]

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]

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 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 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]

Created attachment 76251 [details]
Patch

[Eric Seidel (no email)]

Comment on attachment 76251 [details]
Patch

Looks sane to me.

[Andreas Kling]

Comment on attachment 76251 [details]
Patch

Looks sane indeed, r=me

[WebKit Commit Bot]

Comment on attachment 76251 [details]
Patch

Clearing flags on attachment: 76251

Committed r74108: <http://trac.webkit.org/changeset/74108>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.