[webkit-reviews] review granted: [Bug 161106] Stop using CookiesStrategy : [Attachment 359051] Patch

[bugzilla-daemon at webkit.org]

Antti Koivisto <koivisto at iki.fi> has granted Alex Christensen
<achristensen at apple.com>'s request for review:
Bug 161106: Stop using CookiesStrategy
https://bugs.webkit.org/show_bug.cgi?id=161106

Attachment 359051: Patch

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




--- Comment #23 from Antti Koivisto <koivisto at iki.fi> ---
Comment on attachment 359051
  --> https://bugs.webkit.org/attachment.cgi?id=359051
Patch

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

> Source/WebCore/ChangeLog:3
> +	   Stop using CookiesStrategy

A word of motivation would be nice.

> Source/WebCore/loader/CookieJar.cpp:74
> +    if (auto* session =
NetworkStorageSession::storageSession(document.sessionID()))
> +	   result = session->cookiesForDOM(document.firstPartyForCookies(),
sameSiteInfo(document), url, frameID, pageID, includeSecureCookies);
>      else
> -	   result =
platformStrategies()->cookiesStrategy()->cookiesForDOM(document.sessionID(),
document.firstPartyForCookies(), sameSiteInfo(document), url, WTF::nullopt,
WTF::nullopt, includeSecureCookies);
> +	   ASSERT_NOT_REACHED();

Is there a particular reason for this defensiveness? Why bother null testing
and asserting if the session really can't be null?

> Source/WebCore/loader/CookieJar.cpp:111
> +    if (auto* session =
NetworkStorageSession::storageSession(document.sessionID()))
> +	   session->setCookiesFromDOM(document.firstPartyForCookies(),
sameSiteInfo(document), url, frameID, pageID, cookieString);
>      else
> -	  
platformStrategies()->cookiesStrategy()->setCookiesFromDOM(document.sessionID()
, document.firstPartyForCookies(), sameSiteInfo(document), url, WTF::nullopt,
WTF::nullopt, cookieString);
> +	   ASSERT_NOT_REACHED();

Also here and in many other places.