[webkit-changes] [250977] trunk/Source/WebCore

Thu, 10 Oct 2019 11:05:35 -0700

Title: [250977] trunk/Source/WebCore
Revision
250977
Author
[email protected]
Date
2019-10-10 11:04:45 -0700 (Thu, 10 Oct 2019)

Log Message

Web Inspector: Timelines: don't call willDispatchEvent/didDispatchEvent unless there is a listener for the event
https://bugs.webkit.org/show_bug.cgi?id=202713

Reviewed by Joseph Pecoraro.

Fixes failing inspector/timeline/timeline-recording.html after r250672. This was because the
`InspectorTimelineAgent` expected a corresponding `willDispatchEvent` before it was told
about the `didDispatchEvent`, which wasn't happening since only `willDispatchEvent` would
early-return if the `DOMWindow` didn't have any event listeners for the dispatched event. By
making the `DOMWindow::dispatchEvent` itself early-return in that case, it now handles both
`willDispatchEvent` and `didDispatchEvent`, ensuring that they are always called in pairs.

* page/DOMWindow.cpp:
(WebCore::DOMWindow::dispatchEvent):

* dom/EventTarget.cpp:
(WebCore::EventTarget::innerInvokeEventListeners):
* inspector/InspectorInstrumentation.h:
(WebCore::InspectorInstrumentation::willDispatchEvent):
(WebCore::InspectorInstrumentation::didDispatchEvent):
(WebCore::InspectorInstrumentation::willDispatchEventOnWindow):
(WebCore::InspectorInstrumentation::didDispatchEventOnWindow):
* inspector/InspectorInstrumentation.cpp:
(WebCore::InspectorInstrumentation::willDispatchEventImpl):
(WebCore::InspectorInstrumentation::didDispatchEventImpl):
(WebCore::InspectorInstrumentation::willDispatchEventOnWindowImpl):
(WebCore::InspectorInstrumentation::didDispatchEventOnWindowImpl):
`InspectorInstrumentation::willDispatchEventImpl` was always called with `hasEventListeners`
as `true`, so there's no reason to keep that parameter around. Similarly, the change inside
`DOMWindow::dispatchEvent` will make it so that the same is true for
`InspectorInstrumentation::willDispatchEventOnWindowImpl` as well.

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (250976 => 250977)


--- trunk/Source/WebCore/ChangeLog	2019-10-10 17:17:51 UTC (rev 250976)
+++ trunk/Source/WebCore/ChangeLog	2019-10-10 18:04:45 UTC (rev 250977)
@@ -1,3 +1,37 @@
+2019-10-10  Devin Rousso  <[email protected]>
+
+        Web Inspector: Timelines: don't call willDispatchEvent/didDispatchEvent unless there is a listener for the event
+        https://bugs.webkit.org/show_bug.cgi?id=202713
+
+        Reviewed by Joseph Pecoraro.
+
+        Fixes failing inspector/timeline/timeline-recording.html after r250672. This was because the
+        `InspectorTimelineAgent` expected a corresponding `willDispatchEvent` before it was told
+        about the `didDispatchEvent`, which wasn't happening since only `willDispatchEvent` would
+        early-return if the `DOMWindow` didn't have any event listeners for the dispatched event. By
+        making the `DOMWindow::dispatchEvent` itself early-return in that case, it now handles both
+        `willDispatchEvent` and `didDispatchEvent`, ensuring that they are always called in pairs.
+
+        * page/DOMWindow.cpp:
+        (WebCore::DOMWindow::dispatchEvent):
+
+        * dom/EventTarget.cpp:
+        (WebCore::EventTarget::innerInvokeEventListeners):
+        * inspector/InspectorInstrumentation.h:
+        (WebCore::InspectorInstrumentation::willDispatchEvent):
+        (WebCore::InspectorInstrumentation::didDispatchEvent):
+        (WebCore::InspectorInstrumentation::willDispatchEventOnWindow):
+        (WebCore::InspectorInstrumentation::didDispatchEventOnWindow):
+        * inspector/InspectorInstrumentation.cpp:
+        (WebCore::InspectorInstrumentation::willDispatchEventImpl):
+        (WebCore::InspectorInstrumentation::didDispatchEventImpl):
+        (WebCore::InspectorInstrumentation::willDispatchEventOnWindowImpl):
+        (WebCore::InspectorInstrumentation::didDispatchEventOnWindowImpl):
+        `InspectorInstrumentation::willDispatchEventImpl` was always called with `hasEventListeners`
+        as `true`, so there's no reason to keep that parameter around. Similarly, the change inside
+        `DOMWindow::dispatchEvent` will make it so that the same is true for
+        `InspectorInstrumentation::willDispatchEventOnWindowImpl` as well.
+
 2019-10-10  Chris Dumez  <[email protected]>
 
         Flaky Test: media/media-source/media-source-page-cache.html

