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

Status: RESOLVED FIXED

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

Description

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

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.

Comment 1

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

fyi ehsan, marshall

Comment 2

[(no longer active)]

Naoki, did you mean to ask a question?

Comment 3

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

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?

Comment 4

[(no longer active)]

(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...

Comment 5

[Marshall Culpepper [:marshall_law]]

(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

Comment 6

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

Attachment: patch rev1

Just to make sure that this won't adversely affect gonk.

Comment 7

[Marshall Culpepper [:marshall_law]]

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 :)

Comment 8

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/01fbd5debd94

Comment 9

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

Pushed to fx-team
https://hg.mozilla.org/integration/fx-team/rev/01fbd5debd94

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/01fbd5debd94