1094362 - Disable checking for updates for Linux Mulet jobs

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia, defect)

Description

[Armen [:armenzg]]

Described in https://bugzilla.mozilla.org/show_bug.cgi?id=1043699#c100

09:18:10     INFO -  UpdatePrompt: Forcing update check
09:18:10     INFO -  *** AUS:SVC Checker: checkForUpdates, force: true
09:18:10     INFO -  *** AUS:SVC getLocale - getting locale from file: resource://gre/update.locale, locale: en-US
09:18:10     INFO -  *** AUS:SVC Checker:getUpdateURL - update URL: https://aus4.mozilla.org/update/3/Firefox/36.0a1/20141104131517/%PRODUCT_DEVICE%/en-US/default/Linux%203.13.0-39-generic%20(GTK%202.24.23)/default/default/update.xml?force=1
09:18:10     INFO -  *** AUS:SVC gCanCheckForUpdates - able to check for updates
09:18:10     INFO -  *** AUS:SVC recordInHealthReport - updateCheckStart - 0
09:18:10     INFO -  *** AUS:SVC Checker:checkForUpdates - sending request to: https://aus4.mozilla.org/update/3/Firefox/36.0a1/20141104131517/%PRODUCT_DEVICE%/en-US/default/Linux%203.13.0-39-generic%20(GTK%202.24.23)/default/default/update.xml?force=1
09:18:10     INFO -  FATAL ERROR: Non-local network connections are disabled and a connection attempt to aus4.mozilla.org (63.245.217.219) was made.
09:18:10     INFO -  You should only access hostnames available via the test networking proxy (if running mochitests) or from a test-specific httpd.js server (if running xpcshell tests). Browser services should be disabled or redirected to a local server.

Comment 1

[Alexandre Poirot [:ochameau]]

I would have imagined app update was disabled thanks to this:
http://mxr.mozilla.org/mozilla-central/source/layout/tools/reftest/reftest-preferences.js#17
     branch.setBoolPref("app.update.enabled", false);
Aren't we using reftest-preferences for mulet?! That sounds very unlikely, as I imagine most reftests rely on these prefs to be set...

Comment 2

[Armen [:armenzg]]

It seems that gerard-majax added some update checking to gaia.
http://hg.mozilla.org/integration/gaia-central/file/1cf711cc0add/tests/atoms/fake_update-checker.js

He asked me to try setting an update pref to an empty string. [1]

I've tried setting the value though b2g_desktop.py and b2g_script.js without success.

All of these sounds like not applicable since we already request updates to be disabled in b2g_start_script.js
> branch.setBoolPref("app.update.enabled", false);

In the logs I see this message "UpdatePrompt: Forcing update check " which leads me here:
http://dxr.mozilla.org/mozilla-central/source/b2g/components/UpdatePrompt.js#415
and here:
http://dxr.mozilla.org/mozilla-central/source/b2g/components/UpdatePrompt.js#20

The next line of output is "*** AUS:SVC Checker: checkForUpdates, force: true" which leads me here:
http://hg.mozilla.org/integration/gaia-central/file/1cf711cc0add/tests/atoms/fake_update-checker.js

[1]
http://dxr.mozilla.org/mozilla-central/search?q=app.update.ur&redirect=true
<gerard-majax> armenzg,  by looking at nsUpdateService.js, that should disable update checking
<gerard-majax> toolkit/mozapps/update/nsUpdateService.js
<gerard-majax> see how getUpdateURL works and is called
<gerard-majax> returns null of the pref string is empty

[2]
forceUpdateCheck: function UP_forceUpdateCheck() {
    log("Forcing update check");

    let checker = Cc["@mozilla.org/updates/update-checker;1"]
                    .createInstance(Ci.nsIUpdateChecker);
    checker.checkForUpdates(this._updateCheckListener, true);
  },

Comment 3

[Armen [:armenzg]]

Who can work on figuring out how to stop the forcing of the updates?

Comment 4

[:gerard-majax]

I had a look at this a couple of days ago, and I found that on some case during boot, we trigger a force check that nothing can stop.

Comment 5

[:gerard-majax]

