1252860 - Disable appcache with e10s

Status: RESOLVED WONTFIX

(Firefox :: General, defect)

Description

[Jim Mathies [:jimm]]

appcache is going away [1], although all the code has yet to be removed. According to bug 621158 appcache support under e10s is untested and most likely broken in some ways.

- Bug 1237782 tracks removing appcache from the repo (filed Jan 2016).
- A pref to turn appcache off was added in bug 1237783.
- The replacement (Service Workers) is now on release channel.

I'd like to propose we flip the disable appcache pref when e10s is turned on such that appcache is disabled under e10s.

[1] https://www.fxsitecompat.com/en-CA/docs/2016/application-cache-support-will-be-removed/

Comment 1

[Mike Taylor [:miketaylr]]

Do we have a plan for what happens with sites calling methods on window.applicationCache? i.e., window.applicationCache.updateCache() will explode once window.applicationCache disappears.

I'm looking over results from top ~25K Alexa pages with JS inlined and it looks like vast majority are doing some kind of object existence checking or coercing to a boolean like Modernizr (!!window.applicationCache).

But code like this is trying to do the right thing, but will still throw a TypeError:

var M = window.applicationCache;
if (M && M.status !== window.applicationCache.UNCACHED) {

(from http://m.tfstatic.com/v-336/build/require/built.js, included in lafourchette.com)

Comment 2

[Mike Taylor [:miketaylr]]

(oops, link to results, if anyone wants to look at it: https://gist.github.com/miketaylr/17dd8d2b9ea73e73bbfb)

Comment 3

[:Gijs (out for now; he/him)]

(In reply to Mike Taylor [:miketaylr] from comment #1)
> Do we have a plan for what happens with sites calling methods on
> window.applicationCache? i.e., window.applicationCache.updateCache() will
> explode once window.applicationCache disappears.
> 
> I'm looking over results from top ~25K Alexa pages with JS inlined and it
> looks like vast majority are doing some kind of object existence checking or
> coercing to a boolean like Modernizr (!!window.applicationCache).
> 
> But code like this is trying to do the right thing, but will still throw a
> TypeError:
> 
> var M = window.applicationCache;
> if (M && M.status !== window.applicationCache.UNCACHED) {

What am I missing here? M is going to be null/undefined, so the evaluation of the if() condition isn't going to actually try to get values for M.status or window.applicationCache.UNCACHED, right? precedence means that this gets evaluated as:

A && (B !== C)

rather than

(A && B) !== C

for which you'd need to add braces.

Comment 4

[Mike Taylor [:miketaylr]]

Nah, you're right Gijs -- I overlooked that. I think we should still possibly consider code that isn't as safe, let me look more closely at the results I have (and probably look at a larger dataset).

Comment 5

[Jeff Griffiths (:canuckistani) (:⚡︎)]

From a side conversation with Ehsan:

<ehsan> whatever we do needs to happen in coordination with other browsers, the evangelism folks and a study of what properties use it and what happens if we disable it
<ehsan> this can break real websites

We just deprecated appcache and usage is still high enough that your proposal ( assuming we shipped e10s over 2-3 cycles to 100% of desktop ) would cause real compat heartburn. As well, we're creating an odd wrinkle to Browser/Version compat where compat is very different between two modes of the same version of the same browser. I think we should avoid that unless absolutely necessary.

I'm a wontfix.

Comment 6

[Jim Mathies [:jimm]]

We're going to remove app cache now. Bug 1237782 tracks the few things we need to do. There's probably not much we need to do here, I'm guessing bug 1237782 will cover e10s as well.

Comment 7

[:Gijs (out for now; he/him)]

(In reply to Jim Mathies [:jimm] from comment #6)
> We're going to remove app cache now. Bug 1237782 tracks the few things we
> need to do. There's probably not much we need to do here, I'm guessing bug
> 1237782 will cover e10s as well.

Do we still want to uplift bug 621158 and the dep to aurora for the initial e10s release? I'm assuming the removal of appcache won't be uplifted.

Comment 8

[Jim Mathies [:jimm]]

(In reply to :Gijs Kruitbosch from comment #7)
> (In reply to Jim Mathies [:jimm] from comment #6)
> > We're going to remove app cache now. Bug 1237782 tracks the few things we
> > need to do. There's probably not much we need to do here, I'm guessing bug
> > 1237782 will cover e10s as well.
> 
> Do we still want to uplift bug 621158 and the dep to aurora for the initial
> e10s release? I'm assuming the removal of appcache won't be uplifted.

I'd suggest uplifting that work to 47 since we know we'll (at least) be doing some experimentation in 47 on beta. I can't say at this time if 47 will be hold the initial e10s rollout, this appcache thing may hold that to 48. I'll bring this up on the e10s rollout weekly that happens today.

Comment 9

[Jim Mathies [:jimm]]

wonfixing again, appcache is going to stick around for a bit based on an email discussion.