[Qt][WebKit2][Mac] WebProcess should exit automatically when UIProcess dies.
Created attachment 108003 [details] patch for feedback.
Created attachment 108336 [details] patch for feedback.
This should already be happening via WebProcess::didClose. Is that not being called for you?
WebProcess::didClose will only be called if the main thread isn't hung. Even if the main thread *is* hung, ChildProcess::didCloseOnConnectionWorkQueue should be called. That function sets things up so that the web process will exit in 10 seconds if the main thread still hasn't responded.
It seems there are several issues with the current implementation. The didClose / didCloseOnConnectionWorkQueue is not working with the Qt port because the unix sockets IPC mechanism does not provide us any meaningful notification when the ui process died. Even when the UIProcess is exited "properly" by the user, this approach does not work due to Leaking a WebContext and therefor not closing the IPC socket properly either. The scheduleWorkOnTermination function does not work either, because we use a SIGNAL/SLOT connection to bind to the QProcess own finished() signal. Which of course never triggers because it would only trigger when the process itself exited already. I will try to come up with a sensible solution at least for the latter problem.
actually i misunderstood the purpose of the scheduleWorkOnTermination function... so at least that one is working properly. :-) ... but it does not serve the purpose i was assuming.
Created attachment 108672 [details] patch for review.
Attachment 108672 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit2/ChangeLog', u'Source/WebKit..." exit_code: 1 Source/WebKit2/WebProcess/qt/WebProcessQt.cpp:70: Missing space after , [whitespace/comma] [3] Total errors found: 1 in 2 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 108673 [details] patch for review - fixed coding style issues i think this is the best thing we can do for now. But unless we want to fix circular dependencies of WebContext, WebProcessProxy and WebPageProxy and change the IPC mechanism of the Qt port, this is probably the way to go.
i will change it from using exit() to qApp->quit() and try that again. ...
Comment on attachment 108673 [details] patch for review - fixed coding style issues Not sure if we will support Leopard but Grand Central Dispatch was added in Snow Leopard.
(In reply to comment #11) > (From update of attachment 108673 [details]) > Not sure if we will support Leopard but Grand Central Dispatch was added in Snow Leopard. We should support whatever Qt 5 will support. I don't think there's a problem adding SL-only things to trunk right now, we can figure this out later if someone decides that Qt 5.0 needs to support Leopard.
(In reply to comment #11) > (From update of attachment 108673 [details]) > Not sure if we will support Leopard but Grand Central Dispatch was added in Snow Leopard. I am aware of that, but i consider it a minor issue in this ecosystem. Do we have any guidelines on that? Or do we know of potential WK2 users using Pre Snow Leopard?
Created attachment 109468 [details] patch for review. - use qApp->quit() instead of exit()
Comment on attachment 109468 [details] patch for review. - use qApp->quit() instead of exit() I think its good as we use the same approach than the Mac port. Jut a nitpick QApplication::quit() it is way nicer to read and understand than this magic hacked qApp pointer :D.
Created attachment 109476 [details] patch for review. - use QCoreApplication::quit() instead of qApp->quit()
Comment on attachment 109476 [details] patch for review. - use QCoreApplication::quit() instead of qApp->quit() LGTM.
Comment on attachment 109476 [details] patch for review. - use QCoreApplication::quit() instead of qApp->quit() Clearing flags on attachment: 109476 Committed r96494: <http://trac.webkit.org/changeset/96494>
All reviewed patches have been landed. Closing bug.