Closed
Bug 962605
Opened 10 years ago
Closed 10 years ago
Enable baseline jit in xpcshell
Categories
(Core :: XPConnect, defect)
Tracking
()
RESOLVED
FIXED
mozilla29
People
(Reporter: bzbarsky, Assigned: bzbarsky)
References
Details
Attachments
(2 files)
1.08 KB,
patch
|
bholley
:
review+
|
Details | Diff | Splinter Review |
1.52 KB,
patch
|
ted
:
review+
|
Details | Diff | Splinter Review |
Ion has the bug 931861 problem, but baseline should be ok and helps some scripts a lot already.
Assignee | ||
Comment 1•10 years ago
|
||
Attachment #8363684 -
Flags: review?(bobbyholley+bmo)
Assignee | ||
Updated•10 years ago
|
Whiteboard: [need review]
Updated•10 years ago
|
Attachment #8363684 -
Flags: review?(bobbyholley+bmo) → review+
Comment 2•10 years ago
|
||
Looping Eddy so that he knows this is happening.
Comment 3•10 years ago
|
||
(Note - this probably warrants a try push if you haven't done one already)
Comment 4•10 years ago
|
||
FYI the xpcshell tests have, for quite some time, been passing `-m -n` to the xpcshell invocation, which *used to* enable the JIT, but hasn't since bug 857845. So there has been at least some testing in this mode, historically. CC gps.
Assignee | ||
Comment 5•10 years ago
|
||
Used to enable a totally different jit. :( I pushed https://tbpl.mozilla.org/?tree=Try&rev=31425d24b979
Assignee | ||
Comment 6•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/c75f13d4f160
Flags: in-testsuite-
Whiteboard: [need review]
Target Milestone: --- → mozilla29
Backed this out in https://hg.mozilla.org/integration/mozilla-inbound/rev/15a9c1e49bab (along with the patches from bug 958576) because one of them seems to have introduced a new permanent failure only on OSX 10.8 m-oth: https://tbpl.mozilla.org/php/getParsedLog.php?id=33415277&tree=Mozilla-Inbound
Assignee | ||
Comment 8•10 years ago
|
||
Try run at https://tbpl.mozilla.org/?tree=Try&rev=ddeab064b5bf is totally green, so: https://hg.mozilla.org/integration/mozilla-inbound/rev/26984019ff59
Comment 9•10 years ago
|
||
Backed out for suspicion of causing OSX 10.8 mochitest-other crashes. https://hg.mozilla.org/integration/mozilla-inbound/rev/c0df37c18e54 https://tbpl.mozilla.org/php/getParsedLog.php?id=33467743&tree=Mozilla-Inbound
Assignee | ||
Comment 10•10 years ago
|
||
Yeah, this time the try runs confirm it... I have no idea why this change causes this orange only on debug 10.8, or even _how_ it manages to cause it. The failure is during dom/tests/mochitest/chrome/test_activation.xul and it looks like this: Assertion failure: aPrincipal, at ../../../content/base/src/nsXMLHttpRequest.h:237 with a stack like so: 00:14:20 INFO - 0 XUL!nsXMLHttpRequest::Construct(nsIPrincipal*, nsIGlobalObject*, nsIURI*) [nsXMLHttpRequest.h:143dbce829fe : 237 + 0x0] 00:14:20 INFO - 1 XUL!nsXMLHttpRequest::Constructor(mozilla::dom::GlobalObject 00:14:20 INFO - 2 XUL!mozilla::dom::XMLHttpRequestBinding::_constructor [XMLHttpRequestBinding.cpp:143dbce829fe : 1518 + 0x4] So basically script did |new XMLHttpRequest|, we QIed the global to nsIScriptObjectPrincipal, but then calling GetPrincipal() on that returned null. It doesn't happen every time, but pretty close. All of the above is happening in browser, not in xpcshell. It's also happening under the interpreter... I'll attach a full stack, for posterity. So here's the interesting thing. In a passing test run with this patch, I get this output in the log: 19:45:59 INFO - 5844 INFO TEST-PASS | chrome://mochitests/content/chrome/dom/tests/mochitest/chrome/test_DOM_element_instanceof.xul | Should be instance of HTMLLabelElement 19:45:59 INFO - 5847 INFO TEST-END | chrome://mochitests/content/chrome/dom/tests/mochitest/chrome/test_DOM_element_instanceof.xul | finished in 258ms 19:45:59 INFO - 5848 INFO TEST-PASS | chrome://mochitests/content/chrome/dom/tests/mochitest/chrome/test_DOM_element_instanceof.xul | report should have been submitted successfully 19:45:59 INFO - 5849 INFO TEST-PASS | chrome://mochitests/content/chrome/dom/tests/mochitest/chrome/test_DOM_element_instanceof.xul | Checking correct topic 19:45:59 INFO - 5850 INFO TEST-PASS | chrome://mochitests/content/chrome/dom/tests/mochitest/chrome/test_DOM_element_instanceof.xul | Subject should be a property bag 19:45:59 INFO - 5851 INFO TEST-PASS | chrome://mochitests/content/chrome/dom/tests/mochitest/chrome/test_DOM_element_instanceof.xul | Should have a server crash ID 19:45:59 INFO - 5852 INFO TEST-PASS | chrome://mochitests/content/chrome/dom/tests/mochitest/chrome/test_DOM_element_instanceof.xul | Server crash ID should not be an empty string 19:45:59 INFO - [1176] WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x80004005: file ../../../../content/base/src/nsXMLHttpRequest.cpp, line 1601 19:45:59 INFO - System JS : ERROR chrome://mochitests/content/chrome/dom/plugins/test/mochitest/test_hang_submit.xul:64 - NS_ERROR_FAILURE: 19:45:59 INFO - 5853 INFO TEST-START | chrome://mochitests/content/chrome/dom/tests/mochitest/chrome/test_activation.xul and then it's all fine. In a failing run, I get: 00:14:06 INFO - 5848 INFO TEST-START | chrome://mochitests/content/chrome/dom/tests/mochitest/chrome/test_activation.xul 00:14:07 INFO - 5849 INFO TEST-PASS | chrome://mochitests/content/chrome/dom/tests/mochitest/chrome/test_activation.xul | report should have been submitted successfully 00:14:07 INFO - 5850 INFO TEST-PASS | chrome://mochitests/content/chrome/dom/tests/mochitest/chrome/test_activation.xul | Checking correct topic 00:14:07 INFO - 5851 INFO TEST-PASS | chrome://mochitests/content/chrome/dom/tests/mochitest/chrome/test_activation.xul | Subject should be a property bag 00:14:07 INFO - 5852 INFO TEST-PASS | chrome://mochitests/content/chrome/dom/tests/mochitest/chrome/test_activation.xul | Should have a server crash ID 00:14:07 INFO - 5853 INFO TEST-PASS | chrome://mochitests/content/chrome/dom/tests/mochitest/chrome/test_activation.xul | Server crash ID should not be an empty string 00:14:07 INFO - [1185] WARNING: No outer window available!: file ../../../dom/base/nsGlobalWindow.cpp, line 11078 and then the crash. We only have two tests that have those "crash ID" strings in them: test_crash_submit.xul and test_hang_submit.xul. And if you look at the "passing" run there you see that it's test_hang_submit that's crapping over later tests, and in particular still running once its window is already gone. All that matters is _how_ gone it is. If it's still there enough to create the XHR object, we bail when we try to do GetContextForEventHandlers(). But if it's really gone we seem to hit that fatal assert. So looking at test_hang_submit, in a log without this patch at all: 02:41:52 INFO - 5722 INFO TEST-START | chrome://mochitests/content/chrome/dom/plugins/test/mochitest/test_hang_submit.xul 02:41:53 INFO - 5723 INFO TEST-KNOWN-FAIL | chrome://mochitests/content/chrome/dom/plugins/test/mochitest/test_hang_submit.xul | Can't test plugin crash notification on OS X 10.7 or 10.8, see bug 705047 02:41:54 INFO - 5726 INFO TEST-END | chrome://mochitests/content/chrome/dom/plugins/test/mochitest/test_hang_submit.xul | finished in 2143ms 02:41:54 INFO - 5727 INFO TEST-PASS | chrome://mochitests/content/chrome/dom/plugins/test/mochitest/test_hang_submit.xul | Plugin crashed notification received 02:41:54 INFO - 5728 INFO TEST-PASS | chrome://mochitests/content/chrome/dom/plugins/test/mochitest/test_hang_submit.xul | event is correct type but then no spurious "report should have been submitted successfully" messages on later tests... In a passing log with this patch, I see the same messages (including the test-pass bits after test-end), but then the test keeps running, apparently.
Target Milestone: mozilla29 → ---
Assignee | ||
Comment 11•10 years ago
|
||
So the test_hang_submit test does this: 27 if (isOSXLion || isOSXMtnLion) { 28 todo(false, "Can't test plugin crash notification on OS X 10.7 or 10.8, see bug 705047"); 29 SimpleTest.finish(); 30 } but then keeps running the test. Is there a reason for that?
Flags: needinfo?(ted)
Flags: needinfo?(georg.fritzsche)
Assignee | ||
Comment 12•10 years ago
|
||
Also, my best guess at this point is that xpcshell baseline compiler just changes timing for the SJS here somehow or something, such that we get a response when we're not actually expecting one. But maybe it also makes toolkit/crashreporter/test/browser/crashreport.sjs run incorrectly.... Hard to tell so far.
Comment 13•10 years ago
|
||
That's just a bug, there should be a return() there. Although given my reading of bug 705047, those tests should be re-enabled now that the crashreporter is working again. Maybe that entire hunk should go?
Comment 14•10 years ago
|
||
Right, if that test runs fine apart from the todo we should just remove that part.
Flags: needinfo?(georg.fritzsche)
Comment 15•10 years ago
|
||
Yeah, let's try just enabling the test and see if that makes this go away. Sounds like there might be a real issue with lifetimes of the XHR etc, but maybe that's not critical to fix?
Flags: needinfo?(ted)
Assignee | ||
Comment 16•10 years ago
|
||
I did some try pushes to that effect, but it still worries me that enabling the JIT on xpcshell changed behavior here... Why were we not getting a response at all before but are with the JIT on?
Comment 17•10 years ago
|
||
Ick. That test disablement is pretty broken. It's racing against the onload="runTests()" call.
Comment 18•10 years ago
|
||
bz: I wonder if the brokenness has to do with the fact that the test is doing a sync XHR? http://hg.mozilla.org/mozilla-central/annotate/bfe4ed6d47ce/dom/plugins/test/mochitest/test_hang_submit.xul#l65
Assignee | ||
Comment 19•10 years ago
|
||
Doesn't seem like that should matter. Again, what we're changing here is the xpcshell behavior. So what I see is that without the xpcshell patch we never get past the early return in the "observe: function(subject, topic, data)" function in the test, but with the patch we do. Assuming that function is even called without the patch... I'm going to do a try push to see if it is.
Assignee | ||
Comment 20•10 years ago
|
||
OK, a bit more data: 1) Just removing the broken disabling of this test seems to be green: https://tbpl.mozilla.org/?tree=Try&rev=00a71e2d9263 2) Doing this patch on top of the removal seems to be green: https://tbpl.mozilla.org/?tree=Try&rev=e66fce02545f 3) Without this patch, on the test with the broken disabling, the observer gets topic 'crash-report-status' and data 'submitting', then nothing. 4) With this patch, on the test with the broken disabling, the observer gets that, and then topic 'crash-report-status' and data 'success', after which it runs those extra steps. I have no idea why JIT for xpcshell affects what the observer gets, but I'm tempted to just remove the broken disabling and move on.
Assignee | ||
Comment 21•10 years ago
|
||
Attachment #8365421 -
Flags: review?(ted)
Assignee | ||
Comment 22•10 years ago
|
||
Oh, for #3 and #4 the try runs are https://tbpl.mozilla.org/?tree=Try&rev=112b3785188b and https://tbpl.mozilla.org/?tree=Try&rev=141af1f166a3 respectively.
Updated•10 years ago
|
Attachment #8365421 -
Flags: review?(ted) → review+
Assignee | ||
Comment 23•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/fdd790518733 https://hg.mozilla.org/integration/mozilla-inbound/rev/c074cbfa54c6
Assignee | ||
Updated•10 years ago
|
Target Milestone: --- → mozilla29
Comment 24•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/fdd790518733 https://hg.mozilla.org/mozilla-central/rev/c074cbfa54c6
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•