Last Comment Bug 328007 - [FIX]root new function objects before JS_DefineUCProperty
: [FIX]root new function objects before JS_DefineUCProperty
Status: RESOLVED FIXED
: fixed1.8.0.4, fixed1.8.1
Product: Core
Classification: Components
Component: XBL (show other bugs)
: Trunk
: All All
: P1 normal (vote)
: mozilla1.9alpha1
Assigned To: Boris Zbarsky [:bz]
: Hixie (not reading bugmail)
Mentors:
Depends on:
Blocks: 307312
  Show dependency treegraph
 
Reported: 2006-02-21 00:16 PST by David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch)
Modified: 2006-05-30 23:22 PDT (History)
1 user (show)
dveditz: blocking1.7.14?
dveditz: blocking‑aviary1.0.9?
dveditz: blocking1.8.0.4+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch (2.82 KB, patch)
2006-03-05 21:32 PST, Boris Zbarsky [:bz]
jonas: review+
brendan: superreview+
jonas: approval‑branch‑1.8.1+
darin.moz: approval1.8.0.4+
Details | Diff | Splinter Review

Description David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch) 2006-02-21 00:16:01 PST
In bug 327712 comment 4, bz wrote:
> but we need similar changes to root newly-cloned function
> objects across JS_DefineUCProperty in nsXBLProtoImplMethod::InstallMember and
> the code added in bug 307040.
Comment 1 Boris Zbarsky [:bz] 2006-03-05 21:32:35 PST
Created attachment 214151 [details] [diff] [review]
Patch
Comment 2 Brendan Eich [:brendan] 2006-03-05 23:46:11 PST
Comment on attachment 214151 [details] [diff] [review]
Patch

If there's a cx in scope, we should pass it into the autoroot thingie as an optional trailing parameter.  Then the impl of that could use a more efficient JS-level rooting technique.

For short-term block-scoped rooting there is no need to lock the runtime's GC state and add and delete an entry in the global root double-hashtable (although that almost always doesn't allocate -- but the locking hurts).

Followup bug?  Or here -- your call.

/be
Comment 3 Boris Zbarsky [:bz] 2006-03-05 23:54:56 PST
I'd prefer a followup.  What API should I be using if we have a cx?  I think we always have one when using nsAutoGCRoot.
Comment 4 Boris Zbarsky [:bz] 2006-04-03 21:36:11 PDT
Fixed.  Filed bug 332648 as the followup.
Comment 5 Boris Zbarsky [:bz] 2006-04-03 21:38:43 PDT
Comment on attachment 214151 [details] [diff] [review]
Patch

We probably want this on branches.  This should be quite safe, and GC fixes seem to be a good idea...
Comment 6 Jonas Sicking (:sicking) PTO Until July 5th 2006-04-03 22:53:36 PDT
Comment on attachment 214151 [details] [diff] [review]
Patch

go for it
Comment 7 Boris Zbarsky [:bz] 2006-04-04 11:28:31 PDT
Fixed on 1.8.1 branch.
Comment 8 Darin Fisher 2006-04-10 12:25:23 PDT
Comment on attachment 214151 [details] [diff] [review]
Patch

a=darin for 1.8.0.3 (on behalf of drivers)
Comment 9 Boris Zbarsky [:bz] 2006-04-10 13:14:52 PDT
Fixed on 1.8.0 branch.

Note You need to log in before you can comment on or make changes to this bug.