This comes from bug #588804, comment #23. If a cached entry on disk has no content, the about:cache description for the entry will claim content is stored in a separate file which doesn't exist. This is simple to observe for e.g. certain 3xx-responses which are cached but do not have content. Not sure what consequences this may have, except for the obvious fact that the about:cache description presented to a user is wrong. Components, add-ons or plugins may all use this info. The code in nsAboutCacheEntry::WriteCacheEntryDescription() uses the attribute nsICacheEntryDescriptor::file to get the filename. IMO, we could extend this interface to throw if the returned file doesn't exist.

Fixing bug #605101 will most likely also handle this one in practice, but we may still want to extend nsICacheEntryDescriptor to throw is the file-attribute denotes a file which doesn't exist.

Simple check in nsDiskCacheDevice Afaics, the only possible issue with this is performance but I doubt it will be a huge problem...

Comment on attachment 483969 [details] [diff] [review] Simple, v1 patch (In reply to comment #1) > Fixing bug #605101 will most likely also handle this one in practice, but we > may still want to extend nsICacheEntryDescriptor to throw is the file-attribute > denotes a file which doesn't exist. Why do we need this if bug #605101 will solve the problem for empty entries (like 304 response)? After fixing #605101 this could happen e.g. in case when user manually removes the file. But this patch doesn't fix this problem. It only changes what about:cache-entry displays.

(In reply to comment #3) > But this patch doesn't fix this problem. It > only changes what about:cache-entry displays. Yes - ref the title of the bug. :) Fixing this is a lot simpler (and has probably infinitely less potential for regression) than fixing bug #605101. In fact, this can be considered a hack to fix the display-issue until a more proper fix is done. Whether or not the display-issue *needs* to be fixed is a decision for whoever approves it for a release, IMO.

(In reply to comment #4) > > But this patch doesn't fix this problem. It > > only changes what about:cache-entry displays. > > Yes - ref the title of the bug. :) So why don't to fix it in nsAboutCacheEntry.cpp? There we could simply differentiate between none and missing (file isn't present but should be!).

Good point - guess I instinctively put the fix as "deep" in the code as reasonable. :\ I've changed the title of this bug to refer to instances of nsICacheEntryDescriptor, broadening the scope of the fix. We agree that the real fix for this problem is bug #605101, but it will modify internals of the disk-cache and I'm not sure we want to do that now. The proposed patch for this bug impacts all users of nsICacheEntryDescriptor, among them the about:cache description. I've pushed the patch to tryserver to get a thorough check because of this.

> I've pushed the patch to tryserver to > get a thorough check because of this. Something weird happened to the push... pushed again.

(In reply to comment #6) > Good point - guess I instinctively put the fix as "deep" in the code as > reasonable. :\ That's a good idea in general, but I still do think that this fix is wrong. In case when the file is missing the user gets a wrong information. Why he should see "none" when even the cache code tries to open some file when loading the resource? I'm proposing a fix inside nsAboutCacheEntry.cpp: 1) if data size is 0, we shouldn't call GetFile() and display any file at all 2) if data size > 0 and GetFile() doesn't return a file, we should display "none" 3) if data size > 0 and Getfile() returns a file, we should display that file even if it doesn't exist IMO this would give a user the most correct information.

This is handled by patch in bug #605101, comment #2.

Comment on attachment 483969 [details] [diff] [review] Simple, v1 patch I don't see a reason to take this fix before we branch, unless there's a specific case that we know is currently failing.

I'm duping this since this is a special case of bug #605101 and the proposed fix handles this case as well.