Summary: | Resource Load Statistics: Re-introduce latch mode for subresource cookie blocking | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | John Wilander <wilander> | ||||||
Component: | WebKit Misc. | Assignee: | John Wilander <wilander> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | achristensen, bfulgham, commit-queue, darin, webkit-bug-importer | ||||||
Priority: | P2 | Keywords: | InRadar | ||||||
Version: | WebKit Nightly Build | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Attachments: |
|
Description
John Wilander
2019-08-02 10:46:33 PDT
Created attachment 375427 [details]
Patch
Comment on attachment 375427 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=375427&action=review > Source/WebKit/NetworkProcess/cocoa/NetworkDataTaskCocoa.mm:142 > + NSHTTPCookieStorage *storage = statelessCookieStorage(); > + [m_task _setExplicitCookieStorage:storage._cookieStorage]; Since we’re collapsing this, I don’t think the local variable adds anything any more. Could be a one liner instead. [m_task _setExplicitCookieStorage:statelessCookieStorage()._cookieStorage]; (In reply to Darin Adler from comment #3) > Comment on attachment 375427 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=375427&action=review > > > Source/WebKit/NetworkProcess/cocoa/NetworkDataTaskCocoa.mm:142 > > + NSHTTPCookieStorage *storage = statelessCookieStorage(); > > + [m_task _setExplicitCookieStorage:storage._cookieStorage]; > > Since we’re collapsing this, I don’t think the local variable adds anything > any more. Could be a one liner instead. > > [m_task > _setExplicitCookieStorage:statelessCookieStorage()._cookieStorage]; You're right. Will fix. I'm also going to rename NetworkDataTaskCocoa::applyCookieBlockingPolicy() to NetworkDataTaskCocoa::blockCookies() to make it clear at the call sites. At first, I wanted to stay with the more general "apply policy" to not have to touch it if we introduce more logic there. But now I think that's wrong. Readability wins. Thanks for the review, Darin. I'll assume you're OK with the function renaming I mention above. Created attachment 375540 [details]
Patch
The renaming ended up restructuring the code and I could remove some duplicate logging. So it's best to get another review. Orange bubbles are unrelated. The mac-debug tree seems red and the test failures on win have nothing to do with this code. Comment on attachment 375540 [details]
Patch
Thanks again, Darin!
Comment on attachment 375540 [details] Patch Clearing flags on attachment: 375540 Committed r248273: <https://trac.webkit.org/changeset/248273> All reviewed patches have been landed. Closing bug. |