46280 2010-09-22 11:03:39 -0700 [Chromium] Fix memory model for dataset 2010-09-22 12:11:15 -0700 1 1 1 Unclassified WebKit New Bugs 528+ (Nightly build) Other OS X 10.5 RESOLVED FIXED P2 Normal --- 1 arv arv antonm japhet oldest_to_newest 283057 0 arv 2010-09-22 11:03:39 -0700 [Chromium] Fix memory model for dataset 283059 1 68396 arv 2010-09-22 11:04:22 -0700 Created attachment 68396 Patch 283066 2 68396 antonm 2010-09-22 11:10:19 -0700 Comment on attachment 68396 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 283067 3 antonm 2010-09-22 11:10:46 -0700 (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 283100 4 68396 japhet 2010-09-22 11:41:13 -0700 Comment on attachment 68396 Patch r+, reiterating antonm's point about adding an isEmpty() check. 283137 5 arv 2010-09-22 12:11:15 -0700 Committed r68064: <http://trac.webkit.org/changeset/68064> 68396 2010-09-22 11:04:22 -0700 2010-09-22 11:41:13 -0700 Patch bug-46280-20100922110420.patch text/plain 1761 arv ZGlmZiAtLWdpdCBhL1dlYkNvcmUvQ2hhbmdlTG9nIGIvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXgg NzcxNTI2NmE2ZGJiMWM5YTMwMjcxYTcyYzRjNzA2NDJmNGIyNDY2YS4uYTQ0M2I0ZmM2MWQzNjky YTlkN2IxYjZjNTg4NWVlYzQxOGFkNjE5NiAxMDA2NDQKLS0tIGEvV2ViQ29yZS9DaGFuZ2VMb2cK KysrIGIvV2ViQ29yZS9DaGFuZ2VMb2cKQEAgLTEsMyArMSwxNSBAQAorMjAxMC0wOS0yMiAgRXJp ayBBcnZpZHNzb24gIDxhcnZAY2hyb21pdW0ub3JnPgorCisgICAgICAgIFJldmlld2VkIGJ5IE5P Qk9EWSAoT09QUyEpLgorCisgICAgICAgIFtDaHJvbWl1bV0gRml4IG1lbW9yeSBtb2RlbCBmb3Ig ZGF0YXNldAorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9 NDYyODAKKworICAgICAgICBUaGlzIGlzIGNvdmVyZWQgYnkgZXhpc3RpbmcgdGVzdHMuCisKKyAg ICAgICAgKiBiaW5kaW5ncy92OC9jdXN0b20vVjhET01TdHJpbmdNYXBDdXN0b20uY3BwOgorICAg ICAgICAoV2ViQ29yZTo6dG9WOCk6CisKIDIwMTAtMDktMjIgIEphbWV5IEhpY2tzICA8amFtZXku aGlja3NAbm9raWEuY29tPgogCiAgICAgICAgIFJldmlld2VkIGJ5IEtlbm5ldGggUm9oZGUgQ2hy aXN0aWFuc2VuLgpkaWZmIC0tZ2l0IGEvV2ViQ29yZS9iaW5kaW5ncy92OC9jdXN0b20vVjhET01T dHJpbmdNYXBDdXN0b20uY3BwIGIvV2ViQ29yZS9iaW5kaW5ncy92OC9jdXN0b20vVjhET01TdHJp bmdNYXBDdXN0b20uY3BwCmluZGV4IGM4YTk3NWIwZmZhZWZmYWJlZTk4ZjdhMzIzNWE1Yzk2NzQx ZmYxZjguLjExM2IxM2FjMzFjZDVlNzYzNjIxOGYxOTFjMTYzMmEzNzBjZDUxNDkgMTAwNjQ0Ci0t LSBhL1dlYkNvcmUvYmluZGluZ3MvdjgvY3VzdG9tL1Y4RE9NU3RyaW5nTWFwQ3VzdG9tLmNwcAor KysgYi9XZWJDb3JlL2JpbmRpbmdzL3Y4L2N1c3RvbS9WOERPTVN0cmluZ01hcEN1c3RvbS5jcHAK QEAgLTM0LDYgKzM0LDcgQEAKICNpbmNsdWRlICJET01TdHJpbmdNYXAuaCIKICNpbmNsdWRlICJW OEJpbmRpbmcuaCIKICNpbmNsdWRlICJWOERPTVdyYXBwZXIuaCIKKyNpbmNsdWRlICJWOEVsZW1l bnQuaCIKIAogbmFtZXNwYWNlIFdlYkNvcmUgewogCkBAIC0xMDEsOCArMTAyLDExIEBAIHY4OjpI YW5kbGU8djg6OlZhbHVlPiB0b1Y4KERPTVN0cmluZ01hcCogaW1wbCkKICAgICB2ODo6SGFuZGxl PHY4OjpPYmplY3Q+IHdyYXBwZXIgPSBWOERPTVN0cmluZ01hcDo6d3JhcChpbXBsKTsKICAgICAv LyBBZGQgYSBoaWRkZW4gcmVmZXJlbmNlIGZyb20gdGhlIGVsZW1lbnQgdG8gdGhlIERPTVN0cmlu Z01hcC4KICAgICBFbGVtZW50KiBlbGVtZW50ID0gaW1wbC0+ZWxlbWVudCgpOwotICAgIGlmICgh d3JhcHBlci5Jc0VtcHR5KCkgJiYgZWxlbWVudCkKLSAgICAgICAgVjhET01XcmFwcGVyOjpzZXRI aWRkZW5XaW5kb3dSZWZlcmVuY2UoZWxlbWVudC0+ZG9jdW1lbnQoKS0+ZnJhbWUoKSwgd3JhcHBl cik7CisgICAgaWYgKCF3cmFwcGVyLklzRW1wdHkoKSAmJiBlbGVtZW50KSB7CisgICAgICAgIHY4 OjpIYW5kbGU8djg6OlZhbHVlPiBlbGVtZW50VmFsdWUgPSB0b1Y4KGVsZW1lbnQpOworICAgICAg ICBpZiAoZWxlbWVudFZhbHVlLT5Jc09iamVjdCgpKQorICAgICAgICAgICAgVjhET01XcmFwcGVy OjpzZXRIaWRkZW5SZWZlcmVuY2Uodjg6OkhhbmRsZTx2ODo6T2JqZWN0Pjo6Q2FzdChlbGVtZW50 VmFsdWUpLCB3cmFwcGVyKTsKKyAgICB9CiAgICAgcmV0dXJuIHdyYXBwZXI7CiB9CiAK