Currently, we are waiting for the Launch Services database to be present on every load in the WebContent process. It should be sufficient to wait only on the first load.
Created attachment 406715 [details] Patch
<rdar://problem/67242794>
Comment on attachment 406715 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=406715&action=review > Source/WebKit/WebProcess/WebPage/Cocoa/WebPageCocoa.mm:69 > + static std::once_flag onceFlag; > + std::call_once(onceFlag, [] { This is a multi-thread-safe way of doing this. Can this be called from multiple threads? If not, a simple global check would suffice.
(In reply to Darin Adler from comment #3) > Comment on attachment 406715 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=406715&action=review > > > Source/WebKit/WebProcess/WebPage/Cocoa/WebPageCocoa.mm:69 > > + static std::once_flag onceFlag; > > + std::call_once(onceFlag, [] { > > This is a multi-thread-safe way of doing this. Can this be called from > multiple threads? If not, a simple global check would suffice. No, I don't believe this can be called from multiple threads. Will fix. Thanks for reviewing!
Created attachment 406742 [details] Patch
Comment on attachment 406742 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=406742&action=review > Source/WebKit/WebProcess/WebPage/Cocoa/WebPageCocoa.mm:79 > + }; This semicolon is not needed.
(In reply to Darin Adler from comment #6) > Comment on attachment 406742 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=406742&action=review > > > Source/WebKit/WebProcess/WebPage/Cocoa/WebPageCocoa.mm:79 > > + }; > > This semicolon is not needed. Thanks, will fix :)
Created attachment 406743 [details] Patch
Committed r265774: <https://trac.webkit.org/changeset/265774> All reviewed patches have been landed. Closing bug and clearing flags on attachment 406743 [details].
Comment on attachment 406743 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=406743&action=review > Source/WebKit/ChangeLog:10 > + increase the maximum wait time from 1s to 5s. I don't think we are ever OK with hanging for 5 seconds. As a matter of fact, during this database update, the process is not processing incoming IPC anymore and gets reported as unresponsive to the client after 3 seconds. Clients such as mail kill the process right away which means that we hung but don't see the time out logging (because the process was terminated before reaching the timeout). There are 2 issues here: 1. We should not be hanging in the first place 2. The 5 seconds timeout is too long. I'd suggest 1 or 2 seconds, something definitely below the 3 second timeout the UIProcess ResponsivenessTimer uses.
Comment on attachment 406743 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=406743&action=review >> Source/WebKit/ChangeLog:10 >> + increase the maximum wait time from 1s to 5s. > > I don't think we are ever OK with hanging for 5 seconds. As a matter of fact, during this database update, the process is not processing incoming IPC anymore and gets reported as unresponsive to the client after 3 seconds. Clients such as mail kill the process right away which means that we hung but don't see the time out logging (because the process was terminated before reaching the timeout). > > There are 2 issues here: > 1. We should not be hanging in the first place > 2. The 5 seconds timeout is too long. I'd suggest 1 or 2 seconds, something definitely below the 3 second timeout the UIProcess ResponsivenessTimer uses. Change it back to the 1 second it was before this patch?
(In reply to Chris Dumez from comment #10) > Comment on attachment 406743 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=406743&action=review > > > Source/WebKit/ChangeLog:10 > > + increase the maximum wait time from 1s to 5s. > > I don't think we are ever OK with hanging for 5 seconds. As a matter of > fact, during this database update, the process is not processing incoming > IPC anymore and gets reported as unresponsive to the client after 3 seconds. > Clients such as mail kill the process right away which means that we hung > but don't see the time out logging (because the process was terminated > before reaching the timeout). > > There are 2 issues here: > 1. We should not be hanging in the first place Yes. This seems to be caused by the entitlement check for "com.apple.private.webkit.use-xpc-endpoint" failing. I am currently looking into this. > 2. The 5 seconds timeout is too long. I'd suggest 1 or 2 seconds, something > definitely below the 3 second timeout the UIProcess ResponsivenessTimer uses. That sounds good, I will change that. Thanks for reviewing!
(In reply to Darin Adler from comment #11) > Comment on attachment 406743 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=406743&action=review > > >> Source/WebKit/ChangeLog:10 > >> + increase the maximum wait time from 1s to 5s. > > > > I don't think we are ever OK with hanging for 5 seconds. As a matter of fact, during this database update, the process is not processing incoming IPC anymore and gets reported as unresponsive to the client after 3 seconds. Clients such as mail kill the process right away which means that we hung but don't see the time out logging (because the process was terminated before reaching the timeout). > > > > There are 2 issues here: > > 1. We should not be hanging in the first place > > 2. The 5 seconds timeout is too long. I'd suggest 1 or 2 seconds, something definitely below the 3 second timeout the UIProcess ResponsivenessTimer uses. > > Change it back to the 1 second it was before this patch? Yes, I will change it back to 1 second. Thanks for reviewing!