Bypass AppCache completely when Service Workers supported & registered

RESOLVED FIXED in Firefox 43

Status

()

RESOLVED FIXED
4 years ago
2 years ago

People

(Reporter: flaki, Assigned: baku)

Tracking

Trunk
mozilla43
Points:
---

Firefox Tracking Flags

(firefox41 affected, firefox43 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

4 years ago
Discussions on enhancing backward-compatibility of the Service Workers spec resulted in allowing AppCache & Service Workers used simultaneously, but a registered service worker script in a supporting browser should override and evict the AppCache completely.

Related spec bug: https://www.w3.org/Bugs/Public/show_bug.cgi?id=27482

Discussion: https://github.com/slightlyoff/ServiceWorker/issues/697

Chrome status (partially implemented): https://code.google.com/p/chromium/issues/detail?id=410665

Found a FixMe by :nsm, but didn't see this filed so I went ahead filing this bug: https://dxr.mozilla.org/mozilla-central/source/dom/workers/ServiceWorkerManager.cpp#
(Reporter)

Updated

4 years ago
Blocks: 1131322
(Reporter)

Updated

4 years ago
Flags: needinfo?(nsm.nikhil)
(Reporter)

Comment 1

4 years ago
(In reply to Szmozsánszky István [:flaki] from comment #0)
> Found a FixMe by :nsm, but didn't see this filed so I went ahead filing this
> bug:

Correct link is https://dxr.mozilla.org/mozilla-central/source/dom/workers/ServiceWorkerManager.cpp#1191
I don't think I'll be working on this immediately. Clearing ni?
Flags: needinfo?(nsm.nikhil)
I think we need to consider this a bit more as well.  I believe there are sites that are built on appcache today, but also want to use a service worker for push (or to get android chrome's install banner).  They can't migrate off appcache, but might have a service worker for other reasons.
I think we might need to do something here for v1.  We need to at least make sure things don't completely blow up for people using SW and appcache simultaneously.

We can move this back to v2 or v3 if people don't think its a critical problem.
Blocks: 1059784
No longer blocks: 1131322

Comment 5

4 years ago
We have telemetry on app cache usage <https://telemetry.mozilla.org/new-pipeline/dist.html#!cumulative=0&end_date=2015-08-26&keys=__none__!__none__!__none__&max_channel_version=release%252F40&measure=HTTP_OFFLINE_CACHE_DOCUMENT_LOAD&min_channel_version=release%252F39&product=Firefox&sanitize=1&sort_keys=submissions&start_date=2015-08-07&table=0&trim=1&use_submission_date=0> and on the release channel, if I'm doing the math correctly, the usage is 0.043% of page loads.

Based on that, I don't really think this should block v1, but if the fix is super simple, it's definitely nice to fix.
(Assignee)

Updated

4 years ago
Assignee: nobody → amarchesini
Status: NEW → ASSIGNED
(Assignee)

Comment 6

4 years ago
Created attachment 8658225 [details] [diff] [review]
appCache.patch
Attachment #8658225 - Flags: feedback?(nsm.nikhil)
(Assignee)

Comment 7

4 years ago
Created attachment 8658797 [details] [diff] [review]
appCache.patch
Attachment #8658225 - Attachment is obsolete: true
Attachment #8658225 - Flags: feedback?(nsm.nikhil)
Attachment #8658797 - Flags: review?(josh)
Comment on attachment 8658797 [details] [diff] [review]
appCache.patch

Review of attachment 8658797 [details] [diff] [review]:
-----------------------------------------------------------------

A test for these changes would be useful, I think.

::: dom/base/nsContentSink.cpp
@@ +1063,5 @@
>    if (!mDocShell) {
>      return;
>    }
>  
> +  // If this document has been interecepted, let's skip the processing of the

Don't bother processing offline manifest for documents with a ServiceWorker.
Attachment #8658797 - Flags: review?(josh) → review+
(Assignee)

Comment 9

4 years ago
I'll file a follow up for the test. I'm going to land this patch.

https://hg.mozilla.org/integration/mozilla-inbound/rev/982d9a863989
https://hg.mozilla.org/mozilla-central/rev/982d9a863989
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
status-firefox43: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla43
You need to log in before you can comment on or make changes to this bug.