[V8] Enumerate Nodes via the V8 heap rather than via a list in WebCore

• Component: New Bugs
• Status: RESOLVED FIXED

[Adam Barth]

[V8] Enumerate Nodes via the V8 heap rather than via a list in WebCore

[Adam Barth]

Created attachment 169989 [details]
Patch

[Eric Seidel (no email)]

Comment on attachment 169989 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=169989&action=review

LGTM.  You may wish to have Kentaro look too, as he's undoubtably more versed in this than I.

> Source/WebCore/bindings/v8/V8DOMMap.cpp:119
> +            v8::Persistent<v8::Object> wrapper = v8::Persistent<v8::Object>::Cast(value);

I assume this ASSERTs, etc. that this is an Object?

[Adam Barth]

Performance results in https://bugs.webkit.org/show_bug.cgi?id=97974#c24

[Adam Barth]

Comment on attachment 169989 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=169989&action=review

>> Source/WebCore/bindings/v8/V8DOMMap.cpp:119
>> +            ASSERT(V8Node::HasInstance(value));
>> +            ASSERT(value->IsObject());
>> +            ASSERT(!value.IsIndependent());
>> +            v8::Persistent<v8::Object> wrapper = v8::Persistent<v8::Object>::Cast(value);
> 
> I assume this ASSERTs, etc. that this is an Object?

It doesn't, but that's why I have the ASSERT on line 117.  :)

[Kentaro Hara]

Comment on attachment 169989 [details]
Patch

LGTM.

[WebKit Review Bot]

Comment on attachment 169989 [details]
Patch

Clearing flags on attachment: 169989

Committed r132242: <http://trac.webkit.org/changeset/132242>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.