[webkit-reviews] review granted: [Bug 194906] Same Site Lax cookies are not sent with cross-site redirect from client-initiated load : [Attachment 362618] Patch and layout test

[bugzilla-daemon at webkit.org]

Brent Fulgham <bfulgham at webkit.org> has granted Daniel Bates
<dbates at webkit.org>'s request for review:
Bug 194906: Same Site Lax cookies are not sent with cross-site redirect from
client-initiated load
https://bugs.webkit.org/show_bug.cgi?id=194906

Attachment 362618: Patch and layout test

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




--- Comment #3 from Brent Fulgham <bfulgham at webkit.org> ---
Comment on attachment 362618
  --> https://bugs.webkit.org/attachment.cgi?id=362618
Patch and layout test

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

Looks good. I thought of some ideas for possible future improvements (see
comments).

> Source/WebCore/loader/FrameLoader.cpp:2889
> +    request.setIsTopSite(isMainResource && m_frame.isMainFrame());

It looks like the concept of 'isTopSite' is used elsewhere in FrameLoader
(e.g., L2871). I wonder if we should have a helper:

bool isTopSite(bool isMainResource) const  { return isMainResource &&
m_frame.isMainFrame(); }

>
LayoutTests/http/tests/cookies/same-site/user-load-cross-site-redirect-expected
.txt:9
> +PASS Has cookie "normal" with value 27.

Do we have a test for Same-Site HTTP-only cookies that are visible servers, but
are not exposed to DOM? That's not really relevant to this Lax cookie case, but
would be good to make sure we never regress HTTP-only cookies being isolated
from the WebContent process.