Closed
Bug 328007
Opened 19 years ago
Closed 19 years ago
[FIX]root new function objects before JS_DefineUCProperty
Categories
(Core :: XBL, defect, P1)
Core
XBL
Tracking
()
RESOLVED
FIXED
mozilla1.9alpha1
People
(Reporter: dbaron, Assigned: bzbarsky)
References
Details
(Keywords: fixed1.8.0.4, fixed1.8.1)
Attachments
(1 file)
(deleted),
patch
|
sicking
:
review+
brendan
:
superreview+
sicking
:
approval-branch-1.8.1+
darin.moz
:
approval1.8.0.4+
|
Details | Diff | Splinter Review |
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.
Assignee | ||
Comment 1•19 years ago
|
||
Assignee: general → bzbarsky
Status: NEW → ASSIGNED
Attachment #214151 -
Flags: superreview?(brendan)
Attachment #214151 -
Flags: review?(bugmail)
Assignee | ||
Updated•19 years ago
|
Priority: -- → P1
Summary: root new function objects before JS_DefineUCProperty → [FIX]root new function objects before JS_DefineUCProperty
Target Milestone: --- → mozilla1.9alpha
Comment 2•19 years ago
|
||
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
Attachment #214151 -
Flags: superreview?(brendan) → superreview+
Assignee | ||
Comment 3•19 years ago
|
||
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.
Attachment #214151 -
Flags: review?(bugmail) → review+
Assignee | ||
Comment 4•19 years ago
|
||
Fixed. Filed bug 332648 as the followup.
Status: ASSIGNED → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 5•19 years ago
|
||
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...
Attachment #214151 -
Flags: approval1.8.0.3?
Attachment #214151 -
Flags: approval-branch-1.8.1?(bugmail)
Comment on attachment 214151 [details] [diff] [review]
Patch
go for it
Attachment #214151 -
Flags: approval-branch-1.8.1?(bugmail) → approval-branch-1.8.1+
Updated•19 years ago
|
Flags: blocking1.8.0.3+
Comment 8•19 years ago
|
||
Comment on attachment 214151 [details] [diff] [review]
Patch
a=darin for 1.8.0.3 (on behalf of drivers)
Attachment #214151 -
Flags: approval1.8.0.3? → approval1.8.0.3+
Updated•19 years ago
|
Flags: blocking1.7.14?
Flags: blocking-aviary1.0.9?
You need to log in
before you can comment on or make changes to this bug.
Description
•