Closed Bug 868040 Opened 8 years ago Closed 8 years ago

GC: Fix some rooting hazards in content/base

Categories

(Core :: DOM: Core & HTML, defect)

x86
macOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla23

People

(Reporter: jonco, Assigned: jonco)

References

Details

Attachments

(1 file, 3 obsolete files)

Attached patch Proposed changes (obsolete) — Splinter Review
Here's a fix for a rooting hazard in nsDocument::GetCustomPrototype().
The first patch in bug 866450 fixes this, no?
(In reply to Boris Zbarsky (:bz) from comment #1)

I don't think it fixes this one in GetCustomPrototype itself:

base/src/nsDocument.h:1049: Function 'JSObject* nsDocument::GetCustomPrototype(nsAString_internal*)' has unrooted 'prototype' of type 'JSObject*' live across GC call 'bool nsBaseHashtable<KeyClass, DataType, UserDataType>::Get(nsBaseHashtable<KeyClass, DataType, UserDataType>::KeyType, UserDataType*) const [with KeyClass = nsStringHashKey; DataType = JSObject*; UserDataType = JSObject*; nsBaseHashtable<KeyClass, DataType, UserDataType>::KeyType = const nsAString_internal&]'
Hmm.  I guess so, yeah.  Seems like a false positive to me, but easier to fix the code than the analysis, and the net result is the same.  You probably want to merge on top of that other bug, though.
Jon, are you still planning to get this landed?
Summary: GC: Fix rooting hazard in nsDocument::GetCustomPrototype() → GC: Fix some rooting hazards in content/base
(In reply to Boris Zbarsky (:bz) from comment #4)

Yes, I thought it wasn't worth it for one hazard so I did some more in that area.
Attached patch Proposed changes (obsolete) — Splinter Review
Attachment #744674 - Attachment is obsolete: true
I think you just duplicated a bunch of the work from bug 869311...
(In reply to Boris Zbarsky (:bz) from comment #7)

Yes, I didn't see that.  I've rebased on top of your changes, and there are just two fixes left. 

I'm going to set you as reviewre, I hope that's ok.
Attached patch Proposed changes (obsolete) — Splinter Review
Attachment #746485 - Attachment is obsolete: true
Attachment #746987 - Flags: review?(bzbarsky)
It's ok, but is that the right patch?  It doesn't have the GetCustomPrototype bits...
Flags: needinfo?(jcoppeard)
Attached patch Proposed changesSplinter Review
Yeah, uploading the right patch would be a good idea... let's try this again.
Attachment #746987 - Attachment is obsolete: true
Attachment #746987 - Flags: review?(bzbarsky)
Attachment #746995 - Flags: review?(bzbarsky)
Flags: needinfo?(jcoppeard)
Comment on attachment 746995 [details] [diff] [review]
Proposed changes

r=me
Attachment #746995 - Flags: review?(bzbarsky) → review+
https://hg.mozilla.org/mozilla-central/rev/0c5f4ae8b572
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla23
You need to log in before you can comment on or make changes to this bug.