1833104 - JOG stats jogfile.json in the appdir on the main thread before opening first browser window for artifact builds but has no exception in `browser_startup_mainthreadio.js`

Status: RESOLVED FIXED

(Toolkit :: Telemetry, defect)

Description

[:Gijs (he/him)]

We should have caught this immediately with the mainthreadio test in bug 1831406 but we didn't because the test accidentally got disabled across all platforms. The stack looks like this:

(root) []
XREMain::XRE_main []
XPCWrappedJS method call []
js::RunScript []
observe [resource://gre/modules/addonManager.js:72:9]
startup [resource://gre/modules/AddonManager.jsm:3665:9]
startup [resource://gre/modules/AddonManager.jsm:609:9]
_startProvider [resource://gre/modules/AddonManager.jsm:545:16]
callProvider [resource://gre/modules/AddonManager.jsm:240:21]
js::RunScript []
startup [resource://gre/modules/addons/XPIProvider.jsm:2496:9]
checkForChanges [resource://gre/modules/addons/XPIProvider.jsm:3043:17]
processFileChanges [resource://gre/modules/addons/XPIDatabase.jsm:3504:20]
js::RunScript []
addMetadata [resource://gre/modules/addons/XPIDatabase.jsm:3062:13]
syncLoadManifest [resource://gre/modules/addons/XPIInstall.jsm:765:25]
awaitPromise [resource://gre/modules/addons/XPIProvider.jsm:211:21]
nsIThreadManager.spinEventLoopUntil []
SpinEventLoopUntil XPIProvider.jsm:awaitPromise []
promise callback []
js::RunScript []
AsyncFunctionNext [self-hosted:894:26]
loadManifestFromWebManifest [resource://gre/modules/addons/XPIInstall.jsm:436:42]
loadManifest [resource://gre/modules/Extension.jsm:1725:20]
parseManifest [resource://gre/modules/Extension.jsm:1329:21]
lazyInit [resource://gre/modules/ExtensionParent.jsm:181:10]
get [resource://gre/modules/ExtensionParent.jsm:2235:11]
getStore [resource://gre/modules/ExtensionParent.jsm:2210:16]
js::RunScript []
get dataPromise [resource://gre/modules/ExtensionParent.jsm:2164:17]
_readData [resource://gre/modules/ExtensionParent.jsm:2139:17]
nsLocalFile::ResolveAndStat []

perhaps this only happens on artifact builds? It's hard to be sure off-hand.

Comment 1

[:Gijs (he/him)]

(In reply to :Gijs (he/him) from comment #0)

perhaps this only happens on artifact builds? It's hard to be sure off-hand.

Looks like this is indeed enforced via a pref and conditioned on MOZ_ARTIFACT_BUILDS. I don't really understand why this uses a pref rather than defining MOZ_ARTIFACT_BUILDS in AppConstants, but I guess it works...

Comment 2

[Chris H-C :chutten]

(In reply to :Gijs (he/him) from comment #1)

(In reply to :Gijs (he/him) from comment #0)

perhaps this only happens on artifact builds? It's hard to be sure off-hand.

Looks like this is indeed enforced via a pref and conditioned on MOZ_ARTIFACT_BUILDS. I don't really understand why this uses a pref rather than defining MOZ_ARTIFACT_BUILDS in AppConstants, but I guess it works...

The short of it is we need this information in C++ and reading AppConstants in C++ didn't appear to be something that is done anywhere. Rather than invent a new pattern, I went with a pref (and this way, if I end up needing it in Rust, too, it's right there). Plus, that makes it easier to manipulate for testing. Documentation for FOG's defines and prefs is here, though it doesn't contain any more information than you now have.

Comment 3

[:Gijs (he/him)]

Attachment: Bug 1833104 - add exception for artifact-only jogfile.json stat, r?mconley

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/256aac2b8a71
add exception for artifact-only jogfile.json stat, r=mconley

Comment 5

[Noemi Erli[:noemi_erli]]

Backed out 4 changesets (Bug 1833104, Bug 1831406) for causing failures in browser_startup_mainthreadio.js CLOSED TREE

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&collapsedPushes=842382&selectedTaskRun=UZkdO2d6RXSlufYXYjJv7w.0&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel&revision=536375224eb1c62bcff369c691ca5b2f1d959a56

Log: https://treeherder.mozilla.org/logviewer?job_id=415850103&repo=autoland&lineNumber=3734

Backout: https://hg.mozilla.org/integration/autoland/rev/69f226b3c41e8fe89cf6567ac2356cccfa19acfc

Comment 6

[:Gijs (he/him)]

Yeah this wasn't actually a problem with this part of the patch, so I'll reland this bit...

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4c3c77ede182
add exception for artifact-only jogfile.json stat, r=mconley

Comment 8

[Iulian Moraru]

https://hg.mozilla.org/mozilla-central/rev/4c3c77ede182