Bug 208399

Summary: ResourceLoadStatistics: Enable cookie blocking and the Storage Access API in ephemeral sessions
Product: WebKit Reporter: John Wilander <wilander>
Component: WebKit Misc.Assignee: John Wilander <wilander>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, bfulgham, cdumez, commit-queue, dpino, ews-watchlist, japhet, katherine_cheney
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=208400
https://bugs.webkit.org/show_bug.cgi?id=208739
Attachments:
Description Flags
Patch
none
Patch for landing
none
Patch none

John Wilander
Reported 2020-02-28 16:34:27 PST
Since full third-party cookie blocking is stateless, it can now be enabled in ephemeral sessions. User interaction as a gate for calling the Storage Access API can be captured in an ephemeral fashion as well to allow authenticated embeds. We should enabled these features.
Attachments
Patch (145.00 KB, patch)
2020-02-28 17:27 PST, John Wilander
no flags
Patch for landing (147.20 KB, patch)
2020-03-02 11:23 PST, John Wilander
no flags
Patch (1.33 KB, patch)
2020-03-23 16:18 PDT, Alex Christensen
no flags
John Wilander
Comment 1 2020-02-28 16:34:41 PST
John Wilander
Comment 2 2020-02-28 17:27:10 PST
Brent Fulgham
Comment 3 2020-02-28 18:19:44 PST
Comment on attachment 392037 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=392037&action=review I think you might have left a backtrace by accident, but otherwise looks great! R=me > Source/WebKit/UIProcess/Network/NetworkProcessProxy.cpp:1223 > + WTFReportBacktrace(); Did you mean to leave this in?
John Wilander
Comment 4 2020-02-28 20:04:12 PST
(In reply to Brent Fulgham from comment #3) > Comment on attachment 392037 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=392037&action=review > > I think you might have left a backtrace by accident, but otherwise looks > great! R=me > > > Source/WebKit/UIProcess/Network/NetworkProcessProxy.cpp:1223 > > + WTFReportBacktrace(); > > Did you mean to leave this in? Nope. And I had removed removed it but not saved the file before uploading. Thanks for the review! I'll wait for green light on the ios-wk2 bot before putting it on the queue.
John Wilander
Comment 5 2020-02-28 20:26:43 PST
We got one curious layout test crash with this trace: 0 com.apple.WebKit 0x000000010fefb3d3 WTFCrashWithInfo(int, char const*, char const*, int) + 19 1 com.apple.WebKit 0x000000011003450e WebKit::WebResourceLoadStatisticsStore::suspend(WTF::CompletionHandler<void ()>&&) + 278 2 com.apple.WebKit 0x00000001100076f6 WTF::Detail::CallableWrapper<WebKit::NetworkProcess::prepareToSuspend(bool, WTF::CompletionHandler<void ()>&&)::$_58, void, WebKit::NetworkSession&>::call(WebKit::NetworkSession&) + 76 3 com.apple.WebKit 0x000000010ffdba5d WebKit::NetworkProcess::forEachNetworkSession(WTF::Function<void (WebKit::NetworkSession&)> const&) + 91 4 com.apple.WebKit 0x000000010ffe3288 WebKit::NetworkProcess::prepareToSuspend(bool, WTF::CompletionHandler<void ()>&&) + 394 5 com.apple.WebKit 0x000000010ff439ac void IPC::handleMessageAsync<Messages::NetworkProcess::PrepareToSuspend, WebKit::NetworkProcess, void (WebKit::NetworkProcess::*)(bool, WTF::CompletionHandler<void ()>&&)>(IPC::Connection&, IPC::Decoder&, WebKit::NetworkProcess*, void (WebKit::NetworkProcess::*)(bool, WTF::CompletionHandler<void ()>&&)) + 186 Looks like something is not happy when suspending. While that might be unrelated, I'll have to look at suspend() and resume() for ephemeral sessions before landing. Then there's an API test that failed at least once on the api-ios bot: WKWebView.InitializingWebViewWithEphemeralStorageDoesNotLog It does not fail on my Mac so I may have to reproduce in a simulator or on a device to see what's logging.
John Wilander
Comment 6 2020-02-28 20:31:33 PST
(In reply to John Wilander from comment #5) > It does not fail on my Mac so I may have to reproduce in a simulator or on a > device to see what's logging. I just realized that the logging is most probably that stray WTFReportBacktrace() call.
John Wilander
Comment 7 2020-02-28 20:32:12 PST
(In reply to John Wilander from comment #5) > We got one curious layout test crash with this trace: > > 0 com.apple.WebKit 0x000000010fefb3d3 WTFCrashWithInfo(int, > char const*, char const*, int) + 19 > 1 com.apple.WebKit 0x000000011003450e > WebKit::WebResourceLoadStatisticsStore::suspend(WTF::CompletionHandler<void > ()>&&) + 278 > 2 com.apple.WebKit 0x00000001100076f6 > WTF::Detail::CallableWrapper<WebKit::NetworkProcess::prepareToSuspend(bool, > WTF::CompletionHandler<void ()>&&)::$_58, void, > WebKit::NetworkSession&>::call(WebKit::NetworkSession&) + 76 > 3 com.apple.WebKit 0x000000010ffdba5d > WebKit::NetworkProcess::forEachNetworkSession(WTF::Function<void > (WebKit::NetworkSession&)> const&) + 91 > 4 com.apple.WebKit 0x000000010ffe3288 > WebKit::NetworkProcess::prepareToSuspend(bool, WTF::CompletionHandler<void > ()>&&) + 394 > 5 com.apple.WebKit 0x000000010ff439ac void > IPC::handleMessageAsync<Messages::NetworkProcess::PrepareToSuspend, > WebKit::NetworkProcess, void (WebKit::NetworkProcess::*)(bool, > WTF::CompletionHandler<void ()>&&)>(IPC::Connection&, IPC::Decoder&, > WebKit::NetworkProcess*, void (WebKit::NetworkProcess::*)(bool, > WTF::CompletionHandler<void ()>&&)) + 186 > > Looks like something is not happy when suspending. While that might be > unrelated, I'll have to look at suspend() and resume() for ephemeral > sessions before landing. The test case editing/async-clipboard/clipboard-change-data-while-getting-type.html does not fail on my Mac.
John Wilander
Comment 8 2020-03-02 10:58:22 PST
I had a conversation with Chris Dumez this morning on how to handle suspend/resume. Easy to do. Will add it to the patch I land.
John Wilander
Comment 9 2020-03-02 11:23:45 PST
Created attachment 392163 [details] Patch for landing
WebKit Commit Bot
Comment 10 2020-03-02 12:23:11 PST
The commit-queue encountered the following flaky tests while processing attachment 392163 [details]: editing/spelling/spellcheck-async-remove-frame.html bug 158401 (authors: morrita@google.com, rniwa@webkit.org, and tony@chromium.org) The commit-queue is continuing to process your patch.
WebKit Commit Bot
Comment 11 2020-03-02 12:24:10 PST
Comment on attachment 392163 [details] Patch for landing Clearing flags on attachment: 392163 Committed r257726: <https://trac.webkit.org/changeset/257726>
WebKit Commit Bot
Comment 12 2020-03-02 12:24:11 PST
All reviewed patches have been landed. Closing bug.
Diego Pino
Comment 13 2020-03-03 04:22:27 PST
Alex Christensen
Comment 14 2020-03-23 16:18:26 PDT
Reopening to attach new patch.
Alex Christensen
Comment 15 2020-03-23 16:18:27 PDT
John Wilander
Comment 16 2020-03-23 16:20:15 PDT
Why are we reusing this bug for a small follow-up fix? This bug used to track a major change and now it looks like a one-liner.
John Wilander
Comment 17 2020-03-23 16:20:59 PDT
Alex Christensen
Comment 18 2020-03-23 17:06:15 PDT
webkit-patch upload automatically obsoletes previous attachments, which I undid making it clear that this is a followup fix to a big change.
EWS
Comment 19 2020-03-23 18:22:41 PDT
Committed r258893: <https://trac.webkit.org/changeset/258893> All reviewed patches have been landed. Closing bug and clearing flags on attachment 394318 [details].
Brent Fulgham
Comment 20 2021-02-10 11:39:21 PST
*** Bug 193728 has been marked as a duplicate of this bug. ***
Note You need to log in before you can comment on or make changes to this bug.