[webkit-reviews] review granted: [Bug 207523] Expose prevalent domains on a per-page basis : [Attachment 390706] Patch

[bugzilla-daemon at webkit.org]

Chris Dumez <cdumez at apple.com> has granted katherine_cheney at apple.com's request
for review:
Bug 207523: Expose prevalent domains on a per-page basis
https://bugs.webkit.org/show_bug.cgi?id=207523

Attachment 390706: Patch

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




--- Comment #27 from Chris Dumez <cdumez at apple.com> ---
Comment on attachment 390706
  --> https://bugs.webkit.org/attachment.cgi?id=390706
Patch

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

r=me with changes

> Source/WebCore/loader/cache/CachedResourceLoader.cpp:794
> +   
frame.loader().client().addLoadedRegistrableDomain(RegistrableDomain(request.re
sourceRequest().url()));

I think this should be moved towards the end of the method to take into account
content extensions/blockers for example (we may block the load or update the
url). Probably by this line at the end:
 m_documentResources.set(resource->url(), resource);

> Source/WebKit/UIProcess/API/Cocoa/WKWebsiteDataStore.mm:426
> +	   apiDomains = WTF::map(prevalentDomains, [](auto& domain) {

this can be on the previous line:
Vector<RefPtr<API::Object>> apiDomains = WTF::map();

> Source/WebKit/WebProcess/WebPage/WebPage.cpp:6678
> +	  
WebProcess::singleton().ensureNetworkProcessConnection().connection().sendWithA
syncReply(Messages::NetworkConnectionToWebProcess::IsPrevalentSubresourceLoad(d
omain), [this, protectedThis = makeRefPtr(*this), domain] (bool isPrevalent) {

makeRef(), not makeRefPtr()

> Source/WebKit/WebProcess/WebPage/WebPage.cpp:6687
> +    

nit: extra blank line