757885 - Background updates sit on applying... forever if the service preference is turned off

Status: RESOLVED FIXED

(Toolkit :: Application Update, defect)

Description

[Brian R. Bondy [:bbondy]]

Steps to reproduce:
1. Download a build that is not the newest

2. Go to Tools | Options -> Advanced, Uncheck the option: Use a background service to install updates.

3. Then go to the about dialog and you should see it sitting on applying... forever.

Expected behavior:
The update should finish and it shouldn't sit on applying... forever.


Note: I actually reproduced this by maxing out the error count to 10, in which case the service option turns off automatically.  But I think it can be reproduced by manually turning the option off. 

I did see someone on a forum with this same issue, they probably don't have the service installed or else have it disabled.

Comment 2

[(no longer active)]

This is the code that is being hit here.  http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/update/updater/updater.cpp#2255

We should check usingService there as well.

Comment 3

[(no longer active)]

I spoke too soon.  This is what really happens: <http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/update/updater/updater.cpp#2417>

This is by design, since we don't want to trigger a UAC dialog when the user is using the browser.  We should just make the refreshUpdateStatus method smarter to not clear the updates directory if the status file is set to pending, as that indicates that the updater is willing to fall back to non-background updates.

Comment 4

[(no longer active)]

Attachment: Patch (v1)

Comment 5

[(no longer active)]

This patch seems to fix the issue for me locally.  I'm getting a cert verification error when the update falls back to the UAC prompt, but that's fine, since my build is unsigned, and this bug is all about us not falling back to the UAC prompt!  :-)

Comment 6

[(no longer active)]

Pushed to the Oak branch to trigger nightlies for testing.

Comment 7

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

Comment on attachment 626522 [details] [diff] [review]
Patch (v1)

So, in this case the download was successful but the staging didn't succeed.

Per bug 307181 comment #160 this should fallback to the current process and let that process handle the error condition if one still exists. Correct?

Comment 8

[(no longer active)]

(In reply to Robert Strong [:rstrong] (do not email) from comment #7)
> Comment on attachment 626522 [details] [diff] [review]
> Patch (v1)
> 
> So, in this case the download was successful but the staging didn't succeed.
> 
> Per bug 307181 comment #160 this should fallback to the current process and
> let that process handle the error condition if one still exists. Correct?

I'm not sure if I understand this comment.  Bug 307181 comment 160 was about the wizard page.  And this patch makes sure that the fallback path acts correctly if there's an update error.

Do you have any specific concerns about this patch since you minused it?

Comment 9

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

So, if the update downloaded correctly (passed the hash check, etc.) and it fails staging you ask the user to download and manually install for the about dialog but not the wizard. There shouldn't be any difference with the update flow of these two. I want them both to be the same so the update flow is more deterministic and if there is a case where the user is able to update using the current update flow but is not when staging the update they are still able to update. I know that there aren't any cases of the above that we know of at this time but there were cases we didn't know about until after this landed as well.

Comment 10

[(no longer active)]

OK, I see.  New patch coming up.

Comment 11

[(no longer active)]

Attachment: Patch (v2)

Comment 12

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

Comment on attachment 626673 [details] [diff] [review]
Patch (v2)

Fairly certain this is correct and as long as manual testing shows that it is r=me

Comment 13

[(no longer active)]

I'll hold off on landing this until we verify that this works on Oak.

Comment 14

[(no longer active)]

https://hg.mozilla.org/mozilla-central/rev/26d651834b0e