tests/browser.js does a lot of stuff with enablePrivilege when these tests are run via jsreftest. This blocks our efforts to gut CAPS, which blocks the effort to remove JSStackFrame inspection, which blocks IonMonkey. It would be simple to convert browser.js to SpecialPowers if it were a mochitest, but unfortunately it's run as a reftest, and I don't believe we have SpecialPowers in mochitests right now. jmaher would know more. I'm working pretty hard to have this stuff ready in time for the IonMonkey landing, so it would be nice if someone on the JS side could do the necessary driving to get the JS tests fixed up. dmandelin, thoughts?

we have no specialpowers in reftest. SpecialPowers is designed to be lightweight, so we could make a reftest version or a portable version. iirc, the reftest guys were not too excited about the idea of specialpowers in reftest as there are only a few calls needed for reftest.

Is there any reason why these js tests run on tinderbox in the browser rather than in the shell? Could we fix that?

For one, I think there are a few JS ref-tests that actually touch the browser. Perhaps these could be turned into xpconnect xpcshell/mochitests, though? In the past, JS reftests in the browser has caught a few bugs dealing with browser/JS interaction (global object, namely), but not too many. But mostly it is a huge pain because we also run the non-browser JS reftests in the shell with a little python script (js/src/tests/jstests.py) and so it is easy to forget you can't depend on shell-only testing functions. I know terrence has talked about unifying src/jit-tests/jit_tests.py and src/tests/jstests.py; perhaps this would be a good opportunity?

(In reply to Luke Wagner [:luke] from comment #3) > I know terrence has talked about unifying src/jit-tests/jit_tests.py and > src/tests/jstests.py; perhaps this would be a good opportunity? SGTM. I think that we've got enough test coverage elsewhere to handle the browser stuff, and CPG probably makes the two environments similar anyway.

(In reply to Bobby Holley (:bholley) from comment #4) > SGTM. I think that we've got enough test coverage elsewhere to handle the > browser stuff, and CPG probably makes the two environments similar anyway. I would tend to agree.

I think this is exactly what I was planning to do, except I was going to polyfill browser.js, instead of using xpcshell/mochitests. Thank you for making my work much simpler! Let me summarize my understanding of the current plan to make sure we are all fully on the same page: 1) Move jstests that use functionality in browser.js to xpcshell/mochitests, removing the "if (feature) {}" guard and just doing the test. 2) Remove the jsreftest browser runs from tbpl. 3) Kill "make jstestbrowser", the manifest generator, browser.js, and other assorted uglyness. 4) Unify shell.js between jit-tests and jsreftests and convert |reftest| notation to |jit-test| notation, thus killing the 387 js shell invocations that must run and succeed before jstests.py can start running tests. 5) Finish backfilling jit-test functionality into jstests. 6) Copy jit-test suite into the tests directory and kill the jittest runners on tbpl because they now do nothing. 7) Throw a party.

Sounds good to me - huzzah!

Just to confirm terrence - are you actively working on this? We're still hoping to have this enablePrivilege stuff landed in time for IM.

No, I'm working full time on GenerationalGC. I have also been doing various work to make our testing situation better, but only 0-10% of the time.

