114322 2013-04-09 17:08:09 -0700 Update Document's event listener type bitfield when adopting a Node 2013-04-10 09:04:19 -0700 1 1 1 Unclassified WebKit New Bugs 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 adamk adamk commit-queue darin rniwa oldest_to_newest 872523 0 adamk 2013-04-09 17:08:09 -0700 Update Document's event listener type bitfield when adopting a Node 872524 1 197189 adamk 2013-04-09 17:10:26 -0700 Created attachment 197189 Patch 872533 2 197189 darin 2013-04-09 17:36:04 -0700 Comment on attachment 197189 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=197189&action=review Looks fine. > Source/WebCore/dom/Node.cpp:2158 > + Vector<AtomicString> types = listenerMap.eventTypes(); If I had more time I’d study the implementation of EventListenerMap::eventTypes, but is it really necessary to copy a vector of atomic strings? Is there any more efficient way to get at the event type strings, perhaps without allocating memory? 872535 3 adamk 2013-04-09 17:40:06 -0700 (In reply to comment #2) > (From update of attachment 197189 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=197189&action=review > > Looks fine. > > > Source/WebCore/dom/Node.cpp:2158 > > + Vector<AtomicString> types = listenerMap.eventTypes(); > > If I had more time I’d study the implementation of EventListenerMap::eventTypes, but is it really necessary to copy a vector of atomic strings? Is there any more efficient way to get at the event type strings, perhaps without allocating memory? I poked around for a bit because I had the same thoughts. The solution would be to expose some sort of iterator interface from EventListenerMap to let one iterate over the types without exposing the implementation (as a vector of pairs). However, I think it's likely not worth doing just for this use, since the normal case is no EventTargetData, or just a few listeners. So the vector isn't likely to be large, even in the rare case when it's allocated. 872548 4 197189 commit-queue 2013-04-09 18:10:20 -0700 Comment on attachment 197189 Patch Clearing flags on attachment: 197189 Committed r148072: <http://trac.webkit.org/changeset/148072> 872549 5 commit-queue 2013-04-09 18:10:23 -0700 All reviewed patches have been landed. Closing bug. 872559 6 197189 darin 2013-04-09 18:49:34 -0700 Comment on attachment 197189 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=197189&action=review >>> Source/WebCore/dom/Node.cpp:2158 >>> + Vector<AtomicString> types = listenerMap.eventTypes(); >> >> If I had more time I’d study the implementation of EventListenerMap::eventTypes, but is it really necessary to copy a vector of atomic strings? Is there any more efficient way to get at the event type strings, perhaps without allocating memory? > > I poked around for a bit because I had the same thoughts. The solution would be to expose some sort of iterator interface from EventListenerMap to let one iterate over the types without exposing the implementation (as a vector of pairs). > > However, I think it's likely not worth doing just for this use, since the normal case is no EventTargetData, or just a few listeners. So the vector isn't likely to be large, even in the rare case when it's allocated. If the common case is a small vector, then an example of an efficient data structure that does not require memory allocation could be Vector<AtomicString, 4>. 872818 7 adamk 2013-04-10 09:04:19 -0700 (In reply to comment #6) > (From update of attachment 197189 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=197189&action=review > > >>> Source/WebCore/dom/Node.cpp:2158 > >>> + Vector<AtomicString> types = listenerMap.eventTypes(); > >> > >> If I had more time I’d study the implementation of EventListenerMap::eventTypes, but is it really necessary to copy a vector of atomic strings? Is there any more efficient way to get at the event type strings, perhaps without allocating memory? > > > > I poked around for a bit because I had the same thoughts. The solution would be to expose some sort of iterator interface from EventListenerMap to let one iterate over the types without exposing the implementation (as a vector of pairs). > > > > However, I think it's likely not worth doing just for this use, since the normal case is no EventTargetData, or just a few listeners. So the vector isn't likely to be large, even in the rare case when it's allocated. > > If the common case is a small vector, then an example of an efficient data structure that does not require memory allocation could be Vector<AtomicString, 4>. Seems like a reasonable follow-up, not sure if I'll have time to put it together myself. 2 seems like it might be the right size, to match the sizing in EventListenerMap::m_entries. C++11 would make such changes easier, as one could use "auto" instead of having to update callers of eventTypes(). 197189 2013-04-09 17:10:26 -0700 2013-04-09 18:49:34 -0700 Patch bug-114322-20130409171008.patch text/plain 5159 adamk U3VidmVyc2lvbiBSZXZpc2lvbjogMTQ4MDU3CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggNWU1MThhNTdjMzJjNjU3 YmVlZDgyN2VmZmRiZDNhNGEzNzE0NDNiMy4uMDZhZDdlZjE0Nzc1NmY0YjA5YWIzZDJkY2EzMTEx MWQ4OTZjNDlmMSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDI1IEBACisyMDEzLTA0LTA5ICBBZGFt IEtsZWluICA8YWRhbWtAY2hyb21pdW0ub3JnPgorCisgICAgICAgIFVwZGF0ZSBEb2N1bWVudCdz IGV2ZW50IGxpc3RlbmVyIHR5cGUgYml0ZmllbGQgd2hlbiBhZG9wdGluZyBhIE5vZGUKKyAgICAg ICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTExNDMyMgorCisgICAg ICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIFdpdGhvdXQgdGhpcywg bW92aW5nIGEgTm9kZSBiZXR3ZWVuIGRvY3VtZW50cyBjYW4gc2lsZW50bHkgZGVhY3RpdmF0ZQor ICAgICAgICBhbiBldmVudCBsaXN0ZW5lciwgaWYgaXQncyBvbmUgb2YgdGhlIHR5cGVzIHRoYXQg d2hvc2UgY3JlYXRpb24gaXMKKyAgICAgICAgb3B0aW1pemVkIGF3YXkgYnkgRG9jdW1lbnQ6Omhh c0xpc3RlbmVyVHlwZS4KKworICAgICAgICBBbiBhbHRlcm5hdGUgYXBwcm9hY2ggd291bGQgYmUg dG8gc2ltcGx5IGNvcHkgdGhlIG9sZCBkb2N1bWVudCdzCisgICAgICAgIGJpdGZpZWxkIG92ZXIu IEl0J3MgYSB0cmFkZW9mZiBiZXR3ZWVuIG1ha2luZyBhZG9wdGlvbiBmYXN0IGFuZCBtYWtpbmcK KyAgICAgICAgdGhlIG9wZXJhdGlvbiBvZiBhbnkgb3BlcmF0aW9uIGRlcGVuZGluZyBvbiB0aGVz ZSBldmVudCB0eXBlcyBmYXN0LgorICAgICAgICBUaGUgbGF0dGVyIHNlZW1zIGxpa2UgdGhlIHJp Z2h0IG9wdGltaXphdGlvbiBnaXZlbiB0aGF0IGFkb3B0aW9uCisgICAgICAgIGRvZXNuJ3QgaGFw cGVuIHZlcnkgb2Z0ZW4uCisKKyAgICAgICAgVGVzdDogZmFzdC9ldmVudHMvZXZlbnQtbGlzdGVu ZXItbW92aW5nLWRvY3VtZW50cy5odG1sCisKKyAgICAgICAgKiBkb20vTm9kZS5jcHA6CisgICAg ICAgIChXZWJDb3JlOjpOb2RlOjpkaWRNb3ZlVG9OZXdEb2N1bWVudCk6IEZvciBlYWNoIGV2ZW50 IGxpc3RlbmVyIHR5cGUgb24gdGhlIGFkb3B0ZWQgbm9kZSwgdXBkYXRlIHRoZSBuZXcgZG9jdW1l bnQncyBsaXN0IG9mIGxpc3RlbmVyIHR5cGVzLgorCiAyMDEzLTA0LTA5ICBCZW0gSm9uZXMtQmV5 ICA8YmpvbmVzYmVAYWRvYmUuY29tPgogCiAgICAgICAgIFtDU1MgRXhjbHVzaW9uc10gUHJvcGVy bHkgcG9zaXRpb24gbXVsdGlwbGUgc3RhY2tlZCBmbG9hdHMgd2l0aCBub24gcmVjdGFuZ3VsYXIg c2hhcGUgb3V0c2lkZQpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYkNvcmUvZG9tL05vZGUuY3BwIGIv U291cmNlL1dlYkNvcmUvZG9tL05vZGUuY3BwCmluZGV4IDcwMzU2ODhlYWMwNmIyMzMwOWQ5Y2Jj ZTBiNjMxOGE4YjU2ZDkzZTQuLjUzM2QwOTI1YzVmNWY1NmZkYTNmOTMyNWM0NGM0YTFiZTNhYjhi YmMgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJDb3JlL2RvbS9Ob2RlLmNwcAorKysgYi9Tb3VyY2Uv V2ViQ29yZS9kb20vTm9kZS5jcHAKQEAgLTIxNTIsMTIgKzIxNTIsMTkgQEAgdm9pZCBOb2RlOjpk aWRNb3ZlVG9OZXdEb2N1bWVudChEb2N1bWVudCogb2xkRG9jdW1lbnQpCiB7CiAgICAgVHJlZVNj b3BlQWRvcHRlcjo6ZW5zdXJlRGlkTW92ZVRvTmV3RG9jdW1lbnRXYXNDYWxsZWQob2xkRG9jdW1l bnQpOwogCisgICAgaWYgKGNvbnN0IEV2ZW50VGFyZ2V0RGF0YSogZXZlbnRUYXJnZXREYXRhID0g dGhpcy0+ZXZlbnRUYXJnZXREYXRhKCkpIHsKKyAgICAgICAgY29uc3QgRXZlbnRMaXN0ZW5lck1h cCYgbGlzdGVuZXJNYXAgPSBldmVudFRhcmdldERhdGEtPmV2ZW50TGlzdGVuZXJNYXA7CisgICAg ICAgIGlmICghbGlzdGVuZXJNYXAuaXNFbXB0eSgpKSB7CisgICAgICAgICAgICBWZWN0b3I8QXRv bWljU3RyaW5nPiB0eXBlcyA9IGxpc3RlbmVyTWFwLmV2ZW50VHlwZXMoKTsKKyAgICAgICAgICAg IGZvciAodW5zaWduZWQgaSA9IDA7IGkgPCB0eXBlcy5zaXplKCk7ICsraSkKKyAgICAgICAgICAg ICAgICBkb2N1bWVudCgpLT5hZGRMaXN0ZW5lclR5cGVJZk5lZWRlZCh0eXBlc1tpXSk7CisgICAg ICAgIH0KKyAgICB9CisKICAgICBpZiAoQVhPYmplY3RDYWNoZTo6YWNjZXNzaWJpbGl0eUVuYWJs ZWQoKSAmJiBvbGREb2N1bWVudCkKICAgICAgICAgaWYgKEFYT2JqZWN0Q2FjaGUqIGNhY2hlID0g b2xkRG9jdW1lbnQtPmV4aXN0aW5nQVhPYmplY3RDYWNoZSgpKQogICAgICAgICAgICAgY2FjaGUt PnJlbW92ZSh0aGlzKTsKIAotICAgIC8vIEZJWE1FOiBFdmVudCBsaXN0ZW5lciB0eXBlcyBmb3Ig dGhpcyBub2RlIHNob3VsZCBiZSBzZXQgb24gdGhlIG5ldyBvd25lciBkb2N1bWVudCBoZXJlLgot CiAgICAgY29uc3QgRXZlbnRMaXN0ZW5lclZlY3RvciYgd2hlZWxMaXN0ZW5lcnMgPSBnZXRFdmVu dExpc3RlbmVycyhldmVudE5hbWVzKCkubW91c2V3aGVlbEV2ZW50KTsKICAgICBmb3IgKHNpemVf dCBpID0gMDsgaSA8IHdoZWVsTGlzdGVuZXJzLnNpemUoKTsgKytpKSB7CiAgICAgICAgIG9sZERv Y3VtZW50LT5kaWRSZW1vdmVXaGVlbEV2ZW50SGFuZGxlcigpOwpkaWZmIC0tZ2l0IGEvTGF5b3V0 VGVzdHMvQ2hhbmdlTG9nIGIvTGF5b3V0VGVzdHMvQ2hhbmdlTG9nCmluZGV4IDQyYzNlYzAzNGVh NzE2NjAwYThhNDkxNDNmZmIwNTY3N2NiM2Y3NzIuLjJkMzNiNjcxMzRhNTFjYWVjZTNlODkyMTBj YjlhZjEzOGNjZjgzODcgMTAwNjQ0Ci0tLSBhL0xheW91dFRlc3RzL0NoYW5nZUxvZworKysgYi9M YXlvdXRUZXN0cy9DaGFuZ2VMb2cKQEAgLTEsMyArMSwxMyBAQAorMjAxMy0wNC0wOSAgQWRhbSBL bGVpbiAgPGFkYW1rQGNocm9taXVtLm9yZz4KKworICAgICAgICBVcGRhdGUgRG9jdW1lbnQncyBl dmVudCBsaXN0ZW5lciB0eXBlIGJpdGZpZWxkIHdoZW4gYWRvcHRpbmcgYSBOb2RlCisgICAgICAg IGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xMTQzMjIKKworICAgICAg ICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICAqIGZhc3QvZXZlbnRzL2V2 ZW50LWxpc3RlbmVyLW1vdmluZy1kb2N1bWVudHMtZXhwZWN0ZWQudHh0OiBBZGRlZC4KKyAgICAg ICAgKiBmYXN0L2V2ZW50cy9ldmVudC1saXN0ZW5lci1tb3ZpbmctZG9jdW1lbnRzLmh0bWw6IEFk ZGVkLgorCiAyMDEzLTA0LTA5ICBTaW1vbiBGcmFzZXIgIDxzaW1vbi5mcmFzZXJAYXBwbGUuY29t PgogCiAgICAgICAgIFJlYmFzZWxpbmUgdGVzdHMgYWZmZWN0ZWQgYnkgcjE0ODA0OSBvciByMTQ4 MDQ4LgpkaWZmIC0tZ2l0IGEvTGF5b3V0VGVzdHMvZmFzdC9ldmVudHMvZXZlbnQtbGlzdGVuZXIt bW92aW5nLWRvY3VtZW50cy1leHBlY3RlZC50eHQgYi9MYXlvdXRUZXN0cy9mYXN0L2V2ZW50cy9l dmVudC1saXN0ZW5lci1tb3ZpbmctZG9jdW1lbnRzLWV4cGVjdGVkLnR4dApuZXcgZmlsZSBtb2Rl IDEwMDY0NAppbmRleCAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwLi41 ODdmNjhiMTA2MjQwYTYyZGQ2NGFlZmU2ZmQ1ZTNmOWMzNWFiNTU4Ci0tLSAvZGV2L251bGwKKysr IGIvTGF5b3V0VGVzdHMvZmFzdC9ldmVudHMvZXZlbnQtbGlzdGVuZXItbW92aW5nLWRvY3VtZW50 cy1leHBlY3RlZC50eHQKQEAgLTAsMCArMSwxMiBAQAorTW92aW5nIGFuIGV2ZW50IGxpc3RlbmVy IGJldHdlZW4gZG9jdW1lbnRzIHNob3VsZCBrZWVwIGl0IGFjdGl2ZQorCitPbiBzdWNjZXNzLCB5 b3Ugd2lsbCBzZWUgYSBzZXJpZXMgb2YgIlBBU1MiIG1lc3NhZ2VzLCBmb2xsb3dlZCBieSAiVEVT VCBDT01QTEVURSIuCisKKworUEFTUyBhZGRlZENhbGxzIGlzIDEKK1BBU1MgYWRkZWRDYWxscyBp cyAyCitQQVNTIHJlbW92ZWRDYWxscyBpcyAxCitQQVNTIHN1Y2Nlc3NmdWxseVBhcnNlZCBpcyB0 cnVlCisKK1RFU1QgQ09NUExFVEUKKwpkaWZmIC0tZ2l0IGEvTGF5b3V0VGVzdHMvZmFzdC9ldmVu dHMvZXZlbnQtbGlzdGVuZXItbW92aW5nLWRvY3VtZW50cy5odG1sIGIvTGF5b3V0VGVzdHMvZmFz dC9ldmVudHMvZXZlbnQtbGlzdGVuZXItbW92aW5nLWRvY3VtZW50cy5odG1sCm5ldyBmaWxlIG1v ZGUgMTAwNjQ0CmluZGV4IDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAu LmE0N2ExNjRiYzYyMmUzNTQxN2M3N2U4NTc0OGIzNWIzMzQ5YzNlZmEKLS0tIC9kZXYvbnVsbAor KysgYi9MYXlvdXRUZXN0cy9mYXN0L2V2ZW50cy9ldmVudC1saXN0ZW5lci1tb3ZpbmctZG9jdW1l bnRzLmh0bWwKQEAgLTAsMCArMSwyMSBAQAorPCFET0NUWVBFIGh0bWw+Cis8Ym9keT4KKzxzY3Jp cHQgc3JjPSIuLi9qcy9yZXNvdXJjZXMvanMtdGVzdC1wcmUuanMiPjwvc2NyaXB0PgorPHNjcmlw dD4KK2Rlc2NyaXB0aW9uKCdNb3ZpbmcgYW4gZXZlbnQgbGlzdGVuZXIgYmV0d2VlbiBkb2N1bWVu dHMgc2hvdWxkIGtlZXAgaXQgYWN0aXZlJyk7CisKK3ZhciBkb2MgPSBkb2N1bWVudC5pbXBsZW1l bnRhdGlvbi5jcmVhdGVIVE1MRG9jdW1lbnQoJycpOwordmFyIGRpdiA9IGRvYy5jcmVhdGVFbGVt ZW50KCdkaXYnKTsKK3ZhciBhZGRlZENhbGxzID0gMDsKK3ZhciByZW1vdmVkQ2FsbHMgPSAwOwor ZGl2LmFkZEV2ZW50TGlzdGVuZXIoJ0RPTU5vZGVJbnNlcnRlZCcsIGZ1bmN0aW9uKCkgeyBhZGRl ZENhbGxzKysgfSk7CitkaXYuYWRkRXZlbnRMaXN0ZW5lcignRE9NTm9kZVJlbW92ZWQnLCBmdW5j dGlvbigpIHsgcmVtb3ZlZENhbGxzKysgfSk7Citkb2N1bWVudC5ib2R5LmFwcGVuZENoaWxkKGRp dik7CitzaG91bGRCZSgnYWRkZWRDYWxscycsICcxJyk7CitkaXYuYXBwZW5kQ2hpbGQoZG9jdW1l bnQuY3JlYXRlRWxlbWVudCgnc3BhbicpKTsKK3Nob3VsZEJlKCdhZGRlZENhbGxzJywgJzInKTsK K2Rpdi5yZW1vdmVDaGlsZChkaXYuZmlyc3RDaGlsZCk7CitzaG91bGRCZSgncmVtb3ZlZENhbGxz JywgJzEnKTsKKzwvc2NyaXB0PgorPHNjcmlwdCBzcmM9Ii4uL2pzL3Jlc291cmNlcy9qcy10ZXN0 LXBvc3QuanMiPjwvc2NyaXB0PgorPC9ib2R5Pgo=