[webkit-reviews] review granted: [Bug 186205] Regression(r230567): Unable to log into twitter.com in private sessions : [Attachment 341783] Patch

[bugzilla-daemon at webkit.org]

youenn fablet <youennf at gmail.com> has granted Chris Dumez <cdumez at apple.com>'s
request for review:
Bug 186205: Regression(r230567): Unable to log into twitter.com in private
sessions
https://bugs.webkit.org/show_bug.cgi?id=186205

Attachment 341783: Patch

https://bugs.webkit.org/attachment.cgi?id=341783&action=review




--- Comment #3 from youenn fablet <youennf at gmail.com> ---
Comment on attachment 341783
  --> https://bugs.webkit.org/attachment.cgi?id=341783
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=341783&action=review

> Source/WebCore/workers/service/server/SWServer.cpp:-75
> -    RELEASE_ASSERT(m_jobQueues.isEmpty());

Shouldn't we also remove the first assert?

> Source/WebKit/UIProcess/WebProcessPool.cpp:734
> +    }

You might be able to use sendToNetworkingProcess/sendToStorageProcess to
improve the readability.
Something like:
if (privateBrowsingEnabled) {
   ...
   return;
}

sendToNetworkingProcess(Messages::NetworkProcess::DestroySession(PAL::SessionID
::legacyPrivateSessionID()), 0);
sendToStorageProcess(Messages::StorageProcess::DestroySession(PAL::SessionID::l
egacyPrivateSessionID()), 0);
sendToAllProcesses(Messages::WebProcess::DestroySession(PAL::SessionID::legacyP
rivateSessionID()));

> Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp:1482
> +	       weakThis->removePrevalentDomains(domainsToRemove);

Lambdas do not seem to make things better here.
How about passing the weakThis once to WebResourceLoadStatisticsStore, which
could then call the
removePrevalentDomains/removeAllStorageAccessHandler/grantStorageAccessHandler/
... methods directly.