Closed
Bug 830588
Opened 13 years ago
Closed 10 years ago
Use VALIDATE_ALWAYS for offline cache manifest loads
Categories
(Core :: Networking: Cache, defect)
Core
Networking: Cache
Tracking
()
RESOLVED
WONTFIX
People
(Reporter: mayhemer, Assigned: mayhemer)
References
Details
Attachments
(1 file, 1 obsolete file)
|
8.82 KB,
patch
|
jduell.mcbugs
:
feedback-
|
Details | Diff | Splinter Review |
We require, as specified by WHATWG, to set Cache-control: no-cache or ETag on app cache manifests. If none of that provided, we just count expiration time that usually prevents request to the server to check on manifest modification.
There are bugs that web developer get stuck on "noupdate" even they have modified the manifest. I always have closed those bugs as invalid, since we were behaving according the spec. But I've ignored the bad user experience.
Chrome (and I believe other browsers too) ignore this part of the spec and do revalidation always.
We should do the same to save troubles.
And we should consider setting the VALIDATE_ALWAYS flag on all explicit and fallback entries as well.
Some bug deps added, but there are IMO more.
| Assignee | ||
Comment 2•13 years ago
|
||
Just a one line patch that makes many many people's lives very much easier!
OpenChannel affect manifest and resource as well.
Assignee: nobody → honzab.moz
Status: NEW → ASSIGNED
Attachment #706660 -
Flags: review?(jduell.mcbugs)
Updated•13 years ago
|
Attachment #706660 -
Flags: review?(jduell.mcbugs) → review+
Comment 4•13 years ago
|
||
Could you ask for approval for landing in b2g18 branch after review ?
| Assignee | ||
Comment 5•13 years ago
|
||
- removed test_changingManifest.html that checks exactly our conformation to the spec in this matter (what we are not with this patch)
- left the changingManifest.sjs and its portions since it is used by another test
Attachment #706660 -
Attachment is obsolete: true
Attachment #706681 -
Flags: review?(jduell.mcbugs)
Comment 6•13 years ago
|
||
(In reply to Honza Bambas (:mayhemer) from comment #5)
> - removed test_changingManifest.html that checks exactly our conformation to
> the spec in this matter (what we are not with this patch)
Honza, I think it is the case that we are still in conformance with the spec with this patch, because the spec just requires us to honor HTTP caching semantics, and HTTP caching semantics allow us to revalidate at any time.
However, I don't understand why we're doing this only for AppCache and not for all HTTP requests. Why should requests for AppCache resources be cached less aggressively than non-AppCache resources?
This change would mean that the common pattern of using query parameters for versioning to avoid unnecessary revalidations would no longer work in Firefox's AppCache implementation.
I am a concerned that we're going to make developers dependent on us always revalidating, and then having their apps break in other browsers that use normal HTTP caching semantics. Are other browsers forcing a revalidation here? Have we asked for the spec. to be changed to require revalidation? This could also lead us to backward-incompatible changes in the future, if we decide that we need to cache more aggressively to get performance parity with other implementations, if this isn't incorporated into the spec. (And, to be clear, I don't think it *should* be incorporated into the spec.)
Also, this change is inconsistent with other work that we've done, or are planning to do, about caching more aggressively. For example, I know we have a bug (perhaps already fixed?) about doing more aggressive heuristic caching. (IMO, heuristic caching is *much* more error-prone than the problem that this patch is trying to fix.) I'm not sure there's agreement about changing course to cache things less aggressively to wallpaper over problems with misconfigured their servers while hurting people that are purposefully optimizing their sites.
This approach is great UX for application developers who are refreshing their apps frequently, but it is bad for performance (and potentially bandwidth usage) for all users of those apps. That seems like a bad tradeoff. IMO, we should instead improve the developer tools for offline apps.
Comment 7•13 years ago
|
||
Another question: Both duplicates are about problems with *heuristic* caching, but this patch affects even applications that send explicit Cache-Control headers. I would be more sympathetic about this patch if it simply disabled heuristic caching while still honoring explicit Cache-Control directives. But, even then, I think it is a mistake to treat AppCache requests differently than other HTTP requests; it just makes things more convenient.
If we thing this is a real concern, then why don't we just log a warning to the web console informing the developer that we're doing heuristic caching because no Cache-Control directives were received? That would help resolve confusion not just for developers working on AppCache-based apps, but also developers working on many other websites.
Comment 8•13 years ago
|
||
(In reply to Brian Smith (:bsmith) from comment #7)
> Another question: Both duplicates are about problems with *heuristic*
> caching, but this patch affects even applications that send explicit
> Cache-Control headers. I would be more sympathetic about this patch if it
> simply disabled heuristic caching while still honoring explicit
> Cache-Control directives. But, even then, I think it is a mistake to treat
> AppCache requests differently than other HTTP requests; it just makes things
> more convenient.
s/convenient/confusing/.
Comment 9•13 years ago
|
||
Comment on attachment 706681 [details] [diff] [review]
v1.1
Review of attachment 706681 [details] [diff] [review]:
-----------------------------------------------------------------
> We require, as specified by WHATWG, to set Cache-control: no-cache or ETag on
> app cache manifests. If none of that provided, we just count expiration time
> that usually prevents request to the server to check on manifest modification.
Confession: I read the bug title and this part of the description and concluded that this was about always revalidating the load of the *manifest*, not the resources pointed to by the manifest. So I +r'd it w/o careful enough review.
Now I see that this patch always validating the item's listed in the cache (do we always revalidate the manifest too? I'd think that's probably a good idea).
I don't see any mention in the WHATWG spec about requiring Cache-control: no-cache or ETag on app cache manifests. Can someone point me at it?
http://www.whatwg.org/specs/web-apps/current-work/#applicationcache
> I would be more sympathetic about this patch if it simply disabled heuristic
> caching while still honoring explicit Cache-Control directives.
I think I agree with Brian on this. For apps with a lot of resources (a wikipedia app, twitter, etc) it seems to make sense to allow explicit caching control to avoid tons of pointless revalidation traffic. But I support getting rid of heuristic caching, so we stop torturing developers who forget how that works. Honza, would it be hard to write code to do that?
Attachment #706681 -
Flags: review?(jduell.mcbugs) → feedback-
Comment 10•13 years ago
|
||
(In reply to Jason Duell (:jduell) from comment #9)
> > I would be more sympathetic about this patch if it simply disabled heuristic
> > caching while still honoring explicit Cache-Control directives.
>
> I think I agree with Brian on this. For apps with a lot of resources (a
> wikipedia app, twitter, etc) it seems to make sense to allow explicit
> caching control to avoid tons of pointless revalidation traffic. But I
> support getting rid of heuristic caching, so we stop torturing developers
> who forget how that works. Honza, would it be hard to write code to do that?
That was also basically what I asked for in Bug 834338 Comment 29.
I'm quite sure we still need the cache heuristics when we download the ressources from an appcache update. That's good for the Web. About always revalidating an appcache, I'd say... why not but I'm not sure. Maybe a middle path would be to always revalidate the appcache when the user press ctrl+F5, as he's used to.
However I talked with some fellows today during Firefox OS App Days and maybe we only really need better logs and better tools. The real problem here is that the developer doesn't know what happens, so we should tell him. Like "The application cache manifest has not expired (Expiration Date is XXX), so we fetch it from our cache".
All appcache best practices ask developers to disable cache for appcache manifests through an Apache config (ExpiresByType text/cache-manifest "access") already so adding a log would be a small step towards developer education.
Another possibility is to make it really apparent in our tools: Firebug and Integrated devtools. Apparent when we're getting the appcache or when it comes from the cache (Firebug does this for normal ressources), and apparent when the ressources come from the application cache too. Right now we see nothing in either tool.
So, here are my current thoughts. What are yours ?
Comment 11•13 years ago
|
||
I filed bug 835079 about adding the warnings to the web/error consoles.
(In reply to Honza Bambas (:mayhemer) from comment #0)
> Chrome (and I believe other browsers too) ignore this part of the spec and
> do revalidation always.
Do you think we should change the spec to say that we must always act like the cache manifest was served with no-cache, and that we must always send "Cache-control: no-cache / Pragma: no-cache" when validating the cache manifest? If so, and if other implementations are already doing that, then I'm willing to go with the flow. In that case, let's file the bug against the spec.
But, I would refer if AppCache minimized the number of special cases it adds for HTTP as much as possible (the redirect rules are bad enough on their own). So, I would prefer to WONTFIX this.
Either way, I think bug 835079 is more important to fix than this.
Comment 12•13 years ago
|
||
I've filed http://code.google.com/p/fbug/issues/detail?id=6221 against firebug and Bug 835274 against Firefox Dev Tools.
| Assignee | ||
Comment 13•13 years ago
|
||
We all agree something needs to be done here. I propose to do something relatively quickly.
This bug is trying to solve appcache problem of type "why doesn't my content update?". There were quit a few bugs recently those took significant amount of time to several people to figure out just "missing header on one's server".
I really want to avoid such INVALID bugs.
I checked with local site sending no ETag but Last-Modified set to some 1995 that Chrome revalidates manifest *and* resources.
IE9 doesn't know what appcache is.
We use VALIDATE_ALWAYS on F5 (normal) refresh, only. Going to direct URL and clicking a link with default preferences will load from cache for my testing case.
Re logging: definitely better logging is a must. I've introduced some logging [1], but apparently not complete. Also, the better logging must w/o problems work on all end-software we have: desktop, mobile, b2g.
The idea of disabling heuristic is what we should do here. Honoring Cache-control: max-age and similar should be preserved, here I agree with Brian and Julien. I was somehow persuaded VALIDATE_ALWAYS is doing exactly this job, but it isn't.
(In reply to Brian Smith (:bsmith) from comment #6)
> However, I don't understand why we're doing this only for AppCache and not
> for all HTTP requests. Why should requests for AppCache resources be cached
> less aggressively than non-AppCache resources?
Since appcache updates only occasionally but has to be up to date at the moment the manifest has updated. This is my opinion based also on what problems I see people experience with app cache.
> This change would mean that the common pattern of using query parameters for
> versioning to avoid unnecessary revalidations would no longer work in
> Firefox's AppCache implementation.
I thought queried requests validate always, maybe I was wrong.
> I am a concerned that we're going to make developers dependent on us always
> revalidating,
Apparently, now many developers don't know this and are blaming Fx from being buggy since Chrome works (at least, didn't check Safari).
> and then having their apps break in other browsers that use
> normal HTTP caching semantics. Are other browsers forcing a revalidation
> here?
Yes.
> Have we asked for the spec. to be changed to require revalidation?
No, but I plan to propose this.
> This could also lead us to backward-incompatible changes in the future, if
> we decide that we need to cache more aggressively to get performance parity
> with other implementations, if this isn't incorporated into the spec. (And,
> to be clear, I don't think it *should* be incorporated into the spec.)
My opinion is it should. Appcache loads are not daily/hourly common visit loads. Appcache loads are occasional, that is one of attributes and purposes of appcache. We can effort to revalidate, IMO.
> Also, this change is inconsistent with other work that we've done, or are
> planning to do, about caching more aggressively. For example, I know we have
> a bug (perhaps already fixed?) about doing more aggressive heuristic
> caching. (IMO, heuristic caching is *much* more error-prone than the problem
> that this patch is trying to fix.)
I may need a bit more explanation here.
> I'm not sure there's agreement about
> changing course to cache things less aggressively to wallpaper over problems
> with misconfigured their servers while hurting people that are purposefully
> optimizing their sites.
It seems like you talk about optimizing site for "normal" common daily load. Optimizing for appcache is probably a bit different.
> This approach is great UX for application developers who are refreshing
> their apps frequently, but it is bad for performance (and potentially
> bandwidth usage) for all users of those apps. That seems like a bad
> tradeoff. IMO, we should instead improve the developer tools for offline
> apps.
You want to prevent server requests for manifests that don't change frequently? What about a security update after a long time of an app not being modified?
(In reply to Jason Duell (:jduell) from comment #9)
> > I would be more sympathetic about this patch if it simply disabled heuristic
> > caching while still honoring explicit Cache-Control directives.
>
> I think I agree with Brian on this. For apps with a lot of resources (a
> wikipedia app, twitter, etc) it seems to make sense to allow explicit
> caching control to avoid tons of pointless revalidation traffic. But I
> support getting rid of heuristic caching, so we stop torturing developers
> who forget how that works. Honza, would it be hard to write code to do that?
I think we have to introduce a new flag (yet we may need to go 64 bit!). It shouldn't be that hard to do IMO.
[1] http://www.janbambas.cz/offline-cache-update-console-logs-introduced-in-firefox-19/
Comment 14•13 years ago
|
||
(In reply to Honza Bambas (:mayhemer) from comment #13)
> We all agree something needs to be done here. I propose to do something
> relatively quickly.
>
> This bug is trying to solve appcache problem of type "why doesn't my content
> update?". There were quit a few bugs recently those took significant amount
> of time to several people to figure out just "missing header on one's
> server".
For Firefox OS at least I added some doc in [1].
>
> I really want to avoid such INVALID bugs.
>
> I checked with local site sending no ETag but Last-Modified set to some 1995
> that Chrome revalidates manifest *and* resources.
>
> IE9 doesn't know what appcache is.
>
> We use VALIDATE_ALWAYS on F5 (normal) refresh, only. Going to direct URL
> and clicking a link with default preferences will load from cache for my
> testing case.
I just checked and it seems (Firefox Aurora 20 here) that we revalidate the appcache when going to direct URL too. The browser in Firefox OS behaves the same.
In the bug I filed for Firefox OS (Bug 834285), we didn't revalidate the appcache when checking for update. I find this very strange that we have 2 different behaviours here and maybe we should reopen Bug 834285.
About the other behaviors, I let you decide :)
[1] https://developer.mozilla.org/en-US/docs/Mozilla/Firefox_OS/Apps/Tips_and_techniques#My_app_doesn%27t_update_even_though_I_have_an_appcache_manifest
Comment 15•13 years ago
|
||
(In reply to Julien Wajsberg [:julienw] from comment #14)
> I just checked and it seems (Firefox Aurora 20 here) that we revalidate the
> appcache when going to direct URL too. The browser in Firefox OS behaves the
> same.
I was testing with a website without any Cache headers, but with ETag and Last-Modified. It is in http://everlong.org/mozilla/appcache/ if you want to check yourself.
| Assignee | ||
Comment 16•13 years ago
|
||
Chrome seems to ignore max-age when requesting resources and the manifest.
There is no difference in Gecko in how we load app cache regarding you go directly to URL or refresh the page.
Comment 17•13 years ago
|
||
ok, but there is a difference when checking updates in B2G.
| Assignee | ||
Comment 18•13 years ago
|
||
Actually it looks like Chrome (26) happily ignores max-age even for normal loads (direct, refresh).
| Assignee | ||
Comment 19•13 years ago
|
||
(In reply to Julien Wajsberg [:julienw] from comment #17)
> ok, but there is a difference when checking updates in B2G.
How do you recognize it exactly?
AFAIK, b2g is using the same C++ implemented API to check for an update and then, if avail, do an update. There is no way to alter the load flags.
Comment 20•13 years ago
|
||
I check my server logs.
That's what I was explaining in Bug 834285. I can do further tests tomorrow.
Comment 21•13 years ago
|
||
(In reply to Honza Bambas (:mayhemer) from comment #13)
> I checked with local site sending no ETag but Last-Modified set to some 1995
> that Chrome revalidates manifest *and* resources.
>
> IE9 doesn't know what appcache is.
FWIW, IE10 does know what AppCache is, and so does Safari.
> (In reply to Brian Smith (:bsmith) from comment #6)
> > This change would mean that the common pattern of using query parameters for
> > versioning to avoid unnecessary revalidations would no longer work in
> > Firefox's AppCache implementation.
>
> I thought queried requests validate always, maybe I was wrong.
See bug 718797. What *should* happen now when there is a query string (before fixing bug 718797) is that we stop doing heuristic caching, but still cache (without revalidation) if the server gives some explicit indication that the resource is cacheable.
> > I am a concerned that we're going to make developers dependent on us always
> > revalidating,
>
> Apparently, now many developers don't know this and are blaming Fx from
> being buggy since Chrome works (at least, didn't check Safari).
I know that, in the *non-AppCache* case, at least, developers complain that Chrome is caching too aggressively.
Heuristic caching is a big footgun. I am open to the idea that we should be more conservative about doing it in general. But, like I said, that's the opposite of what we seemed to be converging to in bug 718797.
> > and then having their apps break in other browsers that use
> > normal HTTP caching semantics. Are other browsers forcing a revalidation
> > here?
>
> Yes.
>
> > Have we asked for the spec. to be changed to require revalidation?
>
> No, but I plan to propose this.
>
I think we should check what IE10 and Safari are doing. I will check the IE10 case as I have it installed. Can somebody else check Safari?
> My opinion is it should. Appcache loads are not daily/hourly common visit
> loads. Appcache loads are occasional, that is one of attributes and
> purposes of appcache. We can effort to revalidate, IMO.
OK. I understand that your point is that, since this revalidation isn't blocking the user in any way, that there is no real negative performance impact to being more conservative. I buy that.
> > Also, this change is inconsistent with other work that we've done, or are
> > planning to do, about caching more aggressively. For example, I know we have
> > a bug (perhaps already fixed?) about doing more aggressive heuristic
> > caching. (IMO, heuristic caching is *much* more error-prone than the problem
> > that this patch is trying to fix.)
>
> I may need a bit more explanation here.
I was referring to bug 718797.
> You want to prevent server requests for manifests that don't change
> frequently? What about a security update after a long time of an app not
> being modified?
AppCache isn't a security mechanism, and non-AppCache apps have the same problems (or worse).
> I think we have to introduce a new flag (yet we may need to go 64 bit!). It
> shouldn't be that hard to do IMO.
I agree that it seems to make the most sense to add some kind of NO_HUERISTIC_FRESHNESS flag.
However, note that this is a very common progression for building an app:
1. Write the app without AppCache.
2. Add the AppCache manifest once the app is working.
The developer will already run into the problems with heuristic caching during the first phase. I guess many must work around it by shift-reloading because they don't understand WTF is happening. That's why I think the logging to the web console is the "real" fix.
Comment 22•13 years ago
|
||
(In reply to Brian Smith (:bsmith) from comment #21)
>
> > My opinion is it should. Appcache loads are not daily/hourly common visit
> > loads. Appcache loads are occasional, that is one of attributes and
> > purposes of appcache. We can effort to revalidate, IMO.
>
> OK. I understand that your point is that, since this revalidation isn't
> blocking the user in any way, that there is no real negative performance
> impact to being more conservative. I buy that.
And don't forget everything is done in background so this doesn't even block a page from loading.
> However, note that this is a very common progression for building an app:
>
> 1. Write the app without AppCache.
> 2. Add the AppCache manifest once the app is working.
>
> The developer will already run into the problems with heuristic caching
> during the first phase. I guess many must work around it by shift-reloading
> because they don't understand WTF is happening. That's why I think the
> logging to the web console is the "real" fix.
A problem will also arise when the developer will want to _update_ its app.
| Assignee | ||
Comment 23•13 years ago
|
||
Update problems may arise after a web app deployment. So, too late to watch for any logs or to add headers. I don't believe logs are a full solution to this.
| Assignee | ||
Comment 24•13 years ago
|
||
(In reply to Julien Wajsberg [:julienw] from comment #14)
> I just checked and it seems (Firefox Aurora 20 here) that we revalidate the
> appcache when going to direct URL too. The browser in Firefox OS behaves the
> same.
>
> In the bug I filed for Firefox OS (Bug 834285), we didn't revalidate the
> appcache when checking for update. I find this very strange that we have 2
> different behaviours here and maybe we should reopen Bug 834285.
I don't understand. You claim FxOS browser does what exactly?
(In reply to Julien Wajsberg [:julienw] from comment #15)
> I was testing with a website without any Cache headers, but with ETag and
> Last-Modified. It is in http://everlong.org/mozilla/appcache/ if you want to
> check yourself.
Then it is no bug. It is expected to revalidate when there is an ETag on any resource, including the app cache manifest.
Comment 25•13 years ago
|
||
(In reply to Honza Bambas (:mayhemer) from comment #24)
> (In reply to Julien Wajsberg [:julienw] from comment #14)
> > I just checked and it seems (Firefox Aurora 20 here) that we revalidate the
> > appcache when going to direct URL too. The browser in Firefox OS behaves the
> > same.
> >
> > In the bug I filed for Firefox OS (Bug 834285), we didn't revalidate the
> > appcache when checking for update. I find this very strange that we have 2
> > different behaviours here and maybe we should reopen Bug 834285.
>
> I don't understand. You claim FxOS browser does what exactly?
Not the browser, but the app update mechanism.
| Assignee | ||
Comment 26•13 years ago
|
||
(In reply to Julien Wajsberg [:julienw] from comment #25)
> Not the browser, but the app update mechanism.
Ah, just being more specific in line "we didn't revalidate the appcache when checking for *OWA* update" would explain this.
I don't know exactly how the OWA update code works, but if I remember correctly we don't check for appcache udpate (whether the appcache manifest has changed) when also the OWA manifest has not changed. W/o OWA update only way to detect appcache update is to refer the manifest in you OWA main html page.
The process of OWA update is (AFAIK) pseudo-coded as:
Update()
{
if (!OWA manifest changed)
return;
Update OWA;
if (!appcache manifest changed)
return;
Update appcache;
}
However, as I checked the code and also my local testing shows, we do appcache update check when an HTML page *NOT* referring appcache manifest is navigated to. This is according the spec, the cache selection algorithm invokes appcache update when the top level doc has been loaded from (its) appcache. I'm not sure you are referring to this case.
| Assignee | ||
Comment 27•13 years ago
|
||
(In reply to Honza Bambas (:mayhemer) from comment #26)
> W/o OWA update
> only way to detect appcache update is to refer the manifest in you OWA main
> html page.
>
Ignore this line please
Comment 28•13 years ago
|
||
We changed to always check for an appcache update even if the owa manifest has not been updated, so we can update the app "content" even if no meta data has changed.
Comment 29•13 years ago
|
||
I'm reopning Bug 834285 as this is now clear to me that for some reason we're doing things different in Firefox OS than in Desktop Firefox, and we need to sort this out.
Comment 30•12 years ago
|
||
1. the file from manifest was updated. And manifest was updated.
2. Firefox loads manifest (expiration is not heuristic for this file), than loads a file from cache (because of heuristic expiration)
3. appcache contains old file.
| Assignee | ||
Updated•12 years ago
|
Attachment #706660 -
Flags: review+
Comment 32•11 years ago
|
||
I confirm that Firefox 34-38 will display stale content, even after fetching the updated manifest and content files, triggering an UPDATEREADY, and having the page reloaded.
More detailed report: https://bugzilla.mozilla.org/show_bug.cgi?id=780197#c6
Comment 33•11 years ago
|
||
This is occurring on Firefox 40 as well. It makes offline apps a risky endeavor.
| Assignee | ||
Comment 34•10 years ago
|
||
appcache is dead
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → WONTFIX
Comment 35•10 years ago
|
||
(In reply to Honza Bambas (:mayhemer) from comment #34)
> appcache is dead
It is not. It will die, but look how long it's taking flash, IE 6 and IE 8 to die. This way of handling offline apps will be the main way for at least another few years. This IS worth fixing.
Comment 36•10 years ago
|
||
@donrhummy
It at least dead in browser which support ServiceWorker, so they do not have to fix AppCache. Everyone should migrate to SW as soon as possible. AppCache is buggy thing, completely unusable unless you do very simple-static-site, which doesn't need AppCache at all, actually.
You need to log in
before you can comment on or make changes to this bug.
Description
•