Try harder to get a JSContext in PostMessageEvent::Run

RESOLVED FIXED in Firefox 6

Status

()

Core
DOM
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: khuey, Assigned: khuey)

Tracking

({regression})

unspecified
mozilla6
regression
Points:
---
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(firefox5 unaffected, firefox6 fixed, firefox7 fixed)

Details

(Whiteboard: [qa-])

Attachments

(1 attachment, 1 obsolete attachment)

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.
tracking-firefox6: ? → ---
Created attachment 534822 [details] [diff] [review]
Patch
Attachment #534822 - Flags: review?(bent.mozilla)
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.
Attachment #534822 - Flags: review?(bent.mozilla) → review+
I fixed that, and reverted the now unnecessary nsresult decl changes.
Created attachment 534868 [details] [diff] [review]
Cleaner patch

Carrying forward r=bent.

We should take this on Aurora to fix a minor memory leak.  Risk here should be low.
Attachment #534822 - Attachment is obsolete: true
Attachment #534868 - Flags: review+
Attachment #534868 - Flags: approval-mozilla-aurora?
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.
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Target Milestone: --- → mozilla7
status-firefox6: --- → affected
status-firefox7: --- → fixed

Updated

6 years ago
Attachment #534868 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
http://hg.mozilla.org/releases/mozilla-aurora/rev/3cb769e43035
status-firefox6: affected → fixed
Target Milestone: mozilla7 → mozilla6
so is it safe to say that this issue was fixed?
Thanks.
qa-: nothing for QA to verify
Whiteboard: [qa-]
You need to log in before you can comment on or make changes to this bug.