Construct less unnecessary temporary WebProcessPool objects in WebsiteDataStore implementation.
Created attachment 392502 [details] Patch
Created attachment 392510 [details] Patch
Comment on attachment 392510 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=392510&action=review > Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp:1380 > - for (auto& processPool : ensureProcessPools()) { > + for (auto& processPool : processPools()) { > if (auto* process = processPool->networkProcess()) { > process->isRegisteredAsSubresourceUnder(m_sessionID, WebCore::RegistrableDomain { subresourceURL }, WebCore::RegistrableDomain { topFrameURL }, WTFMove(completionHandler)); > - break; > - } else { > - ASSERT_NOT_REACHED(); > - completionHandler(false); > - break; > + return; > } > + break; > } > + completionHandler(false); I am not sure what this function is supposed to do. Does it expect (processPools().size() == 1 and processPools()[0]->networkProcess() != nullptr)? if this is the case why do we use a for-loop? > Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp:1481 > + completionHandler(); completionHandler is moved above when creating callbackAggregator.
(In reply to Said Abou-Hallawa from comment #3) > Comment on attachment 392510 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=392510&action=review > > > Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp:1380 > > - for (auto& processPool : ensureProcessPools()) { > > + for (auto& processPool : processPools()) { > > if (auto* process = processPool->networkProcess()) { > > process->isRegisteredAsSubresourceUnder(m_sessionID, WebCore::RegistrableDomain { subresourceURL }, WebCore::RegistrableDomain { topFrameURL }, WTFMove(completionHandler)); > > - break; > > - } else { > > - ASSERT_NOT_REACHED(); > > - completionHandler(false); > > - break; > > + return; > > } > > + break; > > } > > + completionHandler(false); > > I am not sure what this function is supposed to do. Does it expect > (processPools().size() == 1 and processPools()[0]->networkProcess() != > nullptr)? if this is the case why do we use a for-loop? > > > Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp:1481 > > + completionHandler(); > > completionHandler is moved above when creating callbackAggregator. Thanks for catching this. Will fix before landing.
Created attachment 392514 [details] Patch
Comment on attachment 392514 [details] Patch Rejecting attachment 392514 [details] from commit-queue. Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-03', 'validate-changelog', '--check-oops', '--non-interactive', 392514, '--port=mac']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit ChangeLog entry in Source/WebKit/ChangeLog contains OOPS!. Full output: https://webkit-queues.webkit.org/results/13333735
Created attachment 392515 [details] Patch
Comment on attachment 392515 [details] Patch Clearing flags on attachment: 392515 Committed r257893: <https://trac.webkit.org/changeset/257893>
All reviewed patches have been landed. Closing bug.
<rdar://problem/60066720>