Summary: | [Chromium] Fix memory model for dataset | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Erik Arvidsson <arv> | ||||
Component: | New Bugs | Assignee: | Erik Arvidsson <arv> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | antonm, japhet | ||||
Priority: | P2 | ||||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | Other | ||||||
OS: | OS X 10.5 | ||||||
Attachments: |
|
Description
Erik Arvidsson
2010-09-22 11:03:39 PDT
Created attachment 68396 [details]
Patch
Comment on attachment 68396 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=68396&action=review Thanks a lot for doing this. Alas, I cannot r+ it, but it's almost LGTM > WebCore/bindings/v8/custom/V8DOMStringMapCustom.cpp:107 > + if (elementValue->IsObject()) I think that if elementValue is an empty handle, that would segfault > WebCore/bindings/v8/custom/V8DOMStringMapCustom.cpp:108 > + V8DOMWrapper::setHiddenReference(v8::Handle<v8::Object>::Cast(elementValue), wrapper); you can use elementValue.As<v8::Object> too. These .As methods are rarely met currently as they were added relatively recently, up to you if you'd like to switch to new API or not (In reply to comment #2) > (From update of attachment 68396 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=68396&action=review > > Thanks a lot for doing this. > > Alas, I cannot r+ it, but it's almost LGTM > > > WebCore/bindings/v8/custom/V8DOMStringMapCustom.cpp:107 > > + if (elementValue->IsObject()) > > I think that if elementValue is an empty handle, that would segfault > > > WebCore/bindings/v8/custom/V8DOMStringMapCustom.cpp:108 > > + V8DOMWrapper::setHiddenReference(v8::Handle<v8::Object>::Cast(elementValue), wrapper); > > you can use elementValue.As<v8::Object> too. These .As methods are rarely met currently as they were added relatively recently, up to you if you'd like to switch to new API or not cannot r+ means I am not a reviewer Comment on attachment 68396 [details]
Patch
r+, reiterating antonm's point about adding an isEmpty() check.
Committed r68064: <http://trac.webkit.org/changeset/68064> |