Addons Manager XPCShellTests that set AddonManager.checkCompatibility too early are failing on Aurora 17 nightly builds

RESOLVED FIXED in Firefox 17

Status

()

Toolkit
Add-ons Manager
--
blocker
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: akeybl, Assigned: Unfocused)

Tracking

(Depends on: 1 bug)

unspecified
mozilla18
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox17- fixed)

Details

(Whiteboard: [qa-])

Attachments

(1 attachment)

(Reporter)

Description

5 years ago
Philor just pointed out that XPCShellTest is failing with PGO builds on Aurora 17 since the merge:

https://tbpl.mozilla.org/?tree=Mozilla-Aurora&onlyunstarred=1

We're planning on enabling Aurora builds tomorrow morning, so we need to decide whether this should be a blocker for the release. My feeling is no, if QA qualification is successful. But I also think Gavin will have a more informed opinion.
(Reporter)

Updated

5 years ago
tracking-firefox17: --- → ?
Not all PGO builds are affected; only nightly builds:
https://tbpl.mozilla.org/?tree=Mozilla-Aurora&rev=e101e4c1dcf8

Both the "B" (dep) and "N" (nightly) builds use the same .mozconfig (as you can verify from the "cat .mozconfig" step in the build log) which has PGO enabled (the PROFILE_GEN_SCRIPT variable is set).

This happens even if the dep build is a forced clobber, as in the Win opt B here:
https://tbpl.mozilla.org/?tree=Mozilla-Aurora&rev=380fdb7249aa&jobname=win
The .mozconfig has a couple of variables that could lead to different configuration depending on the build environment; in particular MOZ_UPDATE_CHANNEL="default" for dep builds and MOZ_UPDATE_CHANNEL="aurora" for nightly builds.  So probably some code or test is breaking when the update channel is set to "aurora".
I think this was caused by bug 741972. The checkCompatibility setter is being called while PREF_EM_CHECK_COMPATIBILITY is undefined (i.e. AddonManagerInternal.startup hasn't been called), so we get:

resource://gre/modules/AddonManager.jsm:1951: NS_ERROR_ILLEGAL_VALUE: Component returned failure code: 0x80070057 (NS_ERROR_ILLEGAL_VALUE) [nsIPrefBranch.setBoolPref]

This is only a problem on Aurora (and beta, esr, nightly) because MOZ_COMPATIBILITY_NIGHTLY isn't defined there, and so PREF_EM_CHECK_COMPATIBILITY doesn't have a default value.
Blocks: 741972
Summary: XPCShellTest is failing on Aurora 17 PGO builds → XPCShellTest is failing on Aurora 17 nightly builds
(In reply to :Gavin Sharp (use gavin@gavinsharp.com for email) from comment #3)
> This is only a problem on Aurora (and beta, esr, nightly) because

Er, not nightly :) It's a problem for any build where MOZ_UPDATE_CHANNEL is one of "aurora beta release esr", per http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/extensions/Makefile.in#12.
AFAICT this is a test-only issue, so it doesn't need to block our re-enabling Aurora updates tomorrow.
(Reporter)

Comment 6

5 years ago
(In reply to :Gavin Sharp (use gavin@gavinsharp.com for email) from comment #5)
> AFAICT this is a test-only issue, so it doesn't need to block our
> re-enabling Aurora updates tomorrow.

Thanks Gavin!
tracking-firefox17: ? → -
Assignee: gavin.sharp → nobody
(In reply to :Gavin Sharp (use gavin@gavinsharp.com for email) from comment #3)
> I think this was caused by bug 741972. The checkCompatibility setter is
> being called while PREF_EM_CHECK_COMPATIBILITY is undefined (i.e.
> AddonManagerInternal.startup hasn't been called), so we get:

Ah, yes, that would make sense. And PREF_EM_CHECK_COMPATIBILITY needs to be set in startup() because the tests need to change the application version between startup and shutdown. Test fix coming up.
Assignee: nobody → bmcbride
Status: NEW → ASSIGNED
Summary: XPCShellTest is failing on Aurora 17 nightly builds → Addons Manager XPCShellTests that set AddonManager.checkCompatibility too early are failing on Aurora 17 nightly builds
Created attachment 657755 [details] [diff] [review]
Patch v1
Attachment #657755 - Flags: review?(dtownsend+bugmail)
Component: General → Add-ons Manager
Product: Firefox → Toolkit
Looks like somebody decided to close the tree over it, so even though it doesn't block enabling nightlies, it does block landing anything.
Severity: normal → blocker
(In reply to Phil Ringnalda (:philor) from comment #9)
> Looks like somebody decided to close the tree over it, so even though it
> doesn't block enabling nightlies, it does block landing anything.

I closed aurora due to people landing on unstarred orange & due to the issue being unknown. Now that we have a bug & patch, I have reopened.
Comment on attachment 657755 [details] [diff] [review]
Patch v1

Would it make sense to have the setter initialize the service as needed, rather than just adjusting all the callers? I guess this is an xpcshell-only problem so that's not worth it?
Comment on attachment 657755 [details] [diff] [review]
Patch v1

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

Can you also add a guard to make the checkCompatibility setter throw if we haven't been through startup yet, that should make sure we don't accidentally hit this again.
Attachment #657755 - Flags: review?(dtownsend+bugmail) → review+
Depends on: 788378
(In reply to :Gavin Sharp (use gavin@gavinsharp.com for email) from comment #11)
> Would it make sense to have the setter initialize the service as needed,
> rather than just adjusting all the callers? I guess this is an xpcshell-only
> problem so that's not worth it?

I want to avoid doing that, as it's an unexpected consequence. But Dave's suggestion would also solve the issue, and fits in nicely with recent work adding similar guards (bug 782881).
https://hg.mozilla.org/integration/fx-team/rev/19450d9f0f4c
https://hg.mozilla.org/releases/mozilla-aurora/rev/6ee9ade752e4
Target Milestone: --- → mozilla17

Updated

5 years ago
status-firefox17: --- → fixed
https://hg.mozilla.org/mozilla-central/rev/19450d9f0f4c
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED

Updated

5 years ago
Target Milestone: mozilla17 → mozilla18
Whiteboard: [qa-]
You need to log in before you can comment on or make changes to this bug.