Closed Bug 551709 Opened 10 years ago Closed 10 years ago

Auto-update add-ons

Categories

(Firefox for Android Graveyard :: General, defect)

x86
Linux
defect
Not set

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: mfinkle, Assigned: mfinkle)

Details

Attachments

(1 file, 2 obsolete files)

Attached patch WIP 1 (obsolete) — Splinter Review
Add-ons installed in Firefox for Mobile should support auto-upgrading. We don't necessarily need many mindless prompts, telling the user that updates are available and would you like to update. AMO seems OK with just silently performing the update and then we will notify.
Attached patch patch (obsolete) — Splinter Review
This patch adds auto-update support. It basically moves the manual update code into a component. That component is driven using the nsIUpdateTimerManager to check for updates. Currently, the update manager is set to check every 5 minutes, at which time we check to see if the add-ons should be checked. Add-ons are checked once a day (86400 seconds).

This patch leaves the manual "Update" in the Add-ons Manager. We can remove it once we decide the auto-system is working OK.

To test this, I used a simple add-on on my people page:
http://people.mozilla.com/~mfinkle/fennec/addons/

* Install fennec-sample-a.xpi
* Force an update check or wait to the auto-update
* You should get a restart prompt and a note that "Fennec Sample" was updated to version 2.0

You can change the timeout for auto-update checks:
* Change "app.update.timer" to 30000  (30 secs)
* Change "extensions.autoupdate.interval" to 60  (60 secs)

You should see and update in a minute
Assignee: nobody → mark.finkle
Attachment #431891 - Attachment is obsolete: true
Attachment #432148 - Flags: review?(21)
Attached patch patch 2Splinter Review
With less debug output
Attachment #432148 - Attachment is obsolete: true
Attachment #432150 - Flags: review?(21)
Attachment #432148 - Flags: review?(21)
Comment on attachment 432150 [details] [diff] [review]
patch 2

Dave, could you give this a quick look over? I added new preferences for auto-updating instead of reusing the existing prefs. I also fire off notifs so the UI can update if needed.


Any feedback is welcome.
Attachment #432150 - Flags: feedback?(dtownsend)
Flags: in-litmus?
We should also make sure do not auto-update when using a slow and/or potentially expensive connection. It appears to be feasible using libiconic (see http://maemo.org/api_refs/4.0/libconic/coniciap_8h.html)
Comment on attachment 432150 [details] [diff] [review]
patch 2

>diff --git a/app/mobile.js b/app/mobile.js
>--- a/app/mobile.js
>+++ b/app/mobile.js
> /* app update prefs */
>+pref("app.update.timer", 300000); // milliseconds (5 mins)
>+
> #ifdef MOZ_UPDATER
> pref("app.update.auto", true);
> pref("app.update.channel", "@MOZ_UPDATE_CHANNEL@");
>-pref("app.update.timer", 600000);
> pref("app.update.mode", 1);
> pref("app.update.silent", false);

Is there any reason for this pref to live outside the preprocessing MOZ_UPDATER instruction?

>diff --git a/components/AddonUpdateService.js b/components/AddonUpdateService.js
>new file mode 100644
>--- /dev/null
>+++ b/components/AddonUpdateService.js


>+  notify: function aus_notify(aTimer) {
>+    if (aTimer && !getPref("getBoolPref", PREF_ADDON_UPDATE_ENABLED, true))
>+      return;
>+
>+    // If we already auto-upgraded and installed new versions, ignore this check
>+    if (gNeedsRestart)
>+      return;
>+

If I understand well this means that once we have do an auto-update we're not going to do another one, even if the user install an other outdated addon and click on the "Update" button? This is not very important because that's an edge case but I just want to be sure to understand.


Code looks good otherwise and works for me.
r+ with questions answered.
Attachment #432150 - Flags: review?(21) → review+
(In reply to comment #5)
> >diff --git a/app/mobile.js b/app/mobile.js
> >+pref("app.update.timer", 300000); // milliseconds (5 mins)
> >+
> > #ifdef MOZ_UPDATER
> 
> Is there any reason for this pref to live outside the preprocessing MOZ_UPDATER

MOZ_UPDATER is defined for updating the entire application, something we only do on WinMo. "app.update.timer" (confusing name) is used to control the update timer - which controls many timers, not just app update. The timer controls the add-on update check too.

> >diff --git a/components/AddonUpdateService.js b/components/AddonUpdateService.js
> 
> >+  notify: function aus_notify(aTimer) {
> >+    if (aTimer && !getPref("getBoolPref", PREF_ADDON_UPDATE_ENABLED, true))
> >+      return;
> >+
> >+    // If we already auto-upgraded and installed new versions, ignore this check
> >+    if (gNeedsRestart)
> >+      return;

> If I understand well this means that once we have do an auto-update we're not
> going to do another one, even if the user install an other outdated addon and
> click on the "Update" button? This is not very important because that's an edge
> case but I just want to be sure to understand.

You are right. I only want to block timer-based auto-updates. Manual update checks should still happen. I'll update the code.

Keep in mind, installing any add-on requires a restart (currently) and so it's not possible to install an add-on and get an update for the add-on without restarting first anyway.
(In reply to comment #4)
> We should also make sure do not auto-update when using a slow and/or
> potentially expensive connection. It appears to be feasible using libiconic
> (see http://maemo.org/api_refs/4.0/libconic/coniciap_8h.html)

Hmm, I'd love to find a way to expose this information
pushed:
http://hg.mozilla.org/mobile-browser/rev/e6085a7a0f4b

with these tweaks:
* Removed the manual "Update" button, the DTD string and the JS code
* Changed app.update.timer to 1 minute

Filed bug 552879 to add wifi detection
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
I've tried the following scenarios and can't get any hint of a add-on update from the add-ons manager using the sample xpi from Mark's people dircetory (mentioned in comment #1)

1.9.2 and trunk maemo builds:

1. app.update.timer = default (60000)
2. app.update.timer = 10000

Has this worked for anyone else on today's nightly build?
I got it to work fairly quickly by only changing:

extensions.autoupdate.interval = 60

That should check for updates every 60 secs. If you only change app.update.timer, you still need to wait a day.
verified FIXED On builds:

Mozilla/5.0 (X11; U; Linux armv7l; Nokia N900; en-US; rv:1.9.2.2pre) Gecko/20100318 Namoroka/3.6.2pre Fennec/1.1a2pre

and

Mozilla/5.0 (X11; U; Linux armv6l; en-US; rv:1.9.3a3pre) Gecko/20100318 Namoroka/3.7a3pre Fennec/1.1a2pre

litmus testcase https://litmus.mozilla.org/show_test.cgi?id=7133 updated to regression test this new feature
Status: RESOLVED → VERIFIED
Flags: in-litmus? → in-litmus+
Attachment #432150 - Flags: feedback?(dtownsend)
You need to log in before you can comment on or make changes to this bug.