Modified: trunk/Source/WebCore/dom/EventTarget.cpp (250976 => 250977)


--- trunk/Source/WebCore/dom/EventTarget.cpp	2019-10-10 17:17:51 UTC (rev 250976)
+++ trunk/Source/WebCore/dom/EventTarget.cpp	2019-10-10 18:04:45 UTC (rev 250977)
@@ -283,7 +283,7 @@
     auto& context = *scriptExecutionContext();
     bool contextIsDocument = is<Document>(context);
     if (contextIsDocument)
-        InspectorInstrumentation::willDispatchEvent(downcast<Document>(context), event, true);
+        InspectorInstrumentation::willDispatchEvent(downcast<Document>(context), event);
 
     for (auto& registeredListener : listeners) {
         if (UNLIKELY(registeredListener->wasRemoved()))
@@ -322,7 +322,7 @@
     }
 
     if (contextIsDocument)
-        InspectorInstrumentation::didDispatchEvent(downcast<Document>(context), event.defaultPrevented());
+        InspectorInstrumentation::didDispatchEvent(downcast<Document>(context), event);
 }
 
 Vector<AtomString> EventTarget::eventTypes()

Modified: trunk/Source/WebCore/inspector/InspectorInstrumentation.cpp (250976 => 250977)


--- trunk/Source/WebCore/inspector/InspectorInstrumentation.cpp	2019-10-10 17:17:51 UTC (rev 250976)
+++ trunk/Source/WebCore/inspector/InspectorInstrumentation.cpp	2019-10-10 18:04:45 UTC (rev 250977)
@@ -395,11 +395,8 @@
         timelineAgent->didCallFunction(frameForScriptExecutionContext(context));
 }
 
-void InspectorInstrumentation::willDispatchEventImpl(InstrumentingAgents& instrumentingAgents, Document& document, const Event& event, bool hasEventListeners)
+void InspectorInstrumentation::willDispatchEventImpl(InstrumentingAgents& instrumentingAgents, Document& document, const Event& event)
 {
-    if (!hasEventListeners)
-        return;
-
     if (auto* timelineAgent = instrumentingAgents.trackingInspectorTimelineAgent())
         timelineAgent->willDispatchEvent(event, document.frame());
 }
@@ -422,25 +419,22 @@
         domDebuggerAgent->didHandleEvent();
 }
 
-void InspectorInstrumentation::didDispatchEventImpl(InstrumentingAgents& instrumentingAgents, bool defaultPrevented)
+void InspectorInstrumentation::didDispatchEventImpl(InstrumentingAgents& instrumentingAgents, const Event& event)
 {
     if (auto* timelineAgent = instrumentingAgents.trackingInspectorTimelineAgent())
-        timelineAgent->didDispatchEvent(defaultPrevented);
+        timelineAgent->didDispatchEvent(event.defaultPrevented());
 }
 
 void InspectorInstrumentation::willDispatchEventOnWindowImpl(InstrumentingAgents& instrumentingAgents, const Event& event, DOMWindow& window)
 {
-    if (!window.hasEventListeners(event.type()))
-        return;
-
     if (auto* timelineAgent = instrumentingAgents.trackingInspectorTimelineAgent())
         timelineAgent->willDispatchEvent(event, window.frame());
 }
 
