Bug 208399 – ResourceLoadStatistics: Enable cookie blocking and the Storage Access API in ephemeral sessions

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

[John Wilander]

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.

[John Wilander]

<rdar://problem/24731650>

[John Wilander]

Created attachment 392037 [details]
Patch

[Brent Fulgham]

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]

(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]

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]

(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]

(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]

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]

Created attachment 392163 [details]
Patch for landing

[WebKit Commit Bot]

The commit-queue encountered the following flaky tests while processing attachment 392163 [details]:

editing/spelling/spellcheck-async-remove-frame.html bug 158401 (authors: [e-mail], [e-mail], and [e-mail])
The commit-queue is continuing to process your patch.

[WebKit Commit Bot]

Comment on attachment 392163 [details]
Patch for landing

Clearing flags on attachment: 392163

Committed r257726: <https://trac.webkit.org/changeset/257726>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Diego Pino]

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

[Alex Christensen]

Reopening to attach new patch.

[Alex Christensen]

Created attachment 394318 [details]
Patch

[John Wilander]

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]

You should use https://bugs.webkit.org/show_bug.cgi?id=208506

[Alex Christensen]

webkit-patch upload automatically obsoletes previous attachments, which I undid making it clear that this is a followup fix to a big change.

[EWS]

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]

*** Bug 193728 has been marked as a duplicate of this bug. ***