Use references in public EventSender functions
authormmaxfield@apple.com <mmaxfield@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 2 Sep 2014 22:47:37 +0000 (22:47 +0000)
committermmaxfield@apple.com <mmaxfield@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 2 Sep 2014 22:47:37 +0000 (22:47 +0000)
https://bugs.webkit.org/show_bug.cgi?id=136463

Reviewed by Dan Bates.

Passing nullptr to EventSender shouldn't be allowed.

No new tests because there is no behavior change.

* dom/EventSender.h:
(WebCore::EventSender::hasPendingEvents):
(WebCore::EventSender<T>::dispatchEventSoon):
(WebCore::EventSender<T>::cancelEvent):
(WebCore::EventSender<T>::dispatchPendingEvents):
* html/HTMLLinkElement.cpp:
(WebCore::HTMLLinkElement::~HTMLLinkElement):
(WebCore::HTMLLinkElement::notifyLoadedSheetAndAllCriticalSubresources):
* html/HTMLStyleElement.cpp:
(WebCore::HTMLStyleElement::~HTMLStyleElement):
(WebCore::HTMLStyleElement::notifyLoadedSheetAndAllCriticalSubresources):
* loader/ImageLoader.cpp:
(WebCore::ImageLoader::~ImageLoader):
(WebCore::ImageLoader::setImageWithoutConsideringPendingLoadEvent):
(WebCore::ImageLoader::updateFromElement):
(WebCore::ImageLoader::notifyFinished):
(WebCore::ImageLoader::dispatchPendingBeforeLoadEvent):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@173190 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/dom/EventSender.h
Source/WebCore/html/HTMLLinkElement.cpp
Source/WebCore/html/HTMLStyleElement.cpp
Source/WebCore/loader/ImageLoader.cpp

index 529c7cc..9dea3eb 100644 (file)
@@ -1,3 +1,32 @@
+2014-09-02  Myles C. Maxfield  <mmaxfield@apple.com>
+
+        Use references in public EventSender functions
+        https://bugs.webkit.org/show_bug.cgi?id=136463
+
+        Reviewed by Dan Bates.
+
+        Passing nullptr to EventSender shouldn't be allowed.
+
+        No new tests because there is no behavior change.
+
+        * dom/EventSender.h:
+        (WebCore::EventSender::hasPendingEvents):
+        (WebCore::EventSender<T>::dispatchEventSoon):
+        (WebCore::EventSender<T>::cancelEvent):
+        (WebCore::EventSender<T>::dispatchPendingEvents):
+        * html/HTMLLinkElement.cpp:
+        (WebCore::HTMLLinkElement::~HTMLLinkElement):
+        (WebCore::HTMLLinkElement::notifyLoadedSheetAndAllCriticalSubresources):
+        * html/HTMLStyleElement.cpp:
+        (WebCore::HTMLStyleElement::~HTMLStyleElement):
+        (WebCore::HTMLStyleElement::notifyLoadedSheetAndAllCriticalSubresources):
+        * loader/ImageLoader.cpp:
+        (WebCore::ImageLoader::~ImageLoader):
+        (WebCore::ImageLoader::setImageWithoutConsideringPendingLoadEvent):
+        (WebCore::ImageLoader::updateFromElement):
+        (WebCore::ImageLoader::notifyFinished):
+        (WebCore::ImageLoader::dispatchPendingBeforeLoadEvent):
+
 2014-09-02  Daniel Bates  <dabates@apple.com>
 
         [iOS] Exclude touch and gesture files when building without ENABLE_TOUCH_EVENTS
index 2cb5aaf..04fe258 100644 (file)
@@ -37,14 +37,14 @@ public:
     explicit EventSender(const AtomicString& eventType);
 
     const AtomicString& eventType() const { return m_eventType; }
-    void dispatchEventSoon(T*);
-    void cancelEvent(T*);
+    void dispatchEventSoon(T&);
+    void cancelEvent(T&);
     void dispatchPendingEvents();
 
 #ifndef NDEBUG
-    bool hasPendingEvents(T* sender) const
+    bool hasPendingEvents(T& sender) const
     {
-        return m_dispatchSoonList.find(sender) != notFound || m_dispatchingList.find(sender) != notFound;
+        return m_dispatchSoonList.find(&sender) != notFound || m_dispatchingList.find(&sender) != notFound;
     }
 #endif
 
@@ -63,26 +63,24 @@ template<typename T> EventSender<T>::EventSender(const AtomicString& eventType)
 {
 }
 