There is an update check forced at https://github.com/mozilla-b2g/gaia/blob/42f6d67e4809333a7ce4ed729c620052c940ac96/apps/system/js/bootstrap.js#L120

Comment 6

[:gerard-majax]

Settings the pref |app.update.enabled| to false, or the URL |app.update.url| to something empty is not enough because at http://hg.mozilla.org/mozilla-central/annotate/c9cfa9b91dea/toolkit/mozapps/update/nsUpdateService.js#l3669 we will still take into account the value of |force|. This is triggered by the Gaia code as documented in comment 5.

Comment 7

[:gerard-majax]

Attachment: bug1094362-disable-forced-updates.diff

Would this approach be okay for all of you? I don't see a nicer way to deal with this than add a new pref that would help us to disable the effect of forcing update checking.

Comment 8

[:gerard-majax]

We could also remove the code from bug 879192 if we land this, and just use the prefs.

Comment 9

[:gerard-majax]

Try run: https://tbpl.mozilla.org/?tree=Try&rev=2e5d45e97295

Comment 10

[:gerard-majax]

Try run with the pref enabled: https://tbpl.mozilla.org/?tree=Try&rev=e63d83632258

Comment 11

[:gerard-majax]

It's red in both cases, but looking at the logs, we see that with the pref enabled, at least, it's not red because of network access due to update checking.

Comment 12

[Armen [:armenzg]]

Comment on attachment 8527299 [details] [diff] [review]
bug1094362-disable-forced-updates.diff

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

It looks sensible. Thanks for looking into this!

If you push it to try you should be able to see if it does the trick.

Comment 13

[Armen [:armenzg]]

> If you push it to try you should be able to see if it does the trick.

nvm me :)

Here's the log referred by gerard-majax:
https://tbpl.mozilla.org/php/getParsedLog.php?id=53296348&tree=Try&full=1#error0

Comment 14

[:gerard-majax]

Looks like we get those disabled also by setting:
> pref("app.update.url", "");
> pref("app.update.url.override", "");
> pref("app.update.enabled", false);

Comment 15

[:gerard-majax]

