Closed Bug 780878 Opened 7 years ago Closed 7 years ago

[ApplicationCache] Manifest file not reloaded

Categories

(Core :: Networking: Cache, defect)

14 Branch
x86_64
Windows 7
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla18

People

(Reporter: adsi, Assigned: sinker)

References

Details

(Keywords: regression, testcase)

Attachments

(3 files)

User Agent: Opera/9.80 (Windows NT 6.1; WOW64; U; en) Presto/2.10.289 Version/12.01

Steps to reproduce:

Hello,

I access a webpage where the attribute manifest is specified on the "html" tag using Firefox 14/15/16.


Actual results:

The first time I go on the webpage, the manifest file is correctly read by Firefox and the content is downloaded.

Then, If I go on the same page again or another webpage specifying the same manifest file, Firefox 14 doesn't load the manifest file at all.

It seems that this problem occurs only when the downloaded content is greater than 50MB (when the "warning" banner appears).


Expected results:

Firefox should always reload the Manifest to see if something has changed.

I used mozregression and I saw that this bug started with nightly 2012-04-01. For all previous versions, it works as expected. 

http://hg.mozilla.org/mozilla-central/pushloghtml?startdate=2012-03-31&enddate=2012-04-01

We created a small demo to illustrate this problem. Every file in the Manifest file is 1MB and are sent by the server using this header : "Cache-Control: max-age=31536000, must-revalidate"

The manifest file itself is sent by the server using this header : "Cache-Control: private, max-age=0, must-revalidate". This header should force Firefox to always fetch the version from the server.

Steps to reproduce:

1) Go to http://www.jimmygilles.net/bugzilla/page_a.htm
2) Accept the manifest download
3) Wait for the manifest to be completly downloaded (54 files of 1MB each)
4) The manifest is now ready
5) go to http://www.jimmygilles.net/bugzilla/page_b.htm (which uses the same manifest file)
6) Firefox 14-15-16 : the manifest is not downloaded at all and never will be again

Thank you
Severity: normal → critical
Severity: critical → normal
According to your regression range, the suspected bug could be:

Bug 674728 - Allow web apps to be "pinned", so that they're guaranteed to be fully cached locally
Component: Untriaged → Networking: Cache
Keywords: regression, testcase
Product: Firefox → Core
Hello,

