1559683 - Addon install cancellation is silently ignored when invoked while AddonInstall's loadManifest is being run

Status: RESOLVED FIXED

(Toolkit :: Add-ons Manager, defect, P3)

Description

[Rob Wu [:robwu]]

It seems that an AddonInstall's cancel method should either cancel the install, or throw an error.

However, there is a small window where this is not the case:

1. When a XPI download request finishhes, DownloadAddonInstall's onStopRequest is invoked, which clears the channel member.
2. While state is still STATE_DOWNLOADING, the asynchronous loadManifest method is invoked, which may take several dozens of milliseconds.
3. If cancel is called before the state is changed, then the cancel invocation is silently ignored (because channel was cleared in step 1).

The expected result is that cancel throws an error,
OR that the install is immediately canceled and cleaned up.

The current logic was introduced in bug 1116629, and the condition of interest is:

We have to wait until the channel closes before deleting it.

If this is the only condition, then it seems that we can run the cleanup logic right away. OTOH, loadManifest reads the contents of the XPI, so throwing an error may be safer.

Comment 1

[Release mgmt bot [:sylvestre / :calixte / :marco for bugbug]]

The priority flag is not set for this bug.
:ddurst, could you have a look please?

For more information, please visit auto_nag documentation.

Comment 3

[[PTO until Jun 3rd] Agi Sferro | :agi | ni? for questions | ⏰ PST | he/him]

Attachment: Bug 1559683 - Reject install.cancel() call when loadManifest is running.

Comment 4

[[PTO until Jun 3rd] Agi Sferro | :agi | ni? for questions | ⏰ PST | he/him]

Attachment: Bug 1559683 - Rename STATE_CHECKING -> STATE_CHECKING_UPDATE.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c1c8a51c4efb
Reject install.cancel() call when loadManifest is running. r=mixedpuppy,robwu
https://hg.mozilla.org/integration/autoland/rev/c26fdb0faea7
Rename STATE_CHECKING -> STATE_CHECKING_UPDATE. r=mixedpuppy

Comment 6

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/c1c8a51c4efb
https://hg.mozilla.org/mozilla-central/rev/c26fdb0faea7