1395990 - Pass "-purgecaches" arg when creating chrome debugging process for artifact builds

Status: RESOLVED FIXED

(DevTools :: General, enhancement, P3)

Description

[Julian Descottes [:jdescottes]]

We are currently relying on Services.appinfo.isOfficial in order to add -purgeCaches to the arguments passed to the chrome debugging process at:

http://searchfox.org/mozilla-central/rev/51b3d67a5ec1758bd2fe7d7b6e75ad6b6b5da223/devtools/client/framework/ToolboxProcess.jsm#261-270

However Services.appinfo.isOfficial is only false for non-artifact builds. Artifact builds actually have it set to true since they are based on a CI-built artifact.

We could rely on the following combination of constants to get a better approximation:

> let isOfficialBuild = AppConstants.NIGHTLY_BUILD ||
>                       AppConstants.RELEASE_OR_BETA ||
>                       AppConstants.MOZ_DEV_EDITION;

Comment 1

[Julian Descottes [:jdescottes]]

Actually this other version fails in the same way. Looks like we have to use: 

> const IS_UNOFFICIAL_BUILD = Services.prefs.getStringPref("app.update.channel") === "default";

to get something that can be used for both artifact and non artifact builds.

Comment 2

[Brian Grinstead [:bgrins]]

We use `#ifndef MOZILLA_OFFICIAL` for turning on the remote debugging prefs in local builds, which works in artifact builds: https://dxr.mozilla.org/mozilla-central/source/modules/libpref/init/all.js#1049

Comment 3

[Brian Grinstead [:bgrins]]

So should be AppConstants.MOZILLA_OFFICIAL

Comment 4

[Julian Descottes [:jdescottes]]

Attachment: Bug 1395990 - use constant MOZILLA_OFFICIAL to check for local build in ToolboxProcess.jsm;

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

Comment 5

[Brian Grinstead [:bgrins]]

Comment on attachment 8903660 [details]
Bug 1395990 - use constant MOZILLA_OFFICIAL to check for local build in ToolboxProcess.jsm;

https://reviewboard.mozilla.org/r/175430/#review180550

Comment 6

[Brian Grinstead [:bgrins]]

With this change it looks like there is only one other usage of "Services.appinfo.isOfficial" (in a test): https://dxr.mozilla.org/mozilla-central/search?q=%22Services.appinfo.isOfficial%22&redirect=false. Mossop, can that property go away?

Comment 7

[Julian Descottes [:jdescottes]]

(thanks for jumping on this and the review Brian!)

Comment 8

[Julian Descottes [:jdescottes]]

Attachment: Bug 1395990 - stop using Services.appinfo.isOfficial in test_TelemetrySession.js;

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

Comment 9

[Julian Descottes [:jdescottes]]

Attachment: Bug 1395990 - remove Services.appinfo.isOfficial;

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

Comment 10

[Dave Townsend [:mossop]]

(In reply to Brian Grinstead [:bgrins] from comment #6)
> With this change it looks like there is only one other usage of
> "Services.appinfo.isOfficial" (in a test):
> https://dxr.mozilla.org/mozilla-central/search?q=%22Services.appinfo.
> isOfficial%22&redirect=false. Mossop, can that property go away?

Yes, we should use AppConstants.MOZILLA_OFFICIAL instead.

Comment 11

[Dave Townsend [:mossop]]

Comment on attachment 8903675 [details]
Bug 1395990 - stop using Services.appinfo.isOfficial in test_TelemetrySession.js;

https://reviewboard.mozilla.org/r/175452/#review180560

Comment 12

[Dave Townsend [:mossop]]

Comment on attachment 8903676 [details]
Bug 1395990 - remove Services.appinfo.isOfficial;

https://reviewboard.mozilla.org/r/175454/#review180562

Comment 13

[Julian Descottes [:jdescottes]]

Comment on attachment 8903675 [details]
Bug 1395990 - stop using Services.appinfo.isOfficial in test_TelemetrySession.js;

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

Comment 14

[Julian Descottes [:jdescottes]]

Comment on attachment 8903676 [details]
Bug 1395990 - remove Services.appinfo.isOfficial;

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

Comment 15

[Julian Descottes [:jdescottes]]

Thanks for the reviews! Try is green at https://treeherder.mozilla.org/#/jobs?repo=try&revision=37d8d621646b361d04ff6d68d9e09aa877e0b9b2

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7d8b0d8d0043
use constant MOZILLA_OFFICIAL to check for local build in ToolboxProcess.jsm;r=bgrins
https://hg.mozilla.org/integration/autoland/rev/fb06b4e04043
stop using Services.appinfo.isOfficial in test_TelemetrySession.js;r=mossop
https://hg.mozilla.org/integration/autoland/rev/de9a050e5ff0
remove Services.appinfo.isOfficial;r=mossop

Comment 17

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/7d8b0d8d0043
https://hg.mozilla.org/mozilla-central/rev/fb06b4e04043
https://hg.mozilla.org/mozilla-central/rev/de9a050e5ff0