450738 - Image cache has a bad expiration policy for local files

Status: RESOLVED WONTFIX

(Core :: Graphics: ImageLib, defect)

Description

[Wladimir Palant]

Attachment: Proposed patch

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.

Comment 1

[Wladimir Palant]

Attachment: Trunk patch

Attaching a trunk patch as well. However, it is not fully tested due the issue I mentioned in bug 430061 comment 31.

Comment 2

[Wladimir Palant]

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.

Comment 3

[Joe Drew (not getting mail)]

Wladimir,

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.

Comment 4

[Wladimir Palant]

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.