Closed Bug 1000338 Opened 11 years ago Closed 10 years ago

nsICacheEntry.lastModified not properly implemented

Categories

(Core :: Networking: Cache, defect)

defect
Not set
normal

Tracking

()

VERIFIED FIXED
mozilla35
Tracking Status
firefox32 --- wontfix
firefox33 --- verified
firefox34 --- verified
firefox35 --- verified
b2g-v2.0 --- fixed
b2g-v2.0M --- fixed
b2g-v2.1 --- fixed
b2g-v2.2 --- fixed

People

(Reporter: mayhemer, Assigned: mayhemer)

References

(Blocks 2 open bugs)

Details

Attachments

(1 file, 1 obsolete file)

This is an internal property, not the value of Last-Modified http header. We never set the value. Looking into the old cache code, apparently this is updated only internally when data or metadata are touched (nsCacheEntry::TouchData(), nsCacheEntry::TouchMetaData()). In the new cache there is however an exposed public method CacheFile::SetLastModified. But it's never called by anyone. I personally tend to let this be an internal property of CacheFile that is updated automatically to PR_Now() in CacheFile directly.
Summary: nsICacheEntry.lastModified not properly implemented, remove it completely? → nsICacheEntry.lastModified not properly implemented
I think we can live with this for the first deployment. This will only influence VALIDATE_ONCE_PER_SESSION loads (will be always re-validated) and some authenticated requests to be re-validated more often. See fromPreviousSession usage in nsHttpChannel::OnCacheEntryCheck. With this bug it's always at |true|.
Blocks: cache2afterenable
No longer blocks: cache2enable
lastFetched is affected as well, but there is no serious consumer for it except about:cache.
Blocks: 1009122
Blocks: 1029782
Assignee: nobody → honzab.moz
Status: NEW → ASSIGNED
Attached patch v1 (obsolete) (deleted) — Splinter Review
- removed SetLM from CacheFile/CacheFileMetadata - LF, FC no longer controlled by CacheFileMetadata - added OnFetched() to CacheFile/CacheFileMetadata that updates: FC++, LF=Now() - MarkDirty() now does LM=Now() in selected cases - CacheEntry calls CacheFile->OnFetched() when it gives itself to the consumer (callback) This way it conforms the cache1 implementation, makes actually sense and also should be in sync with the IDL comments now.
Attachment #8482928 - Flags: review?(michal.novotny)
Blocks: 1064040
Michal, ping on review here. This is causing serious trouble. Thanks.
Comment on attachment 8482928 [details] [diff] [review] v1 Review of attachment 8482928 [details] [diff] [review]: ----------------------------------------------------------------- ::: netwerk/cache2/CacheFileMetadata.cpp @@ -720,5 @@ > mBufSize = 0; > } > mOffset = 0; > mMetaHdr.mVersion = kCacheEntryVersion; > - mMetaHdr.mFetchCount = 1; We must set something here since the value is in unknown state. So we should assign here 0. ::: netwerk/test/unit/test_cache2-28-last-access-attrs.js @@ +4,5 @@ > + function NowSeconds() { > + return parseInt((new Date()).getTime() / 1000); > + } > + function do_check_time(a, b) { > + do_check_true(Math.abs(a - b) < 0.5); I think this can be unreliable. Couldn't we take one timestamp before calling asyncOpenCacheEntry() the second in OpenCallback() and check that the entry's timestamp falls within this range?
Attachment #8482928 - Flags: review?(michal.novotny) → review+
Attached patch v1.1 (deleted) — Splinter Review
Attachment #8482928 - Attachment is obsolete: true
Attachment #8487801 - Flags: review+
sorry also backedout for xpcshell-2 test failures like https://tbpl.mozilla.org/php/getParsedLog.php?id=47873428&tree=Mozilla-Inbound
Blocks: 1067931
Dear Honza Bambas, thank you for your reply explaining me that the bug 1064040 is related to this one. As you already know, the problem of the BACK button not working occurs in all sites with basic authentication, from Firefox ver. 32.0 on, and I guess this should be a major issue. I notice that bug 1000338 was first open in April 2014 and is still not patched, I hope it won't take so much time to solve this issue. Also, please tell me if you still need the debug site I prepared at address http://95.110.161.29:42002/ or if I can close it. Thank you very much, Alessandro
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
Comment on attachment 8487801 [details] [diff] [review] v1.1 Approval Request Comment [Feature/regressing bug #]: cache2enable [User impact if declined]: ill behavior when using back/forth navigation (unexpected document load errors) and probably more ; the significance of this metadata has been underestimated! [Describe test coverage new/current, TBPL]: none (test needs to be be tuned up - it's time check based, i.e. not 100% reliable) [Risks and why]: small, only risk would be some mistake in the code leading to e.g. a crash or something, but that would already be discovered, since this code is exercised by the whole platform and a tun of other tests [String/UUID change made/needed]: none
Attachment #8487801 - Flags: approval-mozilla-beta?
Attachment #8487801 - Flags: approval-mozilla-aurora?
Attachment #8487801 - Flags: approval-mozilla-beta?
Attachment #8487801 - Flags: approval-mozilla-beta+
Attachment #8487801 - Flags: approval-mozilla-aurora?
Attachment #8487801 - Flags: approval-mozilla-aurora+
Honza, could you provide a way to reproduce this issue for QA? Thanks
Flags: needinfo?(honzab.moz)
(In reply to Sylvestre Ledru [:sylvestre] from comment #15) > Honza, could you provide a way to reproduce this issue for QA? Thanks Let them use bug 1064040. There is a very good test case!
Flags: needinfo?(honzab.moz)
(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #16) > https://hg.mozilla.org/releases/mozilla-aurora/rev/e69b76df294c > https://hg.mozilla.org/releases/mozilla-beta/rev/b88069789828 > > Do we need to track this for B2G v2.0 (b2g32)? I think it's worth too.
Attachment #8487801 - Flags: approval-mozilla-b2g32?
Flags: qe-verify+
Reproduced the issue on Firefox 32.0.2 build, verified as fixed on latest Aurora (build id:20140919030202), latest Nightly (build id: 20140919030202) and Firefox 33 Beta 5 (build id: 20140918174809) under: Windows 7 64bit, Mac OS X 10.9 and Ubuntu 64bit.
Very nice job, Honza Bambas. The debug / fixing / testing process was fast. Now we just wait for the official release of Firefox 33. Thank you to all the team. Alessandro Pasta
Honza, for v2.0(based on gecko 32) we are only uplifting release show-stopper bugs. Can you give us your input on user impact/risk for FxOS to evaluate how critical this is at this point the cycle given we are very close to ship. Else this may have to fixed in 2.1 had the Same question for 1066726..
Flags: needinfo?(honzab.moz)
(In reply to bhavana bajaj [:bajaj] from comment #23) > Honza, > > for v2.0(based on gecko 32) we are only uplifting release show-stopper bugs. > Can you give us your input on user impact/risk for FxOS to evaluate how > critical this is at this point the cycle given we are very close to ship. > Else this may have to fixed in 2.1 > > had the Same question for 1066726.. See bug 1063431 and then duplicates of bug 1066726. Those are major regressions you may get in to. This is worth uplifting.
Flags: needinfo?(honzab.moz)
Attachment #8487801 - Flags: approval-mozilla-b2g32? → approval-mozilla-b2g32+
Blocks: 1066726
No longer blocks: 1066726
This bug does not seem to be fixed in Firefox 33.1.1 (Mozilla Firefox EU euballot - 1.1). With this version (which is the latest released version), running on Windows 7 64 bits, I still get some invalid entries in the cache (about:cache?storage=disk&context=), which shows as: Data size = "0 bytes" Last Modifed = "No last modified time (bug 1000338)" Expires = "No expiration time" I have the problem even after I empty the cache and far before the cache reaches its size limit. So this bug should not be written as "fixed" or "verified" for v33. It's still not fixed.
(In reply to François R-Champ from comment #28) > This bug does not seem to be fixed in Firefox 33.1.1 (Mozilla Firefox EU Do you have any problems with page loads or something unexpected that could be related to this bug? If not I consider this bug as fixed. What you see *may* be a bug, and if so, definitely a different bug than this one. Please file it if you believe it's worth an attention and don't comment on closed bugs. Thanks.
(In reply to Honza Bambas (:mayhemer) from comment #29) > (In reply to François R-Champ from comment #28) > > This bug does not seem to be fixed in Firefox 33.1.1 (Mozilla Firefox EU > > Do you have any problems with page loads or something unexpected that could > be related to this bug? If not I consider this bug as fixed. What you see > *may* be a bug, and if so, definitely a different bug than this one. Please > file it if you believe it's worth an attention and don't comment on closed > bugs. Thanks. 33.1.1 still have that bug. Using alessandro.pasta's test link http://95.110.161.29:42002/, user=test, pwd=bug "Expired document" appeared. http://i.imgur.com/mwlM988.png And the "0 bytes" in about:cache?storage=disk&context= http://i.imgur.com/pbwI2Q5.png My system is win7 x64.
(In reply to kxmpex from comment #30) To reproduce it must have big disk cache entries. I reproduced it when my disk cache entries is about 13210.
(In reply to kxmpex from comment #30) > (In reply to Honza Bambas (:mayhemer) from comment #29) > > (In reply to François R-Champ from comment #28) > > > This bug does not seem to be fixed in Firefox 33.1.1 (Mozilla Firefox EU > > > > Do you have any problems with page loads or something unexpected that could > > be related to this bug? If not I consider this bug as fixed. What you see > > *may* be a bug, and if so, definitely a different bug than this one. Please > > file it if you believe it's worth an attention and don't comment on closed > > bugs. Thanks. > > 33.1.1 still have that bug. > Using alessandro.pasta's test link http://95.110.161.29:42002/, user=test, > pwd=bug > "Expired document" appeared. > http://i.imgur.com/mwlM988.png > And the "0 bytes" in about:cache?storage=disk&context= > http://i.imgur.com/pbwI2Q5.png > > My system is win7 x64. Rechecked test case from bug 1064040 comment 2 on: - a debug nightly build. Works for me. - 33.1.1. Works for me. - 33.1.1 with a lowered cache limit and cache full, works for me. - 32.X. Broken, easy to reproduce. I checked the actual fix is on 33.1.1 (current release sources). It has also been checked by our QA this is fixed on 33. I think you are experiencing bug 1060082.
(In reply to Honza Bambas (:mayhemer) from comment #32) Year,and it's interesting. It seems only happened to fat32 filesystem.
(In reply to kxmpex from comment #33) Fat32 have max files in a folder limit. And differend seems have different limit..... Max files in a folder with 4k cluster have a limit is about 13xxx!! I think that why i have that weird experience.
(In reply to Honza Bambas (:mayhemer) from comment #29) > (In reply to François R-Champ from comment #28) > > This bug does not seem to be fixed in Firefox 33.1.1 (Mozilla Firefox EU > > Do you have any problems with page loads or something unexpected that could > be related to this bug? If not I consider this bug as fixed. What you see > *may* be a bug, and if so, definitely a different bug than this one. Please > file it if you believe it's worth an attention and don't comment on closed > bugs. Thanks. Yes, the problem I am having is that some pages in cache do not load and of course it is a bug. When I look at the cache (about:cache?storage=disk&context=), all the cache entries of the pages which do not load have the following values : « Data size = "0 bytes" Last Modifed = "No last modified time (bug 1000338)" Expires = "No expiration time" » As you can see Firefox itself is saying that this is bug number 1000338!
(In reply to François R-Champ from comment #35) > (In reply to Honza Bambas (:mayhemer) from comment #29) > > (In reply to François R-Champ from comment #28) > > > This bug does not seem to be fixed in Firefox 33.1.1 (Mozilla Firefox EU > > > > Do you have any problems with page loads or something unexpected that could > > be related to this bug? If not I consider this bug as fixed. What you see > > *may* be a bug, and if so, definitely a different bug than this one. Please > > file it if you believe it's worth an attention and don't comment on closed > > bugs. Thanks. > > Yes, the problem I am having is that some pages in cache do not load and of > course it is a bug. When I look at the cache > (about:cache?storage=disk&context=), all the cache entries of the pages > which do not load have the following values : > « Data size = "0 bytes" > Last Modifed = "No last modified time (bug 1000338)" > Expires = "No expiration time" » > > As you can see Firefox itself is saying that this is bug number 1000338! François, thanks for your comment. Finally I understand what's going on here. Sorry for later answer, I was indisposed for a time. Apparently, it sometimes may happen we store an empty cache entry that we then use for rendering a page somehow. Definitely a bug and definitely in or around the new cache2. Connection to bug 1000338 is just coincidental. The message in about:cache had to be removed with this bug (filed a new bug 1119394 on that). What you see is a different, new bug (reported as bug 1119406).
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: