https://bugs.webkit.org/show_bug.cgi?id=181580
<rdar://problem/
36461309>
Reviewed by Brian Burg.
Source/WebCore:
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:
LayoutTests:
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:
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@231659
268f45cc-cd09-0410-ab3c-
d52691b4dbfc
+2018-05-10 Matt Baker <mattbaker@apple.com>
+
+ 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 <cdumez@apple.com>
'Cross-Origin-Options header implementation follow-up
-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.
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:
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);
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({
+2018-05-10 Matt Baker <mattbaker@apple.com>
+
+ 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 <cdumez@apple.com>
'Cross-Origin-Options header implementation follow-up
}
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;
}
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;
}
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);
}
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);
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);
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)
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;
}
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)
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&);