Bug 175401 - Disable Beacon API on WK1 DRT and WK2 when not using NETWORK_SESSION
Summary: Disable Beacon API on WK1 DRT and WK2 when not using NETWORK_SESSION
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Chris Dumez
URL:
Keywords: InRadar
Depends on:
Blocks: 147885
  Show dependency treegraph
 
Reported: 2017-08-09 16:07 PDT by Chris Dumez
Modified: 2017-08-22 10:41 PDT (History)
8 users (show)

See Also:


Attachments
Patch (12.36 KB, patch)
2017-08-09 16:22 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews101 for mac-elcapitan (1.07 MB, application/zip)
2017-08-09 17:10 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews107 for mac-elcapitan-wk2 (1.29 MB, application/zip)
2017-08-09 17:16 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews116 for mac-elcapitan (1.76 MB, application/zip)
2017-08-09 17:52 PDT, Build Bot
no flags Details
Patch (111.71 KB, patch)
2017-08-09 18:48 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (111.81 KB, patch)
2017-08-09 19:46 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Dumez 2017-08-09 16:07:22 PDT
Disable Beacon API on WK1 DRT and WK2 when not using NETWORK_SESSION.
Comment 1 Chris Dumez 2017-08-09 16:22:57 PDT
Created attachment 317756 [details]
Patch
Comment 2 Build Bot 2017-08-09 17:10:23 PDT
Comment on attachment 317756 [details]
Patch

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

New failing tests:
fast/dom/navigator-detached-no-crash.html
imported/w3c/web-platform-tests/url/failure.html
Comment 3 Build Bot 2017-08-09 17:10:24 PDT
Created attachment 317764 [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 4 Build Bot 2017-08-09 17:15:59 PDT
Comment on attachment 317756 [details]
Patch

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

New failing tests:
fast/dom/navigator-detached-no-crash.html
imported/w3c/web-platform-tests/url/failure.html
Comment 5 Build Bot 2017-08-09 17:16:00 PDT
Created attachment 317765 [details]
Archive of layout-test-results from ews107 for mac-elcapitan-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107  Port: mac-elcapitan-wk2  Platform: Mac OS X 10.11.6
Comment 6 Build Bot 2017-08-09 17:52:46 PDT
Comment on attachment 317756 [details]
Patch

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

New failing tests:
fast/dom/navigator-detached-no-crash.html
Comment 7 Build Bot 2017-08-09 17:52:48 PDT
Created attachment 317768 [details]
Archive of layout-test-results from ews116 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews116  Port: mac-elcapitan  Platform: Mac OS X 10.11.6
Comment 8 Chris Dumez 2017-08-09 18:48:31 PDT
Created attachment 317772 [details]
Patch
Comment 9 Chris Dumez 2017-08-09 19:46:33 PDT
Created attachment 317776 [details]
Patch
Comment 10 Brady Eidson 2017-08-09 19:51:02 PDT
Comment on attachment 317776 [details]
Patch

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

> LayoutTests/platform/win/TestExpectations:4054
> +# Beacon is not supported on WK1.
> +http/tests/blink/sendbeacon/ [ Skip ]
> +http/wpt/beacon/ [ Skip ]
> +imported/blink/fast/beacon/ [ Skip ]
> +imported/w3c/web-platform-tests/beacon/ [ Skip ]

Instead of checking in the verbose failing results for windows wk1, should also just Skip them.
Comment 11 Chris Dumez 2017-08-09 20:07:48 PDT
(In reply to Brady Eidson from comment #10)
> Comment on attachment 317776 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=317776&action=review
> 
> > LayoutTests/platform/win/TestExpectations:4054
> > +# Beacon is not supported on WK1.
> > +http/tests/blink/sendbeacon/ [ Skip ]
> > +http/wpt/beacon/ [ Skip ]
> > +imported/blink/fast/beacon/ [ Skip ]
> > +imported/w3c/web-platform-tests/beacon/ [ Skip ]
> 
> Instead of checking in the verbose failing results for windows wk1, should
> also just Skip them.

The reason I did not skip them is that they are not beacon tests, they test other things.
Comment 12 Brady Eidson 2017-08-09 20:16:30 PDT
Comment on attachment 317776 [details]
Patch

Makes sense.  r+ assuming bots are happy.
Comment 13 WebKit Commit Bot 2017-08-09 21:19:38 PDT
Comment on attachment 317776 [details]
Patch

Clearing flags on attachment: 317776

Committed r220507: <http://trac.webkit.org/changeset/220507>
Comment 14 WebKit Commit Bot 2017-08-09 21:19:39 PDT
All reviewed patches have been landed.  Closing bug.
Comment 15 Radar WebKit Bug Importer 2017-08-09 21:20:33 PDT
<rdar://problem/33820090>
Comment 16 Sam Weinig 2017-08-10 17:08:05 PDT
Is this a temporary measure, or is the idea that Beacon won't be supported in those configurations going forward?
Comment 17 Chris Dumez 2017-08-10 18:24:30 PDT
(In reply to Sam Weinig from comment #16)
> Is this a temporary measure, or is the idea that Beacon won't be supported
> in those configurations going forward?

Probably will not be supported on these configurations.
Comment 18 Sam Weinig 2017-08-21 16:11:58 PDT
(In reply to Chris Dumez from comment #17)
> (In reply to Sam Weinig from comment #16)
> > Is this a temporary measure, or is the idea that Beacon won't be supported
> > in those configurations going forward?
> 
> Probably will not be supported on these configurations.

Bummer.
Comment 19 Chris Dumez 2017-08-21 16:14:58 PDT
(In reply to Sam Weinig from comment #18)
> (In reply to Chris Dumez from comment #17)
> > (In reply to Sam Weinig from comment #16)
> > > Is this a temporary measure, or is the idea that Beacon won't be supported
> > > in those configurations going forward?
> > 
> > Probably will not be supported on these configurations.
> 
> Bummer.

If WK1 gets ported to NETWORK_SESSION, we can reconsider.
Comment 20 Sam Weinig 2017-08-21 16:59:19 PDT
That seems like a tall bar, and one that seems unlikely given the use of NSURLProtocol with Legacy WebKit clients.  It seems unfortunate to bifurcate the feature set. And something that shouldn't be done with consideration, we have been bitten by it before (see IndexedDB).
Comment 21 Geoffrey Garen 2017-08-22 10:15:38 PDT
(In reply to Sam Weinig from comment #20)
> we have been bitten by it before (see IndexedDB).

What happened with IndexedDB?
Comment 22 Sam Weinig 2017-08-22 10:41:41 PDT
(In reply to Geoffrey Garen from comment #21)
> (In reply to Sam Weinig from comment #20)
> > we have been bitten by it before (see IndexedDB).
> 
> What happened with IndexedDB?

People got annoyed / confused that they couldn't use it in UIWebView. 

The philosophy I was trying to champion for the last few years was that the web platform we expose should be as uniform, web-exposed feature wise, as possible to create a coherent story for developers.