Bug 175280

Summary: Update sendBeacon() to rely on FetchBody instead of the whole FetchRequest
Product: WebKit Reporter: Chris Dumez <cdumez>
Component: WebCore Misc.Assignee: Chris Dumez <cdumez>
Status: RESOLVED FIXED    
Severity: Normal CC: buildbot, commit-queue, rniwa, webkit-bug-importer, youennf
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
URL: https://w3c.github.io/beacon/#sec-processing-model
Bug Depends on:    
Bug Blocks: 175264    
Attachments:
Description Flags
Patch
none
Patch
none
Archive of layout-test-results from ews101 for mac-elcapitan
none
Patch none

Description Chris Dumez 2017-08-07 13:00:50 PDT
Update sendBeacon() to rely on FetchBody instead of the whole FetchRequest. FetchBody for data destruction is really the only thing we need at the moment.
The new code also properly sets the CORS mode, which will be needed for Bug 175264.
Comment 1 Chris Dumez 2017-08-07 13:03:31 PDT
Created attachment 317454 [details]
Patch
Comment 2 youenn fablet 2017-08-07 14:11:26 PDT
Comment on attachment 317454 [details]
Patch

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

> Source/WebCore/ChangeLog:9
> +        for data destruction is really the only thing we need at the moment.

data extraction?

> Source/WebCore/Modules/beacon/NavigatorBeacon.cpp:35
>  #include <runtime/JSCJSValue.h>

Do you still need that include?

> Source/WebCore/Modules/beacon/NavigatorBeacon.cpp:64
> +    options.redirect = FetchOptions::Redirect::Follow;

No need to set redirect to follow, this is the default and is not spelled out in the spec explicitly.

> Source/WebCore/Modules/beacon/NavigatorBeacon.cpp:65
> +    options.cache = FetchOptions::Cache::NoCache;

I do not see that option set in the spec.
I forgot to ask why it was set in previous patches.
Comment 3 Chris Dumez 2017-08-07 14:17:56 PDT
Created attachment 317463 [details]
Patch
Comment 4 Build Bot 2017-08-07 15:27:30 PDT
Comment on attachment 317463 [details]
Patch

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

New failing tests:
http/tests/blink/sendbeacon/beacon-same-origin.html
Comment 5 Build Bot 2017-08-07 15:27:31 PDT
Created attachment 317480 [details]
Archive of layout-test-results from ews101 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101  Port: mac-elcapitan  Platform: Mac OS X 10.11.6
Comment 6 Chris Dumez 2017-08-07 15:30:03 PDT
Created attachment 317482 [details]
Patch
Comment 7 WebKit Commit Bot 2017-08-07 16:22:15 PDT
Comment on attachment 317482 [details]
Patch

Clearing flags on attachment: 317482

Committed r220366: <http://trac.webkit.org/changeset/220366>
Comment 8 WebKit Commit Bot 2017-08-07 16:22:16 PDT
All reviewed patches have been landed.  Closing bug.
Comment 9 Radar WebKit Bug Importer 2017-08-07 16:23:18 PDT
<rdar://problem/33764771>