1149590 - Update/Upgrade for application dowloaded and installed, setting to ask/prompt for action ignored

Status: RESOLVED FIXED

(Toolkit :: Application Update, defect)

Description

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Firefox 39.0a1 and Thunderbird 39.0a1, both 20150330 on Windows 8.1

Both are set in Settings > Advanced > Update to check for updates, but prompt the user for the action (download & install or ignore).

Open the application and let it idle. The download billboard will open after some time, download the update and tell you to restart to complete the installation

[Tracking Requested - why for this release]: regression

Comment 1

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

Did you download and install 39.0a1? If you did please provide a link to the Firefox download.

Comment 2

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

Also, Do you remember if you used a second profile at any point? If you did download and install please provide the steps you used.

Comment 3

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

Attachment: patch

Figured out what the problem is

Comment 4

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

Pushed to fx-team
https://hg.mozilla.org/integration/fx-team/rev/202034003fe1

Comment 5

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

Comment on attachment 8586242 [details] [diff] [review]
patch

Approval Request Comment
[Feature/regressing bug #]: Bug 1137447
[User impact if declined]: B2G will auto update (the default is prompt in B2G) and desktop users will auto update when they have set their preference to notify.
[Describe test coverage new/current, TreeHerder]: Manually tested and I am writing a test for in tree coverage.
[Risks and why]: Minimal to none. This reverts a change made in Bug 1137447.
[String/UUID change made/needed]: None

Comment 6

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/202034003fe1

Comment 8

[Liz Henry (:lizzard Please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8586242 [details] [diff] [review]
patch

Approving this for 39 since it looks low-risk and can affect updates.

Comment 9

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

Pushed to mozilla-aurora
https://hg.mozilla.org/releases/mozilla-aurora/rev/cbfc7d6d54b7

Comment 10

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

Attachment: New test

Stephen, I verified this test works locally and here is a try run. I suspect that it will fail on gonk since gonk uses a custom nsIUpdatePrompt in which case I'll exclude gonk in the xpcshell.ini

https://treeherder.mozilla.org/#/jobs?repo=try&revision=67fc983dc607

Comment 11

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

Attachment: test patch

Forgot to use Assert in one place

Comment 12

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

Attachment: test patch

bah! I need to slow down... forgot to remove DEBUG_AUS_TEST!

Comment 13

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

Comment on attachment 8587798 [details] [diff] [review]
test patch

meh, crashes on Linux

Comment 14

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

Attachment: test patch

I think this should fix the crash (Stephen, sorry about the bug spam)

Try run
https://treeherder.mozilla.org/#/jobs?repo=try&revision=2c69167d2459

Comment 15

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

Also pushed without the fix for this bug to check if it fails on all platforms, etc.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=5d8c9bd800b5

Comment 16

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

Attachment: patch test

Comment 17

[Stephen A Pohl [:spohl]]

Comment on attachment 8587868 [details] [diff] [review]
patch test

Unfortunately, this seems to time out on B2G...

https://treeherder.mozilla.org/logviewer.html#?job_id=6233132&repo=try

Comment 18

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

Comment on attachment 8587868 [details] [diff] [review]
patch test

Because of the custom nsIUpdatePrompt and I disabled it in b2g in the patch as shown below

>diff --git a/toolkit/mozapps/update/tests/unit_aus_update/xpcshell.ini b/toolkit/mozapps/update/tests/unit_aus_update/xpcshell.ini
>--- a/toolkit/mozapps/update/tests/unit_aus_update/xpcshell.ini
>+++ b/toolkit/mozapps/update/tests/unit_aus_update/xpcshell.ini
>@@ -32,8 +32,11 @@ skip-if = toolkit != 'gonk'
> skip-if = toolkit == 'gonk'
> reason = custom nsIUpdatePrompt
> [uiOnlyAllowOneWindow.js]
> skip-if = toolkit == 'gonk'
> reason = custom nsIUpdatePrompt
> [uiUnsupportedAlreadyNotified.js]
> skip-if = toolkit == 'gonk'
> reason = custom nsIUpdatePrompt
>+[uiAutoPref.js]
>+skip-if = toolkit == 'gonk'
>+reason = custom nsIUpdatePrompt

Comment 19

[Stephen A Pohl [:spohl]]

Ah, sorry. Missed comment 10. Thanks!

Comment 20

[Stephen A Pohl [:spohl]]

Comment on attachment 8587868 [details] [diff] [review]
patch test

Review of attachment 8587868 [details] [diff] [review]:
-----------------------------------------------------------------

::: toolkit/mozapps/update/tests/unit_aus_update/uiAutoPref.js
@@ +87,5 @@
> +    return { getInterface: XPCOMUtils.generateQI([Ci.nsIDOMWindow]) };
> +  },
> +
> +  QueryInterface: XPCOMUtils.generateQI([Ci.nsIWindowMediator])
> +}

nit: add ';' after '}'

Comment 21

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

(In reply to Stephen Pohl [:spohl] from comment #20)
> Comment on attachment 8587868 [details] [diff] [review]
> patch test
> 
> Review of attachment 8587868 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: toolkit/mozapps/update/tests/unit_aus_update/uiAutoPref.js
> @@ +87,5 @@
> > +    return { getInterface: XPCOMUtils.generateQI([Ci.nsIDOMWindow]) };
> > +  },
> > +
> > +  QueryInterface: XPCOMUtils.generateQI([Ci.nsIWindowMediator])
> > +}
> 
> nit: add ';' after '}'
Forgot to do this. I'll fix that and other instances of the same in bug 1149276

Comment 22

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

Pushed test to fx-team
https://hg.mozilla.org/integration/fx-team/rev/cfabb51c4a30

Comment 23

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

I'll uplift the test to aurora later

Comment 24

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/cfabb51c4a30

Comment 25

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

Pushed test to mozilla-aurora
https://hg.mozilla.org/releases/mozilla-aurora/rev/70b67abc4699

Comment 26

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Test was failing on Aurora, backed it out at rstrong's request:

https://hg.mozilla.org/releases/mozilla-aurora/rev/85071beda936
https://treeherder.mozilla.org/logviewer.html#?job_id=749079&repo=mozilla-aurora

Comment 27

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

Attachment: aurora test patch

I'll land this tomorrow

Comment 28

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

Pushed fixed test patch to mozilla-aurora
https://hg.mozilla.org/releases/mozilla-aurora/rev/c4fb854ba5fe