Bug 216564 - Remove runtime setting for enabling/disabling the fetch keepalive attribute
Summary: Remove runtime setting for enabling/disabling the fetch keepalive attribute
Status: RESOLVED DUPLICATE of bug 264154
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Sam Weinig
URL:
Keywords: InRadar
Depends on:
Blocks: 216512
  Show dependency tree / graph
 
Reported: 2020-09-15 09:42 PDT by Sam Weinig
Modified: 2023-11-22 01:08 PST (History)
9 users (show)

See Also:

Attachments
Patch (24.49 KB, patch)
2020-09-17 09:52 PDT, Sam Weinig 		sam: review-
Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Sam Weinig 2020-09-15 09:42:08 PDT 
Remove runtime setting for enabling/disabling the Fetch keepalive attribute API. It should always be enabled at this point.
Comment 1 Sam Weinig 2020-09-15 09:44:54 PDT 
Looks like this was put behind the flag about 3 years ago in https://trac.webkit.org/changeset/227079/webkit, and then turned on by default 17 months ago, https://trac.webkit.org/changeset/244931/webkit.
Comment 2 Sam Weinig 2020-09-17 09:52:35 PDT 
Created attachment 409046 [details]
Patch
Comment 3 youenn fablet 2020-09-17 09:55:46 PDT 
Comment on attachment 409046 [details]
Patch

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

> Source/WebKitLegacy/mac/ChangeLog:9
> +        on by default and there is no known reason one would need to disable it.

Isn’’t it off by default in wk1
Comment 4 Sam Weinig 2020-09-17 10:01:37 PDT 
(In reply to youenn fablet from comment #3)
> Comment on attachment 409046 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=409046&action=review
> 
> > Source/WebKitLegacy/mac/ChangeLog:9
> > +        on by default and there is no known reason one would need to disable it.
> 
> Isn’’t it off by default in wk1

It looks like it was, yes. Do you think turning it on will be a problem?
Comment 5 Sam Weinig 2020-09-17 16:01:58 PDT 
Youenn, is your concern with the ChangeLog comment being inaccurate or do you think we should not do this?
Comment 6 youenn fablet 2020-09-18 00:15:00 PDT 
(In reply to Sam Weinig from comment #5)
> Youenn, is your concern with the ChangeLog comment being inaccurate or do
> you think we should not do this?

Mostly the latter.
We only made sure to implement fetch keepAlive correctly for WK2.
See for instance LayoutTests/platform/mac-wk1/imported/w3c/web-platform-tests/fetch/api/basic/keepalive-expected.txt vs. LayoutTests/imported/w3c/web-platform-tests/fetch/api/basic/keepalive-expected.txt.
Comment 7 Sam Weinig 2020-09-18 07:59:11 PDT 
(In reply to youenn fablet from comment #6)
> (In reply to Sam Weinig from comment #5)
> > Youenn, is your concern with the ChangeLog comment being inaccurate or do
> > you think we should not do this?
> 
> Mostly the latter.
> We only made sure to implement fetch keepAlive correctly for WK2.
> See for instance
> LayoutTests/platform/mac-wk1/imported/w3c/web-platform-tests/fetch/api/basic/
> keepalive-expected.txt vs.
> LayoutTests/imported/w3c/web-platform-tests/fetch/api/basic/keepalive-
> expected.txt.

Eek. That definitely seems like a reason not to do this. 

Pretty big bummer that we are not implementing these things for both WebKit1 and WebKit2. Seems like that is likely to create a more complicated testing matrix long term.

Do you know what work remains to make keepalive work with WebKit1?
Comment 8 Radar WebKit Bug Importer 2020-09-22 09:43:13 PDT 
<rdar://problem/69375709>
Comment 9 Anne van Kesteren 2023-11-22 01:08:45 PST 
I ended up removing the preference in bug 264154. As far as I can tell the preference doesn't end up guarding WebKitLegacy support. It just isn't implemented in WebKitLegacy's networking code.

*** This bug has been marked as a duplicate of bug 264154 ***