Closed
Bug 1157714
Opened 10 years ago
Closed 10 years ago
browser_monitorUncaught.js is going to permafail when Gecko 40 merges to Aurora
Categories
(Core :: DOM: Core & HTML, defect)
Core
DOM: Core & HTML
Tracking
()
VERIFIED
FIXED
mozilla41
People
(Reporter: RyanVM, Assigned: Yoric)
References
Details
Attachments
(1 file, 1 obsolete file)
39 bytes,
text/x-review-board-request
|
Details |
https://treeherder.mozilla.org/logviewer.html#?job_id=6870722&repo=try
23:18:02 INFO - 292 INFO Entering test test_globals
23:18:02 INFO - 293 INFO TEST-UNEXPECTED-FAIL | dom/promise/tests/browser_monitorUncaught.js | We are testing DOM Promise. - "function ()\n{\n\"use strict\";\n\n return new Deferred();\n}" == "undefined" - JS frame :: chrome://mochitests/content/browser/dom/promise/tests/browser_monitorUncaught.js :: test_globals :: line 10
23:18:02 INFO - Stack trace:
23:18:02 INFO - chrome://mochitests/content/browser/dom/promise/tests/browser_monitorUncaught.js:test_globals:10
23:18:02 INFO - self-hosted:next:624
23:18:02 INFO - Tester_execTest@chrome://mochikit/content/browser-test.js:742:9
23:18:02 INFO - Tester.prototype.nextTest</<@chrome://mochikit/content/browser-test.js:665:7
23:18:02 INFO - SimpleTest.waitForFocus/waitForFocusInner/focusedOrLoaded/<@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:744:59
23:18:02 INFO - 294 INFO TEST-PASS | dom/promise/tests/browser_monitorUncaught.js | PromiseDebugging is available. - "function PromiseDebugging() {\n [native code]\n}" != "undefined"
etc...
Flags: needinfo?(dteller)
Assignee | ||
Comment 1•10 years ago
|
||
Weird. There's nothing in that directory that should be related to the channel.
Assignee | ||
Comment 2•10 years ago
|
||
Wait, what?
This log means that Promise.jsm somehow overwrites DOM Promise in browser tests, at least in this test. That's bad, but that doesn't seem to be caused by DOM Promise. I'll need to investigate some more.
Flags: needinfo?(dteller)
Reporter | ||
Comment 3•10 years ago
|
||
If Yoric is gone through 6-May, that leaves very little time to investigate and fix this prior to the next uplift. Vladan, is there someone else on your team that can look in the mean time?
Flags: needinfo?(vdjeric)
Comment 4•10 years ago
|
||
Roberto, could you take a look at this issue while I'm away?
Flags: needinfo?(vdjeric) → needinfo?(rvitillo)
Comment 6•10 years ago
|
||
I am having an hard time reproducing this on a build with a similar configuration.
Comment 7•10 years ago
|
||
It seems that the following happens with Nightly when compiled with the Aurora build configuration:
1. contentAreautils.js imports lazily Promise.jsm in the chrome window [1]
2. when the Promise object is first accessed in browser_monitorUncaught.js [2], Promise.jsm is imported in the window object
In the Nightly build, step 2 never happens as the native Promise implementation is used. The only way there to force the use of Promise.jsm is to access window.Promise. When Nightly is compiled as Aurora though, it seems as if the native Promise implementation is missing.
Vladan, Yoric, do you have any idea what might be causing the behaviour?
[1] https://dxr.mozilla.org/mozilla-central/source/toolkit/content/contentAreaUtils.js?from=contentAreaUtils.js&case=true#20
[2] https://dxr.mozilla.org/mozilla-central/source/dom/promise/tests/browser_monitorUncaught.js?from=browser_monitorUncaught.js&case=true#10
Flags: needinfo?(vdjeric)
Flags: needinfo?(dteller)
Assignee | ||
Comment 8•10 years ago
|
||
Are you sure that the native implementation is missing? It was my impression that it was somehow overwritten with Promise.jsm.
Flags: needinfo?(dteller)
Comment 9•10 years ago
|
||
Deferring to Yoric on this one
Comment 10•10 years ago
|
||
Yoric, you can instrument defineLazyModuleGetter and verify that comment 7 outlines precisely what's happening.
After a conversation with paolo, I came to realize that what happens in Aurora is likely the desired behaviour as Promise.jsm is imported lazily in the global object before the test is executed. It's not clear to me why that very same lazy import has no effect in Nightly though.
Flags: needinfo?(vdjeric)
Assignee | ||
Comment 11•10 years ago
|
||
Overshadowing DOM Promise with Promise.jsm strikes me as odd. Paolo, what makes you think that this is the right behavior? Also, bz, any idea how we could force DOM Promise to take back its place if it is masked?
Flags: needinfo?(paolo.mozmail)
Flags: needinfo?(bzbarsky)
![]() |
||
Comment 12•10 years ago
|
||
There's basically nothing magic about DOM Promise: it's just a property on the global object. The property is writable, configurable, etc, so people can override it, like any other built-in constructor. That's what Promise.jsm does.
We could certainly add something that would redefine the global's .Promise to DOM Promise (e.g. on windowutils or some equivalent). But who would use it and for what purpose?
Flags: needinfo?(bzbarsky)
Comment 13•10 years ago
|
||
(In reply to David Rajchenbach-Teller [:Yoric] (use "needinfo") from comment #11)
> Overshadowing DOM Promise with Promise.jsm strikes me as odd. Paolo, what
> makes you think that this is the right behavior?
Browser-chrome tests are supposed to be run in the environment of the browser XUL window, they are not supposed to see an environment that is substantially different from the one we have in production.
In the browser XUL window, we load Promise.jsm, so tests would need to see Promise.jsm as well. We use Promise.jsm since it is still ahead of DOM Promise with regard to handling of uncaught rejections.
Promise.jsm is loaded by contentAreaUtils and maybe other scripts. This is done using a <script> tag in a XUL overlay loaded over "browser.xul". The testing infrastructure is also loaded as an overlay of "browser.xul".
I haven't investigated this in detail, but the difference when compiling with the Aurora flags might be caused by some kind of Nightly-only overlay isolation feature in the JavaScript environment?
That said, I've taken a look and the test doesn't seem to depend on having the browser window environment available, or to be testing the browser-chrome framework itself (it's only testing the debugging API and not its use in the testing framework).
Converting this to an xpcshell test might simplify things. On the other hand, I guess there was a reason in the first place why this wasn't an xpcshell test to begin with.
Flags: needinfo?(paolo.mozmail)
![]() |
||
Comment 14•10 years ago
|
||
> might be caused by some kind of Nightly-only overlay isolation feature
Ah, yes. dom.compartment_per_addon is only set to true if E10S_TESTING_ONLY is defined, and that's only defined if NIGHTLY_BUILD in configure.in. I bet that's the issue here, if the testing infra overlay comes from an addon. If so, it would get its own global scope on nightly, but share one with contentAreaUtils on aurora.
Comment 15•10 years ago
|
||
(In reply to Not doing reviews right now from comment #14)
> Ah, yes. dom.compartment_per_addon is only set to true if E10S_TESTING_ONLY
> is defined, and that's only defined if NIGHTLY_BUILD in configure.in. I bet
> that's the issue here, if the testing infra overlay comes from an addon. If
> so, it would get its own global scope on nightly, but share one with
> contentAreaUtils on aurora.
Hm, so if this is the case, maybe a simple solution to get consistency would be for the testing framework, instead of calling loadSubScript directly on the test and head files, to call a helper function in "browser.js" that in turn calls loadSubScript? If I understand correctly, the script would get the global active at the time loadSubScript is called, which would be the one for "browser.js".
The browser_monitorUncaught test would need to be modified to get a reference to DOM Promise from somewhere else than the browser's global scope (or converted to xpcshell).
Assignee | ||
Comment 16•10 years ago
|
||
I'll try and see if it can be converted to xpcshell.
Assignee | ||
Comment 17•10 years ago
|
||
/r/9025 - Bug 1157714 - Moving browser_monitorUncaught to a xpcshell test;r=self
Pull down this commit:
hg pull -r 7a5d3de1959c831ac8980729145437fc073f6a31 https://reviewboard-hg.mozilla.org/gecko/
Assignee | ||
Comment 18•10 years ago
|
||
Comment on attachment 8607465 [details]
MozReview Request: bz://1157714/Yoric
/r/9025 - Bug 1157714 - Moving browser_monitorUncaught to a xpcshell test;r=self
Pull down this commit:
hg pull -r 6a62bcd962f7302b6068aeac138784be47fabe23 https://reviewboard-hg.mozilla.org/gecko/
Comment 19•10 years ago
|
||
Comment on attachment 8607465 [details]
MozReview Request: bz://1157714/Yoric
Assuming you've published this after testing locally, it looks good to me, if you'd like a second opinion. Thanks for doing this!
Glad to see how using Assert.jsm assertions made the conversion easy.
Assignee | ||
Comment 20•10 years ago
|
||
Thanks, Paolo.
Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=6a62bcd962f7
Keywords: checkin-needed
Comment 21•10 years ago
|
||
Keywords: checkin-needed
Assignee: nobody → dteller
Status: NEW → RESOLVED
Closed: 10 years ago
status-firefox41:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
Reporter | ||
Updated•10 years ago
|
Flags: needinfo?(ryanvm)
Reporter | ||
Comment 24•10 years ago
|
||
Flags: in-testsuite+
Assignee | ||
Comment 25•10 years ago
|
||
Attachment #8607465 -
Attachment is obsolete: true
Assignee | ||
Comment 26•10 years ago
|
||
Reporter | ||
Updated•10 years ago
|
Updated•6 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•