Diff
Modified: trunk/LayoutTests/ChangeLog (231658 => 231659)
--- trunk/LayoutTests/ChangeLog 2018-05-10 20:54:12 UTC (rev 231658)
+++ trunk/LayoutTests/ChangeLog 2018-05-10 20:59:07 UTC (rev 231659)
@@ -1,3 +1,17 @@
+2018-05-10 Matt Baker <[email protected]>
+
+ Web Inspector: ASSERT_NOT_REACHED in PageDebuggerAgent::didAddEventListener when page adds attribute event listener
+ https://bugs.webkit.org/show_bug.cgi?id=181580
+ <rdar://problem/36461309>
+
+ Reviewed by Brian Burg.
+
+ Add new test covering the case where adding an attribute event listener
+ causes an existing attribute event listener to be replaced.
+
+ * inspector/debugger/async-stack-trace-expected.txt:
+ * inspector/debugger/async-stack-trace.html:
+
2018-05-10 Chris Dumez <[email protected]>
'Cross-Origin-Options header implementation follow-up
Modified: trunk/LayoutTests/inspector/debugger/async-stack-trace-expected.txt (231658 => 231659)
--- trunk/LayoutTests/inspector/debugger/async-stack-trace-expected.txt 2018-05-10 20:54:12 UTC (rev 231658)
+++ trunk/LayoutTests/inspector/debugger/async-stack-trace-expected.txt 2018-05-10 20:59:07 UTC (rev 231659)
@@ -1,4 +1,4 @@
-CONSOLE MESSAGE: line 55: Unable to post message to http://example.com. Recipient has origin .
+CONSOLE MESSAGE: line 83: Unable to post message to http://example.com. Recipient has origin .
Tests for async stack traces.
@@ -80,6 +80,26 @@
5: [F] testAddEventListener
6: [P] Global Code
+-- Running test case: CheckAsyncStackTrace.AddAttributeEventListener
+PAUSE #1
+CALL STACK:
+0: [F] pauseThenFinishTest
+1: [F] handleClick
+2: [F] testAttributeEventListener
+3: [P] Global Code
+4: [F] testAttributeEventListener
+5: [P] Global Code
+
+-- Running test case: CheckAsyncStackTrace.ReplaceAttributeEventListener
+PAUSE #1
+CALL STACK:
+0: [F] pauseThenFinishTest
+1: [F] handleClick2
+2: [F] testReplaceAttributeEventListener
+3: [P] Global Code
+4: [F] testReplaceAttributeEventListener
+5: [P] Global Code
+
-- Running test case: CheckAsyncStackTrace.PostMessage
PAUSE #1
CALL STACK:
Modified: trunk/LayoutTests/inspector/debugger/async-stack-trace.html (231658 => 231659)
--- trunk/LayoutTests/inspector/debugger/async-stack-trace.html 2018-05-10 20:54:12 UTC (rev 231658)
+++ trunk/LayoutTests/inspector/debugger/async-stack-trace.html 2018-05-10 20:59:07 UTC (rev 231659)
@@ -50,6 +50,34 @@
document.body.click();
}
+function testAttributeEventListener() {
+ let previousListener = document.body.onclick;
+
+ function handleClick() {
+ document.body._onclick_ = previousListener;
+ pauseThenFinishTest();
+ }
+
+ document.body._onclick_ = handleClick;
+ document.body.click();
+}
+
+function testReplaceAttributeEventListener() {
+ let previousListener = document.body.onclick;
+
+ function handleClick1() {}
+
+ function handleClick2() {
+ document.body._onclick_ = previousListener;
+ pauseThenFinishTest();
+ }
+
+ document.body._onclick_ = handleClick1;
+ document.body.addEventListener("click", handleClick1, {once: true});
+ document.body._onclick_ = handleClick2;
+ document.body.click();
+}
+
function testPostMessage(targetOrigin = "*") {
let childFrame = document.getElementById("postMessageTestFrame");
childFrame.contentWindow.postMessage("<message>", targetOrigin);
@@ -121,6 +149,11 @@
addSimpleTestCase("ChainedRequestAnimationFrame", "testChainedRequestAnimationFrame()");
addSimpleTestCase("ReferenceCounting", "testReferenceCounting()");
addSimpleTestCase("AddEventListener", "testAddEventListener()");
+ // FIXME: <https://webkit.org/b/183236> Web Inspector: Async stack trace for an on-event handler doesn't include a boundary frame.
+ // Update test results once this has been addressed.
+ addSimpleTestCase("AddAttributeEventListener", "testAttributeEventListener()");
+ // Test for <https://webkit.org/b/181580> Web Inspector: ASSERT_NOT_REACHED in PageDebuggerAgent::didAddEventListener when page adds attribute event listener.
+ addSimpleTestCase("ReplaceAttributeEventListener", "testReplaceAttributeEventListener()");
addSimpleTestCase("PostMessage", "testPostMessage()");
suite.addTestCase({
Modified: trunk/Source/WebCore/ChangeLog (231658 => 231659)
--- trunk/Source/WebCore/ChangeLog 2018-05-10 20:54:12 UTC (rev 231658)
+++ trunk/Source/WebCore/ChangeLog 2018-05-10 20:59:07 UTC (rev 231659)
@@ -1,3 +1,30 @@
+2018-05-10 Matt Baker <[email protected]>
+
+ Web Inspector: ASSERT_NOT_REACHED in PageDebuggerAgent::didAddEventListener when page adds attribute event listener
+ https://bugs.webkit.org/show_bug.cgi?id=181580
+ <rdar://problem/36461309>
+
+ Reviewed by Brian Burg.
+
+ EventTarget should pass newly added EventListeners to InspectorInstrumentation,
+ instead of PageDebuggerAgent assuming the last item in the EventListenerVector
+ is the most recently added listener. This assumption does not hold when
+ the new listener replaces an existing listener.
+
+ * dom/EventTarget.cpp:
+ (WebCore::EventTarget::addEventListener):
+ (WebCore::EventTarget::setAttributeEventListener):
+
+ * 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:
+
2018-05-10 Chris Dumez <[email protected]>
'Cross-Origin-Options header implementation follow-up
Modified: trunk/Source/WebCore/dom/EventTarget.cpp (231658 => 231659)
--- trunk/Source/WebCore/dom/EventTarget.cpp 2018-05-10 20:54:12 UTC (rev 231658)
+++ trunk/Source/WebCore/dom/EventTarget.cpp 2018-05-10 20:59:07 UTC (rev 231659)
@@ -75,12 +75,13 @@
}
bool listenerCreatedFromScript = listener->type() == EventListener::JSEventListenerType && !listener->wasCreatedFromMarkup();
+ auto listenerRef = listener.copyRef();
if (!ensureEventTargetData().eventListenerMap.add(eventType, WTFMove(listener), { options.capture, passive.value_or(false), options.once }))
return false;
if (listenerCreatedFromScript)
- InspectorInstrumentation::didAddEventListener(*this, eventType);
+ InspectorInstrumentation::didAddEventListener(*this, eventType, listenerRef.get(), options.capture);
return true;
}
@@ -135,9 +136,10 @@
if (existingListener) {
InspectorInstrumentation::willRemoveEventListener(*this, eventType, *existingListener, false);
+ auto listenerPointer = listener.copyRef();
eventTargetData()->eventListenerMap.replace(eventType, *existingListener, listener.releaseNonNull(), { });
- InspectorInstrumentation::didAddEventListener(*this, eventType);
+ InspectorInstrumentation::didAddEventListener(*this, eventType, *listenerPointer, false);
return true;
}
Modified: trunk/Source/WebCore/inspector/InspectorInstrumentation.cpp (231658 => 231659)
--- trunk/Source/WebCore/inspector/InspectorInstrumentation.cpp 2018-05-10 20:54:12 UTC (rev 231658)
+++ trunk/Source/WebCore/inspector/InspectorInstrumentation.cpp 2018-05-10 20:59:07 UTC (rev 231659)
@@ -313,10 +313,10 @@
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, EventListener& listener, bool capture)
{
if (PageDebuggerAgent* pageDebuggerAgent = instrumentingAgents.pageDebuggerAgent())
- pageDebuggerAgent->didAddEventListener(target, eventType);
+ pageDebuggerAgent->didAddEventListener(target, eventType, listener, capture);
if (InspectorDOMAgent* domAgent = instrumentingAgents.inspectorDOMAgent())
domAgent->didAddEventListener(target);
}
Modified: trunk/Source/WebCore/inspector/InspectorInstrumentation.h (231658 => 231659)
--- trunk/Source/WebCore/inspector/InspectorInstrumentation.h 2018-05-10 20:54:12 UTC (rev 231658)
+++ trunk/Source/WebCore/inspector/InspectorInstrumentation.h 2018-05-10 20:59:07 UTC (rev 231659)
@@ -146,7 +146,7 @@
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, EventListener&, bool capture);
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 @@
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, EventListener&, bool capture);
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 @@
didRemoveTimerImpl(*instrumentingAgents, timerId, context);
}
-inline void InspectorInstrumentation::didAddEventListener(EventTarget& target, const AtomicString& eventType)
+inline void InspectorInstrumentation::didAddEventListener(EventTarget& target, const AtomicString& eventType, EventListener& listener, bool capture)
{
FAST_RETURN_IF_NO_FRONTENDS(void());
if (InstrumentingAgents* instrumentingAgents = instrumentingAgentsForContext(target.scriptExecutionContext()))
- didAddEventListenerImpl(*instrumentingAgents, target, eventType);
+ didAddEventListenerImpl(*instrumentingAgents, target, eventType, listener, capture);
}
inline void InspectorInstrumentation::willRemoveEventListener(EventTarget& target, const AtomicString& eventType, EventListener& listener, bool capture)
Modified: trunk/Source/WebCore/inspector/agents/page/PageDebuggerAgent.cpp (231658 => 231659)
--- trunk/Source/WebCore/inspector/agents/page/PageDebuggerAgent.cpp 2018-05-10 20:54:12 UTC (rev 231658)
+++ trunk/Source/WebCore/inspector/agents/page/PageDebuggerAgent.cpp 2018-05-10 20:59:07 UTC (rev 231659)
@@ -164,14 +164,20 @@
setSuppressAllPauses(false);
}
-void PageDebuggerAgent::didAddEventListener(EventTarget& target, const AtomicString& eventType)
+void PageDebuggerAgent::didAddEventListener(EventTarget& target, const AtomicString& eventType, EventListener& listener, bool capture)
{
if (!breakpointsActive())
return;
auto& eventListeners = target.eventListeners(eventType);
- const RefPtr<RegisteredEventListener>& listener = eventListeners.last();
- if (m_registeredEventListeners.contains(listener.get())) {
+ auto position = eventListeners.findMatching([&](auto& registeredListener) {
+ return ®isteredListener->callback() == &listener && registeredListener->useCapture() == capture;
+ });
+ if (position == notFound)
+ return;
+
+ auto& registeredListener = eventListeners.at(position);
+ if (m_registeredEventListeners.contains(registeredListener.get())) {
ASSERT_NOT_REACHED();
return;
}
@@ -181,9 +187,9 @@
return;
int identifier = m_nextEventListenerIdentifier++;
- m_registeredEventListeners.set(listener.get(), identifier);
+ m_registeredEventListeners.set(registeredListener.get(), identifier);
- didScheduleAsyncCall(scriptState, InspectorDebuggerAgent::AsyncCallType::EventListener, identifier, listener->isOnce());
+ didScheduleAsyncCall(scriptState, InspectorDebuggerAgent::AsyncCallType::EventListener, identifier, registeredListener->isOnce());
}
void PageDebuggerAgent::willRemoveEventListener(EventTarget& target, const AtomicString& eventType, EventListener& listener, bool capture)
Modified: trunk/Source/WebCore/inspector/agents/page/PageDebuggerAgent.h (231658 => 231659)
--- trunk/Source/WebCore/inspector/agents/page/PageDebuggerAgent.h 2018-05-10 20:54:12 UTC (rev 231658)
+++ trunk/Source/WebCore/inspector/agents/page/PageDebuggerAgent.h 2018-05-10 20:59:07 UTC (rev 231659)
@@ -61,7 +61,7 @@
void willFireAnimationFrame(int callbackId);
void didCancelAnimationFrame(int callbackId);
- void didAddEventListener(EventTarget&, const AtomicString& eventType);
+ void didAddEventListener(EventTarget&, const AtomicString& eventType, EventListener&, bool capture);
void willRemoveEventListener(EventTarget&, const AtomicString& eventType, EventListener&, bool capture);
void willHandleEvent(const RegisteredEventListener&);
