Comment on attachment 8872089 [details] [diff] [review] Make test_nukeContentWindow.html correctly wait for the window to be destroyed instead of relying on the scheduling of the corresponding event Review of attachment 8872089 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/xpconnect/tests/mochitest/test_nukeContentWindow.html @@ +24,5 @@ > + SpecialPowers.removeObserver(observer, "outer-window-destroyed"); > + SpecialPowers.executeSoon(callback); > + } > + }; > + SpecialPowers.addObserver(observer, "outer-window-destroyed"); I'd rather we use TestUtils.topicObserved for this, but if you'd prefer to keep this, I'm OK with that too. Please use an observer function rather than an object, though.

I couldn't figure out how I'm supposed to use TestUtils.jsm from a mochitest, do you know?

(In reply to :Ehsan Akhgari (needinfo please, extremely long backlog) from comment #3) > I couldn't figure out how I'm supposed to use TestUtils.jsm from a > mochitest, do you know? Unfortunately, you need to do something like: var {TestUtils} = SpecialPowers.Cu.import("resource://testing-common/TestUtils.jsm", {});

I tried that, and that throws: Permission denied for <http://mochi.test:8888> to create wrapper for object of class UnnamedClass Switching to: var {TestUtils} = SpecialPowers.unwrap(SpecialPowers.Cu.import("resource://testing-common/TestUtils.jsm", {})); gives: Permission denied to access property "TestUtils" at @http://mochi.test:8888/tests/js/xpconnect/tests/mochitest/test_nukeContentWindow.html:21:19

(Note that this test runs as a non-privileged mochitest-plain)

I just tested, and this works as expected: var {TestUtils} = SpecialPowers.Cu.import("resource://testing-common/TestUtils.jsm", {}); ... yield TestUtils.topicObserved("outer-window-destroyed"); SpecialPowers.unwrap would unwrap the module global to a privileged object you have no permissions for, and unfortunately, SpecialPowers isn't smart enough to handle wrapping objects that Cu.import implicitly exports to the global, so pulling symbols off the returned wrapped module scope is the only option. Would probably be nice to fix that...

(In reply to Kris Maglione [:kmag] (busy; behind on reviews) from comment #7) > I just tested, and this works as expected: > > > var {TestUtils} = > SpecialPowers.Cu.import("resource://testing-common/TestUtils.jsm", {}); > > ... > > yield TestUtils.topicObserved("outer-window-destroyed"); While this works, this is the real code I need: yield TestUtils.topicObserved("outer-window-nuked", (subject, data) => { let id = subject.QueryInterface(SpecialPowers.Ci.nsISupportsPRUint64).data; return id == winID; }); Passing a function here raises the "Permission denied for <http://mochi.test:8888> to create wrapper for object of class UnnamedClass" error I mentioned before, irrespective of what's in the body of this function... I think it's really not worth it to use this helper in this test honestly. :-)

Ah, interesting. Yeah, probably not worth it, in that case. But your original patch didn't check the window ID. :)

Ah yeah sorry I forgot to mention. I hit some intermittent test failures on the try server which were being caused because the topic observer could be handling the observer notification coming from a window being nuked from a previous test run and therefore the callback would end up running too early as a result. As a solution I'm checking the window ID to make sure the callback only handles the correct window ID.

(In reply to Kris Maglione [:kmag] (busy; behind on reviews) from comment #2) > Please use an observer function rather than > an object, though. That also doesn't work, FWIW. It will throw: JavaScript error: , line 0: Error: Permission denied for <http://mochi.test:8888> to create wrapper for object of class UnnamedClass The SpecialPowers API is unfortunately pretty unfriendly to use as far as idiomatic JS goes.

Pushed by eakhgari@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/8def89fd63a0 Make test_nukeContentWindow.html correctly wait for the window to be destroyed instead of relying on the scheduling of the corresponding event; r=kmag