Attachment 339247 Details for Bug 178198 – Patch

[patch] Patch

bug-178198-20180501174617.patch (text/plain), 10.75 KB, created by Ross Kirsling on 2018-05-01 17:46:17 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Ross Kirsling

Created: 2018-05-01 17:46:17 PDT

Size: 10.75 KB

patch

obsolete

>Subversion Revision: 231111
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 4e696177b1691b146c1fef75dc69d23c9652d18d..d5e4b649490262b32bbf022a203ea115e382ea70 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,34 @@
>+2018-05-01  Ross Kirsling  <ross.kirsling@sony.com>
>+
>+        Web Inspector: ASSERT_NOT_REACHED hit in PageDebuggerAgent::didAddEventListener
>+        https://bugs.webkit.org/show_bug.cgi?id=178198
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        No new tests (OOPS!).
>+
>+        * dom/EventTarget.cpp:
>+        (WebCore::EventTarget::setAttributeEventListener):
>+        Pass isolatedWorld down to didAddEventListener in the "onevent" attribute case.
>+
>+        (WebCore::EventTarget::attributeRegisteredEventListener):
>+        (WebCore::EventTarget::attributeEventListener): Deleted.
>+        Don't just throw away the RegisteredEventListener when looking for an "onevent" callback...
>+
>+        * dom/EventTarget.h:
>+        (WebCore::EventTarget::attributeEventListener):
>+        ...instead, make attributeEventListener an inline wrapper for attributeRegisteredEventListener.
>+
>+        * inspector/InspectorInstrumentation.cpp:
>+        (WebCore::InspectorInstrumentation::didAddEventListenerImpl):
>+        * inspector/InspectorInstrumentation.h:
>+        (WebCore::InspectorInstrumentation::didAddEventListener):
>+        * inspector/agents/page/PageDebuggerAgent.cpp:
>+        (WebCore::PageDebuggerAgent::didAddEventListener):
>+        * inspector/agents/page/PageDebuggerAgent.h:
>+        When overwriting an "onevent" attribute, the new event listener replaces the old one,
>+        so we need to search through the list for it instead of assuming that it was appended.
>+
> 2018-04-27  Chris Dumez  <cdumez@apple.com>
> 
>         Regression(r222392?): Events can have a negative timestamp which causes app breakage
>diff --git a/Source/WebCore/dom/EventTarget.cpp b/Source/WebCore/dom/EventTarget.cpp
>index 2f60c319298ed25e2ce893e67670d97c5207e765..add59bd6bce4b6a98a0a40f790999eb20258b05f 100644
>--- a/Source/WebCore/dom/EventTarget.cpp
>+++ b/Source/WebCore/dom/EventTarget.cpp
>@@ -137,14 +137,14 @@ bool EventTarget::setAttributeEventListener(const AtomicString& eventType, RefPt
> 
>         eventTargetData()->eventListenerMap.replace(eventType, *existingListener, listener.releaseNonNull(), { });
> 
>-        InspectorInstrumentation::didAddEventListener(*this, eventType);
>+        InspectorInstrumentation::didAddEventListener(*this, eventType, &isolatedWorld);
> 
>         return true;
>     }
>     return addEventListener(eventType, listener.releaseNonNull());
> }
> 
>-EventListener* EventTarget::attributeEventListener(const AtomicString& eventType, DOMWrapperWorld& isolatedWorld)
>+RefPtr<RegisteredEventListener> EventTarget::attributeRegisteredEventListener(const AtomicString& eventType, DOMWrapperWorld& isolatedWorld)
> {
>     for (auto& eventListener : eventListeners(eventType)) {
>         auto& listener = eventListener->callback();
>@@ -153,7 +153,7 @@ EventListener* EventTarget::attributeEventListener(const AtomicString& eventType
> 
>         auto& listenerWorld = downcast<JSEventListener>(listener).isolatedWorld();
>         if (&listenerWorld == &isolatedWorld)
>-            return &listener;
>+            return eventListener;
>     }
> 
>     return nullptr;
>diff --git a/Source/WebCore/dom/EventTarget.h b/Source/WebCore/dom/EventTarget.h
>index 90b122ae74cafc5e22bcb66336d8d150f3ef589e..20e15e3ca3a92c98835b057d1e46b303876729a2 100644
>--- a/Source/WebCore/dom/EventTarget.h
>+++ b/Source/WebCore/dom/EventTarget.h
>@@ -102,6 +102,7 @@ public:
> 
>     // Used for legacy "onevent" attributes.
>     bool setAttributeEventListener(const AtomicString& eventType, RefPtr<EventListener>&&, DOMWrapperWorld&);
>+    RefPtr<RegisteredEventListener> attributeRegisteredEventListener(const AtomicString& eventType, DOMWrapperWorld&);
>     EventListener* attributeEventListener(const AtomicString& eventType, DOMWrapperWorld&);
> 
>     bool hasEventListeners() const;
>@@ -171,4 +172,10 @@ inline bool EventTarget::hasCapturingEventListeners(const AtomicString& eventTyp
>     return data && data->eventListenerMap.containsCapturing(eventType);
> }
> 
>+inline EventListener* EventTarget::attributeEventListener(const AtomicString& eventType, DOMWrapperWorld& isolatedWorld)
>+{
>+    const auto& registeredEventListener = attributeRegisteredEventListener(eventType, isolatedWorld);
>+    return registeredEventListener ? &registeredEventListener->callback() : nullptr;
>+}
>+
> } // namespace WebCore
>diff --git a/Source/WebCore/inspector/InspectorInstrumentation.cpp b/Source/WebCore/inspector/InspectorInstrumentation.cpp
>index 3bd4bc0a48fa8b6f75f29a10de457ed0e19689cc..2ff30f130498125987f86a7d38b1cd8901e478c1 100644
>--- a/Source/WebCore/inspector/InspectorInstrumentation.cpp
>+++ b/Source/WebCore/inspector/InspectorInstrumentation.cpp
>@@ -313,10 +313,10 @@ void InspectorInstrumentation::didRemoveTimerImpl(InstrumentingAgents& instrumen
>         timelineAgent->didRemoveTimer(timerId, frameForScriptExecutionContext(context));
> }
> 
>-void InspectorInstrumentation::didAddEventListenerImpl(InstrumentingAgents& instrumentingAgents, EventTarget& target, const AtomicString& eventType)
>+void InspectorInstrumentation::didAddEventListenerImpl(InstrumentingAgents& instrumentingAgents, EventTarget& target, const AtomicString& eventType, DOMWrapperWorld* isolatedWorld)
> {
>     if (PageDebuggerAgent* pageDebuggerAgent = instrumentingAgents.pageDebuggerAgent())
>-        pageDebuggerAgent->didAddEventListener(target, eventType);
>+        pageDebuggerAgent->didAddEventListener(target, eventType, isolatedWorld);
>     if (InspectorDOMAgent* domAgent = instrumentingAgents.inspectorDOMAgent())
>         domAgent->didAddEventListener(target);
> }
>diff --git a/Source/WebCore/inspector/InspectorInstrumentation.h b/Source/WebCore/inspector/InspectorInstrumentation.h
>index 8f93f4537fc2150ea7d80cf4df83a51ee3c186aa..4b03fb46033332665a548887970e5fa669f194ba 100644
>--- a/Source/WebCore/inspector/InspectorInstrumentation.h
>+++ b/Source/WebCore/inspector/InspectorInstrumentation.h
>@@ -146,7 +146,7 @@ public:
> 
>     static InspectorInstrumentationCookie willCallFunction(ScriptExecutionContext*, const String& scriptName, int scriptLine);
>     static void didCallFunction(const InspectorInstrumentationCookie&, ScriptExecutionContext*);
>-    static void didAddEventListener(EventTarget&, const AtomicString& eventType);
>+    static void didAddEventListener(EventTarget&, const AtomicString& eventType, DOMWrapperWorld* isolatedWorld = nullptr);
>     static void willRemoveEventListener(EventTarget&, const AtomicString& eventType, EventListener&, bool capture);
>     static bool isEventListenerDisabled(EventTarget&, const AtomicString& eventType, EventListener&, bool capture);
>     static InspectorInstrumentationCookie willDispatchEvent(Document&, const Event&, bool hasEventListeners);
>@@ -330,7 +330,7 @@ private:
> 
>     static InspectorInstrumentationCookie willCallFunctionImpl(InstrumentingAgents&, const String& scriptName, int scriptLine, ScriptExecutionContext*);
>     static void didCallFunctionImpl(const InspectorInstrumentationCookie&, ScriptExecutionContext*);
>-    static void didAddEventListenerImpl(InstrumentingAgents&, EventTarget&, const AtomicString& eventType);
>+    static void didAddEventListenerImpl(InstrumentingAgents&, EventTarget&, const AtomicString& eventType, DOMWrapperWorld* isolatedWorld);
>     static void willRemoveEventListenerImpl(InstrumentingAgents&, EventTarget&, const AtomicString& eventType, EventListener&, bool capture);
>     static bool isEventListenerDisabledImpl(InstrumentingAgents&, EventTarget&, const AtomicString& eventType, EventListener&, bool capture);
>     static InspectorInstrumentationCookie willDispatchEventImpl(InstrumentingAgents&, Document&, const Event&, bool hasEventListeners);
>@@ -683,11 +683,11 @@ inline void InspectorInstrumentation::didRemoveTimer(ScriptExecutionContext& con
>         didRemoveTimerImpl(*instrumentingAgents, timerId, context);
> }
> 
>-inline void InspectorInstrumentation::didAddEventListener(EventTarget& target, const AtomicString& eventType)
>+inline void InspectorInstrumentation::didAddEventListener(EventTarget& target, const AtomicString& eventType, DOMWrapperWorld* isolatedWorld)
> {
>     FAST_RETURN_IF_NO_FRONTENDS(void());
>     if (InstrumentingAgents* instrumentingAgents = instrumentingAgentsForContext(target.scriptExecutionContext()))
>-        didAddEventListenerImpl(*instrumentingAgents, target, eventType);
>+        didAddEventListenerImpl(*instrumentingAgents, target, eventType, isolatedWorld);
> }
> 
> inline void InspectorInstrumentation::willRemoveEventListener(EventTarget& target, const AtomicString& eventType, EventListener& listener, bool capture)
>diff --git a/Source/WebCore/inspector/agents/page/PageDebuggerAgent.cpp b/Source/WebCore/inspector/agents/page/PageDebuggerAgent.cpp
>index 29cdda29ee4c0d4f9cb07a4085d2b1add56b628e..c7f7661a2cc60d208806275061075c18bf3b494c 100644
>--- a/Source/WebCore/inspector/agents/page/PageDebuggerAgent.cpp
>+++ b/Source/WebCore/inspector/agents/page/PageDebuggerAgent.cpp
>@@ -33,6 +33,7 @@
> #include "PageDebuggerAgent.h"
> 
> #include "CachedResource.h"
>+#include "DOMWrapperWorld.h"
> #include "Document.h"
> #include "EventListener.h"
> #include "EventTarget.h"
>@@ -164,13 +165,12 @@ void PageDebuggerAgent::mainFrameNavigated()
>     setSuppressAllPauses(false);
> }
> 
>-void PageDebuggerAgent::didAddEventListener(EventTarget& target, const AtomicString& eventType)
>+void PageDebuggerAgent::didAddEventListener(EventTarget& target, const AtomicString& eventType, DOMWrapperWorld* isolatedWorld)
> {
>     if (!breakpointsActive())
>         return;
> 
>-    auto& eventListeners = target.eventListeners(eventType);
>-    const RefPtr<RegisteredEventListener>& listener = eventListeners.last();
>+    const auto& listener = isolatedWorld ? target.attributeRegisteredEventListener(eventType, *isolatedWorld) : target.eventListeners(eventType).last();
>     if (m_registeredEventListeners.contains(listener.get())) {
>         ASSERT_NOT_REACHED();
>         return;
>diff --git a/Source/WebCore/inspector/agents/page/PageDebuggerAgent.h b/Source/WebCore/inspector/agents/page/PageDebuggerAgent.h
>index d2b8b89a4afc239177f8a3af86a9d9bdc6a882f0..2dd17aa0e454d924efbbb1ef2760bbacc75b3066 100644
>--- a/Source/WebCore/inspector/agents/page/PageDebuggerAgent.h
>+++ b/Source/WebCore/inspector/agents/page/PageDebuggerAgent.h
>@@ -61,7 +61,7 @@ public:
>     void willFireAnimationFrame(int callbackId);
>     void didCancelAnimationFrame(int callbackId);
> 
>-    void didAddEventListener(EventTarget&, const AtomicString& eventType);
>+    void didAddEventListener(EventTarget&, const AtomicString& eventType, DOMWrapperWorld* isolatedWorld);
>     void willRemoveEventListener(EventTarget&, const AtomicString& eventType, EventListener&, bool capture);
>     void willHandleEvent(const RegisteredEventListener&);
>

Actions: View | Formatted Diff | Diff

Attachments on bug 178198: 339247