1358342 - Remove aushelper system add-on

Status: RESOLVED FIXED

(Toolkit :: Application Update, enhancement, P1)

Description

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

Using a system add-on to modify the update url has had several problems with it a couple of which are listed in this bug's dependencies. System add-ons are also not pushed to clients that have app update disabled.

I'd like to use a new field in the update url instead of %OS_VERSION% since this shouldn't change other code that have %OS_VERSION% and use UpdateUtils.jsm.

Ben, is this possible now?

Comment 1

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

Ben ^^

Comment 3

[[e-mail] (:bhearsum)]

We'll need to tweak the Balrog code to handle it, but it shouldn't be a big deal. Just let me know what we're calling the new field, and the format of the new URL.

Comment 5

[:shell escalante]

Hi Rob,
Hi Ben,

Is there anyone who is planning to do this work before boostrapping goes away?  I'm not sure what channels aushelper code is used on (Nightly, Beta, or Release only).

Bootstrapped code will start being removed from 65 Nightly after October 22 (completed before Dec 11).  Bootstrapped add-ons won't work in 65 Beta (on Dec 11) or 65 Release (on Jan 29th).

Comment 6

[[e-mail] (:bhearsum)]

(In reply to :shell escalante from comment #5)
> Hi Rob,
> Hi Ben,
> 
> Is there anyone who is planning to do this work before boostrapping goes
> away?  I'm not sure what channels aushelper code is used on (Nightly, Beta,
> or Release only).
> 
> Bootstrapped code will start being removed from 65 Nightly after October 22
> (completed before Dec 11).  Bootstrapped add-ons won't work in 65 Beta (on
> Dec 11) or 65 Release (on Jan 29th).

This is client side work, so it would be Robert or someone else who works in that area. I believe we'll need a little bit of support in Balrog for it, too, which might end being me on me.

Comment 7

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

This shouldn't be a problem and I will be talking about this with my manager to get this work prioritized.

Changing summary of bug to reflect that the code is no longer needed. We might want to add more detailed CPU detection support than what is in aushelper in UpdateUtils.jsm but that would be done in a different bug.

This will be coordinated with releng for any balrog work that needs to be done.

Comment 8

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

Attachment: Bug 1358342 - Remove telemetry for the aushelper system add-on. r?chutten

The aushelper add-on is no longer needed and needs to be removed before bootstraping for add-ons is removed

Comment 9

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

Attachment: Bug 1358342 - Remove the aushelper system add-on. r?mhowell

The aushelper add-on is no longer needed and needs to be removed before bootstraping for add-ons is removed

Depends on D5338

Comment 10

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

Pushed to try
https://treeherder.mozilla.org/#/jobs?repo=try&revision=2fc143f2401f34da0283c553db8776390fea1d5d

Comment 11

[Molly Howell (she/her) (mostly inactive)]

Comment on attachment 9007395 [details]
Bug 1358342 - Remove the aushelper system add-on. r?mhowell

Matt Howell [:mhowell] has approved the revision.

Comment 12

[Chris H-C :chutten]

Comment on attachment 9007394 [details]
Bug 1358342 - Remove telemetry for the aushelper system add-on. r?chutten

Chris H-C :chutten has approved the revision.

Comment 13

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

Ben, can you check balrog to see if it will be able to handle requests after this lands? Thanks!

Comment 14

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

I'll take a look at this and get back to you.

Comment 15

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

So we're removing the helper which appended information about crashy Intel CPUs (bug 1296630) and Websense version to %OS_VERSION% on Windows. %OS_VERSION% in the update query will change from values like 
   Windows_NT%2010.0.0.0.17134.228%20(x64)(noBug1296630v1)(nowebsense)
back to
   Windows_NT%2010.0.0.0.17134.228%20(x64)

I've checked Balrog and we no longer have any rules that reference either '1296630' or 'websense', except for one comment which I've scheduled an correction for. There's no code to remove either, as we'll retain the 'foo && bar' matching in %OS_VERSION% which was added.

So all clear from our side.

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/030e811bfc34
Remove telemetry for the aushelper system add-on. r=chutten
https://hg.mozilla.org/integration/autoland/rev/f1e4fc4339f7
Remove the aushelper system add-on. r=mhowell

Comment 17

[Daniel Varga [:dvarga]]

Push: https://hg.mozilla.org/integration/autoland/rev/458e5b24da2f3da65254f69ee718233e68573c0e

Log: https://treeherder.mozilla.org/logviewer.html#?job_id=198957428&repo=autoland&lineNumber=3066 

