659215 - Try harder to get a JSContext in PostMessageEvent::Run

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

The patch from Bug 553125 causes a leak and an assertion if we can't get a JSContext from the window.  It turns out that that can be hit if one calls postMessage on a window that's closed.  Our testsuite actually does this.

We should try harder to get a JSContext to use.

Comment 1

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Attachment: Patch

Comment 2

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Comment on attachment 534822 [details] [diff] [review]
Patch

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

::: dom/base/nsGlobalWindow.cpp
@@ +5903,5 @@
> +    if (cxStack) {
> +      rv = cxStack->GetSafeJSContext(&cx);
> +    }
> +
> +    if (!cxStack || NS_FAILED(rv) || !cx) {

I think you can get away with just checking !cx here, and don't worry about getting a return value from GetSafeJSContext.

Comment 3

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

I fixed that, and reverted the now unnecessary nsresult decl changes.

Comment 4

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Attachment: Cleaner patch

Carrying forward r=bent.

We should take this on Aurora to fix a minor memory leak.  Risk here should be low.

Comment 5

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

http://hg.mozilla.org/mozilla-central/rev/a2b3d055d1f8

Sadly we can't test this since assertions are not fatal in any test suite that can use window.open.

Comment 6

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

http://hg.mozilla.org/releases/mozilla-aurora/rev/3cb769e43035

Comment 7

[Trif Andrei-Alin[:AlinT]]

so is it safe to say that this issue was fixed?
Thanks.

Comment 8

[u279076]

qa-: nothing for QA to verify