115964 2013-05-11 11:12:40 -0700 When possible, terminate web processes immediately when closing their last page. 2013-05-12 18:07:51 -0700 1 1 1 Unclassified WebKit WebKit2 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 kling kling andersca ap darin ggaren kling oldest_to_newest 887805 0 kling 2013-05-11 11:12:40 -0700 For <rdar://problem/10103795>. 887806 1 201465 kling 2013-05-11 11:13:55 -0700 Created attachment 201465 Patch 887810 2 201465 ggaren 2013-05-11 11:37:45 -0700 Comment on attachment 201465 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=201465&action=review r=me!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! > Source/WebKit2/UIProcess/WebProcessProxy.cpp:204 > + // Terminate the web process immediately if we have enough information to confidently do so. > + // This only works if we're using a network process. Otherwise we have to wait for the web process to clear caches. I don't understand what "clear caches" means, but I guess that's OK. 887812 3 kling 2013-05-11 11:52:37 -0700 Committed r149934: <http://trac.webkit.org/changeset/149934> 887814 4 kling 2013-05-11 11:56:40 -0700 (In reply to comment #2) > (From update of attachment 201465 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=201465&action=review > > r=me!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! Thanks!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! > > Source/WebKit2/UIProcess/WebProcessProxy.cpp:204 > > + // Terminate the web process immediately if we have enough information to confidently do so. > > + // This only works if we're using a network process. Otherwise we have to wait for the web process to clear caches. > > I don't understand what "clear caches" means, but I guess that's OK. I changed that to "clean up." When doing networking in the web process, we have to wait for it to run [[NSURLCache sharedURLCache] removeAllCachedResponses] before exiting. (See WebProcess::platformTerminate()) 887850 5 201465 darin 2013-05-11 16:08:08 -0700 Comment on attachment 201465 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=201465&action=review >>> Source/WebKit2/UIProcess/WebProcessProxy.cpp:204 >>> + // This only works if we're using a network process. Otherwise we have to wait for the web process to clear caches. >> >> I don't understand what "clear caches" means, but I guess that's OK. > > I changed that to "clean up." When doing networking in the web process, we have to wait for it to run [[NSURLCache sharedURLCache] removeAllCachedResponses] before exiting. (See WebProcess::platformTerminate()) I think the key phrase would probably be “caches on disk” or “in the file system” or some phrase like that. > Source/WebKit2/UIProcess/WebProcessProxy.h:181 > + bool canTerminateChildProcess(); Should it be const? 887852 6 kling 2013-05-11 16:09:39 -0700 (In reply to comment #5) > (From update of attachment 201465 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=201465&action=review > > > Source/WebKit2/UIProcess/WebProcessProxy.h:181 > > + bool canTerminateChildProcess(); > > Should it be const? I wanted it to be, but that made the patch a bit huge. I'll fix separately. 887864 7 ap 2013-05-11 16:52:22 -0700 I'm surprised that this is a good idea. Even not considering other persistent state that may be inconsistent, a web page at least needs to send a ping request from unload. 887877 8 kling 2013-05-11 18:05:31 -0700 (In reply to comment #7) > I'm surprised that this is a good idea. Even not considering other persistent state that may be inconsistent, a web page at least needs to send a ping request from unload. How about pushing information about whether a web process has registered unload event handlers to the UI process, and then disabling this short-circuit in that case? What other persistent state do you have in mind? Do you think it would be better to start a watchdog timer when sending WebPage::Close and deferring this mechanism until that fires? (and canceling it if the child process terminates.) 888028 9 kling 2013-05-12 16:22:00 -0700 Web processes with unload event handlers registered will now prevent the UI process from killing them (bug 115988.) 888045 10 ap 2013-05-12 18:07:51 -0700 Unload (and beforeunload) are necessary to run for JS applications to cleanly shut down themselves. WebProcess itself may also be in the middle of writing to a file, and terminating it without a chance to finish that is not nice. This includes Keychain, Web SQL database, and possibly more. > For <rdar://problem/10103795>. Perhaps the right way forward is to chat about this change on IRC. There are already mechanisms that are supposed to prevent this kind of behavior, and it's unclear to me from this bug why they were found inadequate. I didn't look into this very deeply, but I was under the impression that we needed a bugfix, not a redesign. 201465 2013-05-11 11:13:55 -0700 2013-05-11 16:08:08 -0700 Patch bug-115964.diff text/plain 3888 kling ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJLaXQyL0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJLaXQyL0No YW5nZUxvZwppbmRleCAzMTc5YTBhLi40Yjk3ZmVhIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViS2l0 Mi9DaGFuZ2VMb2cKKysrIGIvU291cmNlL1dlYktpdDIvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMjgg QEAKKzIwMTMtMDUtMTEgIEFuZHJlYXMgS2xpbmcgIDxha2xpbmdAYXBwbGUuY29tPgorCisgICAg ICAgIFdoZW4gcG9zc2libGUsIHRlcm1pbmF0ZSB3ZWIgcHJvY2Vzc2VzIGltbWVkaWF0ZWx5IHdo ZW4gY2xvc2luZyB0aGVpciBsYXN0IHBhZ2UuCisgICAgICAgIDxodHRwOi8vd2Via2l0Lm9yZy9i LzExNTk2ND4KKyAgICAgICAgPHJkYXI6Ly9wcm9ibGVtLzEwMTAzNzk1PgorCisgICAgICAgIFJl dmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIFdoZW4gd2UncmUgdXNpbmcgYSBu ZXR3b3JrIHByb2Nlc3MsIHRoZXJlJ3Mgbm8gbmVlZCBmb3IgdGhlIFVJIHByb2Nlc3MgdG8gd2Fp dCBmb3Igd2ViIHByb2Nlc3NlcworICAgICAgICB0byBjbGVhciByZXNvdXJjZSBjYWNoZXMgYW5k IHRlcm1pbmF0ZSBuaWNlbHkuCisKKyAgICAgICAgV2UgY2FuIGp1c3Qga2lsbCB0aGVtIG9mZiBy aWdodCBhd2F5IGluIFdlYlByb2Nlc3NQcm94eTo6cmVtb3ZlV2ViUGFnZSgpIHdoZW4gdGhlIGxh c3QgcGFnZSBpcyBjbG9zZWQuCisKKyAgICAgICAgKiBVSVByb2Nlc3MvV2ViUHJvY2Vzc1Byb3h5 LmNwcDoKKyAgICAgICAgKFdlYktpdDo6V2ViUHJvY2Vzc1Byb3h5OjpyZW1vdmVXZWJQYWdlKToK KworICAgICAgICAgICAgVGVybWluYXRlIHRoZSBjaGlsZCBwcm9jZXNzIGltbWVkaWF0ZWx5IGlm IHBvc3NpYmxlLgorCisgICAgICAgICogVUlQcm9jZXNzL1dlYlByb2Nlc3NQcm94eS5oOgorICAg ICAgICAoV2ViS2l0OjpXZWJQcm9jZXNzUHJveHk6OmNhblRlcm1pbmF0ZUNoaWxkUHJvY2Vzcyk6 CisgICAgICAgIChXZWJLaXQ6OldlYlByb2Nlc3NQcm94eTo6c2hvdWxkVGVybWluYXRlKToKKwor ICAgICAgICAgICAgQnJva2Ugb3V0IHRoZSBsb2dpYyBmcm9tIHRoZSBzaG91bGRUZXJtaW5hdGUo KSBJUEMgbWVzc2FnZSBoYW5kbGVyIGludG8gYSBzZXBhcmF0ZSBmdW5jdGlvbgorICAgICAgICAg ICAgc28gd2UgY2FuIGNhbGwgaXQgZnJvbSByZW1vdmVXZWJQYWdlKCkuCisKIDIwMTMtMDUtMTEg IEFuZGVycyBDYXJsc3NvbiAgPGFuZGVyc2NhQGFwcGxlLmNvbT4KIAogICAgICAgICBDcmFzaCB3 aGVuIHRlcm1pbmF0aW5nIGEgcHJvY2VzcyB0aGF0IGhhcyBub3QgYmVlbiBmdWxseSBsYXVuY2hl ZApkaWZmIC0tZ2l0IGEvU291cmNlL1dlYktpdDIvVUlQcm9jZXNzL1dlYlByb2Nlc3NQcm94eS5j cHAgYi9Tb3VyY2UvV2ViS2l0Mi9VSVByb2Nlc3MvV2ViUHJvY2Vzc1Byb3h5LmNwcAppbmRleCAx ZDYyZDM1Li4wN2I1NGY4IDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViS2l0Mi9VSVByb2Nlc3MvV2Vi UHJvY2Vzc1Byb3h5LmNwcAorKysgYi9Tb3VyY2UvV2ViS2l0Mi9VSVByb2Nlc3MvV2ViUHJvY2Vz c1Byb3h5LmNwcApAQCAtMTk4LDYgKzE5OCwxMyBAQCB2b2lkIFdlYlByb2Nlc3NQcm94eTo6cmVt b3ZlV2ViUGFnZSh1aW50NjRfdCBwYWdlSUQpCiAgICAgbV9wcm9jZXNzU3VwcHJlc3NpYmxlUGFn ZXMucmVtb3ZlKHBhZ2VJRCk7CiAgICAgdXBkYXRlUHJvY2Vzc1N1cHByZXNzaW9uU3RhdGUoKTsK ICNlbmRpZgorCisjaWYgRU5BQkxFKE5FVFdPUktfUFJPQ0VTUykKKyAgICAvLyBUZXJtaW5hdGUg dGhlIHdlYiBwcm9jZXNzIGltbWVkaWF0ZWx5IGlmIHdlIGhhdmUgZW5vdWdoIGluZm9ybWF0aW9u IHRvIGNvbmZpZGVudGx5IGRvIHNvLgorICAgIC8vIFRoaXMgb25seSB3b3JrcyBpZiB3ZSdyZSB1 c2luZyBhIG5ldHdvcmsgcHJvY2Vzcy4gT3RoZXJ3aXNlIHdlIGhhdmUgdG8gd2FpdCBmb3IgdGhl IHdlYiBwcm9jZXNzIHRvIGNsZWFyIGNhY2hlcy4KKyAgICBpZiAoY2FuVGVybWluYXRlQ2hpbGRQ cm9jZXNzKCkgJiYgbV9jb250ZXh0LT51c2VzTmV0d29ya1Byb2Nlc3MoKSkKKyAgICAgICAgcmVx dWVzdFRlcm1pbmF0aW9uKCk7CisjZW5kaWYKIH0KIAogVmVjdG9yPFdlYlBhZ2VQcm94eSo+IFdl YlByb2Nlc3NQcm94eTo6cGFnZXMoKSBjb25zdApAQCAtNTA1LDE3ICs1MTIsMjcgQEAgc2l6ZV90 IFdlYlByb2Nlc3NQcm94eTo6ZnJhbWVDb3VudEluUGFnZShXZWJQYWdlUHJveHkqIHBhZ2UpIGNv bnN0CiAgICAgcmV0dXJuIHJlc3VsdDsKIH0KIAotdm9pZCBXZWJQcm9jZXNzUHJveHk6OnNob3Vs ZFRlcm1pbmF0ZShib29sJiBzaG91bGRUZXJtaW5hdGUpCitib29sIFdlYlByb2Nlc3NQcm94eTo6 Y2FuVGVybWluYXRlQ2hpbGRQcm9jZXNzKCkKIHsKLSAgICBpZiAoIW1fcGFnZU1hcC5pc0VtcHR5 KCkgfHwgKG1fZG93bmxvYWRQcm94eU1hcCAmJiAhbV9kb3dubG9hZFByb3h5TWFwLT5pc0VtcHR5 KCkpIHx8ICFtX2NvbnRleHQtPnNob3VsZFRlcm1pbmF0ZSh0aGlzKSkgewotICAgICAgICBzaG91 bGRUZXJtaW5hdGUgPSBmYWxzZTsKLSAgICAgICAgcmV0dXJuOwotICAgIH0KKyAgICBpZiAoIW1f cGFnZU1hcC5pc0VtcHR5KCkpCisgICAgICAgIHJldHVybiBmYWxzZTsKIAotICAgIHNob3VsZFRl cm1pbmF0ZSA9IHRydWU7CisgICAgaWYgKG1fZG93bmxvYWRQcm94eU1hcCAmJiAhbV9kb3dubG9h ZFByb3h5TWFwLT5pc0VtcHR5KCkpCisgICAgICAgIHJldHVybiBmYWxzZTsKIAotICAgIC8vIFdl IGtub3cgdGhhdCB0aGUgd2ViIHByb2Nlc3MgaXMgZ29pbmcgdG8gdGVybWluYXRlIHNvIGRpc2Nv bm5lY3QgaXQgZnJvbSB0aGUgY29udGV4dC4KLSAgICBkaXNjb25uZWN0KCk7CisgICAgaWYgKCFt X2NvbnRleHQtPnNob3VsZFRlcm1pbmF0ZSh0aGlzKSkKKyAgICAgICAgcmV0dXJuIGZhbHNlOwor CisgICAgcmV0dXJuIHRydWU7Cit9CisKK3ZvaWQgV2ViUHJvY2Vzc1Byb3h5OjpzaG91bGRUZXJt aW5hdGUoYm9vbCYgc2hvdWxkVGVybWluYXRlKQoreworICAgIHNob3VsZFRlcm1pbmF0ZSA9IGNh blRlcm1pbmF0ZUNoaWxkUHJvY2VzcygpOworICAgIGlmIChzaG91bGRUZXJtaW5hdGUpIHsKKyAg ICAgICAgLy8gV2Uga25vdyB0aGF0IHRoZSB3ZWIgcHJvY2VzcyBpcyBnb2luZyB0byB0ZXJtaW5h dGUgc28gZGlzY29ubmVjdCBpdCBmcm9tIHRoZSBjb250ZXh0LgorICAgICAgICBkaXNjb25uZWN0 KCk7CisgICAgfQogfQogCiB2b2lkIFdlYlByb2Nlc3NQcm94eTo6dXBkYXRlVGV4dENoZWNrZXJT dGF0ZSgpCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0Mi9VSVByb2Nlc3MvV2ViUHJvY2Vzc1By b3h5LmggYi9Tb3VyY2UvV2ViS2l0Mi9VSVByb2Nlc3MvV2ViUHJvY2Vzc1Byb3h5LmgKaW5kZXgg NmJkMmNkZi4uOGE1YWI2NyAxMDA2NDQKLS0tIGEvU291cmNlL1dlYktpdDIvVUlQcm9jZXNzL1dl YlByb2Nlc3NQcm94eS5oCisrKyBiL1NvdXJjZS9XZWJLaXQyL1VJUHJvY2Vzcy9XZWJQcm9jZXNz UHJveHkuaApAQCAtMTc4LDYgKzE3OCw4IEBAIHByaXZhdGU6CiAgICAgdm9pZCBkaWRSZWNlaXZl V2ViUHJvY2Vzc1Byb3h5TWVzc2FnZShDb3JlSVBDOjpDb25uZWN0aW9uKiwgQ29yZUlQQzo6TWVz c2FnZURlY29kZXImKTsKICAgICB2b2lkIGRpZFJlY2VpdmVTeW5jV2ViUHJvY2Vzc1Byb3h5TWVz c2FnZShDb3JlSVBDOjpDb25uZWN0aW9uKiwgQ29yZUlQQzo6TWVzc2FnZURlY29kZXImLCBPd25Q dHI8Q29yZUlQQzo6TWVzc2FnZUVuY29kZXI+Jik7CiAKKyAgICBib29sIGNhblRlcm1pbmF0ZUNo aWxkUHJvY2VzcygpOworCiAgICAgUmVzcG9uc2l2ZW5lc3NUaW1lciBtX3Jlc3BvbnNpdmVuZXNz VGltZXI7CiAgICAgCiAgICAgUmVmUHRyPFdlYkNvbm5lY3Rpb25Ub1dlYlByb2Nlc3M+IG1fd2Vi Q29ubmVjdGlvbjsK