The following assertion failure is seen on iOS Simulator debug bots with layout tests fast/dom/event-handler-attributes.html and legacy-animation-engine/fast/dom/event-handler-attributes.html: ASSERTION FAILED: !m_originalNode ./bindings/js/JSLazyEventListener.cpp(90) : virtual void WebCore::JSLazyEventListener::checkValidityForEventTarget(WebCore::EventTarget &) 1 0x46b3bd3b9 WTFCrash 2 0x46fe24cdb WTFCrashWithInfo(int, char const*, char const*, int) 3 0x471caa79e WebCore::JSLazyEventListener::checkValidityForEventTarget(WebCore::EventTarget&) 4 0x4722c0499 WebCore::EventTarget::addEventListener(WTF::AtomicString const&, WTF::Ref<WebCore::EventListener, WTF::DumbPtrTraits<WebCore::EventListener> >&&, WebCore::EventTarget::AddEventListenerOptions const&) 5 0x472ce04df WebCore::DOMWindow::addEventListener(WTF::AtomicString const&, WTF::Ref<WebCore::EventListener, WTF::DumbPtrTraits<WebCore::EventListener> >&&, WebCore::EventTarget::AddEventListenerOptions const&) 6 0x47233b0ad WebCore::tryAddEventListener(WebCore::Node*, WTF::AtomicString const&, WTF::Ref<WebCore::EventListener, WTF::DumbPtrTraits<WebCore::EventListener> >&&, WebCore::EventTarget::AddEventListenerOptions const&) 7 0x47233ae98 WebCore::Node::addEventListener(WTF::AtomicString const&, WTF::Ref<WebCore::EventListener, WTF::DumbPtrTraits<WebCore::EventListener> >&&, WebCore::EventTarget::AddEventListenerOptions const&) 8 0x4722c1137 WebCore::EventTarget::setAttributeEventListener(WTF::AtomicString const&, WTF::RefPtr<WebCore::EventListener, WTF::DumbPtrTraits<WebCore::EventListener> >&&, WebCore::DOMWrapperWorld&) 9 0x47226de46 WebCore::Element::setAttributeEventListener(WTF::AtomicString const&, WebCore::QualifiedName const&, WTF::AtomicString const&) 10 0x4725eb7f2 WebCore::HTMLElement::parseAttribute(WebCore::QualifiedName const&, WTF::AtomicString const&) 11 0x4722693dd WebCore::Element::attributeChanged(WebCore::QualifiedName const&, WTF::AtomicString const&, WTF::AtomicString const&, WebCore::Element::AttributeModificationReason) 12 0x4723e0eac WebCore::StyledElement::attributeChanged(WebCore::QualifiedName const&, WTF::AtomicString const&, WTF::AtomicString const&, WebCore::Element::AttributeModificationReason) 13 0x47226f812 WebCore::Element::didAddAttribute(WebCore::QualifiedName const&, WTF::AtomicString const&) 14 0x47226f753 WebCore::Element::addAttributeInternal(WebCore::QualifiedName const&, WTF::AtomicString const&, WebCore::Element::SynchronizationOfLazyAttribute) 15 0x472268b81 WebCore::Element::setAttributeInternal(unsigned int, WebCore::QualifiedName const&, WTF::AtomicString const&, WebCore::Element::SynchronizationOfLazyAttribute) 16 0x472268e3d WebCore::Element::setAttribute(WTF::AtomicString const&, WTF::AtomicString const&) 17 0x4708107ce WebCore::jsElementPrototypeFunctionSetAttributeBody(JSC::ExecState*, WebCore::JSElement*, JSC::ThrowScope&) 18 0x4707f79d0 long long WebCore::IDLOperation<WebCore::JSElement>::call<&(WebCore::jsElementPrototypeFunctionSetAttributeBody(JSC::ExecState*, WebCore::JSElement*, JSC::ThrowScope&)), (WebCore::CastedThisErrorBehavior)0>(JSC::ExecState&, char const*) 19 0x4707f76bc WebCore::jsElementPrototypeFunctionSetAttribute(JSC::ExecState*) 20 0x424bcf00116b 21 0x424bcf02fe01 22 0x424bcf0300fc 23 0x424bcf03a921 24 0x424bcf039f65 25 0x46b785d94 llint_entry 26 0x46b7729e3 vmEntryToJavaScript 27 0x46c0fa0b7 JSC::JITCode::execute(JSC::VM*, JSC::ProtoCallFrame*) 28 0x46c0f5870 JSC::Interpreter::execute(JSC::EvalExecutable*, JSC::ExecState*, JSC::JSValue, JSC::JSScope*) 29 0x46c0f404b JSC::eval(JSC::ExecState*) 30 0x46c19337d operationCallEval 31 0x424bcf028ac5 LEAK: 1 WebPageProxy https://build.webkit.org/results/Apple%20iOS%2012%20Simulator%20Debug%20WK2%20(Tests)/r245052%20(3616)/results.html
<rdar://problem/50586956>
Oh great... #if PLATFORM(IOS_FAMILY) if (targetNode == &targetNode->document() && eventType == eventNames().scrollEvent) targetNode->document().domWindow()->incrementScrollEventListenersCount(); // FIXME: Would it be sufficient to special-case this code for <body> and <frameset>? // // This code was added to address <rdar://problem/5846492> Onorientationchange event not working for document.body. // Forward this call to addEventListener() to the window since these are window-only events. if (eventType == eventNames().orientationchangeEvent || eventType == eventNames().resizeEvent) targetNode->document().domWindow()->addEventListener(eventType, WTFMove(listener), options); #if ENABLE(TOUCH_EVENTS) if (eventNames().isTouchRelatedEventType(targetNode->document(), eventType)) targetNode->document().addTouchEventListener(*targetNode); #endif #endif // PLATFORM(IOS_FAMILY) So we create a LazyEventListener for an Element (e.g. body) and then add it to the window. This explains the crashes we've seen on iOS with the reziseEvent lazy listeners.
Per spec, if you set the onresize or onorientationchange attribute on the body, it will also set the event listener on the window. This is implemented in HTMLBodyElement::parseAttribute(). The guilty code, tryAddEventListener() in Node.cpp was meant to make it so that setting an event listener via body.addEventListener('resize') or body.addEventListener('orientationchange') would also set the event listener on the Window. As far as I can tell, this is not standard behavior and is a weird WebKit quirk. The implementation is also wrong and is the source of crashes like <rdar://problem/24314027>. You cannot take a JSLazyEventListener from the body and then simply set it on the Window without consequences since the JSLazyEventListener has a raw pointer to its element. Personally, I'd be tempted to drop this quirk altogether.
Blink does not have this non-standard quirk so I will simply drop it from WebKit.
Created attachment 369429 [details] Patch
Created attachment 369448 [details] Patch
Comment on attachment 369448 [details] Patch Clearing flags on attachment: 369448 Committed r245086: <https://trac.webkit.org/changeset/245086>
All reviewed patches have been landed. Closing bug.
Created attachment 369993 [details] Test that is a millions times better or more fun IMO Here's a way more 🎉 test case that I wrote. In my opinion it is a million times better than the test that landed with this patch, but I could be biased haha 😀
(In reply to Daniel Bates from comment #9) > Created attachment 369993 [details] > Test that is a millions times better or more fun IMO > > Here's a way more 🎉 test case that I wrote. In my opinion it is a million > times better than the test that landed with this patch, but I could be > biased haha 😀 Agree, huge fan of blue.. a lots more better...