(In reply to Terrence Cole [:terrence] from comment #9) > No, I'm working full time on GenerationalGC. I have also been doing various > work to make our testing situation better, but only 0-10% of the time. Given the amount of work I and others are doing to get this stuff ready for IM, I think it's reasonable to ask that the JS team fixes its own tests here. Can we find the necessary resources to make this happen?

(In reply to Bobby Holley (:bholley) from comment #10) I don't know if you missed it, but in on your js-internals email thread, dvander pointed out that enablePrivilege-removal doesn't block IM: as long as we are fine with only calling JS_SetFrameAnnotate on the current frame (innermost), which I think is the case, we have a straightforward, even simple, temporary solution. That said, GGC, isn't exactly around the corner, so it would be good to do this sooner rather than later if it was a relatively quick task...

I'm pretty against moving jstests to the shell. I've found a bunch of GC-related bugs with jsreftests. They don't reproduce in the shell because it's too simple. Often they also reproduce in Mochitests, but it seems to be a lot easier to reproduce things with jsreftests than with Mochitests. Mochitests appear to have a lot more non-determinism.

Sounds like we'd better stick to the original plan of running jsreftests in both places then. Can we replace the code in browser.js with equivalents that don't depend on enablePrivilege? For the shell, most of those methods are builtins and I thought those were also available to chrome in the browser as well?

(In reply to Luke Wagner [:luke] from comment #11) > (In reply to Bobby Holley (:bholley) from comment #10) > I don't know if you missed it, but in on your js-internals email thread, > dvander pointed out that enablePrivilege-removal doesn't block IM: as long > as we are fine with only calling JS_SetFrameAnnotate on the current frame > (innermost), which I think is the case, we have a straightforward, even > simple, temporary solution. I was under the impression that the temporary solution was still undesirable. More to the point, it will be a de-facto permanent solution unless we have a story here. And there's no time like the present. (In reply to Terrence Cole [:terrence] from comment #13) > Sounds like we'd better stick to the original plan of running jsreftests in > both places then. Can we replace the code in browser.js with equivalents > that don't depend on enablePrivilege? For the shell, most of those methods > are builtins and I thought those were also available to chrome in the > browser as well? I'm not aware of any such thing. I kind of doubt it, given that dump() is implemented with a lot of special-case code in nsGlobalWindow.cpp. The browser tests are run as reftests, and reftest doesn't support SpecialPowers (last I heard the reftest owners were opposed to adding SpecialPowers on the grounds that they felt it wasn't necessary). So if the reftest owners don't want SpecialPowers in reftests and bill is opposed to moving jstests to either the shell or mochitests, we're kind of short on options. The easiest thing to do is probably just to move the one test that fails with my interim solution (js1_5/Regress/regress-328897.js) to a mochitest. Once bug 757046 lands, the current setup implies that _all_ js tests will be running with chrome privileges. I assume that's ok?

alternatively we could run jsreftests with specialpowers and leave reftests proper alone. I have a patch that works except for 1 test case, I should be able to figure that test case out tomorrow and get this patch ready for review. This would require releng to add a --specialPowers flag to the runreftests.py harness.

(In reply to Joel Maher (:jmaher) from comment #15) Thank you for working on this!

This patch does a lot of stuff, but the main thing it does is add specialpowers to the reftest harness and ports over the browser.js to use specialpowers. Ideally we would add this in via a --specialPowers commandline option. I am posting it now because this test fails, but the rest of the jsreftests pass: http://mxr.mozilla.org/mozilla-central/source/js/src/tests/js1_5/extensions/regress-369696-02.js#51 Some bits to clean up with this are ensuring mochitests work fine (including browser-chrome, chrome, a11y), and marionette which uses specialpowers as well. If someone can figure out how to get that test case to pass, then we can move forward on getting this checked in.

(In reply to Joel Maher (:jmaher) from comment #17) > I am posting it now because this test fails, but the rest of the jsreftests > pass: > http://mxr.mozilla.org/mozilla-central/source/js/src/tests/js1_5/extensions/ > regress-369696-02.js#51 > > Some bits to clean up with this are ensuring mochitests work fine (including > browser-chrome, chrome, a11y), and marionette which uses specialpowers as > well. If someone can figure out how to get that test case to pass, then we > can move forward on getting this checked in. So what happens is that we invoke Object.toSource on |window|, which does a deep traversal of the object. So we first get window.SpecialPowers, then we get window.DOMWindowUtils, and then we try invoking those getters (which have been rebound in specialPowersAPI). But some of the getters on DOMWindowUtils actually throw in certain cases. Hence the failure. Object.prototype.toSource.apply(window) isn't really meant to happen, because window is a WN and has its own toSource hook. Anyway, the easy solution here is just to set window.SpecialPowers = null at the top of this test. ;-)

(In reply to Bobby Holley (:bholley) from comment #18) > Anyway, the easy solution here is just to set window.SpecialPowers = null at > the top of this test. ;-) And make sure to add a comment linking to this bug!

there is a crash in debug builds only: https://tbpl.mozilla.org/php/getParsedLog.php?id=12971513&tree=Try&full=1#error0 specifically in e4x/QName/regress-619529.js. Otherwise opt builds are green for jsreftest, a few mochitest-other cleanup pieces left.

since e4x is going away with this release anyway, we should be safe turning off this one testcase.

This patch works on try server for all tests. I had to disable 3 additional jsreftests due to crashes: ecma_5/Global/adding-global-var-nonextensible-error.js js1_8_5/extensions/set-property-non-extensible.js js1_8_5/regress/regress-633741.js :bholley, can we confirm that it is ok to skip these tests?

(In reply to Joel Maher (:jmaher) from comment #22) > This patch works on try server for all tests. I had to disable 3 additional > jsreftests due to crashes: > > ecma_5/Global/adding-global-var-nonextensible-error.js > js1_8_5/extensions/set-property-non-extensible.js > js1_8_5/regress/regress-633741.js > > :bholley, can we confirm that it is ok to skip these tests? Erm, those aren't e4x tests, so I don't think we should skip those. Terrance, do you have time to take a quick look and see why these tests fail with the given changes in the testing infrastructure?

Those look like they may be important tests. I'll take a look.

any data on the 3 crashes that are 'skip' in my patch? Would like to keep the ball rolling on this patch.

Yeah, sorry, I got stuck. With the patch from this bug applied, I'm hitting this build error immediately: Makefile:20: config/autoconf.mk: No such file or directory /home/terrence/moz/branch/investigate_specialprivs_test_failures/mozilla/config/config.mk:27: config/autoconf.mk: No such file or directory /bin/sh: ./config.status: No such file or directory make: *** [config/autoconf.mk] Error 127 It looks like configure isn't processing config/autoconf.mk.in. Any ideas?

doesn't ring a bell. I will apply it and build again. Maybe something has landed that is causing this to fail.

I just pulled from m-i and rebuilt, all is well. The patch does some hg move commands, maybe those don't work well across qimports and similar commands.

Yes, must have been the hg moves. I had previously done |patch < foo.diff|, qimport worked much better. Currently building, will take a look after lunch.

Okay, so these all fail for the same reason: the window has been marked as non-extensible when we do a SaveWindowState, which tries to adjust the proto of the window. Looking at the tests, they all contain Object.preventExtensions(this). One of the three -- ecma_5/Global/adding-global-var-nonextensible-error.js -- looks like it was intended to be disabled in the browser, except that there is the text "HACK" in front of the |reftest|, which breaks the disabling code. We should probably just remove this text here. It may be appropriate to do the same for the other two tests as well. Unfortunately, I'm not sure what any of what I just said actually means. Bobby, can you explain why setting the window non-extensible would ever work and why it stops working with special powers? Here is the backtrace I saw, if that helps: #0 0x00007ffff3b43f3a in js::SetProto (cx=0x7fffd55d4040, obj=..., proto=..., checkForCycles=false) at /home/terrence/moz/branch/investigate_specialprivs_test_failures/mozilla/js/src/jsobj.cpp:3743 #1 0x00007ffff3a4057a in JS_SetPrototype (cx=0x7fffd55d4040, obj_=0x7fffdde89080, proto_=0x7fffdde85070) at /home/terrence/moz/branch/investigate_specialprivs_test_failures/mozilla/js/src/jsapi.cpp:3261 #2 0x00007ffff2207200 in nsGlobalWindow::SaveWindowState (this=0x7fffd4908000, aState=0x7fffffffa490) at /home/terrence/moz/branch/investigate_specialprivs_test_failures/mozilla/dom/base/nsGlobalWindow.cpp:10304 #3 0x00007ffff293d131 in nsDocShell::CaptureState (this=0x7fffd4907000) at /home/terrence/moz/branch/investigate_specialprivs_test_failures/mozilla/docshell/base/nsDocShell.cpp:6818 #4 0x00007ffff2940f88 in nsDocShell::SetupNewViewer (this=0x7fffd4907000, aNewViewer=0x7fffd47f8fb0) at /home/terrence/moz/branch/investigate_specialprivs_test_failures/mozilla/docshell/base/nsDocShell.cpp:7758 #5 0x00007ffff2939d0b in nsDocShell::Embed (this=0x7fffd4907000, aContentViewer=0x7fffd47f8fb0, aCommand=0x7ffff42239c2 "", aExtraInfo=0x0) at /home/terrence/moz/branch/investigate_specialprivs_test_failures/mozilla/docshell/base/nsDocShell.cpp:5927 #6 0x00007ffff2940866 in nsDocShell::CreateContentViewer (this=0x7fffd4907000, aContentType=0x7fffd3743588 "text/html", request=0x7fffd4738e50, aContentHandler=0x7fffd3752f80) at /home/terrence/moz/branch/investigate_specialprivs_test_failures/mozilla/docshell/base/nsDocShell.cpp:7635 #7 0x00007ffff295cacc in nsDSURIContentListener::DoContent (this=0x7fffd55ff920, aContentType=0x7fffd3743588 "text/html", aIsContentPreferred=false, request=0x7fffd4738e50, aContentHandler=0x7fffd3752f80, aAbortProcess=0x7fffffffac0f) at /home/terrence/moz/branch/investigate_specialprivs_test_failures/mozilla/docshell/base/nsDSURIContentListener.cpp:119 #8 0x00007ffff2964d53 in nsDocumentOpenInfo::TryContentListener (this=0x7fffd3752f60, aListener=0x7fffd55ff920, aChannel=0x7fffd4738e50) at /home/terrence/moz/branch/investigate_specialprivs_test_failures/mozilla/uriloader/base/nsURILoader.cpp:678 #9 0x00007ffff2963949 in nsDocumentOpenInfo::DispatchContent (this=0x7fffd3752f60, request=0x7fffd4738e50, aCtxt=0x0) at /home/terrence/moz/branch/investigate_specialprivs_test_failures/mozilla/uriloader/base/nsURILoader.cpp:375 #10 0x00007ffff29632b1 in nsDocumentOpenInfo::OnStartRequest (this=0x7fffd3752f60, request=0x7fffd4738e50, aCtxt=0x0) at /home/terrence/moz/branch/investigate_specialprivs_test_failures/mozilla/uriloader/base/nsURILoader.cpp:263 #11 0x00007ffff16dddec in nsBaseChannel::OnStartRequest (this=0x7fffd4738e00, request=0x7fffd37a1e80, ctxt=0x0) at /home/terrence/moz/branch/investigate_specialprivs_test_failures/mozilla/netwerk/base/src/nsBaseChannel.cpp:704 #12 0x00007ffff16f2afb in nsInputStreamPump::OnStateStart (this=0x7fffd37a1e80) at /home/terrence/moz/branch/investigate_specialprivs_test_failures/mozilla/netwerk/base/src/nsInputStreamPump.cpp:416 #13 0x00007ffff16f28c4 in nsInputStreamPump::OnInputStreamReady (this=0x7fffd37a1e80, stream=0x7fffd472fb38) at /home/terrence/moz/branch/investigate_specialprivs_test_failures/mozilla/netwerk/base/src/nsInputStreamPump.cpp:367 #14 0x00007ffff3051e73 in nsInputStreamReadyEvent::Run (this=0x7fffd45edb40) at /home/terrence/moz/branch/investigate_specialprivs_test_failures/mozilla/xpcom/io/nsStreamUtils.cpp:82 #15 0x00007ffff3074a70 in nsThread::ProcessNextEvent (this=0x7ffff6b6efc0, mayWait=false, result=0x7fffffffb1df) at /home/terrence/moz/branch/investigate_specialprivs_test_failures/mozilla/xpcom/threads/nsThread.cpp:624 #16 0x00007ffff3007691 in NS_ProcessNextEvent_P (thread=0x7ffff6b6efc0, mayWait=false) at /home/terrence/moz/branch/investigate_specialprivs_test_failures/br-dbg-gcc-x64/xpcom/build/nsThreadUtils.cpp:217 #17 0x00007ffff2e4d1f2 in mozilla::ipc::MessagePump::Run (this=0x7ffff6bf0f40, aDelegate=0x7ffff6bdf3d0) at /home/terrence/moz/branch/investigate_specialprivs_test_failures/mozilla/ipc/glue/MessagePump.cpp:82 #18 0x00007ffff30c60d3 in MessageLoop::RunInternal (this=0x7ffff6bdf3d0) at /home/terrence/moz/branch/investigate_specialprivs_test_failures/mozilla/ipc/chromium/src/base/message_loop.cc:208 #19 0x00007ffff30c6064 in MessageLoop::RunHandler (this=0x7ffff6bdf3d0) at /home/terrence/moz/branch/investigate_specialprivs_test_failures/mozilla/ipc/chromium/src/base/message_loop.cc:201 #20 0x00007ffff30c603d in MessageLoop::Run (this=0x7ffff6bdf3d0) at /home/terrence/moz/branch/investigate_specialprivs_test_failures/mozilla/ipc/chromium/src/base/message_loop.cc:175 #21 0x00007ffff2cdcc3a in nsBaseAppShell::Run (this=0x7fffe13d6710) at /home/terrence/moz/branch/investigate_specialprivs_test_failures/mozilla/widget/xpwidgets/nsBaseAppShell.cpp:163 #22 0x00007ffff2a1fc36 in nsAppStartup::Run (this=0x7fffe13b9e20) at /home/terrence/moz/branch/investigate_specialprivs_test_failures/mozilla/toolkit/components/startup/nsAppStartup.cpp:257 #23 0x00007ffff169833f in XREMain::XRE_mainRun (this=0x7fffffffb750) at /home/terrence/moz/branch/investigate_specialprivs_test_failures/mozilla/toolkit/xre/nsAppRunner.cpp:3787 #24 0x00007ffff1698617 in XREMain::XRE_main (this=0x7fffffffb750, argc=6, argv=0x7fffffffdbb8, aAppData=0x633c20) at /home/terrence/moz/branch/investigate_specialprivs_test_failures/mozilla/toolkit/xre/nsAppRunner.cpp:3864 #25 0x00007ffff1698853 in XRE_main (argc=6, argv=0x7fffffffdbb8, aAppData=0x633c20, aFlags=0) at /home/terrence/moz/branch/investigate_specialprivs_test_failures/mozilla/toolkit/xre/nsAppRunner.cpp:3940 #26 0x0000000000402c7a in do_main (argc=6, argv=0x7fffffffdbb8) at /home/terrence/moz/branch/investigate_specialprivs_test_failures/mozilla/browser/app/nsBrowserApp.cpp:160 #27 0x0000000000402ec9 in main (argc=6, argv=0x7fffffffdbb8) at /home/terrence/moz/branch/investigate_specialprivs_test_failures/mozilla/browser/app/nsBrowserApp.cpp:298

Thanks, this sounds promising as a root cause. I believe I was the reason for the 'HACK' inserted before the reftest.

(In reply to Terrence Cole [:terrence] from comment #30) > Unfortunately, I'm not sure what any of what I just said actually means. > Bobby, can you explain why setting the window non-extensible would ever work > and why it stops working with special powers? The window object is an outer object, implemented as a proxy. Experimenting, I see that Object.isExtensible(window) === true, and that I can Object.preventExtensions(window), after which point Object.isExtensible(window) === false. But it has no effect on the underlying object, presumably because it's a proxy. From my reading of the code, preventExtensions just mucks with the shape tree, which AIUI is ignored for proxies. Is this correct?

I know next to nothing about proxies, but the code that is failing in nsGlobalWindow::SaveWindowState is explicitly setting them and failing when it does so.

(In reply to Terrence Cole [:terrence] from comment #33) > I know next to nothing about proxies, but the code that is failing in > nsGlobalWindow::SaveWindowState is explicitly setting them and failing when > it does so. Not sure if it's helpful, but I can confirm that calling Object::preventExtensions on a proxy will not cause the object being proxied to to become non-extensible. What's more, subsequent calls to isExtensible will report that the proxy is non-extensible, even though the object being proxied to is not (because we only look at the NOT_EXTENSIBLE flag). Bug 674195 is related to this. According to jorendorff, the proposed solution to this is to add an explicit preventExtensions/isExtensible trap to proxies (although this has not been specced yet). I plan on doing this as part of the direct proxy refactor.

all we are waiting on is a review and recheck on try server. Since this adjusts mochitest, reftest and specialpowers along with some build hacking for OSX, I really think :ted should be reviewing this patch. I am fine with other suggestions as I know :ted has a lot in his review queue.

Yes, lets move ahead with review (note: I'm *not* volunteering :-) -- it looks like people who know what they are doing are already on the other issues. Joel, please open a bug to re-enable these tests after this bug lands and cc :ejpbruel, :jorendorff, :bholley, and :terrence. Thanks!

Comment on attachment 642421 [details] [diff] [review] move specialpowers to testing/specialpowers and fix jsreftests to use it (1.0) Review of attachment 642421 [details] [diff] [review]: ----------------------------------------------------------------- r- just for a couple of things. ::: js/src/tests/browser.js @@ +113,4 @@ > { > try > { > + SpecialPowers.forceGC(); You should be able to ditch the try/catch here. ::: layout/tools/reftest/runreftest.py @@ +79,5 @@ > profileDir, > "reftest@mozilla.org") > > + # I would prefer to use "--install-extension reftest/specialpowers", but that requires tight coordination with > + # release engineering and landing on multiple branches at once. Ideally we'd just have some sort of --enable-specialpowers flag. Although, having the harness do the right thing automatically seems ideal. This is just a little magical for my taste, since there's no clear division between JS tests and reftests. I agree that this makes the most sense given the pain of deployment. ::: testing/mochitest/Makefile.in @@ +107,5 @@ > _DEST_DIR = $(DEPTH)/_tests/$(relativesrcdir) > > +special-powers: > + $(NSINSTALL) -D $(_DEST_DIR)/extensions/specialpowers > + cp -RL $(DEPTH)/testing/specialpowers/specialpowers $(_DEST_DIR)/extensions We're trying to lower the number of custom rules in Makefiles. Can't we just have testing/specialpowers/Makefile.in do this install? ::: testing/mochitest/tests/SimpleTest/specialpowersAPI.js @@ +1164,5 @@ > + }, > + > + setUtilsProperty: function(prop, value) { > + Components.utils[prop] = value; > + }, These seem like they should be unnecessary given Specialpowers.wrap(). Can't you just do var utils = Specialpowers.wrap(Components.utils); ? @@ +1171,5 @@ > + var jsdIDebuggerService = Components.interfaces.jsdIDebuggerService; > + var service = Components.classes['@mozilla.org/js/jsd/debugger-service;1']. > + getService(jsdIDebuggerService); > + service.GC(); > + }, Same here, although if this gets used a lot I can see having it as a useful shorthand.

updated patch to address all review comments except the addition of --enable-specialpowers as that is not realistic in our current buildbot infrastructure :(

Comment on attachment 645741 [details] [diff] [review] move specialpowers to testing/specialpowers and fix jsreftests to use it (2.0) Review of attachment 645741 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/tests/browser.js @@ +145,4 @@ > aOptionName = 'xml'; > > if (aOptionName && aOptionName !== 'allow_xml') { > + if (!SpecialPowers.isPropertyInUtils(aOptionName)) Can you not write this as: if (!(aOptionName in SpecialPowers.wrap(Components).utils)) {} ? ::: testing/mochitest/specialpowers/Makefile.in @@ +37,5 @@ > (cd $(DIST)/xpi-stage && tar $(TAR_CREATE_FLAGS) - $(XPI_NAME)) | (cd $(TEST_EXTENSIONS_DIR) && tar -xf -) > + $(NSINSTALL) -D $(DEPTH)/_tests/testing/mochitest/extensions/specialpowers > + cp -RL $(DEPTH)/testing/specialpowers/specialpowers $(DEPTH)/_tests/testing/mochitest/extensions > + $(NSINSTALL) -D $(DEPTH)/_tests/reftest/specialpowers > + cp -RL $(DEPTH)/testing/specialpowers/specialpowers $(DEPTH)/_tests/reftest We are trying to get rid of custom rules from Makefiles. I'm not sure if there's a generic rule you can use here, perhaps gps or glandium would know. ::: testing/mochitest/tests/SimpleTest/specialpowersAPI.js @@ +1158,5 @@ > + }, > + > + getUtilsProperty: function(prop) { > + return Components.utils[prop]; > + }, I still don't think any of these API changes are necessary, I think you ought to be able to replace these by using SpecialPowers.wrap(Components).utils.

bug 773202 might let you replace those makefile rules, I'm not 100% sure.

have another run on try server with the specialpowers nits addressed. I don't have a solution for the makefile as of yet. There is one test on windows debug that I am concerned about: js1_8_5/extensions/recursion.js, this failed on my last try run, but didn't fail on the previous try run. So something I am changing is causing problems, I will investigate more if it fails on this run. https://tbpl.mozilla.org/?tree=Try&rev=6c8ecd0ef9c4

Running my original patch on try server this test fails also. I couldn't reproduce it on a local win7 vm, Seems like something changed in the product that is causing this to fail in the last couple weeks.

now running my patch fails on crashtest and jsreftest on various platforms. Something must have changed in the last couple weeks.

landed on inbound (without turning on for jsreftests due to continued oranges on debug builds across the board): https://hg.mozilla.org/integration/mozilla-inbound/rev/e21f9042824f we will be turning this on for crashtests though, that is the reason to land this.

This causes js1_5/extensions/regress-369696-02.js to fail on shell builds, because |window| isn't defined in the shell. Just removing the |window.SpecialPowers = null;| line fixes the problem...

What are shell builds? I saw all green on tbpl, so I assumed I was in the clear. Can we do: if (window != undefined) { window.SpecialPowers = null; }

The "shell" is a stand-alone build of SpiderMonkey: e.g. no browser. It is able to run JavaScript code that does not depend on browser features, like |window|. On TBPL, only the jit-tests get run in the shell, the jsreftest suite gets run in the browser using -reftest. Despite this, the reftests suite is incredibly useful to run in a shell locally. We've obviously been meaning to fix this, but nobody has adequate time to work on the testsuite. Also, the fix you are proposing should work.

> Also, the fix you are proposing should work. It doesn't, we still get "ReferenceError: window is not defined".

Kill the annoying shell failure

(In reply to Nicholas Nethercote [:njn] from comment #50) > > Also, the fix you are proposing should work. > > It doesn't, we still get "ReferenceError: window is not defined". What is going on then? I remember having seen very similar code in the jsreftest suite to check if browser features are available.

Thanks, sfink!