[task 2018-09-12T23:24:04.935Z] 23:24:04     INFO - TEST-PASS | browser/base/content/test/performance/browser_preferences_usage.js | media.block-autoplay-until-in-foreground should not be accessed more than 40 times. - 6 <= 40 - 
[task 2018-09-12T23:24:04.936Z] 23:24:04     INFO - TEST-PASS | browser/base/content/test/performance/browser_preferences_usage.js | dom.ipc.tabs.disabled should not be accessed more than 40 times. - 6 <= 40 - 
[task 2018-09-12T23:24:04.942Z] 23:24:04     INFO - TEST-PASS | browser/base/content/test/performance/browser_preferences_usage.js | browser.cache.check_doc_frequency should not be accessed more than 40 times. - 6 <= 40 - 
[task 2018-09-12T23:24:04.943Z] 23:24:04     INFO - TEST-PASS | browser/base/content/test/performance/browser_preferences_usage.js | docshell.device_size_is_page_size should not be accessed more than 40 times. - 6 <= 40 - 
[task 2018-09-12T23:24:04.945Z] 23:24:04     INFO - TEST-PASS | browser/base/content/test/performance/browser_preferences_usage.js | dom.ipc.processCount should not be accessed more than 40 times. - 6 <= 40 - 
[task 2018-09-12T23:24:04.946Z] 23:24:04     INFO - TEST-PASS | browser/base/content/test/performance/browser_preferences_usage.js | pdfjs.disabled should not be accessed more than 40 times. - 6 <= 40 - 
[task 2018-09-12T23:24:04.947Z] 23:24:04     INFO - TEST-PASS | browser/base/content/test/performance/browser_preferences_usage.js | intl.charset.detector should not be accessed more than 40 times. - 6 <= 40 - 
[task 2018-09-12T23:24:04.948Z] 23:24:04     INFO - Buffered messages finished
[task 2018-09-12T23:24:04.950Z] 23:24:04     INFO - TEST-UNEXPECTED-FAIL | browser/base/content/test/performance/browser_preferences_usage.js | Whitelist item extensions.getAddons.cache.enabled should be accessed at least 7 times. - 7 <= 6 - JS frame :: chrome://mochitests/content/browser/browser/base/content/test/performance/browser_preferences_usage.js :: checkPrefGetters :: line 42
[task 2018-09-12T23:24:04.951Z] 23:24:04     INFO - Stack trace:
[task 2018-09-12T23:24:04.954Z] 23:24:04     INFO - chrome://mochitests/content/browser/browser/base/content/test/performance/browser_preferences_usage.js:checkPrefGetters:42
[task 2018-09-12T23:24:04.956Z] 23:24:04     INFO - chrome://mochitests/content/browser/browser/base/content/test/performance/browser_preferences_usage.js:startup:109
[task 2018-09-12T23:24:04.957Z] 23:24:04     INFO - chrome://mochikit/content/browser-test.js:Tester_execTest/<:1102
[task 2018-09-12T23:24:04.958Z] 23:24:04     INFO - chrome://mochikit/content/browser-test.js:Tester_execTest:1093
[task 2018-09-12T23:24:04.961Z] 23:24:04     INFO - chrome://mochikit/content/browser-test.js:nextTest/<:995
[task 2018-09-12T23:24:04.962Z] 23:24:04     INFO - chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:SimpleTest.waitForFocus/waitForFocusInner/focusedOrLoaded/<:795
[task 2018-09-12T23:24:04.964Z] 23:24:04     INFO - TEST-PASS | browser/base/content/test/performance/browser_preferences_usage.js | Whitelist item extensions.getAddons.cache.enabled should be accessed at most 55 times. - 6 <= 55 - 
[task 2018-09-12T23:24:04.965Z] 23:24:04     INFO - TEST-PASS | browser/base/content/test/performance/browser_preferences_usage.js | browser.newtabpage.activity-stream.feeds.telemetry should not be accessed more than 40 times. - 5 <= 40 - 
[task 2018-09-12T23:24:04.968Z] 23:24:04     INFO - TEST-PASS | browser/base/content/test/performance/browser_preferences_usage.js | browser.newtabpage.activity-stream.feeds.favicon should not be accessed more than 40 times. - 5 <= 40 - 
[task 2018-09-12T23:24:04.970Z] 23:24:04     INFO - TEST-PASS | browser/base/content/test/performance/browser_preferences_usage.js | browser.newtabpage.activity-stream.feeds.asrouterfeed should not be accessed more than 40 times. - 5 <= 40 - 
[task 2018-09-12T23:24:04.973Z] 23:24:04     INFO - TEST-PASS | browser/base/content/test/performance/browser_preferences_usage.js | toolkit.cosmeticAnimations.enabled should not be accessed more than 40 times. - 5 <= 40 - 
[task 2018-09-12T23:24:04.975Z] 23:24:04     INFO - TEST-PASS | browser/base/content/test/performance/browser_preferences_usage.js | dom.fileHandle.enabled should not be accessed more than 40 times. - 5 <= 40 - 
[task 2018-09-12T23:24:04.977Z] 23:24:04     INFO - TEST-PASS | browser/base/content/test/performance/browser_preferences_usage.js | layout.css.individual-transform.enabled should not be accessed more than 40 times. - 5 <= 40 - 
[task 2018-09-12T23:24:04.979Z] 23:24:04     INFO - TEST-PASS | browser/base/content/test/performance/browser_preferences_usage.js | browser.newtabpage.activity-stream.feeds.sections should not be accessed more than 40 times. - 5 <= 40 - 
[task 2018-09-12T23:24:04.985Z] 23:24:04     INFO - TEST-PASS | browser/base/content/test/performance/browser_preferences_usage.js | identity.fxaccounts.enabled should not be accessed more than 40 times. - 5 <= 40 - 
[task 2018-09-12T23:24:04.992Z] 23:24:04     INFO - TEST-PASS | browser/base/content/test/performance/browser_preferences_usage.js | layout.css.overscroll-behavior.enabled should not be accessed more than 40 times. - 5 <= 40

