Closed
Bug 490718
Opened 16 years ago
Closed 16 years ago
XPCWrappedNativeScope creates a needless cycle with its principal provider
Categories
(Core :: XPConnect, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: bent.mozilla, Assigned: bent.mozilla)
Details
Attachments
(1 file)
7.36 KB,
patch
|
peterv
:
review+
jst
:
superreview+
beltzner
:
approval1.9.1-
|
Details | Diff | Splinter Review |
We could just use a raw ptr, patch attached.
Attachment #375102 -
Flags: superreview?(jst)
Attachment #375102 -
Flags: review?(peterv)
Updated•16 years ago
|
Attachment #375102 -
Flags: review?(peterv) → review+
Comment 1•16 years ago
|
||
Comment on attachment 375102 [details] [diff] [review]
Patch
>diff --git a/js/src/xpconnect/src/xpcwrappednativescope.cpp b/js/src/xpconnect/src/xpcwrappednativescope.cpp
>@@ -447,17 +452,19 @@ XPCWrappedNativeScope::FinishedMarkPhase
>+#ifndef XPCONNECT_STANDALONE
>+ cur->mScriptObjectPrincipal = nsnull;
>+#endif
> // Move this scope from the live list to the dying list.
Nit: add a newline after the endif.
Assignee | ||
Comment 2•16 years ago
|
||
jsteeeeeeeee!
Comment 3•16 years ago
|
||
Comment on attachment 375102 [details] [diff] [review]
Patch
Duh, totally forgot about this one :(
Attachment #375102 -
Flags: superreview?(jst) → superreview+
Assignee | ||
Updated•16 years ago
|
Attachment #375102 -
Flags: approval1.9.1?
Assignee | ||
Comment 4•16 years ago
|
||
Comment on attachment 375102 [details] [diff] [review]
Patch
I'd love to get this in for 1.9.1 as it helps windows get collected sooner.
Comment 5•16 years ago
|
||
Needs to bake on trunk before we'd consider for 191, which means that it might be fodder for a security and stability release, though!
Flags: wanted1.9.1.x?
Updated•16 years ago
|
Attachment #375102 -
Flags: approval1.9.1?
Comment 6•16 years ago
|
||
Comment on attachment 375102 [details] [diff] [review]
Patch
Please request approval when it has baked on trunk.
Assignee | ||
Comment 7•16 years ago
|
||
Oops, I landed this yesterday, changeset 6dd95b1cba8e.
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Assignee | ||
Updated•16 years ago
|
Attachment #375102 -
Flags: approval1.9.1?
Assignee | ||
Comment 8•16 years ago
|
||
Comment on attachment 375102 [details] [diff] [review]
Patch
Sorry guys, I neglected to indicate that I had landed already. Dunno how much baking you'd want this to have, though I think we would have noticed right away if this patch caused problems.
Comment 9•16 years ago
|
||
Ben: any idea of risk here?
Updated•16 years ago
|
Attachment #375102 -
Flags: approval1.9.1? → approval1.9.1-
Comment 10•16 years ago
|
||
Comment on attachment 375102 [details] [diff] [review]
Patch
Should ask again for 3.5.1, but I don't know if we understand the full risk here. Feel free to renom if you're willing to let us know.
You need to log in
before you can comment on or make changes to this bug.
Description
•