812573 - hotfix to change update interval for existing pre-17 builds

Status: RESOLVED FIXED

(Firefox :: General, defect)

Description

[:Gavin Sharp [email: [e-mail]]]

In bug 803181 we made a tweak to the update download interval to speed up background update downloads. Assuming feedback from 17 is good, we should consider "backporting" that simple pref change to pre-17 builds via an add-on hotfix, to help users of older builds get updated faster.

Comment 1

[Alex Keybl [:akeybl]]

If we do this, it should probably be after we confirm there is no undesirable impact for FF17->FFnext(17.0.1, 18.0), or at the very least FF17.0beta->FF18.0beta. Perhaps I'm being too cautious - let me know what you all think.

Comment 2

[:Gavin Sharp [email: [e-mail]]]

(In reply to Alex Keybl [:akeybl] from comment #1)
> If we do this, it should probably be after we confirm there is no
> undesirable impact for FF17->FFnext(17.0.1, 18.0), or at the very least
> FF17.0beta->FF18.0beta. Perhaps I'm being too cautious - let me know what
> you all think.

No, I think that's right - that's what I was trying to suggest by "feedback from 17 is good", but you're right to point out that that actually means "feedback from the 17->17+1 upgrades".

Comment 3

[hannu.nyman]

Would the proposed change impact also the development channels?

I keeping wondering about the long tail in the nightly, aurora and beta channels. I assume that most users have automatic updates enabled (at least in aurora and nightly), but still a surprisingly large portion of users use older builds (even inside the 18 & 19 series). 

Based on a quick glance at https://metrics.mozilla.com/stats/firefox.shtml , it looks like 1/3 of the Aurora channel users have a build older than a week. And a large amount is still using pre-17 version. Taking into account the small user population of nightly and Aurora, that looks a bit wasteful from testing perspective.

I understand that this specific bug is targeting pre-17 builds, but you might at the same time review the current settings on the development channels.

Comment 4

[:Gavin Sharp [email: [e-mail]]]

The update download interval for Nightly (and Aurora) has been lower since 2009 (bug 477908) - this really doesn't apply to them. Bug 803181 only changed the pref on the release channel.

Comment 5

[:Gavin Sharp [email: [e-mail]]]

Hotfixes can reach Firefox versions >=10, as far as I know. Do we have enough users on those old releases (>=10, <latest) for this to be worth investing in?

Comment 7

[Jared Wein [:jaws] (please needinfo? me)]

(In reply to :Gavin Sharp (use [e-mail] for email) from comment #5)
> Hotfixes can reach Firefox versions >=10, as far as I know. Do we have
> enough users on those old releases (>=10, <latest) for this to be worth
> investing in?

Yes we do. From looking at the stats right now:

#1 version is 19 around 103m users today
#2 version is 18 around 6.5m users today
#3 version is 16 around 5.2m users today
#4 version is 12 around 5.2m users today
#5 version is 3.6 around 4.5m users today
#6 version is 17 around 4.1m users today
... and so on.

We should undoubtedly make this change for as many versions of Firefox as we can. We have users who are stuck on older versions and do not appear to be upgrading at all.

Comment 8

[Marco Bonardo [:mak]]

Many would benefit from this.

Though it won't be enough for everyone, many are unable to upgrade due to exotic reasons (mostly AV), I think we should also invest in a hotfix for bug 837300 for those.  I keep finding persons that are not notified updates at all.

Comment 9

[:Gavin Sharp [email: [e-mail]]]

OK, let's stop considering and start doing :) Who wants to take this?

Comment 10

[:Felipe Gomes (needinfo for replies!)]

I'll take it!

Comment 11

[:Felipe Gomes (needinfo for replies!)]

Attachment: Hotfix

Asking for general feedback on the following points before requesting review:

I was conservative and with this patch the hotfix only applies for:

- users who are on the "release" channel (this therefore excludes users from ESR)
- users who have both prefs being updated set to their default values
- wasn't sure which should be the minVersion so I set it between 13.0 and 16.*

prefs being updated:
app.update.interval -> 8 hours
app.update.download.backgroundInterval -> 60 seconds

Comment 12

[:Gavin Sharp [email: [e-mail]]]

(In reply to :Felipe Gomes from comment #11)
> - users who are on the "release" channel (this therefore excludes users from
> ESR)

What's the reasoning for this?

> - users who have both prefs being updated set to their default values

Makes sense.

> - wasn't sure which should be the minVersion so I set it between 13.0 and
> 16.*

10 is the first version to support hotfixes. Any reason not to use that?

Comment 13

[:Felipe Gomes (needinfo for replies!)]

Neither of these have very strong reasoning, I just picked some justifiable values to get more feedback on which ones they should actually be.

(In reply to :Gavin Sharp (use [e-mail] for email) from comment #12)
> (In reply to :Felipe Gomes from comment #11)
> > - users who are on the "release" channel (this therefore excludes users from
> > ESR)
> 
> What's the reasoning for this?

Because users on ESR are more likely to be managed by someone else who has more control over updates and will trigger than manually (at least in principle -- i was thinking of the target use case for ESR here).

> 
> > - wasn't sure which should be the minVersion so I set it between 13.0 and
> > 16.*
> 
> 10 is the first version to support hotfixes. Any reason not to use that?

I set it to 13 because that's around the time when silent updates landed (in 12). But no big reason to not go as far as 10, I think.

Comment 14

[Jared Wein [:jaws] (please needinfo? me)]

With more details to add to comment #7,

On 3/14 we had:
5.2m on v12
2.1m on v11
3m on v10
whereas we had 103m on v19 (latest).

Moving users on versions 10-12 to latest would help us 110%.

Even though our updates may not have been silent with those old versions, once the user gets updated they will be silent from then on. Anything we can do to help them over that hurdle would be good.

Comment 15

[:Felipe Gomes (needinfo for replies!)]

Attachment: Hotfix, take 2

This version of the hotfix applies to the following users: (all 3 criteria must be met):

- between 10.0 and 16.*
- with "app.update.channel" either being "release" or "esr"
- with default values for the prefs to be changed

And the change it does is:
sets app.update.interval to 28800 (8 hours)
sets app.update.download.backgroundInterval 60 (60 seconds)

Comment 16

[:Felipe Gomes (needinfo for replies!)]

Comment on attachment 729890 [details] [diff] [review]
Hotfix, take 2

Alex, see comment 15

Comment 17

[Matthew N. [:MattN]]

Comment on attachment 729890 [details] [diff] [review]
Hotfix, take 2

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

r- because I think the update interval is being set to the wrong value and I have some questions.

::: README
@@ +25,5 @@
>  v20121128.01 - Bug 816197 - Notify Firefox 17 users affected by Tab Mix Plus issues.
>  v20130211.01 - Bug 839239 - Disable pdf.js for Firefox 18 and 19 users in the 'release'
> +               channel in all platforms. NOTE: this hotfix was not released. To release
> +               it in the future, it needs to have its version id (folder name) bumped.
> +v20130322.01 - Bug 812573 - Increase update interval for pre-17 users in the 'release'

Nit: ESR is missing and technically the interval between checks is decreasing. You could reword this to get rid of the ambiguity.

::: v20120910.01/bootstrap.js
@@ +29,5 @@
>      return;
>    }
>  
>    try {
> +    Services.prefs.setIntPref(UPDATE_INTERVAL_PREF, 28800); // 8 hours

This should be 12 hours for release and beta, 8 hours for Aurora AFAICT.

@@ +47,5 @@
>  
>  /**
>   * @return boolean whether the hotfix applies to the application.
>   */
>  function shouldHotfixApp() {

What is stopping this from going to unofficial builds? Those have different default values.  It seems like the extensions.hotfix.* prefs probably belong in firefox-branding.js rather than firefox.js but since they aren't we may want to check whether we have an official build in the hotfix.

@@ +55,5 @@
>      return false;
>    }
>  
> +  // Only update users on the release channel.
> +  // Note: this excludes users on ESR.

Nit: This comment is outdated

@@ +58,5 @@
> +  // Only update users on the release channel.
> +  // Note: this excludes users on ESR.
> +  if (Services.prefs.getPrefType(UPDATE_CHANNEL_PREF) != Services.prefs.PREF_STRING ||
> +      (Services.prefs.getCharPref(UPDATE_CHANNEL_PREF) != "release" &&
> +       Services.prefs.getCharPref(UPDATE_CHANNEL_PREF) != "esr")) {

I personally think this hotfix should go to all channels (except maybe "default"). Having people on old Nightly/Aurora/Beta builds isn't useful and they can already disable application updates if they don't want to update. Note that Aurora has a different update interval.

::: v20120910.01/install.rdf
@@ +10,5 @@
>      <em:strictCompatibility>true</em:strictCompatibility>
>  
>      <!-- Front End MetaData -->
>      <em:name>Firefox Update Hotfix</em:name>
> +    <em:description>Increase update interval for existing pre-17 Firefox users.</em:description>

Nit: s/Increase/Decrease the/
Something like the follow is easier to understand IMO:
Decrease the update interval for Firefox 10 through 17.

Comment 18

[Matthew N. [:MattN]]

(In reply to Matthew N. [:MattN] from comment #17)
> What is stopping this from going to unofficial builds?

I filed bug 855148 to look into changing this in Firefox.

Comment 19

[:Felipe Gomes (needinfo for replies!)]

Attachment: Hotfi, take 3

After talking to Matt we arrived at:

- applying the hotfix for users of beta, esr, release and release-* (to include partner builds)
- only applies the hotfix if the app.update.url contains mozilla.org. This is the way we found to check for "official" branding as to not interfere with other possible deployments that wouldn't want this change.

And it sets the pref value of app.update.interval to 12 hours. It was incorrectly being set to 8 on the previous version of the patch, as 8 is only the update interval for Aurora but 12 is the right value for beta or release

Comment 20

[Matthew N. [:MattN]]

Comment on attachment 735972 [details] [diff] [review]
Hotfi, take 3

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

Thanks Felipe. r=me

::: README
@@ +26,5 @@
>  v20130211.01 - Bug 839239 - Disable pdf.js for Firefox 18 and 19 users in the 'release'
> +               channel in all platforms. NOTE: this hotfix was not released. To release
> +               it in the future, it needs to have its version id (folder name) bumped.
> +v20130322.01 - Bug 812573 - Decrease the update interval for pre-17 users of beta, esr
> +               and release channels (of official builds) in all platforms.

Nit: Replace "(of official builds)" with "using mozilla.org servers".

::: v20120910.01/bootstrap.js
@@ +61,2 @@
>    try {
> +    let channel = Services.prefs.getCharPref(UPDATE_CHANNEL_PREF);

Normally this check is done on the default branch, is there a reason you changed it to check the user branch?

@@ +61,3 @@
>    try {
> +    let channel = Services.prefs.getCharPref(UPDATE_CHANNEL_PREF);
> +    if (!/^beta$|^esr$|^release/.test(channel)) {

I think our release channels for partner builds normally have a hyphen after "release" but maybe this is not always the case.  If so, this could be:
!/^beta$|^esr$|^release-?/.test(channel)    (untested)

@@ +68,5 @@
>    }
>  
> +  // Only update prefs if the app updates against mozilla.org servers.
> +  // That is to check for official branding and avoid interfering with
> +  // other deployments that could be unprepared for this change.

Nit: I think the mentions of official branding are misleading because it's not as much about the branding and more about server load. I think an extension of the first line is enough:
// Only update prefs if the app updates against mozilla.org servers because they can handle the increased traffic.

Comment 21

[:Felipe Gomes (needinfo for replies!)]

Attachment: Hotfix, take 4

Final version using the defaultBranch for the pref check. Updated all nits, except:

> I think our release channels for partner builds normally have a hyphen after
> "release" but maybe this is not always the case.  If so, this could be:
> !/^beta$|^esr$|^release-?/.test(channel)    (untested)

I didn't update this one because it has the same effect as the previous one (it wouldn't prevent "release-" from matching, nor "releasefoo". I think it's not necessary to make it more complicated to account for that)

Comment 22

[:Felipe Gomes (needinfo for replies!)]

Attachment: Hotfix .xpi

Hotfix .xpi for testing

Comment 23

[:Felipe Gomes (needinfo for replies!)]

The hotfix should be ready to be tested

Comment 24

[:Felipe Gomes (needinfo for replies!)]

Comment on attachment 736060 [details]
Hotfix .xpi

Also flagging review from release-mgmt to get the ball rolling on testing this hotfix.

Comment 25

[u279076]

Assigning myself as QA Contact. I assume comment 19 is an accurate description of the use case for this hotfix?

Comment 26

[:Felipe Gomes (needinfo for replies!)]

(In reply to Anthony Hughes, Mozilla QA (:ashughes) from comment #25)
> Assigning myself as QA Contact. I assume comment 19 is an accurate
> description of the use case for this hotfix?

That is correct. Comment 15 has more details, some of it superseded by comment 19. But here it is all together:

- all platforms
- between 10.0 and 16.*
- app.update.channel is beta, esr, release, or release-*
- app.update.url must contain mozilla.org
- the prefs to be changed must have default values

With all the above met, the changes will then be:
sets app.update.interval to 43200 (12 hours)
sets app.update.download.backgroundInterval to 60 (60 seconds)

Comment 27

[u279076]

(In reply to :Felipe Gomes from comment #26)
> - app.update.channel is beta, esr, release, or release-*

Is release-* used somewhere in the wild? Do we have a real-world example of what * might be?

Comment 28

[:Felipe Gomes (needinfo for replies!)]

My understanding is that it's used for the partner builds, like release-yahoo or release-twitter

Comment 29

[Jared Wein [:jaws] (please needinfo? me)]

(In reply to :Felipe Gomes from comment #28)
> My understanding is that it's used for the partner builds, like
> release-yahoo or release-twitter

Yes, that is correct. Some other examples:
release-cck-euballot
release-cck-yandex
release-cck-mozillaonline
release-cck-yahoo

I was unable to determine what 'cck' stands for though :)

Comment 30

[u279076]

Thanks Felipe and Jared. I'll begin mocking up a mini-testplan to track testing this.

Comment 31

[Jared Wein [:jaws] (please needinfo? me)]

Ah, more info on what CCK is for those interested :P

https://wiki.mozilla.org/CCK

Comment 32

[Nick Thomas [:nthomas] (UTC+12)]

CC jakem from IT as a heads up on extra traffic.

ashughes, you can get some 16.0.2 partner repacks from http://ftp.mozilla.org/pub/mozilla.org/firefox/candidates/16.0.2-candidates/build1/partner-repacks/. Tomcat may be able to help with older releases if you need them.

re partner builds, the default pref branch is left with app.update.url being 'release', so what you have in the hotfix looks fine. At update query time the -cck-foo stuff is appended, see http://mxr.mozilla.org/mozilla-release/source/toolkit/content/UpdateChannel.jsm#28

Comment 33

[bhavana bajaj [:bajaj]]

(In reply to :Felipe Gomes from comment #24)
> Comment on attachment 736060 [details]
> Hotfix .xpi
> 
> Also flagging review from release-mgmt to get the ball rolling on testing
> this hotfix.

This was discussed in today's channel meeting and QA is on it . Once we have the needed verification by QA (mostly by next week as it involves testing on a lot of versions ) we should be good to get the .xpi signed and get the ball rolling.

Jakem, please let us know if there are any concern's related to the extra traffic here.Thanks !

Comment 34

[u279076]

Here is my draft testplan for qualifying this hotfix:
https://wiki.mozilla.org/QA/Desktop_Firefox/Test_Plans/Bug_812573

Felipe, can you please review it? I need to know if:
A) this provides sufficient test coverage for the hotfix
B) any information is incorrect or inaccurate
C) any information or test scenarios are missing and should be covered

Thank you

Comment 35

[Nick Thomas [:nthomas] (UTC+12)]

In https://wiki.mozilla.org/QA/Desktop_Firefox/Test_Plans/Bug_812573#Partner, no modification of the channel is necessary. They'll all say release in channel-prefs.js, but actually ask for updates with release-cck-<foo>.

Comment 36

[u279076]

Thanks for the clarification Nick, I've updated the testplan with that info.

We've made some progress on testing but we are still a long way off. One thing that we noticed is that the hotfix add-on does not appear in the add-ons manager, though it is applying the changes. Is this expected? Hotfixes in the past would show in the add-ons manager until the changes were applied and removed on the next restart.

Comment 37

[:Felipe Gomes (needinfo for replies!)]

(In reply to Anthony Hughes, Mozilla QA (:ashughes) from comment #36)
> We've made some progress on testing but we are still a long way off. One
> thing that we noticed is that the hotfix add-on does not appear in the
> add-ons manager, though it is applying the changes. Is this expected?
> Hotfixes in the past would show in the add-ons manager until the changes
> were applied and removed on the next restart.

Yeah this is expected, the hotfix immediately uninstalls itself after running.

I'm looking into the test plan doc

Comment 38

[:Felipe Gomes (needinfo for replies!)]

I thought of one extra test, but it doesn't really matter and it's impossible to verify:

Firefox 18+ with compatibility override, which will make the hotfix install, but it should not alter any prefs.

However, the values that the hotfix would set are the already-default values in 18+. Perhaps verifying that the values weren't changed to something else, but I don't see how they would.


And in the Expected results section, the hotfix does not need a Firefox restart to uninstall. The net result is that it won't be seem in the add-ons manager, as it runs and uninstalls itself.

Comment 39

[u279076]

We've wrapped up the testing and a couple of issues were found:

> Hotfix appears in add-ons manager on subsequent installs.
The steps to reproduce this are:
1. Install the hotfix
2. Open the addons manager > no hotfix visible
3. Keep the addons manager open and install the hotfix again
4. Hotfix appears in the addons manager disabled
5. Reload the addons manager tab > hotfix is removed

I don't think this issue is concerning since the prefs are still changed. It appears to be an edge case which does not impact browser functionality, does not break updates, and is trivial to work around. 

> There is no checkbox in the Firefox installer to set Firefox as the default browser
> http://www.screencast.com/users/AlexandraLucinet/folders/Default/media/25c9dd3f-7623-4bb0-a18a-94405763803b
I was not able to personally reproduce this but I don't see this as a hotfix bug. If anything it's a bug in the Firefox installer code. I've asked the person who reproduced this to file a bug against the Firefox Installer.

> Updates seem to be failing to be found under some circumstances
> http://www.screencast.com/users/AlexandraLucinet/folders/Default/media/8634e7e6-c93c-4ac7-b6be-e42f8e5b087d
I was not able to personally reproduce this but it does not appear that the hotfix is involved at all in this behaviour. I've asked the person who reproduced this to file a Release Engineering bug to investigate this further.

In short, the only issue found which I can tie to the hotfix is so trivial I don't think it's worth worrying about. Felipe, if it's trivial to fix then I suppose we should wait for your fix, otherwise I won't block on it.

Bhavana, Felipe, what do you think?

Comment 40

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

(In reply to Anthony Hughes, Mozilla QA (:ashughes) from comment #39)
<snip>
> > There is no checkbox in the Firefox installer to set Firefox as the default browser
> > http://www.screencast.com/users/AlexandraLucinet/folders/Default/media/25c9dd3f-7623-4bb0-a18a-94405763803b
> I was not able to personally reproduce this but I don't see this as a hotfix
> bug. If anything it's a bug in the Firefox installer code. I've asked the
> person who reproduced this to file a bug against the Firefox Installer.
There are a couple of pre-conditions that are checked before we display the option to set as the default browser and I highly suspect the person who reproduced this behavior has one or more of those conditions.

> 
> > Updates seem to be failing to be found under some circumstances
> > http://www.screencast.com/users/AlexandraLucinet/folders/Default/media/8634e7e6-c93c-4ac7-b6be-e42f8e5b087d
> I was not able to personally reproduce this but it does not appear that the
> hotfix is involved at all in this behaviour. I've asked the person who
> reproduced this to file a Release Engineering bug to investigate this
> further.
Note that the UI showed "Updates available at" etc. which indicates an update was found but could not be applied... likely due to not all of the current requirements to apply an update being met.

Comment 41

[u279076]

(In reply to Robert Strong [:rstrong] (do not email) from comment #40)
> There are a couple of pre-conditions that are checked before we display the
> option to set as the default browser and I highly suspect the person who
> reproduced this behavior has one or more of those conditions.

Upon further investigation this was indeed the case. Thanks Rob.

> Note that the UI showed "Updates available at" etc. which indicates an
> update was found but could not be applied... likely due to not all of the
> current requirements to apply an update being met.

Turns out this was only reproducible on one machine by one tester. Upon further investigation we identified it as a local permissions issue. 

In short, neither of these issues are Firefox bugs. All we need now is a decision on whether to fix the "double install" issue or move forward with the hotfix as is.

Thanks again Rob for your analysis.

Comment 42

[:Felipe Gomes (needinfo for replies!)]

Thanks Anthony and Rob for clarifications into those possible issues. The "double install" issue also seems a minor thing and likely unrelated to this specific hotfix (an edge case for restartless add-ons in the add-ons manager).

So I think we're good to go ahead. I pushed the hotfix to the repo:
https://hg.mozilla.org/releases/firefox-hotfixes/rev/10b315fb3401

And will file a bug to get it signed

Comment 43

[:Felipe Gomes (needinfo for replies!)]

(In reply to :Felipe Gomes from comment #42)
> And will file a bug to get it signed

Bug 865873