Bug 175443 – [Beacon] Add support for quota limitation

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

[Chris Dumez]

Add support for quota limitation as per:
- https://w3c.github.io/beacon/#sec-processing-model (step 5)

which points to:
- https://fetch.spec.whatwg.org/#http-network-or-cache-fetch (step 9)

[Chris Dumez]

<rdar://problem/33729002>

[Chris Dumez]

Created attachment 318298 [details]
WIP Patch

[Chris Dumez]

Created attachment 318299 [details]
WIP Patch

[Build Bot]

Comment on attachment 318299 [details]
WIP Patch

Attachment 318299 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/4326946

New failing tests:
http/tests/security/contentSecurityPolicy/user-style-sheet-font-crasher.php

[Build Bot]

Created attachment 318309 [details]
Archive of layout-test-results from ews104 for mac-elcapitan-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104  Port: mac-elcapitan-wk2  Platform: Mac OS X 10.11.6

[Build Bot]

Comment on attachment 318299 [details]
WIP Patch

Attachment 318299 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/4326970

New failing tests:
http/tests/security/contentSecurityPolicy/user-style-sheet-font-crasher.php

[Build Bot]

Created attachment 318311 [details]
Archive of layout-test-results from ews122 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews122  Port: ios-simulator-wk2  Platform: Mac OS X 10.12.5

[Chris Dumez]

Created attachment 318399 [details]
WIP Patch

[Chris Dumez]

Created attachment 318442 [details]
WIP Patch

[Chris Dumez]

Created attachment 318451 [details]
Patch

[youenn fablet]

Comment on attachment 318451 [details]
Patch

Looks good to me.
Two questions though:

What happens if the network process crashes?
Is there a way to call the completion handlers so as to free WebProcess pings?

Since we are having a completion handler, would it be worth passing an error if ping load fails?
That might be helpful to do some logging or use this in Web Inspector.

[Chris Dumez]

(In reply to youenn fablet from comment #11)
> Comment on attachment 318451 [details]
> Patch
> 
> Looks good to me.
> Two questions though:
> 
> What happens if the network process crashes?
> Is there a way to call the completion handlers so as to free WebProcess
> pings?

I'll look into this.

> 
> Since we are having a completion handler, would it be worth passing an error
> if ping load fails?
> That might be helpful to do some logging or use this in Web Inspector.

For now this is not useful. It would be trivial to add it in a follow-up if we need it.

[Chris Dumez]

Created attachment 318457 [details]
Patch

[Chris Dumez]

Comment on attachment 318457 [details]
Patch

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

> Source/WebKit/WebProcess/Network/WebLoaderStrategy.cpp:355
> +    for (auto& pingLoadCompletionHandler : pingLoadCompletionHandlers.values())

Now dealing with network process crashes better.

[youenn fablet]

Comment on attachment 318457 [details]
Patch

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

> Source/WebKitLegacy/WebCoreSupport/WebResourceLoadScheduler.h:62
> +    void startPingLoad(WebCore::NetworkingContext*, WebCore::ResourceRequest&, const WebCore::HTTPHeaderMap&, Ref<WebCore::SecurityOrigin>&& sourceOrigin, WebCore::ContentSecurityPolicy*, const WebCore::FetchOptions&, WTF::Function<void()>&& completionHandler) override;

Should be final probably. Do we really need sourceOrigin parameter name here also?

On another topic related to the handling of headers, do we have a test checking referrer value after a cross-origin redirection and with a referrer policy like origin-when-cross-origin?
Looking at the code, I am not sure we are doing the right thing here.

[youenn fablet]

> > Since we are having a completion handler, would it be worth passing an error
> > if ping load fails?
> > That might be helpful to do some logging or use this in Web Inspector.
> 
> For now this is not useful. It would be trivial to add it in a follow-up if
> we need it.

Adding some web inspector console logging would be good when ping load fails.
That might be handy for web developers to debug things like CORS checks, at least until web inspector fully supports ping loads in the network panel.
This would also mirror what is done for fetch.

[Chris Dumez]

(In reply to youenn fablet from comment #16)
> > > Since we are having a completion handler, would it be worth passing an error
> > > if ping load fails?
> > > That might be helpful to do some logging or use this in Web Inspector.
> > 
> > For now this is not useful. It would be trivial to add it in a follow-up if
> > we need it.
> 
> Adding some web inspector console logging would be good when ping load fails.
> That might be handy for web developers to debug things like CORS checks, at
> least until web inspector fully supports ping loads in the network panel.
> This would also mirror what is done for fetch.

I started working on error reporting but this is a larger change. I'll upload a separate follow-up patch for it.

[Chris Dumez]

Created attachment 318516 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 318516 [details]
Patch

Clearing flags on attachment: 318516

Committed r220922: <http://trac.webkit.org/changeset/220922>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.