1460095 - Always flush the JAR cache for AddonInstall temp XPIs

Status: RESOLVED FIXED

(Toolkit :: Add-ons Manager, enhancement)

Description

[Kris Maglione [:kmag]]

There are weird corner cases where we sometimes attempt to re-use the temp filename from an older install attempt for a new install, and therefore wind up reading the cached data in the jar cache from a different add-on. This probably isn't possible on Windows (but may prevent us from deleting the temp files), and is probably extremely unlikely to happen in the real world. But we should probably fix it anyway. If nothing else, it would probably save some resident memory.

Comment 1

[Andrew Swan [:aswan]]

Is there a reason not to flush here unconditionally:
https://searchfox.org/mozilla-central/rev/5ff2d7683078c96e4b11b8a13674daded935aa44/toolkit/mozapps/extensions/internal/XPIInstall.jsm#357-359

There are a few things that touch the zip reader that don't set that flag (eg findEntries() ), I'm not sure exactly what does and doesn't require a flush.  But the bigger issue is that we bypass the XPIPackage instance and just use a jar url here:
https://searchfox.org/mozilla-central/rev/5ff2d7683078c96e4b11b8a13674daded935aa44/toolkit/mozapps/extensions/internal/XPIInstall.jsm#829

I guess we could manually set needFlush on the package at that line, that feels clumsy though.  Is there some scenario where an unconditional flush would add too much unnecessary overhead?

Comment 2

[Kris Maglione [:kmag]]

We should probably just flush unconditionally, yeah. I added the `needFlush` property because I was trying to preserve the old behavior when I added a new fetch call that wound up putting the XPI in the jar cache. But for WebExtensions, where we already loaded the manifest from jar: channels, this problem already existed.

Comment 3

[Andrew Swan [:aswan]]

Attachment: Bug 1460095 Always flush the jar cache after looking at an xpi file

Review commit: https://reviewboard.mozilla.org/r/242874/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/242874/

Comment 4

[Kris Maglione [:kmag]]

Comment on attachment 8974537 [details]
Bug 1460095 Always flush the jar cache after looking at an xpi file

https://reviewboard.mozilla.org/r/242874/#review248722

Thanks

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7758499ad01f
Always flush the jar cache after looking at an xpi file r=kmag

Comment 6

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/7758499ad01f

Comment 7

[marius.santa]

Is manual testing required on this bug? If Yes, please provide some STR and the proper webextension(if required), if No set the “qe-verify-“ flag.