<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[185232] trunk/Source/WebCore</title>
</head>
<body>
<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; }
#msg dl a { font-weight: bold}
#msg dl a:link { color:#fc3; }
#msg dl a:active { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/185232">185232</a></dd>
<dt>Author</dt> <dd>simon.fraser@apple.com</dd>
<dt>Date</dt> <dd>2015-06-04 18:23:56 -0700 (Thu, 04 Jun 2015)</dd>
</dl>
<h3>Log Message</h3>
<pre>Crash in EventDispatcher::dispatchEvent entering a location on Google Maps
https://bugs.webkit.org/show_bug.cgi?id=145677
rdar://problem/20698280
Reviewed by Dean Jackson.
If a transition is running on a pseudo-element, and the host element is removed
from the DOM just as the transition ends, and there is a transition event listener,
then we'd crash with a null dereference in event dispatch code.
AnimationController tries to clean up running animations when renderers are destroyed,
but omitted to remove the element from two vectors that store element references.
Elements are only added to these vectors briefly on animation end, before firing
events, but failure to remove the vector entries could result in attempting
to fire an event on a pseudo-element with no host element.
Also convert EventDispatcher code to be more robust to potentially null event
targets, since it's not clear that eventTargetRespectingTargetRules() can always
manage to return a non-null node.
Hard to make a test because this is timing sensitive.
* dom/EventDispatcher.cpp:
(WebCore::eventTargetRespectingTargetRules):
(WebCore::EventDispatcher::dispatchScopedEvent):
(WebCore::EventDispatcher::dispatchEvent):
(WebCore::EventPath::EventPath):
* page/animation/AnimationController.cpp:
(WebCore::AnimationControllerPrivate::clear):</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceWebCoreChangeLog">trunk/Source/WebCore/ChangeLog</a></li>
<li><a href="#trunkSourceWebCoredomEventDispatchercpp">trunk/Source/WebCore/dom/EventDispatcher.cpp</a></li>
<li><a href="#trunkSourceWebCorepageanimationAnimationControllercpp">trunk/Source/WebCore/page/animation/AnimationController.cpp</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (185231 => 185232)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/ChangeLog 2015-06-05 01:07:51 UTC (rev 185231)
+++ trunk/Source/WebCore/ChangeLog 2015-06-05 01:23:56 UTC (rev 185232)
</span><span class="lines">@@ -1,3 +1,35 @@
</span><ins>+2015-06-04 Simon Fraser <simon.fraser@apple.com>
+
+ Crash in EventDispatcher::dispatchEvent entering a location on Google Maps
+ https://bugs.webkit.org/show_bug.cgi?id=145677
+ rdar://problem/20698280
+
+ Reviewed by Dean Jackson.
+
+ If a transition is running on a pseudo-element, and the host element is removed
+ from the DOM just as the transition ends, and there is a transition event listener,
+ then we'd crash with a null dereference in event dispatch code.
+
+ AnimationController tries to clean up running animations when renderers are destroyed,
+ but omitted to remove the element from two vectors that store element references.
+ Elements are only added to these vectors briefly on animation end, before firing
+ events, but failure to remove the vector entries could result in attempting
+ to fire an event on a pseudo-element with no host element.
+
+ Also convert EventDispatcher code to be more robust to potentially null event
+ targets, since it's not clear that eventTargetRespectingTargetRules() can always
+ manage to return a non-null node.
+
+ Hard to make a test because this is timing sensitive.
+
+ * dom/EventDispatcher.cpp:
+ (WebCore::eventTargetRespectingTargetRules):
+ (WebCore::EventDispatcher::dispatchScopedEvent):
+ (WebCore::EventDispatcher::dispatchEvent):
+ (WebCore::EventPath::EventPath):
+ * page/animation/AnimationController.cpp:
+ (WebCore::AnimationControllerPrivate::clear):
+
</ins><span class="cx"> 2015-06-04 Hunseop Jeong <hs85.jeong@samsung.com>
</span><span class="cx">
</span><span class="cx"> Replace 0 with nullptr in WebCore/Page.
</span></span></pre></div>
<a id="trunkSourceWebCoredomEventDispatchercpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/dom/EventDispatcher.cpp (185231 => 185232)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/dom/EventDispatcher.cpp 2015-06-05 01:07:51 UTC (rev 185231)
+++ trunk/Source/WebCore/dom/EventDispatcher.cpp 2015-06-05 01:23:56 UTC (rev 185232)
</span><span class="lines">@@ -202,26 +202,24 @@
</span><span class="cx"> #endif
</span><span class="cx"> };
</span><span class="cx">
</span><del>-inline EventTarget& eventTargetRespectingTargetRules(Node& referenceNode)
</del><ins>+inline EventTarget* eventTargetRespectingTargetRules(Node& referenceNode)
</ins><span class="cx"> {
</span><del>- if (is<PseudoElement>(referenceNode)) {
- ASSERT(downcast<PseudoElement>(referenceNode).hostElement());
- return *downcast<PseudoElement>(referenceNode).hostElement();
- }
</del><ins>+ if (is<PseudoElement>(referenceNode))
+ return downcast<PseudoElement>(referenceNode).hostElement();
</ins><span class="cx">
</span><span class="cx"> // Events sent to elements inside an SVG use element's shadow tree go to the use element.
</span><span class="cx"> if (is<SVGElement>(referenceNode)) {
</span><span class="cx"> if (auto* useElement = downcast<SVGElement>(referenceNode).correspondingUseElement())
</span><del>- return *useElement;
</del><ins>+ return useElement;
</ins><span class="cx"> }
</span><span class="cx">
</span><del>- return referenceNode;
</del><ins>+ return &referenceNode;
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> void EventDispatcher::dispatchScopedEvent(Node& node, PassRefPtr<Event> event)
</span><span class="cx"> {
</span><span class="cx"> // We need to set the target here because it can go away by the time we actually fire the event.
</span><del>- event->setTarget(&eventTargetRespectingTargetRules(node));
</del><ins>+ event->setTarget(eventTargetRespectingTargetRules(node));
</ins><span class="cx"> ScopedEventQueue::singleton().enqueueEvent(event);
</span><span class="cx"> }
</span><span class="cx">
</span><span class="lines">@@ -340,9 +338,13 @@
</span><span class="cx">
</span><span class="cx"> ChildNodesLazySnapshot::takeChildNodesLazySnapshot();
</span><span class="cx">
</span><del>- event->setTarget(&eventTargetRespectingTargetRules(*node));
</del><ins>+ EventTarget* target = eventTargetRespectingTargetRules(*node);
+ event->setTarget(target);
+ if (!event->target())
+ return true;
+
</ins><span class="cx"> ASSERT_WITH_SECURITY_IMPLICATION(!NoEventDispatchAssertion::isEventDispatchForbidden());
</span><del>- ASSERT(event->target());
</del><ins>+
</ins><span class="cx"> WindowEventContext windowEventContext(node.get(), eventPath.lastContextIfExists());
</span><span class="cx">
</span><span class="cx"> InputElementClickState clickHandlingState;
</span><span class="lines">@@ -352,7 +354,7 @@
</span><span class="cx"> if (!event->propagationStopped() && !eventPath.isEmpty())
</span><span class="cx"> dispatchEventInDOM(*event, eventPath, windowEventContext);
</span><span class="cx">
</span><del>- event->setTarget(&eventTargetRespectingTargetRules(*node));
</del><ins>+ event->setTarget(eventTargetRespectingTargetRules(*node));
</ins><span class="cx"> event->setCurrentTarget(nullptr);
</span><span class="cx"> event->setEventPhase(0);
</span><span class="cx">
</span><span class="lines">@@ -419,22 +421,22 @@
</span><span class="cx"> #if ENABLE(TOUCH_EVENTS) && !PLATFORM(IOS)
</span><span class="cx"> bool isTouchEvent = event.isTouchEvent();
</span><span class="cx"> #endif
</span><del>- EventTarget* target = 0;
</del><ins>+ EventTarget* target = nullptr;
</ins><span class="cx">
</span><span class="cx"> Node* node = nodeOrHostIfPseudoElement(&targetNode);
</span><span class="cx"> while (node) {
</span><span class="cx"> if (!target || !isSVGElement) // FIXME: This code doesn't make sense once we've climbed out of the SVG subtree in a HTML document.
</span><del>- target = &eventTargetRespectingTargetRules(*node);
</del><ins>+ target = eventTargetRespectingTargetRules(*node);
</ins><span class="cx"> for (; node; node = node->parentNode()) {
</span><del>- EventTarget& currentTarget = eventTargetRespectingTargetRules(*node);
</del><ins>+ EventTarget* currentTarget = eventTargetRespectingTargetRules(*node);
</ins><span class="cx"> if (isMouseOrFocusEvent)
</span><del>- m_path.append(std::make_unique<MouseOrFocusEventContext>(node, &currentTarget, target));
</del><ins>+ m_path.append(std::make_unique<MouseOrFocusEventContext>(node, currentTarget, target));
</ins><span class="cx"> #if ENABLE(TOUCH_EVENTS) && !PLATFORM(IOS)
</span><span class="cx"> else if (isTouchEvent)
</span><del>- m_path.append(std::make_unique<TouchEventContext>(node, &currentTarget, target));
</del><ins>+ m_path.append(std::make_unique<TouchEventContext>(node, currentTarget, target));
</ins><span class="cx"> #endif
</span><span class="cx"> else
</span><del>- m_path.append(std::make_unique<EventContext>(node, &currentTarget, target));
</del><ins>+ m_path.append(std::make_unique<EventContext>(node, currentTarget, target));
</ins><span class="cx"> if (!inDocument)
</span><span class="cx"> return;
</span><span class="cx"> if (is<ShadowRoot>(*node))
</span></span></pre></div>
<a id="trunkSourceWebCorepageanimationAnimationControllercpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/page/animation/AnimationController.cpp (185231 => 185232)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/page/animation/AnimationController.cpp 2015-06-05 01:07:51 UTC (rev 185231)
+++ trunk/Source/WebCore/page/animation/AnimationController.cpp 2015-06-05 01:23:56 UTC (rev 185232)
</span><span class="lines">@@ -100,6 +100,17 @@
</span><span class="cx"> {
</span><span class="cx"> ASSERT(renderer.isCSSAnimating());
</span><span class="cx"> ASSERT(m_compositeAnimations.contains(&renderer));
</span><ins>+
+ Element* element = renderer.element();
+
+ m_eventsToDispatch.removeAllMatching([element] (const EventToDispatch& info) {
+ return info.element == element;
+ });
+
+ m_elementChangesToDispatch.removeAllMatching([element] (const Ref<Element>& currElement) {
+ return &currElement.get() == element;
+ });
+
</ins><span class="cx"> // Return false if we didn't do anything OR we are suspended (so we don't try to
</span><span class="cx"> // do a setNeedsStyleRecalc() when suspended).
</span><span class="cx"> RefPtr<CompositeAnimation> animation = m_compositeAnimations.take(&renderer);
</span></span></pre>
</div>
</div>
</body>
</html>