Bug 203428 – Storage Access API: Make the API work with the experimental 3rd-party cookie blocking

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Misc.

[John Wilander]

The experimental 3rd-party cookie blocking that landed in <https://trac.webkit.org/changeset/251467> needs support in the Storage Access API.

[Radar WebKit Bug Importer]

<rdar://problem/56626949>

[John Wilander]

Created attachment 381956 [details]
Patch

[Kate Cheney]

Comment on attachment 381956 [details]
Patch

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

Looks good to me! I'd give it an r+ if I could :)

> Source/WebKit/NetworkProcess/Classifier/ResourceLoadStatisticsDatabaseStore.cpp:1810
> +

Since ensureResourceStatisticsForRegistrableDomain(subFrameDomain) is always called before this function, I don't think the hasNoEntry check is needed (I know you didn't write this code, it was probably me who did this). Removing it would simplify this function a bit and let it match the memory store almost exactly. That being said, it doesn't hurt anyone to keep it in.

[Kate Cheney]

(In reply to Katherine_cheney from comment #3)
> Comment on attachment 381956 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=381956&action=review
> 
> Looks good to me! I'd give it an r+ if I could :)
> 
> > Source/WebKit/NetworkProcess/Classifier/ResourceLoadStatisticsDatabaseStore.cpp:1810
> > +
> 
> Since ensureResourceStatisticsForRegistrableDomain(subFrameDomain) is always
> called before this function, I don't think the hasNoEntry check is needed (I
> know you didn't write this code, it was probably me who did this). Removing
> it would simplify this function a bit and let it match the memory store
> almost exactly. That being said, it doesn't hurt anyone to keep it in.

"this function" being cookieAccess

[John Wilander]

(In reply to Katherine_cheney from comment #3)
> Comment on attachment 381956 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=381956&action=review
> 
> Looks good to me! I'd give it an r+ if I could :)

Thanks for taking a look. You’re always a bit worried touching code someone else has been working a lot on.

> > Source/WebKit/NetworkProcess/Classifier/ResourceLoadStatisticsDatabaseStore.cpp:1810
> > +
> 
> Since ensureResourceStatisticsForRegistrableDomain(subFrameDomain) is always
> called before this function, I don't think the hasNoEntry check is needed (I
> know you didn't write this code, it was probably me who did this). Removing
> it would simplify this function a bit and let it match the memory store
> almost exactly. That being said, it doesn't hurt anyone to keep it in.

I has that thought but figured there was a reason for the code to be there. I’ll try to remove before landing.

[John Wilander]

Thank you, Alex! Let's see if I need to rebase or not. :)

[WebKit Commit Bot]

Comment on attachment 381956 [details]
Patch

Clearing flags on attachment: 381956

Committed r251663: <https://trac.webkit.org/changeset/251663>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.