-void InspectorInstrumentation::didDispatchEventOnWindowImpl(InstrumentingAgents& instrumentingAgents, bool defaultPrevented)
+void InspectorInstrumentation::didDispatchEventOnWindowImpl(InstrumentingAgents& instrumentingAgents, const Event& event)
 {
     if (auto* timelineAgent = instrumentingAgents.trackingInspectorTimelineAgent())
-        timelineAgent->didDispatchEvent(defaultPrevented);
+        timelineAgent->didDispatchEvent(event.defaultPrevented());
 }
 
 void InspectorInstrumentation::eventDidResetAfterDispatchImpl(InstrumentingAgents& instrumentingAgents, const Event& event)

Modified: trunk/Source/WebCore/inspector/InspectorInstrumentation.h (250976 => 250977)


--- trunk/Source/WebCore/inspector/InspectorInstrumentation.h	2019-10-10 17:17:51 UTC (rev 250976)
+++ trunk/Source/WebCore/inspector/InspectorInstrumentation.h	2019-10-10 18:04:45 UTC (rev 250977)
@@ -161,12 +161,12 @@
     static void didAddEventListener(EventTarget&, const AtomString& eventType, EventListener&, bool capture);
     static void willRemoveEventListener(EventTarget&, const AtomString& eventType, EventListener&, bool capture);
     static bool isEventListenerDisabled(EventTarget&, const AtomString& eventType, EventListener&, bool capture);
-    static void willDispatchEvent(Document&, const Event&, bool hasEventListeners);
-    static void didDispatchEvent(Document&, bool defaultPrevented);
+    static void willDispatchEvent(Document&, const Event&);
+    static void didDispatchEvent(Document&, const Event&);
     static void willHandleEvent(ScriptExecutionContext&, Event&, const RegisteredEventListener&);
     static void didHandleEvent(ScriptExecutionContext&);
     static void willDispatchEventOnWindow(Frame*, const Event&, DOMWindow&);
-    static void didDispatchEventOnWindow(Frame*, bool defaultPrevented);
+    static void didDispatchEventOnWindow(Frame*, const Event&);
     static void eventDidResetAfterDispatch(const Event&);
     static void willEvaluateScript(Frame&, const String& url, int lineNumber, int columnNumber);
     static void didEvaluateScript(Frame&);
@@ -371,12 +371,12 @@
     static void didAddEventListenerImpl(InstrumentingAgents&, EventTarget&, const AtomString& eventType, EventListener&, bool capture);
     static void willRemoveEventListenerImpl(InstrumentingAgents&, EventTarget&, const AtomString& eventType, EventListener&, bool capture);
     static bool isEventListenerDisabledImpl(InstrumentingAgents&, EventTarget&, const AtomString& eventType, EventListener&, bool capture);
-    static void willDispatchEventImpl(InstrumentingAgents&, Document&, const Event&, bool hasEventListeners);
+    static void willDispatchEventImpl(InstrumentingAgents&, Document&, const Event&);
     static void willHandleEventImpl(InstrumentingAgents&, Event&, const RegisteredEventListener&);
     static void didHandleEventImpl(InstrumentingAgents&);
-    static void didDispatchEventImpl(InstrumentingAgents&, bool defaultPrevented);
+    static void didDispatchEventImpl(InstrumentingAgents&, const Event&);
     static void willDispatchEventOnWindowImpl(InstrumentingAgents&, const Event&, DOMWindow&);
