83850 2012-04-12 18:50:07 -0700 JSDocument::setLocation does too much bare-handed lifting 2012-04-13 09:29:07 -0700 1 1 1 Unclassified WebKit New Bugs 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 abarth abarth eric webkit.review.bot oldest_to_newest 601740 0 abarth 2012-04-12 18:50:07 -0700 JSDocument::setLocation does too much bare-handed lifting 601741 1 137022 abarth 2012-04-12 18:51:51 -0700 Created attachment 137022 Patch 601924 2 137022 webkit.review.bot 2012-04-13 01:57:52 -0700 Comment on attachment 137022 Patch Clearing flags on attachment: 137022 Committed r114098: <http://trac.webkit.org/changeset/114098> 601925 3 webkit.review.bot 2012-04-13 01:57:56 -0700 All reviewed patches have been landed. Closing bug. 602130 4 137022 darin 2012-04-13 09:20:03 -0700 Comment on attachment 137022 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=137022&action=review > Source/WebCore/bindings/js/JSDocumentCustom.cpp:74 > + UString locationString = value.toString(exec)->value(exec); > + if (exec->hadException()) > + return; Is there no exception possible in toString? If there is, I think we’d need two separate return statements. 602131 5 137022 darin 2012-04-13 09:20:49 -0700 Comment on attachment 137022 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=137022&action=review >> Source/WebCore/bindings/js/JSDocumentCustom.cpp:74 >> + return; > > Is there no exception possible in toString? If there is, I think we’d need two separate return statements. Or maybe toString can have an exception, but value can't. 602138 6 abarth 2012-04-13 09:29:07 -0700 (In reply to comment #5) > (From update of attachment 137022 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=137022&action=review > > >> Source/WebCore/bindings/js/JSDocumentCustom.cpp:74 > >> + return; > > > > Is there no exception possible in toString? If there is, I think we’d need two separate return statements. > > Or maybe toString can have an exception, but value can't. I believe that's the case, but I'll verify by reading the code. 137022 2012-04-12 18:51:51 -0700 2012-04-13 09:20:49 -0700 Patch bug-83850-20120412185150.patch text/plain 2301 abarth SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDExNDA2OCkKKysrIFNvdXJjZS9XZWJDb3JlL0NoYW5n ZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE4IEBACisyMDEyLTA0LTEyICBBZGFtIEJh cnRoICA8YWJhcnRoQHdlYmtpdC5vcmc+CisKKyAgICAgICAgSlNEb2N1bWVudDo6c2V0TG9jYXRp b24gZG9lcyB0b28gbXVjaCBiYXJlLWhhbmRlZCBsaWZ0aW5nCisgICAgICAgIGh0dHBzOi8vYnVn cy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD04Mzg1MAorCisgICAgICAgIFJldmlld2VkIGJ5 IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIEFzIHBhcnQgb2YgYXVkaXRpbmcgYWxsIHRoZSB3 YXlzIG9mIGtpY2tpbmcgb2ZmIGEgbmF2aWdhdGlvbiwgSQorICAgICAgICBoYXBwZW5lZCB0byBy ZWFkIEpTRG9jdW1lbnQ6OnNldExvY2F0aW9uLCB3aGljaCB1c2VzIHZlcnkgb2xkIHBhdHRlcm5z LgorICAgICAgICBUaGlzIHBhdGNoIHVwZGF0ZXMgaXQgdG8gZG8gdGhpbmdzIHRoZSAibW9kZXJu IiB3YXkuICBUaGVyZSBzaG91bGRuJ3QKKyAgICAgICAgYmUgYW55IGJlaGF2aW9yIGNoYW5nZS4K KworICAgICAgICAqIGJpbmRpbmdzL2pzL0pTRG9jdW1lbnRDdXN0b20uY3BwOgorICAgICAgICAo V2ViQ29yZTo6SlNEb2N1bWVudDo6c2V0TG9jYXRpb24pOgorCiAyMDEyLTA0LTEyICBEYXZpZCBE b3J3aW4gIDxkZG9yd2luQGNocm9taXVtLm9yZz4KIAogICAgICAgICBBZGQgRW5jcnlwdGVkIE1l ZGlhIEV4dGVuc2lvbnMgZXZlbnRzIGFuZCBlcnJvcnMgdG8gSFRNTE1lZGlhRWxlbWVudApJbmRl eDogU291cmNlL1dlYkNvcmUvYmluZGluZ3MvanMvSlNEb2N1bWVudEN1c3RvbS5jcHAKPT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PQotLS0gU291cmNlL1dlYkNvcmUvYmluZGluZ3MvanMvSlNEb2N1bWVudEN1c3RvbS5jcHAJ KHJldmlzaW9uIDExNDA1NikKKysrIFNvdXJjZS9XZWJDb3JlL2JpbmRpbmdzL2pzL0pTRG9jdW1l bnRDdXN0b20uY3BwCSh3b3JraW5nIGNvcHkpCkBAIC02OSwxOCArNjksMTIgQEAgdm9pZCBKU0Rv Y3VtZW50OjpzZXRMb2NhdGlvbihFeGVjU3RhdGUqIAogICAgIGlmICghZnJhbWUpCiAgICAgICAg IHJldHVybjsKIAotICAgIFN0cmluZyBzdHIgPSB1c3RyaW5nVG9TdHJpbmcodmFsdWUudG9TdHJp bmcoZXhlYyktPnZhbHVlKGV4ZWMpKTsKLQotICAgIEZyYW1lKiBsZXhpY2FsRnJhbWUgPSBhc0pT RE9NV2luZG93KGV4ZWMtPmxleGljYWxHbG9iYWxPYmplY3QoKSktPmltcGwoKS0+ZnJhbWUoKTsK KyAgICBVU3RyaW5nIGxvY2F0aW9uU3RyaW5nID0gdmFsdWUudG9TdHJpbmcoZXhlYyktPnZhbHVl KGV4ZWMpOworICAgIGlmIChleGVjLT5oYWRFeGNlcHRpb24oKSkKKyAgICAgICAgcmV0dXJuOwog Ci0gICAgLy8gSUUgYW5kIE1vemlsbGEgYm90aCByZXNvbHZlIHRoZSBVUkwgcmVsYXRpdmUgdG8g dGhlIHNvdXJjZSBmcmFtZSwKLSAgICAvLyBub3QgdGhlIHRhcmdldCBmcmFtZS4KLSAgICBGcmFt ZSogYWN0aXZlRnJhbWUgPSBhc0pTRE9NV2luZG93KGV4ZWMtPmR5bmFtaWNHbG9iYWxPYmplY3Qo KSktPmltcGwoKS0+ZnJhbWUoKTsKLSAgICBzdHIgPSBhY3RpdmVGcmFtZS0+ZG9jdW1lbnQoKS0+ Y29tcGxldGVVUkwoc3RyKS5zdHJpbmcoKTsKLQotICAgIGJvb2wgbG9ja0hpc3RvcnkgPSAhU2Ny aXB0Q29udHJvbGxlcjo6cHJvY2Vzc2luZ1VzZXJHZXN0dXJlKCk7Ci0gICAgZnJhbWUtPm5hdmln YXRpb25TY2hlZHVsZXIoKS0+c2NoZWR1bGVMb2NhdGlvbkNoYW5nZShsZXhpY2FsRnJhbWUtPmRv Y3VtZW50KCktPnNlY3VyaXR5T3JpZ2luKCksCi0gICAgICAgIHN0ciwgYWN0aXZlRnJhbWUtPmxv YWRlcigpLT5vdXRnb2luZ1JlZmVycmVyKCksIGxvY2tIaXN0b3J5LCBmYWxzZSk7CisgICAgaWYg KExvY2F0aW9uKiBsb2NhdGlvbiA9IGZyYW1lLT5kb21XaW5kb3coKS0+bG9jYXRpb24oKSkKKyAg ICAgICAgbG9jYXRpb24tPnNldEhyZWYodXN0cmluZ1RvU3RyaW5nKGxvY2F0aW9uU3RyaW5nKSwg YWN0aXZlRE9NV2luZG93KGV4ZWMpLCBmaXJzdERPTVdpbmRvdyhleGVjKSk7CiB9CiAKIEpTVmFs dWUgdG9KUyhFeGVjU3RhdGUqIGV4ZWMsIEpTRE9NR2xvYmFsT2JqZWN0KiBnbG9iYWxPYmplY3Qs IERvY3VtZW50KiBkb2N1bWVudCkK