Closed Bug 1252860 Opened 8 years ago Closed 8 years ago

Disable appcache with e10s

Categories

(Firefox :: General, defect)

defect
Not set
normal

Tracking

()

RESOLVED WONTFIX
Tracking Status
e10s ? ---

People

(Reporter: jimm, Unassigned)

References

Details

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/
Flags: needinfo?(jgriffiths)
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)
(oops, link to results, if anyone wants to look at it: https://gist.github.com/miketaylr/17dd8d2b9ea73e73bbfb)
(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.
Flags: needinfo?(miket)
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).
Flags: needinfo?(miket)
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.
Status: NEW → RESOLVED
Closed: 8 years ago
Flags: needinfo?(jgriffiths)
Resolution: --- → WONTFIX
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.
Status: RESOLVED → REOPENED
Depends on: 1237782
Resolution: WONTFIX → ---
(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.
Flags: needinfo?(jmathies)
(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.
Flags: needinfo?(jmathies)
wonfixing again, appcache is going to stick around for a bit based on an email discussion.
Status: REOPENED → RESOLVED
Closed: 8 years ago8 years ago
Resolution: --- → WONTFIX
Depends on: 1619673
You need to log in before you can comment on or make changes to this bug.