-    static void didDispatchEventOnWindowImpl(InstrumentingAgents&, bool defaultPrevented);
+    static void didDispatchEventOnWindowImpl(InstrumentingAgents&, const Event&);
     static void eventDidResetAfterDispatchImpl(InstrumentingAgents&, const Event&);
     static void willEvaluateScriptImpl(InstrumentingAgents&, Frame&, const String& url, int lineNumber, int columnNumber);
     static void didEvaluateScriptImpl(InstrumentingAgents&, Frame&);
@@ -829,18 +829,18 @@
         didCallFunctionImpl(*instrumentingAgents, context);
 }
 
-inline void InspectorInstrumentation::willDispatchEvent(Document& document, const Event& event, bool hasEventListeners)
+inline void InspectorInstrumentation::willDispatchEvent(Document& document, const Event& event)
 {
     FAST_RETURN_IF_NO_FRONTENDS(void());
     if (auto* instrumentingAgents = instrumentingAgentsForDocument(document))
-        willDispatchEventImpl(*instrumentingAgents, document, event, hasEventListeners);
+        willDispatchEventImpl(*instrumentingAgents, document, event);
 }
 
-inline void InspectorInstrumentation::didDispatchEvent(Document& document, bool defaultPrevented)
+inline void InspectorInstrumentation::didDispatchEvent(Document& document, const Event& event)
 {
     FAST_RETURN_IF_NO_FRONTENDS(void());
     if (auto* instrumentingAgents = instrumentingAgentsForDocument(document))
-        didDispatchEventImpl(*instrumentingAgents, defaultPrevented);
+        didDispatchEventImpl(*instrumentingAgents, event);
 }
 
 inline void InspectorInstrumentation::willHandleEvent(ScriptExecutionContext& context, Event& event, const RegisteredEventListener& listener)
@@ -864,11 +864,11 @@
         willDispatchEventOnWindowImpl(*instrumentingAgents, event, window);
 }
 
-inline void InspectorInstrumentation::didDispatchEventOnWindow(Frame* frame, bool defaultPrevented)
+inline void InspectorInstrumentation::didDispatchEventOnWindow(Frame* frame, const Event& event)
 {
     FAST_RETURN_IF_NO_FRONTENDS(void());
     if (auto* instrumentingAgents = instrumentingAgentsForFrame(frame))
-        didDispatchEventOnWindowImpl(*instrumentingAgents, defaultPrevented);
+        didDispatchEventOnWindowImpl(*instrumentingAgents, event);
 }
 
 inline void InspectorInstrumentation::eventDidResetAfterDispatch(const Event& event)

Modified: trunk/Source/WebCore/page/DOMWindow.cpp (250976 => 250977)


--- trunk/Source/WebCore/page/DOMWindow.cpp	2019-10-10 17:17:51 UTC (rev 250976)
+++ trunk/Source/WebCore/page/DOMWindow.cpp	2019-10-10 18:04:45 UTC (rev 250977)
@@ -2187,13 +2187,22 @@
     event.setEventPhase(Event::AT_TARGET);
     event.resetBeforeDispatch();
 
-    auto* protectedFrame = frame();
-    InspectorInstrumentation::willDispatchEventOnWindow(protectedFrame, event, *this);
+    Frame* protectedFrame = nullptr;
+    bool hasListenersForEvent = false;
+    if (UNLIKELY(InspectorInstrumentation::hasFrontends())) {
+        protectedFrame = frame();
+        hasListenersForEvent = hasEventListeners(event.type());
+        if (hasListenersForEvent)
+            InspectorInstrumentation::willDispatchEventOnWindow(protectedFrame, event, *this);
+    }
+
     // FIXME: We should use EventDispatcher everywhere.
     fireEventListeners(event, EventInvokePhase::Capturing);
     fireEventListeners(event, EventInvokePhase::Bubbling);
-    InspectorInstrumentation::didDispatchEventOnWindow(protectedFrame, event.defaultPrevented());
 
+    if (hasListenersForEvent)
+        InspectorInstrumentation::didDispatchEventOnWindow(protectedFrame, event);
+
     event.resetAfterDispatch();
 }
 

_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to