Try harder to get a JSContext in PostMessageEvent::Run

RESOLVED FIXED in Firefox 6

Status

()

defect
RESOLVED FIXED
8 years ago
2 months ago

People

(Reporter: khuey, Assigned: khuey)

Tracking

({regression})

unspecified
mozilla6
Points:
---
Dependency tree / graph
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.
Posted 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.
Posted 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
Last Resolved: 8 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
Product: Core → Core
You need to log in before you can comment on or make changes to this bug.