The WebProcess should close itself when the client disconnected. We need to do this because the close message can not be delivered to the WebProcess when the client exited - at least I discovered it in Qt. In regard of this change the termination mechanism should be changed, for example wait for the WebProcess to exit instead of immediately killing it. I would implement it in a follow up patch.
Created attachment 60624 [details] proposed patch This change contains the process closing mechanism and some final cleanup logic.
Comment on attachment 60624 [details] proposed patch Looks goood. In the future I plan to have a special didClose handler that is executed directly on the work queue. This way we can still exit even if the web process is spinning.
regarding destroySharedMemory(), since we are using mmap, maybe unmapMemory would be a better name.
Created attachment 60640 [details] proposed patch Ranamed destroySharedMemory to cleanup, create the temporal files in the temp dir. The changes was suggested by kenneth.
Comment on attachment 60640 [details] proposed patch Rejecting patch 60640 from commit-queue. Failed to run "[u'/Users/eseidel/Projects/CommitQueue/WebKitTools/Scripts/svn-apply', u'--reviewer', u'Kenneth Rohde Christiansen', u'--force']" exit_code: 1 Last 500 characters of output: pp patching file WebKit2/Shared/qt/UpdateChunk.h can't find file to patch at input line 5 Perhaps you used the wrong -p or --strip option? The text leading up to this was: -------------------------- |Index: WebKit2/UIProcess/Launcher/qt/WebProcessLauncherQt.cpp |index 5e1e110..306eea3 100644 |--- WebKit2/UIProcess/Launcher/qt/WebProcessLauncherQt.cpp |+++ WebKit2/UIProcess/Launcher/qt/WebProcessLauncherQt.cpp -------------------------- No file to patch. Skipping patch. 3 out of 3 hunks ignored Full output: http://webkit-commit-queue.appspot.com/results/3381344
Comment on attachment 60624 [details] proposed patch Cleared Anders Carlsson's review+ from obsolete attachment 60624 [details] so that this bug does not appear in http://webkit.org/pending-commit.
Created attachment 60848 [details] proposed patch - this time created on the trunk The last one was created in the rebase-after-upstream branch, that's why it did not apply on trunk.
Comment on attachment 60640 [details] proposed patch Cleared Kenneth Rohde Christiansen's review+ from obsolete attachment 60640 [details] so that this bug does not appear in http://webkit.org/pending-commit.
Created attachment 62582 [details] proposed patch (updated) This time I kept only those changes that are related to the bug title - handling of the disconnection. I will push the rest of the previous patch into a new bug.
Comment on attachment 62582 [details] proposed patch (updated) > > +void Connection::disconnectHandler() > +{ > + m_client->didClose(this); > +} > + On which thread is this called?
> On which thread is this called? Oops, on the WorkQueue's thread. It is strange that the patch worked for me (at least it did not crashed). I will fix it in the next patch.
(In reply to comment #11) > > On which thread is this called? > > Oops, on the WorkQueue's thread. It is strange that the patch worked for me (at least it did not crashed). I will fix it in the next patch. We actually quit the run loop when the last web page has been closed. Is there a reason why the web process doesn't exit when that happens?
> > We actually quit the run loop when the last web page has been closed. Is there a reason why the web process doesn't exit when that happens? Huh, seems like I get confused while working on this. There was various problems with our implementation of Connection but it seems like these have been fixed. So, to be able to exit from the web process when it gets the close message, we should just implement RunLoop::exit. The other parts of the patch assures that the web process would exit even when the client did not tear down correctly (killed or crashed). Don't you think this is something we should implement? Actually, I would introduce a common method of Connection what would look like this: void didDisconnect() { m_client->didClose(); } Maybe didClose would be a better name. This way all port can schedule to call this on the main run loop when the connection became broken. Do you like the idea? (I mean the whole concept not just the new function.)
Oh, I have just realized that connectionDidClose and dispatchConnectionDidClose fit my needs!
Created attachment 62678 [details] proposed patch
Comment on attachment 62678 [details] proposed patch Clearing flags on attachment: 62678 Committed r64628: <http://trac.webkit.org/changeset/64628>
All reviewed patches have been landed. Closing bug.