(In reply to Alexandre LISSY :gerard-majax from comment #14)
> Looks like we get those disabled also by setting:
> > pref("app.update.url", "");
> > pref("app.update.url.override", "");
> > pref("app.update.enabled", false);

But I have no idea where we should set those prefs.

Comment 16

[:gerard-majax]

Attachment: Completely disable update prefs for reftest r=...

When running reftests on Mulet, we do not want that forced update
checking gets triggered. We can block this by setting the following
prefs:
 - app.update.enabled to false
 - app.update.url.override and app.update.url to empty string

Comment 17

[:gerard-majax]

Try: https://tbpl.mozilla.org/?tree=Try&rev=398e28cb31c7

Comment 18

[:gerard-majax]

(In reply to Alexandre LISSY :gerard-majax from comment #17)
> Try: https://tbpl.mozilla.org/?tree=Try&rev=398e28cb31c7

So the prefs are not applied, according to the failure and the log.

Armen or Johnathan, is there a trick to set prefs ?

Comment 19

[Armen [:armenzg]]

I don't know how the pref system for the browser works like.

Maybe jgriffin knows who might know?

Comment 20

[Jonathan Griffin (:jgriffin)]

The file you're modifying gets loaded sometime after startup, but the update prefs cause an immediate update check upon launch.

To get the effect you're looking for, we need to modify the prefs _in the profile_, before launching Mulet.

You should set these preferences here, instead:

http://dxr.mozilla.org/mozilla-central/source/layout/tools/reftest/b2g_desktop.py#108

Comment 21

[:gerard-majax]

Attachment: Completely disable update prefs for reftest r=...

When running reftests on Mulet, we do not want that forced update
checking gets triggered. We can block this by setting the following
prefs:
 - app.update.enabled to false
 - app.update.url.override and app.update.url to empty string
We also disable tiles-related pinging and downloading by setting empy
values for:
 - browser.newtabpage.directory.source
 - browser.newtabpage.directory.ping

Comment 22

[:gerard-majax]

Thanks Johnathan, this is making progress.

This try is green https://tbpl.mozilla.org/?tree=Try&rev=c468a2fe6519 relative to the issue we are trying to fix: disabling updates.

Comment 23

[:gerard-majax]

Ok, and now I have teh same marketplace.firefox.com issues than in bug 1104644.

Comment 24

[:gerard-majax]

The reason is documented in bug 1104644. Basically, I don't see a better way than to add a pref that we could toggle to disable webapps update checking.

Comment 25

[:gerard-majax]

Try: https://tbpl.mozilla.org/?tree=Try&rev=a0f93192d70c

This one is with the new pref added and set to false for reftests. It should not be red because of any network connections

Comment 26

[:gerard-majax]

Attachment: Completely disable update prefs for reftest r=...

When running reftests on Mulet, we do not want that forced update
checking gets triggered. We can block this by setting the following
prefs:
 - app.update.enabled to false
 - app.update.url.override and app.update.url to empty string
We also disable tiles-related pinging and downloading by setting empy
values for:
 - browser.newtabpage.directory.source
 - browser.newtabpage.directory.ping

Comment 27

[:gerard-majax]

For some reason, some of the reftests are still doing update checks ?!

Comment 28

[:gerard-majax]

Attachment: Completely disable update prefs for reftest r=...

When running reftests on Mulet, we do not want that forced update
checking gets triggered. We can block this by setting the following
prefs:
 - app.update.enabled to false
 - app.update.url.override and app.update.url to empty string
We also disable tiles-related pinging and downloading by setting empy
values for:
 - browser.newtabpage.directory.source
 - browser.newtabpage.directory.ping

Comment 29

[:gerard-majax]

See https://tbpl.mozilla.org/?tree=Try&rev=07cce692214e

So I changed the profile, but it's still doing some update checking. And the added debug shows that my prefs are not used.

Comment 30

[:gerard-majax]

Forcing the prefs in b2g.js and in runreftestb2g.py does not help ...

Comment 31

[:gerard-majax]

Prefs in b2g.js were not properly set: I used 'prefs()' instead of 'pref()' ...

Pushed a new try run that hardcodes all those prefs to b2g.js (and correctly, checked locally). This one should not be red because of remove connections. We will still need to figure out why settings prefs with the other ways do not work.
Try: https://tbpl.mozilla.org/?tree=Try&rev=2940fef82f7f

Comment 32

[:gerard-majax]

So on https://tbpl.mozilla.org/?tree=Try&rev=2940fef82f7f I see crashes, but it looks like the prefs set in b2g.js are effective.

Johnathan, any idea why doing it in b2g_desktop.py and runreftestb2g.py does not work?

Comment 33

[:gerard-majax]

Comment on attachment 8529106 [details] [diff] [review]
Completely disable update prefs for reftest r=...

Vivien, I'd need a quick review on the part that adds the new pref: webapps.update.enabled

This is intended to allow us to block webapps updates. Those are triggered via a force-update-check event sent from the System app.

Comment 34

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Comment on attachment 8529106 [details] [diff] [review]
Completely disable update prefs for reftest r=...

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

The b2g/ part looks good to me. You will need someone else for the b2g_desktop.py part thought.

Comment 35

[Jonathan Griffin (:jgriffin)]

(In reply to Alexandre LISSY :gerard-majax from comment #29)
> See https://tbpl.mozilla.org/?tree=Try&rev=07cce692214e
> 
> So I changed the profile, but it's still doing some update checking. And the
> added debug shows that my prefs are not used.

I'm not sure, passing the buck to ahal who might be able to help here.

Comment 36

[Andrew Halberstadt [:ahal]]

I'm not entirely sure what the question is. Possibly those debug statements aren't working because the pref in b2g.js is overwriting the one set in the profile? If you see tests running at all it means that the b2g_desktop.py prefs are being loaded, without them nothing would work.

Comment 37

[:gerard-majax]

Attachment: Completely disable update prefs for reftest r=...

When running reftests on Mulet, we do not want that forced update
checking gets triggered. We can block this by setting the following
prefs:
 - app.update.enabled to false
 - app.update.url.override and app.update.url to empty string
We also disable tiles-related pinging and downloading by setting empy
values for:
 - browser.newtabpage.directory.source
 - browser.newtabpage.directory.ping

Comment 38

[:gerard-majax]

Try: https://tbpl.mozilla.org/?tree=Try&rev=44698bd751ff

This one is for attachment 8532078 [details] [diff] [review]: after offline discussion with andrew, it looks like this approach makes sense, and comment 31 was already a good try with those changes ; crash was not related to non local network connections.

Comment 39

[:gerard-majax]

Attachment: Completely disable update prefs for reftest r=vingtetun,jgriffin,ahal

When running reftests on Mulet, we do not want that forced update
checking gets triggered. We can block this by setting the following
prefs:
 - app.update.enabled to false
 - app.update.url.override and app.update.url to empty string
We also disable tiles-related pinging and downloading by setting empy
values for:
 - browser.newtabpage.directory.source
 - browser.newtabpage.directory.ping
Gaia also does force check app updates at some point, so we introduce a
new pref, webapps.update.enabled, that we can use to make sure webapps
update won't be triggered.

Comment 40

[:gerard-majax]

(In reply to Alexandre LISSY :gerard-majax from comment #38)
> Try: https://tbpl.mozilla.org/?tree=Try&rev=44698bd751ff
> 
> This one is for attachment 8532078 [details] [diff] [review]: after offline
> discussion with andrew, it looks like this approach makes sense, and comment
> 31 was already a good try with those changes ; crash was not related to non
> local network connections.

Please read that this is attachment 8532084 [details] [diff] [review].

Comment 41

[:gerard-majax]

Comment on attachment 8532084 [details] [diff] [review]
Completely disable update prefs for reftest r=vingtetun,jgriffin,ahal

Carrying r+ from :21 for the added pref.

Comment 42

[Jonathan Griffin (:jgriffin)]

Comment on attachment 8532084 [details] [diff] [review]
Completely disable update prefs for reftest r=vingtetun,jgriffin,ahal

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

lgtm

Comment 43

[:gerard-majax]

(In reply to Alexandre LISSY :gerard-majax from comment #41)
> Comment on attachment 8532084 [details] [diff] [review]
> Completely disable update prefs for reftest r=vingtetun,jgriffin,ahal
> 
> Carrying r+ from :21 for the added pref.

New try, since it seems infra is having a hard time ... : https://tbpl.mozilla.org/?tree=Try&rev=1ad18ee841e2

Comment 44

[:gerard-majax]

Attachment: Completely disable update prefs for reftest r=vingtetun,jgriffin

When running reftests on Mulet, we do not want that forced update
checking gets triggered. We can block this by setting the following
prefs:
 - app.update.enabled to false
 - app.update.url.override and app.update.url to empty string
We also disable tiles-related pinging and downloading by setting empy
values for:
 - browser.newtabpage.directory.source
 - browser.newtabpage.directory.ping
Gaia also does force check app updates at some point, so we introduce a
new pref, webapps.update.enabled, that we can use to make sure webapps
update won't be triggered.

Comment 45

[:gerard-majax]

Comment on attachment 8532536 [details] [diff] [review]
Completely disable update prefs for reftest r=vingtetun,jgriffin

Carrying r+ from vingtetun and jgriffin.

Comment 46

[:gerard-majax]

https://tbpl.mozilla.org/?tree=Try&rev=8d00493e6809
https://tbpl.mozilla.org/?tree=Try&rev=d9f98e16df7c

Red, but not related to those changes

Comment 47

[:gerard-majax]

This try should trigger gaia js integration including the prefs on gaia side: https://tbpl.mozilla.org/?tree=Try&rev=854780f9efc2

This try should trigger gaia js integration WITHOUT the prefs on gaia side: https://tbpl.mozilla.org/?tree=Try&rev=b81d63df7369

Comment 48

[:gerard-majax]

Added mulet.

without gaia-side prefs https://tbpl.mozilla.org/?tree=Try&rev=d54fb9969961
with gaia-side prefs https://tbpl.mozilla.org/?tree=Try&rev=9adc8124f2fa

Comment 49

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/b2g-inbound/rev/cb79eb1c1244

Comment 50

[Ryan VanderMeulen [:RyanVM]]

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