To avoid the main thread getting stuck, the Launch Services database should be injected before NSApplication is initialized, since the initialization now depends on the database.
<rdar://87468788>
Created attachment 449067 [details] Patch
Comment on attachment 449067 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=449067&action=review r=me > Source/WebKit/WebProcess/cocoa/LaunchServicesDatabaseManager.mm:87 > + auto startTime = WallTime::now(); Should probably be using MonotonicTime::now() > Source/WebKit/WebProcess/cocoa/LaunchServicesDatabaseManager.mm:89 > + auto elapsedTime = WallTime::now() - startTime; ditto. > Source/WebKit/WebProcess/cocoa/LaunchServicesDatabaseManager.mm:91 > + RELEASE_LOG(Loading, "Waiting for Launch Services database update took %f seconds", elapsedTime.value()); Can we use RELEASE_LOG_ERROR so that it stands out in the logs? Any delay during process initialization is very visible to the user and should be avoided.
Created attachment 449072 [details] Patch
(In reply to Chris Dumez from comment #3) > Comment on attachment 449067 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=449067&action=review > > r=me > > > Source/WebKit/WebProcess/cocoa/LaunchServicesDatabaseManager.mm:87 > > + auto startTime = WallTime::now(); > > Should probably be using MonotonicTime::now() > > > Source/WebKit/WebProcess/cocoa/LaunchServicesDatabaseManager.mm:89 > > + auto elapsedTime = WallTime::now() - startTime; > > ditto. > > > Source/WebKit/WebProcess/cocoa/LaunchServicesDatabaseManager.mm:91 > > + RELEASE_LOG(Loading, "Waiting for Launch Services database update took %f seconds", elapsedTime.value()); > > Can we use RELEASE_LOG_ERROR so that it stands out in the logs? Any delay > during process initialization is very visible to the user and should be > avoided. Done! Thanks for reviewing!
Comment on attachment 449072 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=449072&action=review > Source/WebKit/WebProcess/cocoa/LaunchServicesDatabaseManager.mm:90 > + if (elapsedTime.value() > 0.5) if (elapsedTime > 0.5_s)
(In reply to Simon Fraser (smfr) from comment #6) > Comment on attachment 449072 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=449072&action=review > > > Source/WebKit/WebProcess/cocoa/LaunchServicesDatabaseManager.mm:90 > > + if (elapsedTime.value() > 0.5) > > if (elapsedTime > 0.5_s) Nicer indeed
(In reply to Simon Fraser (smfr) from comment #6) > Comment on attachment 449072 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=449072&action=review > > > Source/WebKit/WebProcess/cocoa/LaunchServicesDatabaseManager.mm:90 > > + if (elapsedTime.value() > 0.5) > > if (elapsedTime > 0.5_s) Will fix! Thanks for reviewing!
Created attachment 449079 [details] Patch
Created attachment 449110 [details] Patch
Created attachment 449171 [details] Patch
Committed r288019 (246045@main): <https://commits.webkit.org/246045@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 449171 [details].
EWS is hitting assertions added in this patch: https://ews-build.s3-us-west-2.amazonaws.com/macOS-AppleSilicon-Big-Sur-Debug-WK2-Tests-EWS/r449267-21271/com.apple.WebKit.WebContent.Development-51883-crash-log.txt
Created attachment 449359 [details] Patch
Reopened, due to debug assert on EWS.
Created attachment 449386 [details] Patch
Comment on attachment 449386 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=449386&action=review > Source/WebKit/WebProcess/cocoa/LaunchServicesDatabaseManager.mm:93 > + fprintf(stderr, "Waiting for Launch Services database update took %f seconds", elapsedTime.value()); WTFLogAlways.
Comment on attachment 449386 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=449386&action=review > Source/WebKit/WebProcess/cocoa/LaunchServicesDatabaseManager.mm:36 > +#import <wtf/NumberOfCores.h> Nit - do we need to import this header?
(In reply to Simon Fraser (smfr) from comment #17) > Comment on attachment 449386 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=449386&action=review > > > Source/WebKit/WebProcess/cocoa/LaunchServicesDatabaseManager.mm:93 > > + fprintf(stderr, "Waiting for Launch Services database update took %f seconds", elapsedTime.value()); > > WTFLogAlways. This patch is for debugging purposes, currently. I am speculating that writing to stderr will give us information about which test is sometimes hitting the debug assert. Thanks for reviewing!
(In reply to Wenson Hsieh from comment #18) > Comment on attachment 449386 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=449386&action=review > > > Source/WebKit/WebProcess/cocoa/LaunchServicesDatabaseManager.mm:36 > > +#import <wtf/NumberOfCores.h> > > Nit - do we need to import this header? Good catch! So far this patch is for debugging purposes, but will remove when a new patch is ready for review. Thanks for reviewing!
Created attachment 449510 [details] Patch
Comment on attachment 449510 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=449510&action=review This patch fails to update GPUProcessProxy.cpp accordingly. We can simplify things there now that sendNetworkProcessXPCEndpointToProcess() no longer needs a data store instance. > Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.h:281 > + static bool sendNetworkProcessXPCEndpointToProcess(AuxiliaryProcessProxy&); Now that it is static and doesn't use any information from the data store, I would suggest moving this to NetworkProcessProxy and make it non-static. Then WebProcessProxy could call `NetworkProcessProxy::ensureDefaultNetworkProcess().sendXPCEndpointToProcess(*this)`
Created attachment 449593 [details] Patch
(In reply to Chris Dumez from comment #22) > Comment on attachment 449510 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=449510&action=review > > This patch fails to update GPUProcessProxy.cpp accordingly. We can simplify > things there now that sendNetworkProcessXPCEndpointToProcess() no longer > needs a data store instance. > > > Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.h:281 > > + static bool sendNetworkProcessXPCEndpointToProcess(AuxiliaryProcessProxy&); > > Now that it is static and doesn't use any information from the data store, I > would suggest moving this to NetworkProcessProxy and make it non-static. > Then WebProcessProxy could call > `NetworkProcessProxy::ensureDefaultNetworkProcess(). > sendXPCEndpointToProcess(*this)` Fixed in latest patch. Thanks for reviewing!
Comment on attachment 449593 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=449593&action=review r=me > Source/WebKit/UIProcess/Network/NetworkProcessProxy.h:274 > + bool sendNetworkProcessXPCEndpointToProcess(AuxiliaryProcessProxy&); Personally, I feel we could drop "NetworkProcess" from the name now that it is located on the NetworkProcessProxy class. Your call. > Source/WebKit/UIProcess/Network/NetworkProcessProxyCocoa.mm:61 > + if (GPUProcessProxy::singletonIfCreated()) if (auto gpuProcess = GPUProcessProxy::singletonIfCreated()) > Source/WebKit/UIProcess/Network/NetworkProcessProxyCocoa.mm:62 > + m_networkProcess->sendNetworkProcessXPCEndpointToProcess(*GPUProcessProxy::singletonIfCreated()); *gpuProcess
Created attachment 449615 [details] Patch
(In reply to Chris Dumez from comment #25) > Comment on attachment 449593 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=449593&action=review > > r=me > > > Source/WebKit/UIProcess/Network/NetworkProcessProxy.h:274 > > + bool sendNetworkProcessXPCEndpointToProcess(AuxiliaryProcessProxy&); > > Personally, I feel we could drop "NetworkProcess" from the name now that it > is located on the NetworkProcessProxy class. Your call. > > > Source/WebKit/UIProcess/Network/NetworkProcessProxyCocoa.mm:61 > > + if (GPUProcessProxy::singletonIfCreated()) > > if (auto gpuProcess = GPUProcessProxy::singletonIfCreated()) > > > Source/WebKit/UIProcess/Network/NetworkProcessProxyCocoa.mm:62 > > + m_networkProcess->sendNetworkProcessXPCEndpointToProcess(*GPUProcessProxy::singletonIfCreated()); > > *gpuProcess Fixed! Thanks for reviewing!
Created attachment 449674 [details] Patch
Committed r288387 (246284@main): <https://commits.webkit.org/246284@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 449674 [details].
*** Bug 235719 has been marked as a duplicate of this bug. ***