Bug 175401

Summary: Disable Beacon API on WK1 DRT and WK2 when not using NETWORK_SESSION
Product: WebKit Reporter: Chris Dumez <cdumez>
Component: WebCore Misc.Assignee: Chris Dumez <cdumez>
Status: RESOLVED FIXED    
Severity: Normal CC: beidson, buildbot, commit-queue, ggaren, rniwa, sam, webkit-bug-importer, youennf
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 147885    
Attachments:
Description Flags
Patch
none
Archive of layout-test-results from ews101 for mac-elcapitan
none
Archive of layout-test-results from ews107 for mac-elcapitan-wk2
none
Archive of layout-test-results from ews116 for mac-elcapitan
none
Patch
none
Patch none

Chris Dumez
Reported 2017-08-09 16:07:22 PDT
Disable Beacon API on WK1 DRT and WK2 when not using NETWORK_SESSION.
Attachments
Patch (12.36 KB, patch)
2017-08-09 16:22 PDT, Chris Dumez 		no flags
DetailsFormatted DiffDiff
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
DetailsFormatted DiffDiff
Patch (111.81 KB, patch)
2017-08-09 19:46 PDT, Chris Dumez 		no flags
DetailsFormatted DiffDiff
Show Obsolete (5) View All Add attachment proposed patch, testcase, etc.
Chris Dumez
Comment 1 2017-08-09 16:22:57 PDT
Created attachment 317756 [details] Patch
Build Bot
Comment 2 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
Build Bot
Comment 3 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
Build Bot
Comment 4 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
Build Bot
Comment 5 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
Build Bot
Comment 6 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
Build Bot
Comment 7 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
Chris Dumez
Comment 8 2017-08-09 18:48:31 PDT
Created attachment 317772 [details] Patch
Chris Dumez
Comment 9 2017-08-09 19:46:33 PDT
Created attachment 317776 [details] Patch
Brady Eidson
Comment 10 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.
Chris Dumez
Comment 11 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.
Brady Eidson
Comment 12 2017-08-09 20:16:30 PDT
Comment on attachment 317776 [details] Patch Makes sense. r+ assuming bots are happy.
WebKit Commit Bot
Comment 13 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>
WebKit Commit Bot
Comment 14 2017-08-09 21:19:39 PDT
All reviewed patches have been landed. Closing bug.
Radar WebKit Bug Importer
Comment 15 2017-08-09 21:20:33 PDT
<rdar://problem/33820090>
Sam Weinig
Comment 16 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?
Chris Dumez
Comment 17 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.
Sam Weinig
Comment 18 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.
Chris Dumez
Comment 19 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.
Sam Weinig
Comment 20 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).
Geoffrey Garen
Comment 21 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?
Sam Weinig
Comment 22 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.
Note You need to log in before you can comment on or make changes to this bug.