Currently, the XPC endpoint for receiving the Launch Services database is being sent to GPUP in GPUProcessProxy::addSession. If GPUP has not finished launching at that point, the endpoint will not be sent.
Created attachment 441938 [details] Patch
<rdar://problem/84480229>
Created attachment 441943 [details] Patch
Comment on attachment 441943 [details] Patch r=me
Created attachment 441944 [details] Patch
Created attachment 441945 [details] Patch
Comment on attachment 441945 [details] Patch Thanks for reviewing!
Comment on attachment 441938 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=441938&action=review Some additional thoughts. > Source/WebKit/UIProcess/GPU/GPUProcessProxy.cpp:439 > + auto it = m_sessionIDs.begin(); > + if (it != m_sessionIDs.end()) { > + auto webSiteDataStore = WebsiteDataStore::existingDataStoreForSessionID(*m_sessionIDs.begin()); This is a slightly unusual pattern. Since m_sessionIDs is a HashSet, choosing the first by iterating gives you an arbitrary member of the set, variable. If that is indeed what you want then I suppose this is OK, but note that this is neither truly random, nor fits any reliable pattern. Almost worth a comment explaining why it’s OK to use only one of the session IDs in a set, because I do not think that is obvious. Maybe just a comment saying "all session IDs are guaranteed to have the same data store". Coding style wise, this calls begin() an extra time even though it uses a local variable. There are two ways I might write this that I think would be good style: // <Comment explaining why choosing an arbitrary session ID is OK, but random is not needed.> if (auto it = m_sessionIDs.begin(); it != m_sessionIDs.end()) { auto store = WebsiteDataStore::existingDataStoreForSessionID(*it); Or: // <Comment explaining why choosing an arbitrary session ID is OK, but random is not needed.> if (!m_sessionIDs.isEmpty()) { auto store = WebsiteDataStore::existingDataStoreForSessionID(m_sessionIDs.begin()); Also, how solid is the guarantee that existingDataStoreForSessionID will not return null?
(In reply to Darin Adler from comment #8) > Comment on attachment 441938 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=441938&action=review > > Some additional thoughts. > > > Source/WebKit/UIProcess/GPU/GPUProcessProxy.cpp:439 > > + auto it = m_sessionIDs.begin(); > > + if (it != m_sessionIDs.end()) { > > + auto webSiteDataStore = WebsiteDataStore::existingDataStoreForSessionID(*m_sessionIDs.begin()); > > This is a slightly unusual pattern. Since m_sessionIDs is a HashSet, > choosing the first by iterating gives you an arbitrary member of the set, > variable. If that is indeed what you want then I suppose this is OK, but > note that this is neither truly random, nor fits any reliable pattern. > Almost worth a comment explaining why it’s OK to use only one of the session > IDs in a set, because I do not think that is obvious. Maybe just a comment > saying "all session IDs are guaranteed to have the same data store". > > Coding style wise, this calls begin() an extra time even though it uses a > local variable. There are two ways I might write this that I think would be > good style: > > // <Comment explaining why choosing an arbitrary session ID is OK, but > random is not needed.> > if (auto it = m_sessionIDs.begin(); it != m_sessionIDs.end()) { > auto store = WebsiteDataStore::existingDataStoreForSessionID(*it); > > Or: > > // <Comment explaining why choosing an arbitrary session ID is OK, but > random is not needed.> > if (!m_sessionIDs.isEmpty()) { > auto store = > WebsiteDataStore::existingDataStoreForSessionID(m_sessionIDs.begin()); > That is a good point. I will upload a revised patch, adopting one of these approaches and adding a comment. The reason any session ID is OK to use, is that we're looking to get the XPC endpoint from any Networking process. All Networking processes has this endpoint, and it does not matter from which Networking process we get it from. > Also, how solid is the guarantee that existingDataStoreForSessionID will not > return null? That is also a good point. Although unlikely, I think it is possible that the data store for this session has been removed at this point, since this session ID is kept in a hash map not controlled by the data store class. I will add a null check here. Thanks for reviewing!
Committed r284582 (243319@main): <https://commits.webkit.org/243319@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 441945 [details].
Reopening to attach new patch.
Created attachment 442032 [details] Patch
Created attachment 442033 [details] Patch
I believe the api-gtk are unrelated to the patch.
Committed r284641 (243361@main): <https://commits.webkit.org/243361@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 442033 [details].