1350064 - System add-ons deployed with a build do not update an existing system add-on that was pushed.

Status: VERIFIED FIXED

(Toolkit :: Add-ons Manager, defect)

Description

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

I think we figured out one of the problems with the aushelper system add-on success rate. Turns out that when a Windows 51.0.1 Firefox client gets the aushelper 1.5 system add-on pushed to them and then updates to 52.0.1 that the aushelper 2.0 system add-on that comes with 52.0.1 is not successfully installed and they continue running the aushelper 1.5 system add-on.

Not sure if there are other cases but this

Comment 1

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

BTW: running a 52.0.1 build using a profile that doesn't have the aushelper 1.5 system add-on installed (e.g. received it as a push) does install the aushelper 2.0 system add-on that was included in the build.

Comment 2

[Chris H-C :chutten]

My query against longitudinal[1] suggests that about 1 in 7 Windows clients on 52.x are still running the old aushelper version.

[1]: https://sql.telemetry.mozilla.org/queries/3850/source

Comment 3

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

(In reply to Robert Strong [:rstrong] (use needinfo to contact me) from comment #0)
> I think we figured out one of the problems with the aushelper system add-on
> success rate. Turns out that when a Windows 51.0.1 Firefox client gets the
> aushelper 1.5 system add-on pushed to them and then updates to 52.0.1 that
> the aushelper 2.0 system add-on that comes with 52.0.1 is not successfully
> installed and they continue running the aushelper 1.5 system add-on.
> 
> Not sure if there are other cases but this
s/Not sure if there are other cases but this/Not sure if there are other cases of this/

Comment 4

[Chris H-C :chutten]

Ack. Only 1 in 7 Windows clients on 52.x are running the _NEW_ aushelper version. Most are stuck on the old one.

Comment 5

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

BTW: Those are likely new installations.

Comment 6

[Robert Helmer [:rhelmer]]

The current set of updates is stored in pref extensions.systemAddonSet - this is supposed to be reset when the application version changes.

Comment 7

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

Which it wasn't for me and it appears that it isn't getting reset for the majority of 52.0.1 users as well. From my profile that I upgraded from 51.0.1 to 52.0.1

{"schema":1,"directory":"{1dfafdf0-3361-4735-a240-466a1dc48b71}","addons":{"[e-mail]":{"version":"1.5"},"[e-mail]":{"version":"1.0"},"[e-mail]":{"version":"1.3"},"[e-mail]":{"version":"1.0"}}}

Comment 8

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

(In reply to Robert Strong [:rstrong] (use needinfo to contact me) from comment #5)
> BTW: Those are likely new installations.
I should have said that the one in seven that are reporting the new aushelper which is aushelper 2.0 are likely new installations (e.g. new profile) since 52.0.1 installed the aushelper 2.0 system add-on for me when I didn't have the aushelper 1.5 system add-on installed.

Comment 9

[Robert Helmer [:rhelmer]]

As a workaround while I look into this, could we serve aushelper 2.0 to 52.0.1 users?

Once we figure out what's going on we can ship a fix and get it uplifted so reverting to the built-in works correctly on the next app update.

Comment 10

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

I sent an email to ddurst and felipe suggesting that we push an aushelper 2.5 to 52.x and possibly future versions as well. I'd prefer going with a higher version so we can differentiate the clients in telemetry. As soon as we have a chance to discuss it I'll let you know what we'll do.

Comment 11

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

(In reply to Robert Strong [:rstrong] (use needinfo to contact me) from comment #10)
> I sent an email to ddurst and felipe suggesting that we push an aushelper
> 2.5 to 52.x and possibly future versions as well. I'd prefer going with a
> higher version so we can differentiate the clients in telemetry. As soon as
> we have a chance to discuss it I'll let you know what we'll do.
For clarity, I'd prefer going with a higher *add-on* version so we can differentiate the clients in telemetry.

Comment 12

[Robert Helmer [:rhelmer]]

sgtm. I can reproduce this, so I suspect it's a regression shipped with 52.

We have a test for this in http://searchfox.org/mozilla-central/rev/c48398abd9f0f074c69f2223260939e30e8f99a8/toolkit/mozapps/extensions/test/xpcshell/test_system_reset.js so I am going to start by trying to trigger this there, and make sure it's not something Windows-specific.

Comment 13

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

Since the test didn't catch it perhaps this bug has been there for longer and might explain some of the low uptake in 47.0.2.

Comment 14

[David Durst [:ddurst]]

I didn't comment on the email, but the 2.5 plan has a thumbs-up from rstrong, felipe, and me, so let's do.

Comment 15

[Robert Helmer [:rhelmer]]

Attachment: Bug 1350064 - test that updated system add-ons are reset on app update

Review commit: https://reviewboard.mozilla.org/r/123312/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/123312/

Comment 16

[Robert Helmer [:rhelmer]]

(In reply to Robert Strong [:rstrong] (use needinfo to contact me) from comment #13)
> Since the test didn't catch it perhaps this bug has been there for longer
> and might explain some of the low uptake in 47.0.2.

It's possible, but I suspect it's not the case since the reset path was much simpler before... certainly possible though. I think we would've seen this behavior in telemetry at least, we should still see a bunch of old versions hanging around.

I'll grab an old build and confirm though instead of speculating.

Comment 17

[Robert Helmer [:rhelmer]]

I'll request uplift once we get this landed - that'll get it fixed for good on the next app update.

In the meantime, we can work around this using the update mechanism - I'll take a look at what's new in http://searchfox.org/mozilla-central/source/browser/extensions that got updates in the 51.* cycle and see what's impacted.

Comment 18

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

Filed bug 1350186 to create the aushelper system add-on that will be pushed to 52.* clients.

Comment 19

[Andrew Swan [:aswan]]

Comment on attachment 8850796 [details]
Bug 1350064 - test that updated system add-ons are reset on app update

https://reviewboard.mozilla.org/r/123312/#review125698

::: commit-message-20018:1
(Diff revision 1)
> +Bug 1350064 - test that updates system add-ons are reset on app update r?aswan

updates -> updated

Comment 20

[Robert Helmer [:rhelmer]]

Comment on attachment 8850796 [details]
Bug 1350064 - test that updated system add-ons are reset on app update

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/123312/diff/1-2/

Comment 21

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/45b68fe4d4be
test that updated system add-ons are reset on app update r=aswan

Comment 22

[Robert Helmer [:rhelmer]]

Attachment: uplift request

Approval Request Comment
[Feature/Bug causing the regression]: bug 1204156
[User impact if declined]: system add-on updates are not cleared on app update, causing extra downloads for users and hassle for system add-on releases
[Is this code covered by automated tests?]: yes
[Has the fix been verified in Nightly?]: no
[Needs manual test from QE? If yes, steps to reproduce]: steps to repro would be to ensure that 51.0.1 has updated system add-ons and then to upgrade to 52.0.1 and observe that updates are still present in about:support (the bug is that aushelper would still be using the 1.0 update served to 51.0.1, rather than the built-in 2.0 version)
[List of other uplifts needed for the feature/fix]: none
[Is the change risky?]: no
[Why is the change risky/not risky?]: trivial/obvious fix, easy to test for
[String changes made/needed]: none

Comment 23

[Robert Helmer [:rhelmer]]

I'd expect this to be a ridealong if uplifted to release as we can work around it in the meantime and it should not cause any harm.

Comment 24

[Iris Hsiao [:ihsiao]]

sorry had to back this out for eslint failure in test_system_reset.js like https://treeherder.mozilla.org/logviewer.html#?job_id=86114398&repo=autoland&lineNumber=4374

https://hg.mozilla.org/integration/autoland/rev/3368df04e574

Comment 25

[Robert Helmer [:rhelmer]]

Comment on attachment 8850796 [details]
Bug 1350064 - test that updated system add-ons are reset on app update

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/123312/diff/2-3/

Comment 26

[Robert Helmer [:rhelmer]]

(In reply to Iris Hsiao [:ihsiao] from comment #24)
> sorry had to back this out for eslint failure in test_system_reset.js like
> https://treeherder.mozilla.org/logviewer.
> html#?job_id=86114398&repo=autoland&lineNumber=4374
> 
> https://hg.mozilla.org/integration/autoland/rev/3368df04e574

Thanks! I pushed the whitespace change to fix this, doing a try run to make sure it sticks this time:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=5b1df9bf5a4c

Comment 27

[Robert Helmer [:rhelmer]]

I took a small sample of the longitudinal table for the last 7 days to get an idea of what system add-ons are active on 52.0.1:

https://sql.telemetry.mozilla.org/queries/2678/source#table

The only add-ons affected by this other than aushelper are these two which are active (their install.rdf has 52.*):

* [e-mail] v1.0
* [e-mail] v1.3

Cory, due to this bug these two were left active in profiles of 51.* users upgrading to 52.*

I've tested that any leftover system add-ons like this that are not part of the update set
get removed restartlessly the next time we push an update, so that is working correctly and 
should clean up any lingering system add-on updates.

It looks like we're planning to ship these same add-on versions, I just wanted to make sure
everyone is aware that these two were activated earlier than expected.

Comment 28

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ec80a3f5cebc
test that updated system add-ons are reset on app update r=aswan

Comment 29

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/ec80a3f5cebc

Comment 30

[Gerry Chang [:gchang]]

Hi Brindusa, could you help find someone to verify if this issue was fixed as expected on a latest Nightly build? Thanks!

Comment 31

[Emil Pasca [:emilpasca]]

Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:55.0) Gecko/20100101 Firefox/55.0

I can confirm the fix on the latest Nightly 55.0a1, build ID 20170326030204 on Windows 10 x64, Ubuntu 14.04 x64 and Mac OS X 10.12. When updating from a earlier Nightly release(51.0a1,52.0a1, etc.) the system addon aushelper is successfully updated from 1.0 to 2.0 version.

Comment 32

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Hi Rhelmer, given the critical nature of this problem, I think we should uplift this fix to Aurora54 and eta53. If you agree, please nominate for uplift to Aurora and Beta.

Comment 33

[Gerry Chang [:gchang]]

Comment on attachment 8850831 [details]
uplift request

Fix a system addon update issue and was verified. Aurora54+ & Beta53+.

Comment 34

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/2365a3af90a4

Comment 35

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-beta/rev/6c66fac00e39

Comment 36

[Mihai Boldan, QA [:mboldan]]

I confirm that the version of Application Update Service Helper is updated from 1.0 to 2.0 when Firefox is updated to latest builds. 
The tests were performed for Firefox Aurora (updated from 51.0a2(2016-11-01) to 54.0a2 (2017-04-06)) and for Firefox Beta (updated from 51.0b9 to 53.0b9).
Tests were performed under Windows 10x64.

Comment 37

[Liz Henry (:lizzard Please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8850831 [details]
uplift request

This got fixed in beta 53, so I'm clearing the m-r approval flag.