Closed Bug 450738 Opened 16 years ago Closed 16 years ago

Image cache has a bad expiration policy for local files


(Core :: Graphics: ImageLib, defect)

1.9.0 Branch
Not set





(Reporter: jwkbugzilla, Assigned: jwkbugzilla)




(2 files)

Attached patch Proposed patchSplinter Review
Image cache has special handling to make sure files expire from cache if they change on disk. Unfortunately, the current algorithm requires the last modification time of the file to be higher than the time when it was previously loaded. This will fail for example if an existing file is copied over another one. Instead, the file should expire if its last modification time changed.

To test you can open an image from your disk in Firefox 3. Copy another image over the original image and open it again - note that you are still being shown the old image. You will only get the new image by pressing Reload or by changing the modification date of the file to something in future. According to bug 383336, this is an issue for Thunderbird. It also affects TomTom HOME because the clock on the navigation device is often out of sync with the PC - so if the device replaces the image you are still being shown the old one after reconnecting it to the PC. Or you get only cache misses if the file has a modification time in future.

Attaching patch that fixed the expiration policy by storing file's modification time in cache metadata. SecondsFromPRTime function is only moved around - it has to be declared before its first use. Tested, with that patch you only get a cached version of the file if its modification time is still exactly the same.
Attachment #333917 - Flags: review?(pavlov)
Attached patch Trunk patchSplinter Review
Attaching a trunk patch as well. However, it is not fully tested due the issue I mentioned in bug 430061 comment 31.
Assignee: nobody → trev.moz
Resolving based on bug 430061 comment c32 - if the images are removed from cache after 10 seconds this bug doesn't matter for us any more. And the problem seems to be rare enough so that nobody else cares.
Closed: 16 years ago
Resolution: --- → WONTFIX
Attachment #333917 - Flags: review?(pavlov)

Wouldn't this problem exist equally as often on http: files? You could just as easily copy a file on the server to change its last modification time.
HTTP has the expiration date for that. Local files have nothing like this which is why there is special provisioning in imgLoader.cpp to make sure changes in files are detected. This special provisioning fails to detect a case we care about, a file gets stuck in cache even though it changed. But that's no longer a problem on trunk, the file won't stay in cache too long so it doesn't matter.
You need to log in before you can comment on or make changes to this bug.