122742 2013-10-14 01:44:28 -0700 Reintroduce PassRefPtr<Event> copy in ScopedEventQueue::dispatchEvent 2013-10-14 10:05:07 -0700 1 1 1 Unclassified WebKit New Bugs 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 zan zan ap berto commit-queue esprehn+autocc kangil.han oldest_to_newest 939379 0 zan 2013-10-14 01:44:28 -0700 Reintroduce PassRefPtr<Event> copy in ScopedEventQueue::dispatchEvent 939388 1 214136 zan 2013-10-14 02:11:34 -0700 Created attachment 214136 Patch 939391 2 berto 2013-10-14 02:30:27 -0700 lgtm 939455 3 214136 zan 2013-10-14 08:43:58 -0700 Comment on attachment 214136 Patch Clearing flags on attachment: 214136 Committed r157401: <http://trac.webkit.org/changeset/157401> 939456 4 zan 2013-10-14 08:44:04 -0700 All reviewed patches have been landed. Closing bug. 939490 5 214136 darin 2013-10-14 10:05:07 -0700 Comment on attachment 214136 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=214136&action=review > Source/WebCore/dom/ScopedEventQueue.cpp:86 > + // Passing the PassRefPtr<Event> object into the method call creates a new copy and also nullifies > + // the original object, which is causing crashes in GCC-compiled code that only after that goes on > + // to retrieve the Event's target, calling Event::target() on the now-null PassRefPtr<Event> object. > + Node* node = event->target()->toNode(); > + EventDispatcher::dispatchEvent(node, event); This is a totally confusing comment. The bug is no surprise. If we put the node computation into the function call we’d be depending on undefined behavior. But the comment should say something more like this: // Put node in local variable to make sure we don’t dereference the event after it's nulled out to pass it in. The mentions of things like "create a new copy" and "crashes in GCC-compiled code" make the comment just landed confusing and it's *so* long for such a minor issue. 214136 2013-10-14 02:11:34 -0700 2013-10-14 10:05:07 -0700 Patch bug-122742-20131014021133.patch text/plain 3072 zan U3VidmVyc2lvbiBSZXZpc2lvbjogMTU3MzUzCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggNmUzMjI3M2U4ZWU4OWEx OWJlYzQxYmM4ZmY5OTBhNmU0OTM4ZjQzOC4uYmVkOWVlNDM2MjE2YjI4NzdjYmJjNWJlYjEyMGIx M2JiZDI2OGFlOSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDI1IEBACisyMDEzLTEwLTE0ICBaYW4g RG9iZXJzZWsgIDx6ZG9iZXJzZWtAaWdhbGlhLmNvbT4KKworICAgICAgICBSZWludHJvZHVjZSBQ YXNzUmVmUHRyPEV2ZW50PiBjb3B5IGluIFNjb3BlZEV2ZW50UXVldWU6OmRpc3BhdGNoRXZlbnQK KyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTEyMjc0Mgor CisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIFRoaXMgaXMg YSBmb2xsb3ctdXAgdG8gcjE1NzIxOSB3aGljaCBpbnRyb2R1Y2VkIGEgd29ya2Fyb3VuZCBmb3Ig dGhlIEdDQydzIHF1aXJreSBiZWhhdmlvciB0aGF0CisgICAgICAgIHdhcyByZXN1bHRpbmcgaW4g Y3Jhc2hlcyBkdWUgdG8gdGhlIFBhc3NSZWZQdHI8RXZlbnQ+IG9iamVjdCBwYXNzZWQgdG8gRXZl bnREaXNwYXRjaGVyOjpkaXNwYXRjaEV2ZW50CisgICAgICAgIGJlaW5nIGNvcGllZCBhbmQgbnVs bGlmaWVkIGZpcnN0IGJlZm9yZSByZXRyaWV2aW5nIHRoZSBFdmVudFRhcmdldCBvZiB0aGUgRXZl bnQgb2JqZWN0IHdyYXBwZWQgaW4gdGhhdAorICAgICAgICBQYXNzUmVmUHRyLgorCisgICAgICAg IFRoZSBpbXBsZW1lbnRhdGlvbiBpcyBub3cgYWRqdXN0ZWQgdG8gZmlyc3QgcmV0cmlldmUgdGhl IHBvaW50ZXIgdG8gdGhlIEV2ZW50J3MgRXZlbnRUYXJnZXQgYW5kIHN0b3JlCisgICAgICAgIGl0 IGluIGEgbG9jYWwgdmFyaWFibGUuIFRoYXQgdmFyaWFibGUgaXMgdGhlbiBwYXNzZWQgYXMgdGhl IGZpcnN0IHBhcmFtZXRlciB0byBFdmVudERpc3BhdGNoZXI6OmRpc3BhdGNoRXZlbnQsCisgICAg ICAgIGFuZCB0aGUgUGFzc1JlZlB0cjxFdmVudD4gcGFzc2VkIGRpcmVjdGx5IGFzIHRoZSBzZWNv bmQgcGFyYW1ldGVyLiBQcmV2aW91c2x5IHRoZSBwb2ludGVyIG9mIHRoYXQgUGFzc1JlZlB0cgor ICAgICAgICBvYmplY3Qgd2FzIHBhc3NlZCBpbiwgd2l0aCBhIG5ldyBQYXNzUmVmUHRyIGJlaW5n IGNyZWF0ZWQgd2hpY2ggd291bGQgaW5jcmVhc2UgdGhlIHJlZmVyZW5jZSBjb3VudCBvZiB0aGUK KyAgICAgICAgcmVmLWNvdW50ZWQgb2JqZWN0LiBQYXNzaW5nIGluIHRoZSBvcmlnaW5hbCBQYXNz UmVmUHRyIGF2b2lkcyB0aGUgdW5uZWNlc3NhcnkgcmVmZXJlbmNlIGNvdW50IGluY3JlYXNlIGJ5 IGNyZWF0aW5nCisgICAgICAgIGEgY29weS4gVGhhdCBzdGlsbCBudWxsaWZpZXMgdGhlIG9yaWdp bmFsIFBhc3NSZWZQdHIsIGJ1dCB0aGF0J3Mgbm90IGEgcHJvYmxlbSBhbnltb3JlLgorCisgICAg ICAgICogZG9tL1Njb3BlZEV2ZW50UXVldWUuY3BwOgorICAgICAgICAoV2ViQ29yZTo6U2NvcGVk RXZlbnRRdWV1ZTo6ZGlzcGF0Y2hFdmVudCk6CisKIDIwMTMtMTAtMTIgIERhcmluIEFkbGVyICA8 ZGFyaW5AYXBwbGUuY29tPgogCiAgICAgICAgIE1vdmUgY29kZSB0byBmaW5kIGVsZW1lbnRzIGlu IHNsaWRlciBzaGFkb3cgdHJlZSBpbnRvIFJhbmdlSW5wdXRUeXBlIGNsYXNzLCBzaW5jZSBpdCBj cmVhdGVzIHRoYXQgdHJlZQpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYkNvcmUvZG9tL1Njb3BlZEV2 ZW50UXVldWUuY3BwIGIvU291cmNlL1dlYkNvcmUvZG9tL1Njb3BlZEV2ZW50UXVldWUuY3BwCmlu ZGV4IGUzZGFkZjE1ZTAyYjRlNmM2ODU3YTU2MGNhZDNmM2VlYmNlNmNkMjcuLjBjMGZhMjkwOTA1 ZTQ0ZjI3MjgyNzhlZDk1Zjg4ZmQ0MjljM2JjZmQgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJDb3Jl L2RvbS9TY29wZWRFdmVudFF1ZXVlLmNwcAorKysgYi9Tb3VyY2UvV2ViQ29yZS9kb20vU2NvcGVk RXZlbnRRdWV1ZS5jcHAKQEAgLTc5LDEwICs3OSwxMSBAQCB2b2lkIFNjb3BlZEV2ZW50UXVldWU6 OmRpc3BhdGNoQWxsRXZlbnRzKCkKIHZvaWQgU2NvcGVkRXZlbnRRdWV1ZTo6ZGlzcGF0Y2hFdmVu dChQYXNzUmVmUHRyPEV2ZW50PiBldmVudCkgY29uc3QKIHsKICAgICBBU1NFUlQoZXZlbnQtPnRh cmdldCgpKTsKLSAgICAvLyBQYXNzaW5nIGEgbmFrZWQgRXZlbnQgcG9pbnRlciBpbnN0ZWFkIG9m IHRoZSBQYXNzUmVmUHRyPEV2ZW50PiBwcmV2ZW50cyB0aGUgUGFzc1JlZlB0ciBjb3B5Ci0gICAg Ly8gdGhhdCBudWxsaWZpZXMgdGhlIG9yaWdpbmFsIFBhc3NSZWZQdHIuIFRoaXMgcHJldmVudHMg Y3Jhc2hlcyBvbiBHQ0MtY29tcGlsZWQgY29kZSBzaW5jZQotICAgIC8vIEdDQyBjcmVhdGVzIHRo ZSBjb3B5IGZpcnN0LCB3aGljaCBsZWF2ZXMgdGhlIGV2ZW50LT50YXJnZXQoKSBjYWxsIHRvIGJl IG1hZGUgb24gYSBudWxsIHBvaW50ZXIuCi0gICAgRXZlbnREaXNwYXRjaGVyOjpkaXNwYXRjaEV2 ZW50KGV2ZW50LT50YXJnZXQoKS0+dG9Ob2RlKCksIGV2ZW50LmdldCgpKTsKKyAgICAvLyBQYXNz aW5nIHRoZSBQYXNzUmVmUHRyPEV2ZW50PiBvYmplY3QgaW50byB0aGUgbWV0aG9kIGNhbGwgY3Jl YXRlcyBhIG5ldyBjb3B5IGFuZCBhbHNvIG51bGxpZmllcworICAgIC8vIHRoZSBvcmlnaW5hbCBv YmplY3QsIHdoaWNoIGlzIGNhdXNpbmcgY3Jhc2hlcyBpbiBHQ0MtY29tcGlsZWQgY29kZSB0aGF0 IG9ubHkgYWZ0ZXIgdGhhdCBnb2VzIG9uCisgICAgLy8gdG8gcmV0cmlldmUgdGhlIEV2ZW50J3Mg dGFyZ2V0LCBjYWxsaW5nIEV2ZW50Ojp0YXJnZXQoKSBvbiB0aGUgbm93LW51bGwgUGFzc1JlZlB0 cjxFdmVudD4gb2JqZWN0LgorICAgIE5vZGUqIG5vZGUgPSBldmVudC0+dGFyZ2V0KCktPnRvTm9k ZSgpOworICAgIEV2ZW50RGlzcGF0Y2hlcjo6ZGlzcGF0Y2hFdmVudChub2RlLCBldmVudCk7CiB9 CiAKIFNjb3BlZEV2ZW50UXVldWUqIFNjb3BlZEV2ZW50UXVldWU6Omluc3RhbmNlKCkK