Backout: https://hg.mozilla.org/integration/autoland/rev/458e5b24da2f3da65254f69ee718233e68573c0e

Comment 18

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f1ac94f94245
Backed out 2 changesets for browser-chrome failures on /test/performance/browser_preferences_usage.js on a CLOSED TREE

Comment 19

[Andrew Swan [:aswan]]

I don't know why this test requires a minimum number of reads of that preference, but there are several system addons on the chopping block right now in addition to aushelper, please just drop the min value down to 0.

Comment 20

[Florian Quèze [:florian]]

(In reply to Andrew Swan [:aswan] from comment #19)
> I don't know why this test requires a minimum number of reads of that
> preference, but there are several system addons on the chopping block right
> now in addition to aushelper, please just drop the min value down to 0.

The minimum is to ensure we notice when a whitelist entry is no longer needed and should be removed. If you are sure the pref won't be accessed more than 40 times during startup, you can just remove the whitelist entry completely.

https://searchfox.org/mozilla-central/rev/de7676288a78b70d2b9927c79493adbf294faad5/browser/base/content/test/performance/browser_preferences_usage.js#80,98-101

Comment 21

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

Attachment: Bug 1358342 - Remove the aushelper system add-on. r?florian

The aushelper add-on is being removed and this test needs the minimum number of values in extensions.getAddons.cache.enabled lowered so it passes

Comment 22

[Florian Quèze [:florian]]

Comment on attachment 9009215 [details]
Bug 1358342 - Remove the aushelper system add-on. r?florian

Florian Quèze [:florian] has approved the revision.

Comment 23

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

Attachment: Bug 1358342 - Remove telemetry for the aushelper system add-on. r?chutten

The aushelper add-on is no longer needed and needs to be removed before bootstraping for add-ons is removed

Comment 24

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

Attachment: Bug 1358342 - Remove the aushelper system add-on. r?mhowell

The aushelper add-on is no longer needed and needs to be removed before bootstraping for add-ons is removed

Depends on D5918

Comment 25

[Molly Howell (she/her) (mostly inactive)]

Comment on attachment 9009294 [details]
Bug 1358342 - Remove the aushelper system add-on. r?mhowell

Matt Howell [:mhowell] has approved the revision.

Comment 26

[Chris H-C :chutten]

Comment on attachment 9009293 [details]
Bug 1358342 - Remove telemetry for the aushelper system add-on. r?chutten

Chris H-C :chutten has approved the revision.

Comment 27

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/aa53c685f994
Remove the aushelper system add-on. r=florian
https://hg.mozilla.org/integration/autoland/rev/b77c77751754
Remove telemetry for the aushelper system add-on. r=chutten
https://hg.mozilla.org/integration/autoland/rev/82e4cee9fb1c
Remove the aushelper system add-on. r=mhowell

Comment 28

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/aa53c685f994
https://hg.mozilla.org/mozilla-central/rev/b77c77751754
https://hg.mozilla.org/mozilla-central/rev/82e4cee9fb1c