Bug 216564 – Remove runtime setting for enabling/disabling the fetch keepalive attribute

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

[Sam Weinig]

Remove runtime setting for enabling/disabling the Fetch keepalive attribute API. It should always be enabled at this point.

[Sam Weinig]

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.

[Sam Weinig]

Created attachment 409046 [details]
Patch

[youenn fablet]

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

[Sam Weinig]

(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?

[Sam Weinig]

Youenn, is your concern with the ChangeLog comment being inaccurate or do you think we should not do this?

[youenn fablet]

(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.

[Sam Weinig]

(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?

[Radar WebKit Bug Importer]

<rdar://problem/69375709>

[Anne van Kesteren]

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 ***