getCanStageUpdates has incorrect checks for Windows and B2G

RESOLVED FIXED in Firefox 38

Status

()

Toolkit
Application Update
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: rstrong, Assigned: rstrong)

Tracking

Trunk
mozilla40
x86_64
Windows 8.1
Points:
---

Firefox Tracking Flags

(firefox37 wontfix, firefox38+ verified, firefox39+ fixed, firefox40 fixed, firefox-esr31 wontfix, firefox-esr38 verified)

Details

Attachments

(4 attachments)

When B2G added staging in bug 764684 the code that was added used the Windows path where it checks if the maintenance service pref is set to true when all it should check for is whether staging is enabled.

For Windows it checks if the maintenance service pref is enabled but it doesn't check if the maintenance service is installed.
Created attachment 8592619 [details] [diff] [review]
patch rev1

I suspect this is the cause of the large number of staging errors I am seeing in telemetry.
Attachment #8592619 - Flags: review?(netzen)
Attachment #8592619 - Flags: review?(netzen) → review?(spohl.mozilla.bugs)
status-firefox37: --- → wontfix
status-firefox38: --- → affected
status-firefox39: --- → affected
status-firefox-esr31: --- → wontfix
status-firefox-esr38: --- → affected
Comment on attachment 8592619 [details] [diff] [review]
patch rev1

Brian, I'd also like your input on this.
Attachment #8592619 - Flags: review?(netzen)
Thought I screwed up the patch when I hadn't... new try push
https://treeherder.mozilla.org/#/jobs?repo=try&revision=9f33e0170b93
Attachment #8592619 - Flags: review?(spohl.mozilla.bugs) → review+
Comment on attachment 8592619 [details] [diff] [review]
patch rev1

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

::: toolkit/mozapps/update/nsUpdateService.js
@@ -651,5 @@
>    // For Gonk, the updater will remount the /system partition to move staged
> -  // files into place and it uses the app.update.service.enabled preference for
> -  // this purpose.
> -  if (AppConstants.platform == "win" || AppConstants.platform == "gonk") {
> -    if (getPref("getBoolPref", PREF_APP_UPDATE_SERVICE_ENABLED, false)) {

whoa strange
Attachment #8592619 - Flags: review?(netzen) → review+
Pushed to fx-team
https://hg.mozilla.org/integration/fx-team/rev/3cb6adf9dc71
Target Milestone: --- → mozilla40

Comment 7

3 years ago
https://hg.mozilla.org/mozilla-central/rev/3cb6adf9dc71
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
status-firefox40: affected → fixed
Resolution: --- → FIXED
[Tracking Requested - why for this release]:
This is in support of the update orphaning project. This bug will cause Windows users without the maintenance service installed and don't have write access to the install directory to try to stage updates when they can't. There is also a large number of staging failures in telemetry that this bug is likely at the least partially if not entirely responsible for.
tracking-firefox38: --- → ?
tracking-firefox39: --- → ?
Robert, is it possible to have an uplift request to aurora & beta? Thanks
tracking-firefox38: ? → +
tracking-firefox39: ? → +
Flags: needinfo?(robert.strong.bugs)
Created attachment 8593972 [details] [diff] [review]
aurora and beta patch

Approval Request Comment
[Feature/regressing bug #]: bug 764684 and bug 757965
[User impact if declined]: the users that don't have the maintenance service installed and don't have write access to the install directory will have update staging errors.
[Describe test coverage new/current, TreeHerder]: Landed on m-c and tested via try and locally.
[Risks and why]: Minimal. This makes it so the code takes the original update process path prior to staging being implemented.
[String/UUID change made/needed]: None
Flags: needinfo?(robert.strong.bugs)
Attachment #8593972 - Flags: review+
Attachment #8593972 - Flags: approval-mozilla-beta?
Attachment #8593972 - Flags: approval-mozilla-aurora?
Comment on attachment 8593972 [details] [diff] [review]
aurora and beta patch

Should be in 38 beta 6.
Attachment #8593972 - Flags: approval-mozilla-beta?
Attachment #8593972 - Flags: approval-mozilla-beta+
Attachment #8593972 - Flags: approval-mozilla-aurora?
Attachment #8593972 - Flags: approval-mozilla-aurora+
Created attachment 8594134 [details] [diff] [review]
revert gonk

This reverts the behavior for gonk back to the way it was before due to bug 1155704
Attachment #8594134 - Flags: review?(spohl.mozilla.bugs)
Attachment #8594134 - Flags: review?(spohl.mozilla.bugs) → review+
Created attachment 8594152 [details] [diff] [review]
updated branch patch as pushed without gonk changes
Pushed the followup to mozilla-central
https://hg.mozilla.org/mozilla-central/rev/fff4d44e1f41

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

I'll push to beta soon
status-firefox39: affected → fixed
status-firefox-esr38: affected → fixed
Pushed to mozilla-beta
https://hg.mozilla.org/releases/mozilla-beta/rev/86d3b1103197
status-firefox38: affected → fixed
It looks like this might have fixed the vast majority of staging STATE_FAILED though STATE_PENDING doesn't seem to have decreased. I'll look at it again in a few more days.
(In reply to Robert Strong [:rstrong] (use needinfo to contact me) from comment #18)
> It looks like this might have fixed the vast majority of staging
> STATE_FAILED though STATE_PENDING doesn't seem to have decreased. I'll look
> at it again in a few more days.
Scratch that, the reduction in failures has to do with bug 1157233 popping up on days when there are multiple nightlys built and then going away on days when there is only one nightly built. :(
Robert, could you please provide some detailed steps to verify this on ESR 38? It would really help us save precious time.
Flags: needinfo?(robert.strong.bugs)
On Win 7 with UAC turned on install under Program Files.
Uninstall the Mozilla Maintenance Service
Verify that the app.update.service.enabled preference is true
Set the app.update.log preference to true
Set the devtools.errorconsole.enabled preference to true
Install an update using the about dialog

Without this patch it should fail the staging of the update (possibly a UAC dialog will be displayed) and with the patch staging should not be attempted. There should be messages in the error console (tools -> web devloper -> error console) that staging failed without the patch and with the patch there should be messages that staging was not attempted.
Flags: needinfo?(robert.strong.bugs)
I've tested with Firefox 38 Beta 2 (buildID: 20150406174117) and I have the following message in Error Console: "Notifying observers that the update was staged".

I've tested with Firefox 38.0 ESR (buildID: 20150505103531) and I have the following message in Error Console: "Unable to stage updates". 

It is expected?
Flags: needinfo?(robert.strong.bugs)
That is the expected logging. Thanks!
Flags: needinfo?(robert.strong.bugs)
Verified fixed on Firefox 38 (buildID: 20150503173159).
status-firefox38: fixed → verified
status-firefox-esr38: fixed → verified
You need to log in before you can comment on or make changes to this bug.