[webkit-reviews] review granted: [Bug 210674] Exempt app-bound domains from ITP's website data deletion and third-party cookie blocking between themselves : [Attachment 398573] Patch

[bugzilla-daemon at webkit.org]

Chris Dumez <cdumez at apple.com> has granted John Wilander <wilander at apple.com>'s
request for review:
Bug 210674: Exempt app-bound domains from ITP's website data deletion and
third-party cookie blocking between themselves
https://bugs.webkit.org/show_bug.cgi?id=210674

Attachment 398573: Patch

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




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

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

> Source/WebKit/Shared/ResourceLoadStatisticsParameters.h:50
>      WebCore::RegistrableDomain standaloneApplicationDomain { };

{ } unnecessary

> Source/WebKit/Shared/ResourceLoadStatisticsParameters.h:51
> +    HashSet<WebCore::RegistrableDomain> appBoundDomains { };

{ } unnecessary

> Source/WebKit/Shared/ResourceLoadStatisticsParameters.h:52
>      WebCore::RegistrableDomain manualPrevalentResource { };

{ } unnecessary

> Source/WebKit/UIProcess/WebProcessPool.cpp:609
> +    WebCore::RegistrableDomain standaloneApplicationDomain { };

{ } unnecessary

> Source/WebKit/UIProcess/WebProcessPool.cpp:610
> +    HashSet<WebCore::RegistrableDomain> appBoundDomains { };

{ } unnecessary

> Source/WebKit/UIProcess/WebProcessPool.cpp:611
>      WebCore::RegistrableDomain manualPrevalentResource { };

{ } unnecessary

> Source/WebKit/UIProcess/WebsiteData/Cocoa/WebsiteDataStoreCocoa.mm:505
> +	   RELEASE_ASSERT(domain.string() == "localhost"_s || domain.string()
== "127.0.0.1"_s);

RegistrableDomain has an operator==() so you should be able to do:
RELEASE_ASSERT(domain == "localhost" || domain == "127.0.0.1");

> Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp:2235
> +    auto optionalAppBoundDomains = appBoundDomainsIfInitialized();

There is no point in calling appBoundDomainsIfInitialized() if
!isAppBoundITPRelaxationEnabled so I would write it like so:
if (isAppBoundITPRelaxationEnabled)
    appBoundDomains =
appBoundDomainsIfInitialized().valueOr(HashSet<WebCore::RegistrableDomain> {
});