Manifest fetched on install shouldn't be cached

RESOLVED FIXED in mozilla17

Status

()

Core
DOM: Apps
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: ianb, Assigned: fabrice)

Tracking

unspecified
mozilla17
Points:
---

Firefox Tracking Flags

(blocking-basecamp:+)

Details

(Whiteboard: [qa-])

Attachments

(1 attachment)

(Reporter)

Description

5 years ago
The manifests are fetched with no special attention to caching, which means Expires headers and the like can cause a manifest to be cached and make it difficult to undo (and there's nothing like a hard reload for this request).

It seems to me that we should avoid caching this particular resource.  Adding a no-cache request header should be sufficient.
Is this a mozapps api bug? If so, migrate this bug to Core --> DOM/Mozilla Extensions.

Updated

5 years ago
Component: General → DOM: Apps
Product: Web Apps → Core
True is that people have reported issues with publishing offline cache manifests too.  Since most of them omitted to add no-cache they were reporting bugs that we didn't update offline cache even though they updated (i.e. changed) their manifest on the server.  All browsers, including Gecko-based behave this way what is according the offline cache spec, though.

Based on this experience, we shouldn't repeat the same mistake for web apps and always refetch the manifest (add e.g. no-cache header artificially) if it hasn't been fetched with ETag.

Updated

5 years ago
Component: DOM: Apps → Networking: Cache

Updated

5 years ago
Component: Networking: Cache → DOM: Apps
Honza: Are you saying that we should or should not always refetch the manifest? I.e. are you recommending that we ignore cache headers?
blocking-basecamp: --- → ?
In triage we didn't have the right people in the room to make a blocking call on this. Can you clarify why this is blocking basecamp?
(In reply to Jonas Sicking (:sicking) from comment #3)
> Honza: Are you saying that we should or should not always refetch the
> manifest? I.e. are you recommending that we ignore cache headers?

We should load app manifests with nsIRequest::VALIDATE_ALWAYS flag.  This is the optimal way.  When there is an ETag header available we use it to revalidate.  When there is none, we just always refetch.
Sounds good to me. And it indeed sounds like something that could cause authors a lot of confusion and frustration.
(Assignee)

Comment 7

5 years ago
We load the manifests using an xhr request. Is setting a |Cache-control: no-cache| enough?
Assignee: nobody → fabrice
(In reply to Fabrice Desré [:fabrice] from comment #7)
> We load the manifests using an xhr request. Is setting a |Cache-control:
> no-cache| enough?

Correct is to set nsIRequest::VALIDATE_ALWAYS on its channel.
(Assignee)

Comment 9

5 years ago
Created attachment 648470 [details] [diff] [review]
patch

Setting the flag on the xhr once it is opened.
Attachment #648470 - Flags: review?(honzab.moz)
Are there also some tests for this?
(Assignee)

Comment 11

5 years ago
Not yet - I filed bug 779991 on this.
Comment on attachment 648470 [details] [diff] [review]
patch

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

Next time please with

[defaults]
diff=-U 8 -p
qdiff=-U 8

[diff]
git=true
showfunc=true
unified=8 

in your hgrc.

::: dom/apps/src/Webapps.js
@@ +125,4 @@
>      let requestID = this.getRequestId(request);
>      let xhr = Cc["@mozilla.org/xmlextras/xmlhttprequest;1"].createInstance(Ci.nsIXMLHttpRequest);
>      xhr.open("GET", aURL, true);
> +    xhr.channel.loadFlags |= Ci.nsIChannel.VALIDATE_ALWAYS;

nsIRequest.VALIDATE_ALWAYS
Attachment #648470 - Flags: review?(honzab.moz) → review+
(Assignee)

Comment 13

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/169186616cd2
blocking-basecamp: ? → +
https://hg.mozilla.org/mozilla-central/rev/169186616cd2
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla17

Updated

5 years ago
Whiteboard: [qa-]

Comment 15

5 years ago
Will this be included in FF 10esr? If not, please consider adding/backporting this. Thank you
You need to log in before you can comment on or make changes to this bug.