Summary: | Overwriting an attribute event listener can lead to wrong event listener firing order | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Chris Dumez <cdumez> | ||||||
Component: | DOM | Assignee: | Chris Dumez <cdumez> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | cdumez, commit-queue, darin, dbates, esprehn+autocc, kangil.han, rniwa, sam | ||||||
Priority: | P2 | Keywords: | WebExposed | ||||||
Version: | WebKit Nightly Build | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Attachments: |
|
Description
Chris Dumez
2016-10-06 14:12:53 PDT
Created attachment 290865 [details]
Patch
Comment on attachment 290865 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=290865&action=review > Source/WebCore/dom/EventListenerMap.cpp:120 > + listeners->at(index)->markAsRemoved(); > + listeners->at(index) = RegisteredEventListener::create(WTFMove(newListener), options); Should we use a reference to avoid calling at() twice? > Source/WebCore/dom/EventListenerMap.h:58 > + void replace(const AtomicString& eventType, EventListener& oldListener, Ref<EventListener>&& newListener, const RegisteredEventListener::Options&); This interface is intrinsically inefficient. Callers always have to call find before you call replace, so will always have to search the map twice! In the future for better efficiency we should consider adding an operation that does exactly the right thing so that setAttributeEventListener can be efficient. This would be analogous to HashMap::add function that either adds if there is no existing item, or returns the location of the existing item if there is one. Created attachment 290871 [details]
Patch
Comment on attachment 290871 [details] Patch Clearing flags on attachment: 290871 Committed r206889: <http://trac.webkit.org/changeset/206889> All reviewed patches have been landed. Closing bug. |