Bugzilla

(In reply to Francesco from comment #9)
> The try build works and shows the correct space saved in the status bar.

Great - thank you for that!

> Just out of curiosity: Why is this an issue now? If the old implementation also checked the file size after compaction, it would have had the same issue on Windows.

The new implementation starts by getting the mbox path from the folder (`folder->GetFilePath()`) as an `nsIFile` object, and holds onto that object for the duration:

https://searchfox.org/comm-central/search?q=mMboxPath&path=&case=false&regexp=false

It calls `nsIFile::GetFileSize()` before it starts, then again after it ends. The problem was that the windows implementation of `nsIFile::GetFileSize()` caches the result and had no way of knowing that the file had been overwritten!

The workaround is to clone the `nsIFile` object before the second `GetFileSize()` call. The cloned object has those cached values cleared.

I've seen a few places in the code where `nsIFile` objects were being cloned for no obvious reason, and I always thought it was odd, but maybe this is the reason! I'll write it up as a separate bug for m-c.

(In reply to Francesco from comment #9)
> The try build works and shows the correct space saved in the status bar.

Great - thank you for that!

> Just out of curiosity: Why is this an issue now? If the old implementation also checked the file size after compaction, it would have had the same issue on Windows.

The new implementation starts by getting the mbox path from the folder (`folder->GetFilePath()`) as an `nsIFile` object, and holds onto that object for the duration:

https://searchfox.org/comm-central/search?q=mMboxPath&path=&case=false&regexp=false

It calls `nsIFile::GetFileSize()` before it starts, then again after it ends. The problem was that the windows implementation of `nsIFile::GetFileSize()` caches the result and had no way of knowing that the file had been overwritten, so the second call just returned the same value!

The workaround is to clone the `nsIFile` object before the second `GetFileSize()` call. The cloned object has those cached values cleared.

I've seen a few places in the code where `nsIFile` objects were being cloned for no obvious reason, and I always thought it was odd, but maybe this is the reason! I'll write it up as a separate bug for m-c.