1506917 - Don't attempt to check for updates from Firefox's nsBrowserGlue.js when running tests

Status: RESOLVED FIXED

(Toolkit :: Application Update, enhancement)

Description

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

I noticed that when running tests that nsBrowserGlue.js was attempting to check for an update. This can be disabled by setting the app.update.checkInstallTime bool pref to false.

Comment 1

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

Attachment: patch rev1

Comment 2

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

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

Comment 3

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

Comment on attachment 9024818 [details] [diff] [review]
patch rev1

Hi Ted, I noticed this when running tests locally. Basically, Firefox has an option to perform a background check in nsBrowserGlue.js that will run when the build ID is 2 days or greater old. Though the disabledForTesting pref prevents it from performing the update check it still hits the app update telemetry code so I'd like to disable this when running tests.

Comment 4

[(not currently active) Ted Mielczarek]

Comment on attachment 9024818 [details] [diff] [review]
patch rev1

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

::: testing/geckodriver/src/prefs.rs
@@ +7,1 @@
>  // so please be careful and get review from a Testing :: geckodriver peer

I don't know that I qualify as a geckodriver peer for these purposes. :)

Comment 5

[(not currently active) Ted Mielczarek]

Holy smokes we still have a lot of places that define testing prefs!

Comment 6

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

Attachment: patch rev2

I removed the change to prefs.rs for geckodriver... I'll file a separate bug for that change.

Carrying forward r+

Comment 7

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

Filed bug 1508283 for geckdriver

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b1c306693151
Don't attempt to check for updates from Firefox's nsBrowserGlue.js when running tests. r=ted

Comment 9

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

Attachment: patch rev2

Patch as landed... had to unbitrot it

Comment 10

[Razvan Maries]

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

Comment 11

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 10/03 - 10/13)]

Comment on attachment 9026104 [details] [diff] [review]
patch rev2

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

::: testing/marionette/components/marionette.js
@@ +59,5 @@
>    // Disable automatically upgrading Firefox.
>    //
>    // This should also be set in the profile prior to starting Firefox,
>    // as it is picked up at runtime.
> +  ["app.update.checkInstallTime", false],

This is a preference which needs to be set during start-up, right? So it should only have been set in geckoinstance.py, but not marionette.js. The latter is even only related to geckodriver and should not have been modified here but at least on bug 1508283. Sadly we are getting into an inconsistent state of prefs again because patches land without our review. :(

Comment 12

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

It is needed during startup. I added it to testing/marionette/components/marionette.js because it had the app.update.disabledForTesting pref along with this comment

https://searchfox.org/mozilla-central/source/testing/marionette/components/marionette.js#61

  // This should also be set in the profile prior to starting Firefox,
  // as it is picked up at runtime.

It looks like you can also remove app.update.disabledForTesting since that should be set during startup.

In the case of geckodriver.py I got a geckodriver peer to review in bug 1508283 because of a comment in the code per comment #4. Perhaps you can do something similar and let the testing owner (ted) and other testing peers know. I've also had to do something similar in 
https://searchfox.org/mozilla-central/source/browser/installer/removed-files.in

and I periodically get pinged from people trying to add to it.

Comment 13

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 10/03 - 10/13)]

Thanks Robert! I will take care of it on bug 1509256, which will improve the documentation, and removes both preferences from marionette.js.