I see you changed the severity to "normal". I can understand that "critical" was a little bit too much (after reading this https://bugzilla.mozilla.org/page.cgi?id=fields.html). In my opinion, we should set the severity to "major" as it completely breaks a big functionality in our website (wwww.coursesmart.com - offline reading).

Thanks!

Alessandro
The good url is http://www.coursesmart.com (with 3 w instead of 4 ;))
(In reply to De Simone Alessandro from comment #2)
> Hello,
> 
> I see you changed the severity to "normal". I can understand that "critical"
> was a little bit too much (after reading this
> https://bugzilla.mozilla.org/page.cgi?id=fields.html).
In general, it's up to devs to choose this field and it's not so important (only for critical/blocker bugs).
Hello,

By checking the attached demo, it seems to be a blocker bug as the manifest is not loaded anymore.

This means that for FF version 14/15/16, the application cache's use is totally broken and won't work for everybody... not only coursesmart.com

As Loic previous comment underlined, the suspect bug could be linked with the "pinned" modifications preventing manifest reload (cf. STATE_OBSOLETE >< STATE_NOUPDATE -> ?)

Thanks for your help,
Nic
I have wrote a xpcshell testcase for this bug.  It update a manifest two times, and check how many times of fetching the manifest.  It seems working fine for me.
(In reply to Thinker Li [:sinker] from comment #6)
> Created attachment 650830 [details] [diff] [review]
> testcase for this bug
> 
> I have wrote a xpcshell testcase for this bug.  It update a manifest two
> times, and check how many times of fetching the manifest.  It seems working
> fine for me.

Hello,

Did you try the demo as indicated in the first comment ?
Can you reproduce this bug if you test the provided demo manually ?

It seems the problem is reproductible when the total size of the downloaded content is greater than 50 MBytes (a warning banner is displayed by firefox when the content exceeds 50 MBytes).

With the demo, I can reproduce the problem every time I follow the steps as indicated (on firefox 14).

Thank you.
Jimmy
(In reply to Jimmy Gilles from comment #7)
> (In reply to Thinker Li [:sinker] from comment #6)
> > Created attachment 650830 [details] [diff] [review]
> > testcase for this bug
> > 
> > I have wrote a xpcshell testcase for this bug.  It update a manifest two
> > times, and check how many times of fetching the manifest.  It seems working
> > fine for me.
> 
> Hello,
> 
> Did you try the demo as indicated in the first comment ?
> Can you reproduce this bug if you test the provided demo manually ?
> 
> It seems the problem is reproductible when the total size of the downloaded
> content is greater than 50 MBytes (a warning banner is displayed by firefox
> when the content exceeds 50 MBytes).

Every file in your manifest is noly 2k.  I can not reproduce your issue.  Can you check it again?
Hello,

Here is a screenshot of the fiddler2 session.
As you can see, the content of each downloaded file is 1 MByte.
53 files are downloaded, so the total size is greater than 50MBytes.

Thank you !
The wireshark told me your web server send a header with "Content-Length: 2970".  It is exactly what is cached in the cache.  But, your server does not return a Content-Length when I get the file with wget.
(In reply to Thinker Li [:sinker] from comment #10)
> The wireshark told me your web server send a header with "Content-Length:
> 2970".  It is exactly what is cached in the cache.  But, your server does
> not return a Content-Length when I get the file with wget.

Hello,

I tested the download at home and, indeed, I was able to reproduce the same problem as you had... very strange because it was working correctly at work.

So, we made some changes to be sure it works correctly.
- We changed the content of each download file because it was not a valid html. Now, each downloaded file is a valid html (checked with the W3C validator).
- I disabled the gzip compression for those files.

I hope you will be able to reproduce the problem we have :)

Thank you for your help and sorry for this very strange behaviour.
Hello,

Were you (or anybody else) able to reproduce this problem?
Please tell me if I can do something more to help to resolve this issue.

Thank you.
Yes, I can reproduce this problem.  As comment #1 had mentioned pinned offline cache, I have break pointed at nsOfflineCacheUpdateService::Schedule(), but it is never called for exceeding the offline-cache.quota.warn.  I think there seems something wrong before calling into nsOfflineCacheUpdateService.  I will do more test for this bug.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Hello,

I know you must be busy but just a quick question, did you do more tests for this bug? Do you know if this bug can be fixed easily or importants changes must be applied to resolve it?

Thank you very much.
Hello,
Is there someone working on it?
Thanks.
Without this checking, nsOfflineCacheUpdateService don't update
offline cache any more if previous updating exceeds usage and showing
a warning.

@Honza: can you review this?
Attachment #664408 - Flags: review?(honzab.moz)
(In reply to Jimmy Gilles from comment #15)
> Hello,
> Is there someone working on it?
> Thanks.

Sorry! I just returned from a vocation.
Is this bug a regression of bug 674728 (according the changelog in comment #0)? I don't see any bug blocked.
Yes! This bug is a regression of bug 674728.
Thanks for confirming. :)
Blocks: 674728
Comment on attachment 664408 [details] [diff] [review]
Check app permission against ALLOW_NO_WARN

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

::: uriloader/prefetch/nsOfflineCacheUpdateService.cpp
@@ +569,1 @@
>          *aAllowed = true;

Hmm.. this has slipped through my review.. good catch!
Attachment #664408 - Flags: review?(honzab.moz) → review+
(In reply to Honza Bambas (:mayhemer) from comment #22)
> Hmm.. this has slipped through my review.. good catch!

Taking back... Thinker, I think you have landed the last patch in bug 674728 w/o a proper review...
(In reply to Honza Bambas (:mayhemer) from comment #23)
> (In reply to Honza Bambas (:mayhemer) from comment #22)
> > Hmm.. this has slipped through my review.. good catch!
> 
> Taking back... Thinker, I think you have landed the last patch in bug 674728
> w/o a proper review...

I think it is backed out.  https://bugzilla.mozilla.org/show_bug.cgi?id=674728#c112
I still remember it was happening when I am on a vocation.
Commit 21 for test report of try-server.
Keywords: checkin-needed
(In reply to Thinker Li [:sinker] from comment #24)
> (In reply to Honza Bambas (:mayhemer) from comment #23)
> > (In reply to Honza Bambas (:mayhemer) from comment #22)
> > > Hmm.. this has slipped through my review.. good catch!
> > 
> > Taking back... Thinker, I think you have landed the last patch in bug 674728
> > w/o a proper review...
> 
> I think it is backed out. 
> https://bugzilla.mozilla.org/show_bug.cgi?id=674728#c112
> I still remember it was happening when I am on a vocation.

I am all time talking about the "Part 1" patch.  You have made some changes that needed one more review round but you didn't ask for it.
(In reply to Honza Bambas (:mayhemer) from comment #26)
> (In reply to Thinker Li [:sinker] from comment #24)
> > (In reply to Honza Bambas (:mayhemer) from comment #23)
> > > (In reply to Honza Bambas (:mayhemer) from comment #22)
> > > > Hmm.. this has slipped through my review.. good catch!
> > > 
> > > Taking back... Thinker, I think you have landed the last patch in bug 674728
> > > w/o a proper review...
> > 
> > I think it is backed out. 
> > https://bugzilla.mozilla.org/show_bug.cgi?id=674728#c112
> > I still remember it was happening when I am on a vocation.
> 
> I am all time talking about the "Part 1" patch.  You have made some changes
> that needed one more review round but you didn't ask for it.

Got it!  I am very sorry!
Should the test be getting checked in too? It doesn't appear it was pushed to Try with the fix.
(In reply to Ryan VanderMeulen from comment #28)
> Should the test be getting checked in too? It doesn't appear it was pushed
> to Try with the fix.

@Ryan, testcase will not be checked in.  The testcase is invalid since mochitest never show warning and checking for data size.
I assume there's no other way to test this then?
(In reply to Ryan VanderMeulen from comment #30)
> I assume there's no other way to test this then?

I will revise the testcase later with setting ALLOW_NO_WARN by testcase itself.  So, we can simulate the situation after showing warning by the browser.
(In reply to Thinker Li [:sinker] from comment #31)
> (In reply to Ryan VanderMeulen from comment #30)
> > I assume there's no other way to test this then?
> 
> I will revise the testcase later with setting ALLOW_NO_WARN by testcase
> itself.  So, we can simulate the situation after showing warning by the
> browser.

You can play with the quota setting in the test.  There are prefs for it, if I recall.  I can help if you need.
https://hg.mozilla.org/mozilla-central/rev/b1da17dd5a92
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla18
You need to log in before you can comment on or make changes to this bug.