868040 - GC: Fix some rooting hazards in content/base

Status: RESOLVED FIXED

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

Description

[Jon Coppeard (:jonco)]

Attachment: Proposed changes

Here's a fix for a rooting hazard in nsDocument::GetCustomPrototype().

Comment 1

[Boris Zbarsky [:bzbarsky]]

The first patch in bug 866450 fixes this, no?

Comment 2

[Jon Coppeard (:jonco)]

(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&]'

Comment 3

[Boris Zbarsky [:bzbarsky]]

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.

Comment 4

[Boris Zbarsky [:bzbarsky]]

Jon, are you still planning to get this landed?

Comment 5

[Jon Coppeard (:jonco)]

(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.

Comment 6

[Jon Coppeard (:jonco)]

Attachment: Proposed changes

Comment 7

[Boris Zbarsky [:bzbarsky]]

I think you just duplicated a bunch of the work from bug 869311...

Comment 8

[Jon Coppeard (:jonco)]

(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.

Comment 9

[Jon Coppeard (:jonco)]

Attachment: Proposed changes

Comment 10

[Boris Zbarsky [:bzbarsky]]

It's ok, but is that the right patch?  It doesn't have the GetCustomPrototype bits...

Comment 11

[Jon Coppeard (:jonco)]

Attachment: Proposed changes

Yeah, uploading the right patch would be a good idea... let's try this again.

Comment 12

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 746995 [details] [diff] [review]
Proposed changes

r=me

Comment 13

[Jon Coppeard (:jonco)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/0c5f4ae8b572

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/0c5f4ae8b572