[FIX]root new function objects before JS_DefineUCProperty

RESOLVED FIXED in mozilla1.9alpha1

Status

()

Core
XBL
P1
normal
RESOLVED FIXED
11 years ago
11 years ago

People

(Reporter: dbaron, Assigned: bz)

Tracking

({fixed1.8.0.4, fixed1.8.1})

Trunk
mozilla1.9alpha1
fixed1.8.0.4, fixed1.8.1
Points:
---
Bug Flags:
blocking1.7.14 ?
blocking-aviary1.0.9 ?
blocking1.8.0.4 +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

11 years ago
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.
Created attachment 214151 [details] [diff] [review]
Patch
Assignee: general → bzbarsky
Status: NEW → ASSIGNED
Attachment #214151 - Flags: superreview?(brendan)
Attachment #214151 - Flags: review?(bugmail)
Priority: -- → P1
Summary: root new function objects before JS_DefineUCProperty → [FIX]root new function objects before JS_DefineUCProperty
Target Milestone: --- → mozilla1.9alpha
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+
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+
Fixed.  Filed bug 332648 as the followup.
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED
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+
Fixed on 1.8.1 branch.
Keywords: fixed1.8.1
Flags: blocking1.8.0.3+

Comment 8

11 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+
Fixed on 1.8.0 branch.
Keywords: fixed1.8.0.3
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.