Add assertions to JSLazyEventListener to help catch the cause of a crash
authorcdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 7 May 2019 00:21:25 +0000 (00:21 +0000)
committercdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 7 May 2019 00:21:25 +0000 (00:21 +0000)
https://bugs.webkit.org/show_bug.cgi?id=197617

Reviewed by Alexey Proskuryakov.

Add assertions to JSLazyEventListener to help catch the cause of <rdar://problem/24314027>.

* bindings/js/JSLazyEventListener.cpp:
(WebCore::JSLazyEventListener::checkValidityForEventTarget):
* bindings/js/JSLazyEventListener.h:
* dom/EventListener.h:
(WebCore::EventListener::checkValidityForEventTarget):
* dom/EventTarget.cpp:
(WebCore::EventTarget::addEventListener):
(WebCore::EventTarget::setAttributeEventListener):
(WebCore::EventTarget::innerInvokeEventListeners):

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

Source/WebCore/ChangeLog
Source/WebCore/bindings/js/JSLazyEventListener.cpp
Source/WebCore/bindings/js/JSLazyEventListener.h
Source/WebCore/dom/EventListener.h
Source/WebCore/dom/EventTarget.cpp

index ac8a26f..42e5e3d 100644 (file)
@@ -1,3 +1,22 @@
+2019-05-06  Chris Dumez  <cdumez@apple.com>
+
+        Add assertions to JSLazyEventListener to help catch the cause of a crash
+        https://bugs.webkit.org/show_bug.cgi?id=197617
+
+        Reviewed by Alexey Proskuryakov.
+
+        Add assertions to JSLazyEventListener to help catch the cause of <rdar://problem/24314027>.
+
+        * bindings/js/JSLazyEventListener.cpp:
+        (WebCore::JSLazyEventListener::checkValidityForEventTarget):
+        * bindings/js/JSLazyEventListener.h:
+        * dom/EventListener.h:
+        (WebCore::EventListener::checkValidityForEventTarget):
+        * dom/EventTarget.cpp:
+        (WebCore::EventTarget::addEventListener):
+        (WebCore::EventTarget::setAttributeEventListener):
+        (WebCore::EventTarget::innerInvokeEventListeners):
+
 2019-05-04  Per Arne Vollan  <pvollan@apple.com>
 
         -[WKWebsiteDataStore removeDataOfTypes:forDataRecords:completionHandler:] doesn't delete _WKWebsiteDataTypeCredentials
index 8ca1336..bfe62c4 100644 (file)
@@ -79,6 +79,18 @@ JSLazyEventListener::JSLazyEventListener(CreationArguments&& arguments, const St
 #endif
 }
 
+#if !ASSERT_DISABLED
+// This is to help find the underlying cause of <rdar://problem/24314027>.
+void JSLazyEventListener::checkValidityForEventTarget(EventTarget& eventTarget)
+{
+    if (eventTarget.isNode()) {
+        ASSERT(m_originalNode);
+        ASSERT(static_cast<EventTarget*>(m_originalNode.get()) == &eventTarget);
+    } else
+        ASSERT(!m_originalNode);
+}
+#endif
+
 JSLazyEventListener::~JSLazyEventListener()
 {
 #ifndef NDEBUG
index 552d077..2d4c068 100644 (file)
@@ -46,6 +46,10 @@ private:
     static RefPtr<JSLazyEventListener> create(CreationArguments&&);
     JSLazyEventListener(CreationArguments&&, const String& sourceURL, const TextPosition&);
 
+#if !ASSERT_DISABLED
+    void checkValidityForEventTarget(EventTarget&) final;
+#endif
+
     JSC::JSObject* initializeJSFunction(ScriptExecutionContext&) const final;
     bool wasCreatedFromMarkup() const final { return true; }
 
index c36136f..f8e2270 100644 (file)
@@ -31,6 +31,7 @@ namespace WebCore {
 
 class ScriptExecutionContext;
 class Event;
+class EventTarget;
 
 class EventListener : public RefCounted<EventListener> {
 public:
@@ -55,6 +56,10 @@ public:
     virtual bool isAttribute() const { return false; }
     Type type() const { return m_type; }
 
+#if !ASSERT_DISABLED
+    virtual void checkValidityForEventTarget(EventTarget&) { }
+#endif
+
 protected:
     explicit EventListener(Type type)
         : m_type(type)
index 6ddde4d..b89394b 100644 (file)
@@ -38,6 +38,7 @@
 #include "HTMLHtmlElement.h"
 #include "InspectorInstrumentation.h"
 #include "JSEventListener.h"
+#include "JSLazyEventListener.h"
 #include "RuntimeEnabledFeatures.h"
 #include "ScriptController.h"
 #include "ScriptDisallowedScope.h"
@@ -69,6 +70,10 @@ bool EventTarget::isPaymentRequest() const
 
 bool EventTarget::addEventListener(const AtomicString& eventType, Ref<EventListener>&& listener, const AddEventListenerOptions& options)
 {
+#if !ASSERT_DISABLED
+    listener->checkValidityForEventTarget(*this);
+#endif
+
     auto passive = options.passive;
 
     if (!passive.hasValue() && eventNames().isTouchScrollBlockingEventType(eventType)) {
@@ -145,6 +150,10 @@ bool EventTarget::setAttributeEventListener(const AtomicString& eventType, RefPt
     if (existingListener) {
         InspectorInstrumentation::willRemoveEventListener(*this, eventType, *existingListener, false);
 
+#if !ASSERT_DISABLED
+        listener->checkValidityForEventTarget(*this);
+#endif
+
         auto listenerPointer = listener.copyRef();
         eventTargetData()->eventListenerMap.replace(eventType, *existingListener, listener.releaseNonNull(), { });
 
@@ -301,6 +310,10 @@ void EventTarget::innerInvokeEventListeners(Event& event, EventListenerVector li
         if (registeredListener->isPassive())
             event.setInPassiveListener(true);
 
+#if !ASSERT_DISABLED
+        registeredListener->callback().checkValidityForEventTarget(*this);
+#endif
+
         InspectorInstrumentation::willHandleEvent(context, event, *registeredListener);
         registeredListener->callback().handleEvent(context, event);
         InspectorInstrumentation::didHandleEvent(context);