38765 2010-05-07 12:13:20 -0700 [Chromium] WebFrame::registerPasswordListener shouldn't assert on duplicate listener 2010-05-08 11:06:53 -0700 1 1 1 Unclassified WebKit WebKit API 528+ (Nightly build) All All RESOLVED FIXED P3 Minor --- 1 jens jens commit-queue fishd webkit.review.bot oldest_to_newest 222529 0 jens 2010-05-07 12:13:20 -0700 The Chromium WebKit API's implementation of WebFrame::registerPasswordListener currently requires that the given input element not already have a listener, and crashes with an ASSERT failure if it does. Unfortunately the method can be called multiple times on the same elements if the page contains duplicate forms with the same action URL. This causes such pages to crash debug builds of Chromium*. The fix is either (a) make the caller defensive and check the current list of listeners before trying to add a new one, or (b) make registerPasswordListener treat duplicate listeners as a recoverable error by returning false. Option (a) requires more code and incurs runtime overhead, so I think (b) is best. Note that since registerPasswordListener's contract is to take ownership of the listener pointer, it must delete it immediately if it can't register it. * http://code.google.com/p/chromium/issues/detail?id=20418 222540 1 55405 jens 2010-05-07 12:19:47 -0700 Created attachment 55405 patch 222543 2 webkit.review.bot 2010-05-07 12:23:13 -0700 Attachment 55405 did not pass style-queue: Failed to run "['WebKitTools/Scripts/check-webkit-style', '--no-squash']" exit_code: 1 WebKit/chromium/src/WebFrameImpl.cpp:1950: Missing space before { [whitespace/braces] [5] Total errors found: 1 in 4 files If any of these errors are false positives, please file a bug against check-webkit-style. 222568 3 fishd 2010-05-07 12:57:05 -0700 Why not just have registerPasswordListener clobber the existing listener? Or, provide a hasPasswordListener method? It might also be nice to support unregistering a listener (perhaps by passing 0 to registerPasswordListener). 222581 4 55413 jens 2010-05-07 13:17:48 -0700 Created attachment 55413 patch 2 Added a space to appease the coding style gods. 222596 5 55413 fishd 2010-05-07 13:48:21 -0700 Comment on attachment 55413 patch 2 OK, I'm convinced. It might be nice to provide a way to unregister a listener, but that doesn't have to be part of this patch, and perhaps we can wait on that until we have a need for it. R=me 222907 6 55413 commit-queue 2010-05-08 11:06:47 -0700 Comment on attachment 55413 patch 2 Clearing flags on attachment: 55413 Committed r59029: <http://trac.webkit.org/changeset/59029> 222908 7 commit-queue 2010-05-08 11:06:53 -0700 All reviewed patches have been landed. Closing bug. 55405 2010-05-07 12:19:47 -0700 2010-05-07 13:17:48 -0700 patch textmate stdin DdRbZt.txt text/plain 3287 jens ZGlmZiAtLWdpdCBhL1dlYktpdC9jaHJvbWl1bS9DaGFuZ2VMb2cgYi9XZWJLaXQvY2hyb21pdW0v Q2hhbmdlTG9nCmluZGV4IDY4MTI5YjAuLjllODc2MzQgMTAwNjQ0Ci0tLSBhL1dlYktpdC9jaHJv bWl1bS9DaGFuZ2VMb2cKKysrIGIvV2ViS2l0L2Nocm9taXVtL0NoYW5nZUxvZwpAQCAtMSwzICsx LDE3IEBACisyMDEwLTA1LTA3ICBKZW5zIEFsZmtlICA8c25lakBjaHJvbWl1bS5vcmc+CisKKyAg ICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgW2Nocm9taXVtXSBX ZWJGcmFtZTo6cmVnaXN0ZXJQYXNzd29yZExpc3RlbmVyIHNob3VsZG4ndCBhc3NlcnQgb24gZHVw bGljYXRlIGxpc3RlbmVyLgorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1 Zy5jZ2k/aWQ9Mzg3NjUKKworICAgICAgICAqIHB1YmxpYy9XZWJGcmFtZS5oOgorICAgICAgICAo V2ViS2l0OjpXZWJGcmFtZUltcGw6OnJlZ2lzdGVyUGFzc3dvcmRMaXN0ZW5lcik6IENoYW5nZWQg cmV0dXJuIHR5cGUgdG8gYm9vbC4gVXBkYXRlZCBjb21tZW50LgorICAgICAgICAqIHNyYy9XZWJG cmFtZUltcGwuY3BwOgorICAgICAgICAoV2ViS2l0OjpXZWJGcmFtZUltcGw6OnJlZ2lzdGVyUGFz c3dvcmRMaXN0ZW5lcik6IFJldHVybiBmYWxzZSBpbnN0ZWFkIG9mIGFzc2VydGluZy4KKyAgICAg ICAgKiBzcmMvV2ViRnJhbWVJbXBsLmg6CisgICAgICAgIChXZWJLaXQ6OldlYkZyYW1lSW1wbDo6 cmVnaXN0ZXJQYXNzd29yZExpc3RlbmVyKTogQ2hhbmdlZCByZXR1cm4gdHlwZSB0byBib29sLgor CiAyMDEwLTA1LTA2ICBLZW50IFRhbXVyYSAgPHRrZW50QGNocm9taXVtLm9yZz4KIAogICAgICAg ICBSZXZpZXdlZCBieSBEaW1pdHJpIEdsYXprb3YuCmRpZmYgLS1naXQgYS9XZWJLaXQvY2hyb21p dW0vcHVibGljL1dlYkZyYW1lLmggYi9XZWJLaXQvY2hyb21pdW0vcHVibGljL1dlYkZyYW1lLmgK aW5kZXggZjExMjQ0Ni4uNDZkZmE0MCAxMDA2NDQKLS0tIGEvV2ViS2l0L2Nocm9taXVtL3B1Ymxp Yy9XZWJGcmFtZS5oCisrKyBiL1dlYktpdC9jaHJvbWl1bS9wdWJsaWMvV2ViRnJhbWUuaApAQCAt NDYyLDggKzQ2MiwxMCBAQCBwdWJsaWM6CiAgICAgLy8gUmVnaXN0ZXJzIGEgbGlzdGVuZXIgZm9y IHRoZSBzcGVjaWZpZWQgdXNlciBuYW1lIGlucHV0IGVsZW1lbnQuIFRoZQogICAgIC8vIGxpc3Rl bmVyIHdpbGwgcmVjZWl2ZSBub3RpZmljYXRpb25zIGZvciBibHVyIGFuZCB3aGVuIGF1dG9jb21w bGV0ZQogICAgIC8vIHNob3VsZCBiZSB0cmlnZ2VyZWQuCi0gICAgLy8gVGhlIFdlYkZyYW1lIGJl Y29tZXMgdGhlIG93bmVyIG9mIHRoZSBwYXNzZWQgbGlzdGVuZXIuCi0gICAgdmlydHVhbCB2b2lk IHJlZ2lzdGVyUGFzc3dvcmRMaXN0ZW5lcigKKyAgICAvLyBBbiBlbGVtZW50IGNhbiBoYXZlIG9u bHkgb25lIGxpc3RlbmVyLiBJZiBhIGxpc3RlbmVyIGFscmVhZHkgZXhpc3RzLAorICAgIC8vIHRo aXMgbWV0aG9kIHJldHVybnMgZmFsc2UgYW5kIGRvZXMgbm90IGFkZCB0aGUgbmV3IG9uZS4KKyAg ICAvLyBFaXRoZXIgd2F5LCB0aGUgV2ViRnJhbWUgYmVjb21lcyB0aGUgb3duZXIgb2YgdGhlIHBh c3NlZCBsaXN0ZW5lci4KKyAgICB2aXJ0dWFsIGJvb2wgcmVnaXN0ZXJQYXNzd29yZExpc3RlbmVy KAogICAgICAgICBXZWJJbnB1dEVsZW1lbnQsCiAgICAgICAgIFdlYlBhc3N3b3JkQXV0b2NvbXBs ZXRlTGlzdGVuZXIqKSA9IDA7CiAKZGlmZiAtLWdpdCBhL1dlYktpdC9jaHJvbWl1bS9zcmMvV2Vi RnJhbWVJbXBsLmNwcCBiL1dlYktpdC9jaHJvbWl1bS9zcmMvV2ViRnJhbWVJbXBsLmNwcAppbmRl eCA3YjZjOGNlLi41YzI5ZTdhIDEwMDY0NAotLS0gYS9XZWJLaXQvY2hyb21pdW0vc3JjL1dlYkZy YW1lSW1wbC5jcHAKKysrIGIvV2ViS2l0L2Nocm9taXVtL3NyYy9XZWJGcmFtZUltcGwuY3BwCkBA IC0xOTQyLDEzICsxOTQyLDE2IEBAIHZvaWQgV2ViRnJhbWVJbXBsOjpzZXRDYW5IYXZlU2Nyb2xs YmFycyhib29sIGNhbkhhdmVTY3JvbGxiYXJzKQogICAgIG1fZnJhbWUtPnZpZXcoKS0+c2V0Q2Fu SGF2ZVNjcm9sbGJhcnMoY2FuSGF2ZVNjcm9sbGJhcnMpOwogfQogCi12b2lkIFdlYkZyYW1lSW1w bDo6cmVnaXN0ZXJQYXNzd29yZExpc3RlbmVyKAorYm9vbCBXZWJGcmFtZUltcGw6OnJlZ2lzdGVy UGFzc3dvcmRMaXN0ZW5lcigKICAgICBXZWJJbnB1dEVsZW1lbnQgaW5wdXRFbGVtZW50LAogICAg IFdlYlBhc3N3b3JkQXV0b2NvbXBsZXRlTGlzdGVuZXIqIGxpc3RlbmVyKQogewogICAgIFJlZlB0 cjxIVE1MSW5wdXRFbGVtZW50PiBlbGVtZW50ID0gaW5wdXRFbGVtZW50Lm9wZXJhdG9yIFBhc3NS ZWZQdHI8SFRNTElucHV0RWxlbWVudD4oKTsKLSAgICBBU1NFUlQobV9wYXNzd29yZExpc3RlbmVy cy5maW5kKGVsZW1lbnQpID09IG1fcGFzc3dvcmRMaXN0ZW5lcnMuZW5kKCkpOwotICAgIG1fcGFz c3dvcmRMaXN0ZW5lcnMuc2V0KGVsZW1lbnQsIGxpc3RlbmVyKTsKKyAgICBpZiAoIW1fcGFzc3dv cmRMaXN0ZW5lcnMuYWRkKGVsZW1lbnQsIGxpc3RlbmVyKS5zZWNvbmQpeworICAgICAgICBkZWxl dGUgbGlzdGVuZXI7CisgICAgICAgIHJldHVybiBmYWxzZTsKKyAgICB9CisgICAgcmV0dXJuIHRy dWU7CiB9CiAKIFdlYlBhc3N3b3JkQXV0b2NvbXBsZXRlTGlzdGVuZXIqIFdlYkZyYW1lSW1wbDo6 Z2V0UGFzc3dvcmRMaXN0ZW5lcigKZGlmZiAtLWdpdCBhL1dlYktpdC9jaHJvbWl1bS9zcmMvV2Vi RnJhbWVJbXBsLmggYi9XZWJLaXQvY2hyb21pdW0vc3JjL1dlYkZyYW1lSW1wbC5oCmluZGV4IDA4 ZDNjYzIuLmZhM2NkOTAgMTAwNjQ0Ci0tLSBhL1dlYktpdC9jaHJvbWl1bS9zcmMvV2ViRnJhbWVJ bXBsLmgKKysrIGIvV2ViS2l0L2Nocm9taXVtL3NyYy9XZWJGcmFtZUltcGwuaApAQCAtMTYwLDcg KzE2MCw3IEBAIHB1YmxpYzoKICAgICB2aXJ0dWFsIHZvaWQgY2FuY2VsUGVuZGluZ1Njb3BpbmdF ZmZvcnQoKTsKICAgICB2aXJ0dWFsIHZvaWQgaW5jcmVhc2VNYXRjaENvdW50KGludCBjb3VudCwg aW50IGlkZW50aWZpZXIpOwogICAgIHZpcnR1YWwgdm9pZCByZXNldE1hdGNoQ291bnQoKTsKLSAg ICB2aXJ0dWFsIHZvaWQgcmVnaXN0ZXJQYXNzd29yZExpc3RlbmVyKAorICAgIHZpcnR1YWwgYm9v bCByZWdpc3RlclBhc3N3b3JkTGlzdGVuZXIoCiAgICAgICAgIFdlYklucHV0RWxlbWVudCwgV2Vi UGFzc3dvcmRBdXRvY29tcGxldGVMaXN0ZW5lciopOwogCiAgICAgdmlydHVhbCBXZWJVUkwgY29t cGxldGVVUkwoY29uc3QgV2ViU3RyaW5nJiB1cmwpIGNvbnN0Owo= 55413 2010-05-07 13:17:48 -0700 2010-05-08 11:06:47 -0700 patch 2 textmate stdin Do44ax.txt text/plain 3288 jens ZGlmZiAtLWdpdCBhL1dlYktpdC9jaHJvbWl1bS9DaGFuZ2VMb2cgYi9XZWJLaXQvY2hyb21pdW0v Q2hhbmdlTG9nCmluZGV4IDY4MTI5YjAuLjllODc2MzQgMTAwNjQ0Ci0tLSBhL1dlYktpdC9jaHJv bWl1bS9DaGFuZ2VMb2cKKysrIGIvV2ViS2l0L2Nocm9taXVtL0NoYW5nZUxvZwpAQCAtMSwzICsx LDE3IEBACisyMDEwLTA1LTA3ICBKZW5zIEFsZmtlICA8c25lakBjaHJvbWl1bS5vcmc+CisKKyAg ICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgW2Nocm9taXVtXSBX ZWJGcmFtZTo6cmVnaXN0ZXJQYXNzd29yZExpc3RlbmVyIHNob3VsZG4ndCBhc3NlcnQgb24gZHVw bGljYXRlIGxpc3RlbmVyLgorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1 Zy5jZ2k/aWQ9Mzg3NjUKKworICAgICAgICAqIHB1YmxpYy9XZWJGcmFtZS5oOgorICAgICAgICAo V2ViS2l0OjpXZWJGcmFtZUltcGw6OnJlZ2lzdGVyUGFzc3dvcmRMaXN0ZW5lcik6IENoYW5nZWQg cmV0dXJuIHR5cGUgdG8gYm9vbC4gVXBkYXRlZCBjb21tZW50LgorICAgICAgICAqIHNyYy9XZWJG cmFtZUltcGwuY3BwOgorICAgICAgICAoV2ViS2l0OjpXZWJGcmFtZUltcGw6OnJlZ2lzdGVyUGFz c3dvcmRMaXN0ZW5lcik6IFJldHVybiBmYWxzZSBpbnN0ZWFkIG9mIGFzc2VydGluZy4KKyAgICAg ICAgKiBzcmMvV2ViRnJhbWVJbXBsLmg6CisgICAgICAgIChXZWJLaXQ6OldlYkZyYW1lSW1wbDo6 cmVnaXN0ZXJQYXNzd29yZExpc3RlbmVyKTogQ2hhbmdlZCByZXR1cm4gdHlwZSB0byBib29sLgor CiAyMDEwLTA1LTA2ICBLZW50IFRhbXVyYSAgPHRrZW50QGNocm9taXVtLm9yZz4KIAogICAgICAg ICBSZXZpZXdlZCBieSBEaW1pdHJpIEdsYXprb3YuCmRpZmYgLS1naXQgYS9XZWJLaXQvY2hyb21p dW0vcHVibGljL1dlYkZyYW1lLmggYi9XZWJLaXQvY2hyb21pdW0vcHVibGljL1dlYkZyYW1lLmgK aW5kZXggZjExMjQ0Ni4uNDZkZmE0MCAxMDA2NDQKLS0tIGEvV2ViS2l0L2Nocm9taXVtL3B1Ymxp Yy9XZWJGcmFtZS5oCisrKyBiL1dlYktpdC9jaHJvbWl1bS9wdWJsaWMvV2ViRnJhbWUuaApAQCAt NDYyLDggKzQ2MiwxMCBAQCBwdWJsaWM6CiAgICAgLy8gUmVnaXN0ZXJzIGEgbGlzdGVuZXIgZm9y IHRoZSBzcGVjaWZpZWQgdXNlciBuYW1lIGlucHV0IGVsZW1lbnQuIFRoZQogICAgIC8vIGxpc3Rl bmVyIHdpbGwgcmVjZWl2ZSBub3RpZmljYXRpb25zIGZvciBibHVyIGFuZCB3aGVuIGF1dG9jb21w bGV0ZQogICAgIC8vIHNob3VsZCBiZSB0cmlnZ2VyZWQuCi0gICAgLy8gVGhlIFdlYkZyYW1lIGJl Y29tZXMgdGhlIG93bmVyIG9mIHRoZSBwYXNzZWQgbGlzdGVuZXIuCi0gICAgdmlydHVhbCB2b2lk IHJlZ2lzdGVyUGFzc3dvcmRMaXN0ZW5lcigKKyAgICAvLyBBbiBlbGVtZW50IGNhbiBoYXZlIG9u bHkgb25lIGxpc3RlbmVyLiBJZiBhIGxpc3RlbmVyIGFscmVhZHkgZXhpc3RzLAorICAgIC8vIHRo aXMgbWV0aG9kIHJldHVybnMgZmFsc2UgYW5kIGRvZXMgbm90IGFkZCB0aGUgbmV3IG9uZS4KKyAg ICAvLyBFaXRoZXIgd2F5LCB0aGUgV2ViRnJhbWUgYmVjb21lcyB0aGUgb3duZXIgb2YgdGhlIHBh c3NlZCBsaXN0ZW5lci4KKyAgICB2aXJ0dWFsIGJvb2wgcmVnaXN0ZXJQYXNzd29yZExpc3RlbmVy KAogICAgICAgICBXZWJJbnB1dEVsZW1lbnQsCiAgICAgICAgIFdlYlBhc3N3b3JkQXV0b2NvbXBs ZXRlTGlzdGVuZXIqKSA9IDA7CiAKZGlmZiAtLWdpdCBhL1dlYktpdC9jaHJvbWl1bS9zcmMvV2Vi RnJhbWVJbXBsLmNwcCBiL1dlYktpdC9jaHJvbWl1bS9zcmMvV2ViRnJhbWVJbXBsLmNwcAppbmRl eCA3YjZjOGNlLi5hM2YwOTVmIDEwMDY0NAotLS0gYS9XZWJLaXQvY2hyb21pdW0vc3JjL1dlYkZy YW1lSW1wbC5jcHAKKysrIGIvV2ViS2l0L2Nocm9taXVtL3NyYy9XZWJGcmFtZUltcGwuY3BwCkBA IC0xOTQyLDEzICsxOTQyLDE2IEBAIHZvaWQgV2ViRnJhbWVJbXBsOjpzZXRDYW5IYXZlU2Nyb2xs YmFycyhib29sIGNhbkhhdmVTY3JvbGxiYXJzKQogICAgIG1fZnJhbWUtPnZpZXcoKS0+c2V0Q2Fu SGF2ZVNjcm9sbGJhcnMoY2FuSGF2ZVNjcm9sbGJhcnMpOwogfQogCi12b2lkIFdlYkZyYW1lSW1w bDo6cmVnaXN0ZXJQYXNzd29yZExpc3RlbmVyKAorYm9vbCBXZWJGcmFtZUltcGw6OnJlZ2lzdGVy UGFzc3dvcmRMaXN0ZW5lcigKICAgICBXZWJJbnB1dEVsZW1lbnQgaW5wdXRFbGVtZW50LAogICAg IFdlYlBhc3N3b3JkQXV0b2NvbXBsZXRlTGlzdGVuZXIqIGxpc3RlbmVyKQogewogICAgIFJlZlB0 cjxIVE1MSW5wdXRFbGVtZW50PiBlbGVtZW50ID0gaW5wdXRFbGVtZW50Lm9wZXJhdG9yIFBhc3NS ZWZQdHI8SFRNTElucHV0RWxlbWVudD4oKTsKLSAgICBBU1NFUlQobV9wYXNzd29yZExpc3RlbmVy cy5maW5kKGVsZW1lbnQpID09IG1fcGFzc3dvcmRMaXN0ZW5lcnMuZW5kKCkpOwotICAgIG1fcGFz c3dvcmRMaXN0ZW5lcnMuc2V0KGVsZW1lbnQsIGxpc3RlbmVyKTsKKyAgICBpZiAoIW1fcGFzc3dv cmRMaXN0ZW5lcnMuYWRkKGVsZW1lbnQsIGxpc3RlbmVyKS5zZWNvbmQpIHsKKyAgICAgICAgZGVs ZXRlIGxpc3RlbmVyOworICAgICAgICByZXR1cm4gZmFsc2U7CisgICAgfQorICAgIHJldHVybiB0 cnVlOwogfQogCiBXZWJQYXNzd29yZEF1dG9jb21wbGV0ZUxpc3RlbmVyKiBXZWJGcmFtZUltcGw6 OmdldFBhc3N3b3JkTGlzdGVuZXIoCmRpZmYgLS1naXQgYS9XZWJLaXQvY2hyb21pdW0vc3JjL1dl YkZyYW1lSW1wbC5oIGIvV2ViS2l0L2Nocm9taXVtL3NyYy9XZWJGcmFtZUltcGwuaAppbmRleCAw OGQzY2MyLi5mYTNjZDkwIDEwMDY0NAotLS0gYS9XZWJLaXQvY2hyb21pdW0vc3JjL1dlYkZyYW1l SW1wbC5oCisrKyBiL1dlYktpdC9jaHJvbWl1bS9zcmMvV2ViRnJhbWVJbXBsLmgKQEAgLTE2MCw3 ICsxNjAsNyBAQCBwdWJsaWM6CiAgICAgdmlydHVhbCB2b2lkIGNhbmNlbFBlbmRpbmdTY29waW5n RWZmb3J0KCk7CiAgICAgdmlydHVhbCB2b2lkIGluY3JlYXNlTWF0Y2hDb3VudChpbnQgY291bnQs IGludCBpZGVudGlmaWVyKTsKICAgICB2aXJ0dWFsIHZvaWQgcmVzZXRNYXRjaENvdW50KCk7Ci0g ICAgdmlydHVhbCB2b2lkIHJlZ2lzdGVyUGFzc3dvcmRMaXN0ZW5lcigKKyAgICB2aXJ0dWFsIGJv b2wgcmVnaXN0ZXJQYXNzd29yZExpc3RlbmVyKAogICAgICAgICBXZWJJbnB1dEVsZW1lbnQsIFdl YlBhc3N3b3JkQXV0b2NvbXBsZXRlTGlzdGVuZXIqKTsKIAogICAgIHZpcnR1YWwgV2ViVVJMIGNv bXBsZXRlVVJMKGNvbnN0IFdlYlN0cmluZyYgdXJsKSBjb25zdDsK