122592 2013-10-10 03:05:09 -0700 REGRESSION(r157210): Crashes in WebCore::ScopedEventQueue::dispatchEvent for platforms using GCC 2013-10-14 02:11:16 -0700 1 1 1 Unclassified WebKit New Bugs 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 zan zan berto commit-queue esprehn+autocc kangil.han kling rniwa sergio oldest_to_newest 938258 0 zan 2013-10-10 03:05:09 -0700 REGRESSION(r157210): Crashes in WebCore::ScopedEventQueue::dispatchEvent for platforms using GCC 938260 1 213863 zan 2013-10-10 03:17:23 -0700 Created attachment 213863 Patch 938264 2 213863 svillar 2013-10-10 03:35:05 -0700 Comment on attachment 213863 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=213863&action=review > Source/WebCore/ChangeLog:18 > + Looks like something wrong happened here :) > Source/WebCore/dom/ScopedEventQueue.cpp:83 > + EventDispatcher::dispatchEvent(node, event); I think this deserves a comment, because it isn't obvious why we're assigning the pointer first to a local variable. Or even better, we can assign the event to a local RefPtr and use that in dispatchEvent() 938269 3 213863 zan 2013-10-10 03:50:30 -0700 Comment on attachment 213863 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=213863&action=review >> Source/WebCore/dom/ScopedEventQueue.cpp:83 >> + EventDispatcher::dispatchEvent(node, event); > > I think this deserves a comment, because it isn't obvious why we're assigning the pointer first to a local variable. > > Or even better, we can assign the event to a local RefPtr and use that in dispatchEvent() How about passing in a naked pointer of the event? This won't destruct the original PassRefPtr. I'm missing support for something like std::forward() here. 938301 4 berto 2013-10-10 05:35:44 -0700 (In reply to comment #3) > How about passing in a naked pointer of the event? This won't > destruct the original PassRefPtr. I think it's unclear either way and deserves a comment. 938302 5 213875 zan 2013-10-10 05:39:47 -0700 Created attachment 213875 Patch 938314 6 213875 zan 2013-10-10 06:28:21 -0700 Comment on attachment 213875 Patch Clearing flags on attachment: 213875 Committed r157219: <http://trac.webkit.org/changeset/157219> 938315 7 zan 2013-10-10 06:28:27 -0700 All reviewed patches have been landed. Closing bug. 938388 8 ap 2013-10-10 09:46:23 -0700 I think that the original proposed patch was better, because it avoided reference churn from creating a new PassRefPtr. 938459 9 berto 2013-10-10 11:58:01 -0700 (In reply to comment #8) > I think that the original proposed patch was better, because it > avoided reference churn from creating a new PassRefPtr. I also think that it was a bit clearer, the .get() solution makes the reader wonder what's going on there, while the original one feels natural. 938524 10 zan 2013-10-10 13:45:51 -0700 I'm indifferent here, but I'll upload the adjustment in a separate bug. 939387 11 zan 2013-10-14 02:11:16 -0700 (In reply to comment #10) > I'm indifferent here, but I'll upload the adjustment in a separate bug. Bug #122742. 213863 2013-10-10 03:17:23 -0700 2013-10-10 05:39:41 -0700 Patch bug-122592-20131010031722.patch text/plain 2170 zan U3VidmVyc2lvbiBSZXZpc2lvbjogMTU3MjExCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggN2QzNDQ0MjY3YmZmYzRk YzY0YjI5ZDUxNzU1NDRhOWFlNTIyZDRlYi4uY2FlMWQ4ODVhZmE2ZjZiOTZkYjIzMDRkOGUxNjJm MmJhNzMzZjEwYSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDIxIEBACisyMDEzLTEwLTEwICBaYW4g RG9iZXJzZWsgIDx6ZG9iZXJzZWtAaWdhbGlhLmNvbT4KKworICAgICAgICBSRUdSRVNTSU9OKHIx NTcyMTApOiBDcmFzaGVzIGluIFdlYkNvcmU6OlNjb3BlZEV2ZW50UXVldWU6OmRpc3BhdGNoRXZl bnQgZm9yIHBsYXRmb3JtcyB1c2luZyBHQ0MKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5v cmcvc2hvd19idWcuY2dpP2lkPTEyMjU5MgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAo T09QUyEpLgorCisgICAgICAgICogZG9tL1Njb3BlZEV2ZW50UXVldWUuY3BwOgorICAgICAgICAo V2ViQ29yZTo6U2NvcGVkRXZlbnRRdWV1ZTo6ZGlzcGF0Y2hFdmVudCk6IFdoZW4gY2FsbGluZyBF dmVudERpc3BhdGNoZXI6OmRpc3BhdGNoRXZlbnQoKSwgdGhlIEdDQy1jb21waWxlZCBjb2RlCisg ICAgICAgIGZpcnN0IGNyZWF0ZXMgYSBjb3B5IG9mIHRoZSBQYXNzUmVmUHRyPEV2ZW50PiBvYmpl Y3QgdGhhdCdzIGJlaW5nIHBhc3NlZCBpbnRvIHRoZSBtZXRob2QgY2FsbC4gVGhlIGNvcHkgd2ls bCBiZSB1c2VkCisgICAgICAgIGluIHRoYXQgbWV0aG9kIHdoaWxlIHRoZSBvcmlnaW5hbCBpcyBs ZWZ0IHdpdGggYSBudWxsIHBvaW50ZXIuIE9ubHkgYWZ0ZXIgdGhhdCBpcyB0aGUgb3JpZ2luYWwg cXVlcmllZCBmb3IgaXRzIHBvaW50ZXIKKyAgICAgICAgd2hpbGUgdHJ5aW5nIHRvIGdldCB0aGUg RXZlbnQncyB0YXJnZXQsIHJlc3VsdGluZyBpbiBhIGNyYXNoIGR1ZSB0byBjYWxsaW5nIHRoZSBF dmVudDo6dGFyZ2V0KCkgb24gYSBudWxsIHBvaW50ZXIuCisgICAgICAgIFRvIGF2b2lkIGl0LCB0 aGUgcG9pbnRlciB0byB0aGUgRXZlbnQncyB0YXJnZXQgKGFzIGEgTm9kZSkgaXMgc3RvcmVkIGlu IGEgbG9jYWwgdmFyaWFibGUgYmVmb3JlIGNhbGxpbmcgRXZlbnREaXNwYXRjaGVyOjpkaXNwYXRj aEV2ZW50LgorCithbmQgbGVhdmluZyB0aGUgb3JpZ2luYWwKKyAgICAgICAgb25lIHdpdGggYSBu dWxsIHBvaW50ZXIuIE9ubHkgYWZ0ZXIgdGhhdCBpcyB0aGUgZXZlbnQncyB0YXJnZXQgcXVlcmll ZCwgd2hpY2ggbm93IGNyYXNoZXMgZHVlIHRvIHVzaW5nIHRoZSBvcmlnaW5hbAorICAgICAgICBQ YXNzUmVmUHRyIHRoYXQKKwogMjAxMy0xMC0wOCAgU2VyZ2lvIFZpbGxhciBTZW5pbiAgPHN2aWxs YXJAaWdhbGlhLmNvbT4KIAogICAgICAgICBbQ1NTIEdyaWQgTGF5b3V0XSBJbXBsZW1lbnQgc3Vw cG9ydCBmb3IgZ3JpZC10ZW1wbGF0ZQpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYkNvcmUvZG9tL1Nj b3BlZEV2ZW50UXVldWUuY3BwIGIvU291cmNlL1dlYkNvcmUvZG9tL1Njb3BlZEV2ZW50UXVldWUu Y3BwCmluZGV4IDYwMDU1YzVjNjc5NDY3NjBjYzNjNzE0OTU0NDEwOGMyMmExMDBmN2QuLjJhYjhk OGQ1NjA3ZDFhN2VmYzIxOTA3NjNjZWJlNWIzY2NjZGI5MGQgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9X ZWJDb3JlL2RvbS9TY29wZWRFdmVudFF1ZXVlLmNwcAorKysgYi9Tb3VyY2UvV2ViQ29yZS9kb20v U2NvcGVkRXZlbnRRdWV1ZS5jcHAKQEAgLTc5LDcgKzc5LDggQEAgdm9pZCBTY29wZWRFdmVudFF1 ZXVlOjpkaXNwYXRjaEFsbEV2ZW50cygpCiB2b2lkIFNjb3BlZEV2ZW50UXVldWU6OmRpc3BhdGNo RXZlbnQoUGFzc1JlZlB0cjxFdmVudD4gZXZlbnQpIGNvbnN0CiB7CiAgICAgQVNTRVJUKGV2ZW50 LT50YXJnZXQoKSk7Ci0gICAgRXZlbnREaXNwYXRjaGVyOjpkaXNwYXRjaEV2ZW50KGV2ZW50LT50 YXJnZXQoKS0+dG9Ob2RlKCksIGV2ZW50KTsKKyAgICBOb2RlKiBub2RlID0gZXZlbnQtPnRhcmdl dCgpLT50b05vZGUoKTsKKyAgICBFdmVudERpc3BhdGNoZXI6OmRpc3BhdGNoRXZlbnQobm9kZSwg ZXZlbnQpOwogfQogCiBTY29wZWRFdmVudFF1ZXVlKiBTY29wZWRFdmVudFF1ZXVlOjppbnN0YW5j ZSgpCg== 213875 2013-10-10 05:39:47 -0700 2013-10-10 06:28:21 -0700 Patch bug-122592-20131010053946.patch text/plain 2287 zan U3VidmVyc2lvbiBSZXZpc2lvbjogMTU3MjExCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggN2QzNDQ0MjY3YmZmYzRk YzY0YjI5ZDUxNzU1NDRhOWFlNTIyZDRlYi4uYWVmZDE3Mzg5YmIzNTE1ZjEyZTYyZWMyZWVhNDgw NjgxYzIzMzBlMiAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE4IEBACisyMDEzLTEwLTEwICBaYW4g RG9iZXJzZWsgIDx6ZG9iZXJzZWtAaWdhbGlhLmNvbT4KKworICAgICAgICBSRUdSRVNTSU9OKHIx NTcyMTApOiBDcmFzaGVzIGluIFdlYkNvcmU6OlNjb3BlZEV2ZW50UXVldWU6OmRpc3BhdGNoRXZl bnQgZm9yIHBsYXRmb3JtcyB1c2luZyBHQ0MKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5v cmcvc2hvd19idWcuY2dpP2lkPTEyMjU5MgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAo T09QUyEpLgorCisgICAgICAgICogZG9tL1Njb3BlZEV2ZW50UXVldWUuY3BwOgorICAgICAgICAo V2ViQ29yZTo6U2NvcGVkRXZlbnRRdWV1ZTo6ZGlzcGF0Y2hFdmVudCk6IFdoZW4gY2FsbGluZyBF dmVudERpc3BhdGNoZXI6OmRpc3BhdGNoRXZlbnQoKSwgdGhlIEdDQy1jb21waWxlZCBjb2RlCisg ICAgICAgIGZpcnN0IGNyZWF0ZXMgYSBjb3B5IG9mIHRoZSBQYXNzUmVmUHRyPEV2ZW50PiBvYmpl Y3QgdGhhdCdzIGJlaW5nIHBhc3NlZCBpbnRvIHRoZSBtZXRob2QgY2FsbC4gVGhlIGNvcHkgd2ls bCBiZSB1c2VkCisgICAgICAgIGluIHRoYXQgbWV0aG9kIHdoaWxlIHRoZSBvcmlnaW5hbCBpcyBs ZWZ0IHdpdGggYSBudWxsIHBvaW50ZXIuIE9ubHkgYWZ0ZXIgdGhhdCBpcyB0aGUgb3JpZ2luYWwg cXVlcmllZCBmb3IgaXRzIHBvaW50ZXIKKyAgICAgICAgd2hpbGUgdHJ5aW5nIHRvIGdldCB0aGUg RXZlbnQncyB0YXJnZXQsIHJlc3VsdGluZyBpbiBhIGNyYXNoIGR1ZSB0byBjYWxsaW5nIHRoZSBF dmVudDo6dGFyZ2V0KCkgb24gYSBudWxsIHBvaW50ZXIuCisgICAgICAgIFRvIGF2b2lkIGl0LCBw YXNzIGEgbmFrZWQgRXZlbnQgcG9pbnRlciB0byB0aGUgbWV0aG9kIGNhbGwuIFRoaXMgd2lsbCBj cmVhdGUgYSBuZXcgUGFzc1JlZlB0ciBvYmplY3Qgd2l0aG91dCBudWxsaWZ5aW5nCisgICAgICAg IHRoZSBvcmlnaW5hbCBvbmUuCisKIDIwMTMtMTAtMDggIFNlcmdpbyBWaWxsYXIgU2VuaW4gIDxz dmlsbGFyQGlnYWxpYS5jb20+CiAKICAgICAgICAgW0NTUyBHcmlkIExheW91dF0gSW1wbGVtZW50 IHN1cHBvcnQgZm9yIGdyaWQtdGVtcGxhdGUKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL2Rv bS9TY29wZWRFdmVudFF1ZXVlLmNwcCBiL1NvdXJjZS9XZWJDb3JlL2RvbS9TY29wZWRFdmVudFF1 ZXVlLmNwcAppbmRleCA2MDA1NWM1YzY3OTQ2NzYwY2MzYzcxNDk1NDQxMDhjMjJhMTAwZjdkLi5l M2RhZGYxNWUwMmI0ZTZjNjg1N2E1NjBjYWQzZjNlZWJjZTZjZDI3IDEwMDY0NAotLS0gYS9Tb3Vy Y2UvV2ViQ29yZS9kb20vU2NvcGVkRXZlbnRRdWV1ZS5jcHAKKysrIGIvU291cmNlL1dlYkNvcmUv ZG9tL1Njb3BlZEV2ZW50UXVldWUuY3BwCkBAIC03OSw3ICs3OSwxMCBAQCB2b2lkIFNjb3BlZEV2 ZW50UXVldWU6OmRpc3BhdGNoQWxsRXZlbnRzKCkKIHZvaWQgU2NvcGVkRXZlbnRRdWV1ZTo6ZGlz cGF0Y2hFdmVudChQYXNzUmVmUHRyPEV2ZW50PiBldmVudCkgY29uc3QKIHsKICAgICBBU1NFUlQo ZXZlbnQtPnRhcmdldCgpKTsKLSAgICBFdmVudERpc3BhdGNoZXI6OmRpc3BhdGNoRXZlbnQoZXZl bnQtPnRhcmdldCgpLT50b05vZGUoKSwgZXZlbnQpOworICAgIC8vIFBhc3NpbmcgYSBuYWtlZCBF dmVudCBwb2ludGVyIGluc3RlYWQgb2YgdGhlIFBhc3NSZWZQdHI8RXZlbnQ+IHByZXZlbnRzIHRo ZSBQYXNzUmVmUHRyIGNvcHkKKyAgICAvLyB0aGF0IG51bGxpZmllcyB0aGUgb3JpZ2luYWwgUGFz c1JlZlB0ci4gVGhpcyBwcmV2ZW50cyBjcmFzaGVzIG9uIEdDQy1jb21waWxlZCBjb2RlIHNpbmNl CisgICAgLy8gR0NDIGNyZWF0ZXMgdGhlIGNvcHkgZmlyc3QsIHdoaWNoIGxlYXZlcyB0aGUgZXZl bnQtPnRhcmdldCgpIGNhbGwgdG8gYmUgbWFkZSBvbiBhIG51bGwgcG9pbnRlci4KKyAgICBFdmVu dERpc3BhdGNoZXI6OmRpc3BhdGNoRXZlbnQoZXZlbnQtPnRhcmdldCgpLT50b05vZGUoKSwgZXZl bnQuZ2V0KCkpOwogfQogCiBTY29wZWRFdmVudFF1ZXVlKiBTY29wZWRFdmVudFF1ZXVlOjppbnN0 YW5jZSgpCg==