-template<typename T> void EventSender<T>::dispatchEventSoon(T* sender)
+template<typename T> void EventSender<T>::dispatchEventSoon(T& sender)
 {
-    m_dispatchSoonList.append(sender);
+    m_dispatchSoonList.append(&sender);
     if (!m_timer.isActive())
         m_timer.startOneShot(0);
 }
 
-template<typename T> void EventSender<T>::cancelEvent(T* sender)
+template<typename T> void EventSender<T>::cancelEvent(T& sender)
 {
     // Remove instances of this sender from both lists.
     // Use loops because we allow multiple instances to get into the lists.
-    size_t size = m_dispatchSoonList.size();
-    for (size_t i = 0; i < size; ++i) {
-        if (m_dispatchSoonList[i] == sender)
-            m_dispatchSoonList[i] = 0;
+    for (auto& event : m_dispatchSoonList) {
+        if (event == &sender)
+            event = nullptr;
     }
-    size = m_dispatchingList.size();
-    for (size_t i = 0; i < size; ++i) {
-        if (m_dispatchingList[i] == sender)
-            m_dispatchingList[i] = 0;
+    for (auto& event : m_dispatchingList) {
+        if (event == &sender)
+            event = nullptr;
     }
 }
 
@@ -99,10 +97,9 @@ template<typename T> void EventSender<T>::dispatchPendingEvents()
     m_dispatchSoonList.checkConsistency();
 
     m_dispatchingList.swap(m_dispatchSoonList);
-    size_t size = m_dispatchingList.size();
-    for (size_t i = 0; i < size; ++i) {
-        if (T* sender = m_dispatchingList[i]) {
-            m_dispatchingList[i] = 0;
+    for (auto& event : m_dispatchingList) {
+        if (T* sender = event) {
+            event = nullptr;
             sender->dispatchPendingEvent(this);
         }
     }
index b851145..8c9ebae 100644 (file)
@@ -94,7 +94,7 @@ HTMLLinkElement::~HTMLLinkElement()
     if (inDocument())
         document().styleSheetCollection().removeStyleSheetCandidateNode(*this);
 
-    linkLoadEventSender().cancelEvent(this);
+    linkLoadEventSender().cancelEvent(*this);
 }
 
 void HTMLLinkElement::setDisabledState(bool disabled)
@@ -380,7 +380,7 @@ void HTMLLinkElement::notifyLoadedSheetAndAllCriticalSubresources(bool errorOccu
     if (m_firedLoad)
         return;
     m_loadedSheet = !errorOccurred;
-    linkLoadEventSender().dispatchEventSoon(this);
+    linkLoadEventSender().dispatchEventSoon(*this);
     m_firedLoad = true;
 }
 
index c59ef16..6806494 100644 (file)
@@ -60,7 +60,7 @@ HTMLStyleElement::~HTMLStyleElement()
     // Therefore we can't ASSERT(m_scopedStyleRegistrationState == NotRegistered).
     m_styleSheetOwner.clearDocumentData(document(), *this);
 
-    styleLoadEventSender().cancelEvent(this);
+    styleLoadEventSender().cancelEvent(*this);
 }
 
 PassRefPtr<HTMLStyleElement> HTMLStyleElement::create(const QualifiedName& tagName, Document& document, bool createdByParser)
@@ -133,7 +133,7 @@ void HTMLStyleElement::notifyLoadedSheetAndAllCriticalSubresources(bool errorOcc
     if (m_firedLoad)
         return;
     m_loadedSheet = !errorOccurred;
-    styleLoadEventSender().dispatchEventSoon(this);
+    styleLoadEventSender().dispatchEventSoon(*this);
     m_firedLoad = true;
 }
 
index 7d94fc8..9bad9b4 100644 (file)
@@ -105,17 +105,17 @@ ImageLoader::~ImageLoader()
     if (m_image)
         m_image->removeClient(this);
 
-    ASSERT(m_hasPendingBeforeLoadEvent || !beforeLoadEventSender().hasPendingEvents(this));
+    ASSERT(m_hasPendingBeforeLoadEvent || !beforeLoadEventSender().hasPendingEvents(*this));
     if (m_hasPendingBeforeLoadEvent)
-        beforeLoadEventSender().cancelEvent(this);
+        beforeLoadEventSender().cancelEvent(*this);
 
-    ASSERT(m_hasPendingLoadEvent || !loadEventSender().hasPendingEvents(this));
+    ASSERT(m_hasPendingLoadEvent || !loadEventSender().hasPendingEvents(*this));
     if (m_hasPendingLoadEvent)
-        loadEventSender().cancelEvent(this);
+        loadEventSender().cancelEvent(*this);
 
-    ASSERT(m_hasPendingErrorEvent || !errorEventSender().hasPendingEvents(this));
+    ASSERT(m_hasPendingErrorEvent || !errorEventSender().hasPendingEvents(*this));
     if (m_hasPendingErrorEvent)
-        errorEventSender().cancelEvent(this);
+        errorEventSender().cancelEvent(*this);
 
     // If the ImageLoader is being destroyed but it is still protecting its image-loading Element,
     // remove that protection here.
@@ -139,15 +139,15 @@ void ImageLoader::setImageWithoutConsideringPendingLoadEvent(CachedImage* newIma
     if (newImage != oldImage) {
         m_image = newImage;
         if (m_hasPendingBeforeLoadEvent) {
-            beforeLoadEventSender().cancelEvent(this);
+            beforeLoadEventSender().cancelEvent(*this);
             m_hasPendingBeforeLoadEvent = false;
         }
         if (m_hasPendingLoadEvent) {
-            loadEventSender().cancelEvent(this);
+            loadEventSender().cancelEvent(*this);
             m_hasPendingLoadEvent = false;
         }
         if (m_hasPendingErrorEvent) {
-            errorEventSender().cancelEvent(this);
+            errorEventSender().cancelEvent(*this);
             m_hasPendingErrorEvent = false;
         }
         m_imageComplete = true;
@@ -205,24 +205,24 @@ void ImageLoader::updateFromElement()
         if (!newImage && !pageIsBeingDismissed(document)) {
             m_failedLoadURL = attr;
             m_hasPendingErrorEvent = true;
-            errorEventSender().dispatchEventSoon(this);
+            errorEventSender().dispatchEventSoon(*this);
         } else
             clearFailedLoadURL();
     } else if (!attr.isNull()) {
         // Fire an error event if the url is empty.
         m_failedLoadURL = attr;
         m_hasPendingErrorEvent = true;
-        errorEventSender().dispatchEventSoon(this);
+        errorEventSender().dispatchEventSoon(*this);
     }
     
     CachedImage* oldImage = m_image.get();
     if (newImage != oldImage) {
         if (m_hasPendingBeforeLoadEvent) {
-            beforeLoadEventSender().cancelEvent(this);
+            beforeLoadEventSender().cancelEvent(*this);
             m_hasPendingBeforeLoadEvent = false;
         }
         if (m_hasPendingLoadEvent) {
-            loadEventSender().cancelEvent(this);
+            loadEventSender().cancelEvent(*this);
             m_hasPendingLoadEvent = false;
         }
 
@@ -231,7 +231,7 @@ void ImageLoader::updateFromElement()
         // this load and we should not cancel the event.
         // FIXME: If both previous load and this one got blocked with an error, we can receive one error event instead of two.
         if (m_hasPendingErrorEvent && newImage) {
-            errorEventSender().cancelEvent(this);
+            errorEventSender().cancelEvent(*this);
             m_hasPendingErrorEvent = false;
         }
 
@@ -245,7 +245,7 @@ void ImageLoader::updateFromElement()
                 if (!document.hasListenerType(Document::BEFORELOAD_LISTENER))
                     dispatchPendingBeforeLoadEvent();
                 else
-                    beforeLoadEventSender().dispatchEventSoon(this);
+                    beforeLoadEventSender().dispatchEventSoon(*this);
             } else
                 updateRenderer();
 
@@ -291,7 +291,7 @@ void ImageLoader::notifyFinished(CachedResource* resource)
         setImageWithoutConsideringPendingLoadEvent(0);
 
         m_hasPendingErrorEvent = true;
-        errorEventSender().dispatchEventSoon(this);
+        errorEventSender().dispatchEventSoon(*this);
 
         DEPRECATED_DEFINE_STATIC_LOCAL(String, consoleMessage, (ASCIILiteral("Cross-origin image load denied by Cross-Origin Resource Sharing policy.")));
         element().document().addConsoleMessage(MessageSource::Security, MessageLevel::Error, consoleMessage);
@@ -312,7 +312,7 @@ void ImageLoader::notifyFinished(CachedResource* resource)
         return;
     }
 
-    loadEventSender().dispatchEventSoon(this);
+    loadEventSender().dispatchEventSoon(*this);
 }
 
 RenderImageResource* ImageLoader::renderImageResource()
@@ -409,7 +409,7 @@ void ImageLoader::dispatchPendingBeforeLoadEvent()
         m_image = 0;
     }
 
-    loadEventSender().cancelEvent(this);
+    loadEventSender().cancelEvent(*this);
     m_hasPendingLoadEvent = false;
     
     if (isHTMLObjectElement(element()))