553967 - Add a direct way to get to the AddonInstall from an Addon that is pending upgrade

Status: VERIFIED FIXED

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

Comment 1

[Dave Townsend [:mossop]]

I think the most consistent way to do this would be to have an "install" property on Addons that are pending install. So Addon.install points to the AddonInstall and AddonInstall.addon points to the new Addon and Addon.pendingUpgrade.install would point to the AddonInstall upgrading an Addon.

Comment 2

[Blair McBride [:Unfocused] (UNAVAILABLE)]

http://hg.mozilla.org/projects/addonsmgr/rev/24ea43f4df86

Comment 3

[Henrik Skupin [:whimboo][⌚️UTC+2]]

I assume we do not need a manual test here. If I'm wrong please re-flag.

Comment 4

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Oops, I missed that. Correct - automated testing covers this completely.

Comment 5

[Dave Townsend [:mossop]]

This doesn't seem to have been implemented in the way described in comment 1, looks like you've put the install property on the Addon that is pending upgrade as opposed to the Addon that the AddonInstall is installing.

Comment 6

[Dave Townsend [:mossop]]

Backed out the original changeset with http://hg.mozilla.org/projects/addonsmgr/rev/3399677bafaa and landed a fixed version at http://hg.mozilla.org/projects/addonsmgr/rev/8aec62049ba2

Comment 7

[Dave Townsend [:mossop]]

Attachment: Add Addon.install property

Adds an Addon.install property to get the AddonInstall that is installing it.

Comment 8

[Robert Strong (they/them - no direct email)]

Comment on attachment 435771 [details] [diff] [review]
Add Addon.install property

Looks good

>diff --git a/toolkit/mozapps/extensions/XPIProvider.jsm b/toolkit/mozapps/extensions/XPIProvider.jsm
>--- a/toolkit/mozapps/extensions/XPIProvider.jsm
>+++ b/toolkit/mozapps/extensions/XPIProvider.jsm
>...
>@@ -3820,6 +3826,14 @@ function AddonWrapper(addon) {
>     addon.updateAutomatically = val;
>   });
> 
>+  this.__defineGetter__("install", function() {
>+    if (!("_install" in addon))
>+      return null;
>+    if (!addon._install)
>+      return null;
Why not check both on one line?

Comment 9

[Dave Townsend [:mossop]]

Attachment: patch rev 2

Ready to land

Comment 10

[Dave Townsend [:mossop]]

http://hg.mozilla.org/mozilla-central/rev/0e4fdfeee54c

Comment 11

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Marking as verified fixed based on check-in and passing automated tests.