Closed Bug 1154591 Opened 5 years ago Closed 5 years ago

getCanStageUpdates has incorrect checks for Windows and B2G

Categories

(Toolkit :: Application Update, defect)

x86_64
Windows 8.1
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla40
Tracking Status
firefox37 --- wontfix
firefox38 + verified
firefox39 + fixed
firefox40 --- fixed
firefox-esr31 --- wontfix
firefox-esr38 --- verified

People

(Reporter: robert.strong.bugs, Assigned: robert.strong.bugs)

References

Details

Attachments

(4 files)

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.
Attached patch patch rev1Splinter Review
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)
Comment on attachment 8592619 [details] [diff] [review]
patch rev1

Brian, I'd also like your input on this.
Attachment #8592619 - Flags: review?(netzen)
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+
https://hg.mozilla.org/mozilla-central/rev/3cb6adf9dc71
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
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.
Robert, is it possible to have an uplift request to aurora & beta? Thanks
Flags: needinfo?(robert.strong.bugs)
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+
Attached patch revert gonkSplinter Review
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+
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).
You need to log in before you can comment on or make changes to this bug.