904927 - Bug 853388 breaks gaia unit tests

Status: RESOLVED FIXED

(Toolkit :: Add-ons Manager, defect)

Description

[Jonathan Griffin (:jgriffin)]

The landing of bug 853388 broke gaia unit tests; see: https://tbpl.mozilla.org/?showall=1&tree=Fx-Team&rev=20d83a7220ca

full log:  https://tbpl.mozilla.org/php/getParsedLog.php?id=26512494&tree=Fx-Team&full=1

With this changeset, it appears that the tests never even start; I'm not sure why.

Comment 1

[:Irving Reid (No longer working on Firefox)]

The changeset you point to was missing reviewer annotations on several of the patches, so I manually cancelled the build and re-pushed at https://tbpl.mozilla.org/?showall=1&tree=Fx-Team&rev=61e0fb09a73c. Are you still seeing problems after that changeset?

Comment 2

[Jonathan Griffin (:jgriffin)]

Yes, this test went perma-red as of this commit, and is completely broken now on all branches after fx-team was merged to trunk branches.

Comment 3

[Jonathan Griffin (:jgriffin)]

I tried running these locally; the problem is that the b2g desktop build doesn't even load gaia successfully with a debug profile; you only see a black screen.  With a non-debug profile, it works ok.  (The gaia unit tests require a debug profile.)

The only error in the jsconsole is this:

[Exception... "'Failure' when calling method: [nsIPromptFactory::getPrompt]"  nsresult: "0x80004005 (NS_ERROR_FAILURE)"  location: "native frame :: <unknown filename> :: <TOP_LEVEL> :: line 0"  data: no]

Comment 4

[Julien Wajsberg [:julienw]]

Maybe Alexandre might have a clue here ?

Comment 5

[:Irving Reid (No longer working on Firefox)]

Attachment: xpiprovider-scope

I have a patch for this, but I'm worried about it - it seems like the problem was caused by differences in variable scope across loaded modules, between the Firefox and B2G environments. Am I misunderstanding, or is this really what's going on?

In any case, the addon manager was trying to log error messages but apparently the JS console isn't ready at the time the addon manager starts up. I've changed the logging to always dump ERROR level messages to stdout to make sure the are seen.

The two issues I had to address were:

1) Variables and functions declared at the root of an included module weren't showing up in the scope returned by the script loader

2) Names in scope in the parent module weren't visible in the sub module

In the patch, changing declarations from "var foo = whatever" and "function bar() ..." to "this.foo = whatever" and "this.bar = function() ..." in the submodule made the symbols visible, and I also needed to explicitly pass in or load names in the submodule that it was getting from the parent scope before.

The thing that bothers me is that this code works just fine if I build the "browser" target from mozilla-central, but not if I build the "b2g" target. Should I really be seeing a difference in behaviour?

In any case, I'll push a try build with this patch.

Comment 6

[:Irving Reid (No longer working on Firefox)]

https://tbpl.mozilla.org/?tree=Try&showall=1&rev=98e952446db1

Appears to be running the Gaia test suite, with at least one test failure. I'd still like to see all the other tests complete to make sure it isn't breaking any other platforms.

Comment 7

[Alexandre Poirot [:ochameau]]

(In reply to :Irving Reid from comment #5)
> Created attachment 791826 [details] [diff] [review]
> xpiprovider-scope
> 
> I have a patch for this, but I'm worried about it - it seems like the
> problem was caused by differences in variable scope across loaded modules,
> between the Firefox and B2G environments. Am I misunderstanding, or is this
> really what's going on?

Yes, JSM are known to behave differently in b2g. I think it was introduced when we started sharing global object for all JSMs (in order to save a bunch of memory).

Comment 8

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Comment on attachment 791826 [details] [diff] [review]
xpiprovider-scope

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

Bah, soon as I saw the bug summary mentioning Gaia, I remembered about this. See the following thread for more context:
https://groups.google.com/forum/?fromgroups=#!searchin/mozilla.dev.platform/jsm/mozilla.dev.platform/Ic2YmqbRyIE/oRDIaAXvMn0J

Not sure why AddonManager.jsm needs imported now but didn't before, but if it works it works.

Comment 9

[:Irving Reid (No longer working on Firefox)]

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

XPIProviderUtils.js was using some AddonManager-defined constants, which it had been inheriting from XPIProvider.jsm; with B2G's scope rules they weren't visible. That's why I'm now importing AddonManager in XPIProviderUtils.

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/7287b269f6f2