Closed Bug 659215 Opened 10 years ago Closed 10 years ago

Try harder to get a JSContext in PostMessageEvent::Run

Categories

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

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla6
Tracking Status
firefox5 --- unaffected
firefox6 --- fixed
firefox7 --- fixed

People

(Reporter: khuey, Assigned: khuey)

References

Details

(Keywords: regression, Whiteboard: [qa-])

Attachments

(1 file, 1 obsolete file)

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.
Attached patch Patch (obsolete) — Splinter Review
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.
Attached patch Cleaner patchSplinter Review
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
Closed: 10 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Target Milestone: --- → mozilla7
Attachment #534868 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
so is it safe to say that this issue was fixed?
Thanks.
qa-: nothing for QA to verify
Whiteboard: [qa-]
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.