100853 2012-10-31 06:55:46 -0700 [V8] Cleanup V8DOMWrapper::getCachedWrapper 2012-11-07 03:15:03 -0800 1 1 1 Unclassified WebKit New Bugs 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 0 dcarney webkit-unassigned abarth haraken japhet webkit.review.bot oldest_to_newest 755250 0 dcarney 2012-10-31 06:55:46 -0700 [V8] Cleanup V8DOMWrapper::getCachedWrapper 755252 1 171643 dcarney 2012-10-31 06:57:22 -0700 Created attachment 171643 Patch 755260 2 171643 haraken 2012-10-31 07:03:36 -0700 Comment on attachment 171643 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=171643&action=review > Source/WebCore/bindings/v8/V8DOMWrapper.h:-115 > - v8::Persistent<v8::Object> wrapper = node->wrapper(); > - if (LIKELY(!wrapper.IsEmpty())) Why is this branch redundant? This branch will fail and has to fall back to the following slow path in a case where we are in the main world and a wrapper has not yet created for a given node, won't it? 755335 3 171643 abarth 2012-10-31 08:42:27 -0700 Comment on attachment 171643 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=171643&action=review >> Source/WebCore/bindings/v8/V8DOMWrapper.h:-115 >> - if (LIKELY(!wrapper.IsEmpty())) > > Why is this branch redundant? This branch will fail and has to fall back to the following slow path in a case where we are in the main world and a wrapper has not yet created for a given node, won't it? You're right Kentaro, but this function is only responsible for getting the cached wrapper. The caller of this function will create the wrapper if this function returns an empty handle. 755347 4 haraken 2012-10-31 08:49:11 -0700 (In reply to comment #3) > > Why is this branch redundant? This branch will fail and has to fall back to the following slow path in a case where we are in the main world and a wrapper has not yet created for a given node, won't it? > > You're right Kentaro, but this function is only responsible for getting the cached wrapper. The caller of this function will create the wrapper if this function returns an empty handle. Makes sense. Thanks. 760496 5 171643 webkit.review.bot 2012-11-07 03:14:59 -0800 Comment on attachment 171643 Patch Clearing flags on attachment: 171643 Committed r133734: <http://trac.webkit.org/changeset/133734> 760497 6 webkit.review.bot 2012-11-07 03:15:03 -0800 All reviewed patches have been landed. Closing bug. 171643 2012-10-31 06:57:22 -0700 2012-11-07 03:14:59 -0800 Patch bug-100853-20121031145545.patch text/plain 2017 dcarney U3VidmVyc2lvbiBSZXZpc2lvbjogMTMzMDIwCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggYWZmYTg2MDZmNDM2Yjg5 ZmIxNWQ2Zjc0OGY5YjE4NWIzYWNhODA1Yi4uZjZlZTkwNjc3ZjY4Y2VjODcwYTI0YzIyN2QyYjY3 YmQzM2IyN2ZhNSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE3IEBACisyMDEyLTEwLTMxICBEYW4g Q2FybmV5ICA8ZGNhcm5leUBnb29nbGUuY29tPgorCisgICAgICAgIFtWOF0gQ2xlYW51cCBWOERP TVdyYXBwZXI6OmdldENhY2hlZFdyYXBwZXIKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5v cmcvc2hvd19idWcuY2dpP2lkPTEwMDg1MworCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAo T09QUyEpLgorCisgICAgICAgIFNsaWdodCBjbGVhbnVwLiBSZW1vdmVkIHJlZHVuZGFudCBicmFu Y2guCisKKyAgICAgICAgTm8gbmV3IHRlc3RzLiBObyBjaGFuZ2UgaW4gZnVuY3Rpb25hbGl0eS4K KworICAgICAgICAqIGJpbmRpbmdzL3Y4L1Y4RE9NV3JhcHBlci5oOgorICAgICAgICAoV2ViQ29y ZTo6VjhET01XcmFwcGVyOjpnZXRDYWNoZWRXcmFwcGVyKToKKwogMjAxMi0xMC0zMSAgTWlrZSBX ZXN0ICA8bWt3c3RAY2hyb21pdW0ub3JnPgogCiAgICAgICAgIFgtRnJhbWUtT3B0aW9ucyBjb25z b2xlIG1lc3NhZ2Ugc2hvdWxkIGJlIGFzc29jaWF0ZWQgd2l0aCBhIHJlcXVlc3QuCmRpZmYgLS1n aXQgYS9Tb3VyY2UvV2ViQ29yZS9iaW5kaW5ncy92OC9WOERPTVdyYXBwZXIuaCBiL1NvdXJjZS9X ZWJDb3JlL2JpbmRpbmdzL3Y4L1Y4RE9NV3JhcHBlci5oCmluZGV4IGE1MjkyM2I1YjIzZjVlNGI4 YzQ0ODRlNmZiMWI4OGQ0OTE2YTBjMTEuLjQ1Mjk0MjBlOTE4YWNlMDA5YzI3NmVhNTE2YzgzYWZm ZWRlMWYwZDIgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJDb3JlL2JpbmRpbmdzL3Y4L1Y4RE9NV3Jh cHBlci5oCisrKyBiL1NvdXJjZS9XZWJDb3JlL2JpbmRpbmdzL3Y4L1Y4RE9NV3JhcHBlci5oCkBA IC0xMTAsMTkgKzExMCwxNCBAQCBuYW1lc3BhY2UgV2ViQ29yZSB7CiAgICAgICAgIHN0YXRpYyB2 ODo6SGFuZGxlPHY4OjpPYmplY3Q+IGdldENhY2hlZFdyYXBwZXIoTm9kZSogbm9kZSkKICAgICAg ICAgewogICAgICAgICAgICAgQVNTRVJUKGlzTWFpblRocmVhZCgpKTsKLSAgICAgICAgICAgIGlm IChMSUtFTFkoIURPTVdyYXBwZXJXb3JsZDo6aXNvbGF0ZWRXb3JsZHNFeGlzdCgpKSkgewotICAg ICAgICAgICAgICAgIHY4OjpQZXJzaXN0ZW50PHY4OjpPYmplY3Q+IHdyYXBwZXIgPSBub2RlLT53 cmFwcGVyKCk7Ci0gICAgICAgICAgICAgICAgaWYgKExJS0VMWSghd3JhcHBlci5Jc0VtcHR5KCkp KQotICAgICAgICAgICAgICAgICAgICByZXR1cm4gd3JhcHBlcjsKLSAgICAgICAgICAgIH0KKyAg ICAgICAgICAgIGlmIChMSUtFTFkoIURPTVdyYXBwZXJXb3JsZDo6aXNvbGF0ZWRXb3JsZHNFeGlz dCgpKSkKKyAgICAgICAgICAgICAgICByZXR1cm4gbm9kZS0+d3JhcHBlcigpOwogCiAgICAgICAg ICAgICBWOERPTVdpbmRvd1NoZWxsKiBjb250ZXh0ID0gVjhET01XaW5kb3dTaGVsbDo6Z2V0RW50 ZXJlZCgpOwogICAgICAgICAgICAgaWYgKExJS0VMWSghY29udGV4dCkpCiAgICAgICAgICAgICAg ICAgcmV0dXJuIG5vZGUtPndyYXBwZXIoKTsKIAotICAgICAgICAgICAgRE9NRGF0YVN0b3JlKiBz dG9yZSA9IGNvbnRleHQtPndvcmxkKCktPmRvbURhdGFTdG9yZSgpOwotICAgICAgICAgICAgRE9N V3JhcHBlck1hcDxOb2RlPiYgZG9tTm9kZU1hcCA9IHN0b3JlLT5kb21Ob2RlTWFwKCk7Ci0gICAg ICAgICAgICByZXR1cm4gZG9tTm9kZU1hcC5nZXQobm9kZSk7CisgICAgICAgICAgICByZXR1cm4g Y29udGV4dC0+d29ybGQoKS0+ZG9tRGF0YVN0b3JlKCktPmRvbU5vZGVNYXAoKS5nZXQobm9kZSk7 CiAgICAgICAgIH0KICAgICB9OwogCg==