Bug 208399 - ResourceLoadStatistics: Enable cookie blocking and the Storage Access API in ephemeral sessions
Summary: ResourceLoadStatistics: Enable cookie blocking and the Storage Access API in ...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: John Wilander
URL:
Keywords: InRadar
: 193728 (view as bug list)
Depends on:
Blocks:
 
Reported: 2020-02-28 16:34 PST by John Wilander
Modified: 2021-02-10 11:39 PST (History)
8 users (show)

See Also:


Attachments
Patch (145.00 KB, patch)
2020-02-28 17:27 PST, John Wilander
no flags Details | Formatted Diff | Diff
Patch for landing (147.20 KB, patch)
2020-03-02 11:23 PST, John Wilander
no flags Details | Formatted Diff | Diff
Patch (1.33 KB, patch)
2020-03-23 16:18 PDT, Alex Christensen
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description John Wilander 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.
Comment 1 John Wilander 2020-02-28 16:34:41 PST
<rdar://problem/24731650>
Comment 2 John Wilander 2020-02-28 17:27:10 PST
Created attachment 392037 [details]
Patch
Comment 3 Brent Fulgham 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?
Comment 4 John Wilander 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.
Comment 5 John Wilander 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.
Comment 6 John Wilander 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.
Comment 7 John Wilander 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.
Comment 8 John Wilander 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.
Comment 9 John Wilander 2020-03-02 11:23:45 PST
Created attachment 392163 [details]
Patch for landing
Comment 10 WebKit Commit Bot 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.
Comment 11 WebKit Commit Bot 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>
Comment 12 WebKit Commit Bot 2020-03-02 12:24:11 PST
All reviewed patches have been landed.  Closing bug.
Comment 13 Diego Pino 2020-03-03 04:22:27 PST
This patch caused an early exit in GTK test bot due to too many crashes (>50).

https://build.webkit.org/builders/GTK%20Linux%2064-bit%20Release%20%28Tests%29/builds/12821/steps/layout-test/logs/stdio

Fix here: https://bugs.webkit.org/show_bug.cgi?id=208506
Comment 14 Alex Christensen 2020-03-23 16:18:26 PDT
Reopening to attach new patch.
Comment 15 Alex Christensen 2020-03-23 16:18:27 PDT
Created attachment 394318 [details]
Patch
Comment 16 John Wilander 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.
Comment 17 John Wilander 2020-03-23 16:20:59 PDT
You should use https://bugs.webkit.org/show_bug.cgi?id=208506
Comment 18 Alex Christensen 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.
Comment 19 EWS 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].
Comment 20 Brent Fulgham 2021-02-10 11:39:21 PST
*** Bug 193728 has been marked as a duplicate of this bug. ***