Closed Bug 1155851 Opened 7 years ago Closed 7 years ago

Gonk incorrectly uses the app.update.service.enabled preference

Categories

(Firefox OS Graveyard :: General, defect, P1)

ARM
Gonk (Firefox OS)
defect

Tracking

(firefox40 fixed)

RESOLVED FIXED
Tracking Status
firefox40 --- fixed

People

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

References

Details

(Whiteboard: [LOE:S])

Attachments

(1 file)

This was introduced in bug 764684.

Removing it caused bug 1155704

Instead, the app.update.staging.enabled pref should be used to enable and disable staging and the app.update.service.enabled preference should be removed from gonk.
fyi ehsan, marshall
Flags: needinfo?(marshall)
Flags: needinfo?(ehsan)
Naoki, did you mean to ask a question?
Flags: needinfo?(ehsan)
In bug 764684 the following change was made in attachment #648046 [details] [diff] [review]
-#ifdef XP_WIN
+#ifdef SKIP_STAGE_UPDATES_TEST
   if (getPref("getBoolPref", PREF_APP_UPDATE_SERVICE_ENABLED, false)) {
     // No need to perform directory write checks, the maintenance service will
     // be able to write to all directories.
     LOG("gCanStageUpdates - able to stage updates because we'll use the service");
     return true;
   }
 #endif

This makes it so gonk will return true when checking if it can stage updates only when the app.update.service.enabled preference is true. The app.update.service.enabled preference is to determine whether the maintenance service should be used to stage updates and the maintenance service is windows only and isn't available on gonk. As stated in comment #0 the app.update.staging.enabled preference should be used to enable / disable staging.

According to bug 1155704 removing that check broke gonk.

Do you know why gonk made use of the app.update.service.enabled in bug 764684?
(In reply to Robert Strong [:rstrong] (use needinfo to contact me) from comment #3)
> Do you know why gonk made use of the app.update.service.enabled in bug
> 764684?

That sounds like a mistake...
(In reply to :Ehsan Akhgari (not reading bugmail, needinfo? me!) from comment #4)
> (In reply to Robert Strong [:rstrong] (use needinfo to contact me) from
> comment #3)
> > Do you know why gonk made use of the app.update.service.enabled in bug
> > 764684?
> 
> That sounds like a mistake...

Yeah, it's been a while since I looked at this code, but I'd have to agree w/ Ehsan. The original implementation probably just used the wrong preference
Flags: needinfo?(marshall)
Attached patch patch rev1Splinter Review
Just to make sure that this won't adversely affect gonk.
Assignee: nobody → robert.strong.bugs
Status: NEW → ASSIGNED
Attachment #8594917 - Flags: review?(marshall)
Comment on attachment 8594917 [details] [diff] [review]
patch rev1

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

Looks like this should remove the dependency on the 'service.enabled' property. I'm not 100% sure what behavior we'll see when we changed to the 'staging.enabled' property, but that is left as an exercise for another bug I suppose :)
Attachment #8594917 - Flags: review?(marshall) → review+
https://hg.mozilla.org/mozilla-central/rev/01fbd5debd94
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Duplicate of this bug: 1155853
You need to log in before you can comment on or make changes to this bug.