Bug 177336 – Non-prevalent resources should be allowed access to existing cookies in a third-party context

• Component: WebKit2
• Status: RESOLVED FIXED

[John Wilander]

Non-prevalent resources should be allowed access to existing cookies in a third-party context. This is to fix a regression introduced in https://bugs.webkit.org/show_bug.cgi?id=175232.

[John Wilander]

<rdar://problem/34578257>

[John Wilander]

Created attachment 321502 [details]
Patch

[Build Bot]

Attachment 321502 [details] did not pass style-queue:


ERROR: Source/WebKit/UIProcess/WebResourceLoadStatisticsStore.h:135:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit/UIProcess/WebResourceLoadStatisticsStore.cpp:148:  Extra space before ( in function call  [whitespace/parens] [4]
Total errors found: 2 in 19 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Brent Fulgham]

Comment on attachment 321502 [details]
Patch

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

> Source/WebCore/loader/ResourceLoadStatistics.h:89
> +    bool isMarkedForCookieBlocking { false };

It might be more logical to use an OptionSet or an enum, since what we really have here is a tri-state condition -- either we are blocking, or we are partitioning, or we are allowing cookies. That might simplify some of the conditionals later on, where you have to test for three states.

> Source/WebCore/platform/network/mac/CookieJarMac.mm:104
> +static bool cookiesInBlockedForURL(const NetworkStorageSession& session, const URL& firstParty, const URL& url)

This name is weird. Should it be "cookiesAreBlockedForURL" ? What does "InBlocked" mean?

[Brent Fulgham]

Comment on attachment 321502 [details]
Patch

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

>> Source/WebCore/loader/ResourceLoadStatistics.h:89
>> +    bool isMarkedForCookieBlocking { false };
> 
> It might be more logical to use an OptionSet or an enum, since what we really have here is a tri-state condition -- either we are blocking, or we are partitioning, or we are allowing cookies. That might simplify some of the conditionals later on, where you have to test for three states.

We should make that change as a separate patch, to speed things along.

[Brent Fulgham]

Comment on attachment 321502 [details]
Patch

r=me, but please fix the typo. Let's do a bit of clean-up in a follow-up patch.

[John Wilander]

Created attachment 321566 [details]
Patch for landing

[John Wilander]

(In reply to Brent Fulgham from comment #6)
> Comment on attachment 321502 [details]
> Patch
> 
> r=me, but please fix the typo. Let's do a bit of clean-up in a follow-up
> patch.

Thanks! Typo fixed.

[WebKit Commit Bot]

Comment on attachment 321566 [details]
Patch for landing

Clearing flags on attachment: 321566

Committed r222396: <http://trac.webkit.org/changeset/222396>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.