Bug 78840

Created attachment 127438 [details] Patch The name "EventSender" is a bit vague. The purpose of this class is to queue up senders of a specific DOM event, say a Load event, and to call each sender back when they should dispatch such an event. Maybe EventSenderScheduler or EventSenderQueue would be a more descriptive name?

Comment on attachment 127438 [details] Patch Clearing review flag as I need to add EventSender.h to all build systems.

Created attachment 127464 [details] Patch

Comment on attachment 127464 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=127464&action=review > Source/WebCore/dom/EventSender.h:88 > + if (m_dispatchSoonList.isEmpty()) > + m_timer.stop(); Do you mean to check whether the list is empty or whether it contains all zeros? I'm not sure how this function could result in m_dispatchSoonList being empty if it wasn't already empty. > Source/WebCore/loader/ImageLoader.cpp:-385 > - if (m_dispatchSoonList.isEmpty()) > - m_timer.stop(); I see this was here already. We might want to ask the original author why they intended here.

(In reply to comment #4) > (From update of attachment 127464 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=127464&action=review > > > Source/WebCore/dom/EventSender.h:88 > > + if (m_dispatchSoonList.isEmpty()) > > + m_timer.stop(); > > Do you mean to check whether the list is empty or whether it contains all zeros? I'm not sure how this function could result in m_dispatchSoonList being empty if it wasn't already empty. As far as I can tell this code isn't necessary anymore and is an artifact of using a QPtrList data structure for this functionality when it was originally in DocumentImpl::removeImage() (see <http://trac.webkit.org/browser/trunk/WebCore/khtml/xml/DocumentImpl.cpp?rev=12515#L2302>). In particular, DocumentImpl::removeImage() actually removed items from its QPtrList m_imageLoadEventDispatchSoonList (as opposed to zeroing out its items as we do today) ;=> m_imageLoadEventDispatchSoonList could become empty. I'll remove this code before landing. CC'ing Darin Adler since he may be interested to see this go by given that he kept this "if (m_dispatchSoonList.isEmpty())" code through both <http://trac.webkit.org/changeset/39296> (changed functionality to zero out list entries) and <http://trac.webkit.org/changeset/41766> (moved functionality out of Document.cpp).

Committed r108561: <http://trac.webkit.org/changeset/108561>

This EventSender should be SuspendableTimer or at least ActiveDOMObject?