Bug 216564

Summary:

Remove runtime setting for enabling/disabling the fetch keepalive attribute

Product:

WebKit

Reporter:

Sam Weinig <sam>

Component:

WebCore Misc.

Assignee:

Sam Weinig <sam>

Status:

RESOLVED DUPLICATE

Severity:

Normal

CC:

achristensen, annevk, cdumez, esprehn+autocc, ews-watchlist, kondapallykalyan, webkit-bug-importer, webkit-unassigned, youennf

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

Bug Blocks:

216512

Attachments:

Description	Flags
Patch	sam: review-

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