<!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>[171283] 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/171283">171283</a></dd>
<dt>Author</dt> <dd>darin@apple.com</dd>
<dt>Date</dt> <dd>2014-07-20 13:11:57 -0700 (Sun, 20 Jul 2014)</dd>
</dl>
<h3>Log Message</h3>
<pre>Crashes seen in wheel event handling
https://bugs.webkit.org/show_bug.cgi?id=135102
Reviewed by Beth Dakin.
Speculative fix based on guesses about what could be crashing.
The crash seems to be calling ref on an event target, and my guess is that this
has something to do with latching.
* page/EventHandler.cpp:
(WebCore::EventHandler::platformPrepareForWheelEvents): Updated argument types.
(WebCore::EventHandler::handleWheelEvent): Refactored a little and made some local
variables use RefPtr instead of raw pointers. Also added some comments.
* page/EventHandler.h: Changed argument types to RefPtr.
* page/mac/EventHandlerMac.mm:
(WebCore::EventHandler::platformPrepareForWheelEvents): Updated argument types.
Also added a FIXME.</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceWebCoreChangeLog">trunk/Source/WebCore/ChangeLog</a></li>
<li><a href="#trunkSourceWebCorepageEventHandlercpp">trunk/Source/WebCore/page/EventHandler.cpp</a></li>
<li><a href="#trunkSourceWebCorepageEventHandlerh">trunk/Source/WebCore/page/EventHandler.h</a></li>
<li><a href="#trunkSourceWebCorepagemacEventHandlerMacmm">trunk/Source/WebCore/page/mac/EventHandlerMac.mm</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (171282 => 171283)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/ChangeLog 2014-07-20 18:00:52 UTC (rev 171282)
+++ trunk/Source/WebCore/ChangeLog 2014-07-20 20:11:57 UTC (rev 171283)
</span><span class="lines">@@ -1,3 +1,25 @@
</span><ins>+2014-07-20 Darin Adler <darin@apple.com>
+
+ Crashes seen in wheel event handling
+ https://bugs.webkit.org/show_bug.cgi?id=135102
+
+ Reviewed by Beth Dakin.
+
+ Speculative fix based on guesses about what could be crashing.
+ The crash seems to be calling ref on an event target, and my guess is that this
+ has something to do with latching.
+
+ * page/EventHandler.cpp:
+ (WebCore::EventHandler::platformPrepareForWheelEvents): Updated argument types.
+ (WebCore::EventHandler::handleWheelEvent): Refactored a little and made some local
+ variables use RefPtr instead of raw pointers. Also added some comments.
+
+ * page/EventHandler.h: Changed argument types to RefPtr.
+
+ * page/mac/EventHandlerMac.mm:
+ (WebCore::EventHandler::platformPrepareForWheelEvents): Updated argument types.
+ Also added a FIXME.
+
</ins><span class="cx"> 2014-07-20 Simon Fraser <simon.fraser@apple.com>
</span><span class="cx">
</span><span class="cx"> Print layerIDs in GraphicsLayer dumps
</span></span></pre></div>
<a id="trunkSourceWebCorepageEventHandlercpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/page/EventHandler.cpp (171282 => 171283)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/page/EventHandler.cpp 2014-07-20 18:00:52 UTC (rev 171282)
+++ trunk/Source/WebCore/page/EventHandler.cpp 2014-07-20 20:11:57 UTC (rev 171283)
</span><span class="lines">@@ -2505,14 +2505,17 @@
</span><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> #if !PLATFORM(GTK)
</span><ins>+
</ins><span class="cx"> bool EventHandler::shouldTurnVerticalTicksIntoHorizontal(const HitTestResult&, const PlatformWheelEvent&) const
</span><span class="cx"> {
</span><span class="cx"> return false;
</span><span class="cx"> }
</span><ins>+
</ins><span class="cx"> #endif
</span><span class="cx">
</span><span class="cx"> #if !PLATFORM(MAC)
</span><del>-void EventHandler::platformPrepareForWheelEvents(const PlatformWheelEvent&, const HitTestResult&, Element*&, ContainerNode*&, ScrollableArea*&, bool&)
</del><ins>+
+void EventHandler::platformPrepareForWheelEvents(const PlatformWheelEvent&, const HitTestResult&, RefPtr<Element>&, RefPtr<ContainerNode>&, ScrollableArea*&, bool&)
</ins><span class="cx"> {
</span><span class="cx"> }
</span><span class="cx">
</span><span class="lines">@@ -2535,15 +2538,15 @@
</span><span class="cx"> {
</span><span class="cx"> return true;
</span><span class="cx"> }
</span><ins>+
</ins><span class="cx"> #endif
</span><span class="cx">
</span><del>-bool EventHandler::handleWheelEvent(const PlatformWheelEvent& e)
</del><ins>+bool EventHandler::handleWheelEvent(const PlatformWheelEvent& event)
</ins><span class="cx"> {
</span><span class="cx"> Document* document = m_frame.document();
</span><del>-
</del><span class="cx"> if (!document->renderView())
</span><span class="cx"> return false;
</span><del>-
</del><ins>+
</ins><span class="cx"> RefPtr<FrameView> protector(m_frame.view());
</span><span class="cx">
</span><span class="cx"> FrameView* view = m_frame.view();
</span><span class="lines">@@ -2552,63 +2555,56 @@
</span><span class="cx">
</span><span class="cx"> m_isHandlingWheelEvent = true;
</span><span class="cx"> setFrameWasScrolledByUser();
</span><del>- LayoutPoint vPoint = view->windowToContents(e.position());
</del><span class="cx">
</span><span class="cx"> HitTestRequest request(HitTestRequest::ReadOnly | HitTestRequest::DisallowShadowContent);
</span><del>- HitTestResult result(vPoint);
</del><ins>+ HitTestResult result(view->windowToContents(event.position()));
</ins><span class="cx"> document->renderView()->hitTest(request, result);
</span><span class="cx">
</span><del>- Element* element = result.innerElement();
-
</del><ins>+ RefPtr<Element> element = result.innerElement();
+ RefPtr<ContainerNode> scrollableContainer;
+ ScrollableArea* scrollableArea = nullptr;
</ins><span class="cx"> bool isOverWidget = result.isOverWidget();
</span><ins>+ platformPrepareForWheelEvents(event, result, element, scrollableContainer, scrollableArea, isOverWidget);
</ins><span class="cx">
</span><del>- ContainerNode* scrollableContainer = nullptr;
- ScrollableArea* scrollableArea = nullptr;
- platformPrepareForWheelEvents(e, result, element, scrollableContainer, scrollableArea, isOverWidget);
-
</del><span class="cx"> #if PLATFORM(COCOA)
</span><del>- if (e.phase() == PlatformWheelEventPhaseNone && e.momentumPhase() == PlatformWheelEventPhaseNone) {
-#else
- if (!e.useLatchedEventElement()) {
</del><ins>+ if (event.phase() == PlatformWheelEventPhaseNone && event.momentumPhase() == PlatformWheelEventPhaseNone)
</ins><span class="cx"> #endif
</span><ins>+ {
</ins><span class="cx"> m_latchedWheelEventElement = nullptr;
</span><span class="cx"> m_previousWheelScrolledElement = nullptr;
</span><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> // FIXME: It should not be necessary to do this mutation here.
</span><del>- // Instead, the handlers should know convert vertical scrolls
- // appropriately.
- PlatformWheelEvent event = e;
- if (m_baseEventType == PlatformEvent::NoType && shouldTurnVerticalTicksIntoHorizontal(result, e))
- event = event.copyTurningVerticalTicksIntoHorizontalTicks();
</del><ins>+ // Instead, the handlers should know convert vertical scrolls appropriately.
+ PlatformWheelEvent adjustedEvent = event;
+ if (m_baseEventType == PlatformEvent::NoType && shouldTurnVerticalTicksIntoHorizontal(result, event))
+ adjustedEvent = event.copyTurningVerticalTicksIntoHorizontalTicks();
</ins><span class="cx">
</span><del>- platformRecordWheelEvent(event);
</del><ins>+ platformRecordWheelEvent(adjustedEvent);
</ins><span class="cx">
</span><span class="cx"> if (element) {
</span><del>- // Figure out which view to send the event to.
- RenderElement* target = element->renderer();
-
- if (isOverWidget && target && target->isWidget()) {
- Widget* widget = toRenderWidget(target)->widget();
- if (widget && passWheelEventToWidget(e, widget)) {
- m_isHandlingWheelEvent = false;
- if (scrollableArea)
- scrollableArea->setScrolledProgrammatically(false);
- if (widget->platformWidget())
- return platformCompletePlatformWidgetWheelEvent(e, scrollableContainer);
- return true;
</del><ins>+ if (isOverWidget) {
+ RenderElement* target = element->renderer();
+ if (target && target->isWidget()) {
+ Widget* widget = toRenderWidget(target)->widget();
+ if (widget && passWheelEventToWidget(event, widget)) {
+ m_isHandlingWheelEvent = false;
+ if (scrollableArea)
+ scrollableArea->setScrolledProgrammatically(false);
+ if (!widget->platformWidget())
+ return true;
+ return platformCompletePlatformWidgetWheelEvent(event, scrollableContainer.get());
+ }
</ins><span class="cx"> }
</span><span class="cx"> }
</span><span class="cx">
</span><del>- if (!element->dispatchWheelEvent(event)) {
</del><ins>+ if (!element->dispatchWheelEvent(adjustedEvent)) {
</ins><span class="cx"> m_isHandlingWheelEvent = false;
</span><del>-
</del><span class="cx"> if (scrollableArea && scrollableArea->isScrolledProgrammatically()) {
</span><del>- // Web developer is controlling scrolling. Don't attempt to latch ourselves:
</del><ins>+ // Web developer is controlling scrolling, so don't attempt to latch.
</ins><span class="cx"> clearLatchedState();
</span><span class="cx"> scrollableArea->setScrolledProgrammatically(false);
</span><span class="cx"> }
</span><del>-
</del><span class="cx"> return true;
</span><span class="cx"> }
</span><span class="cx"> }
</span><span class="lines">@@ -2616,7 +2612,7 @@
</span><span class="cx"> if (scrollableArea)
</span><span class="cx"> scrollableArea->setScrolledProgrammatically(false);
</span><span class="cx">
</span><del>- return platformCompleteWheelEvent(e, element, scrollableContainer, scrollableArea);
</del><ins>+ return platformCompleteWheelEvent(event, element.get(), scrollableContainer.get(), scrollableArea);
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> void EventHandler::clearLatchedState()
</span></span></pre></div>
<a id="trunkSourceWebCorepageEventHandlerh"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/page/EventHandler.h (171282 => 171283)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/page/EventHandler.h 2014-07-20 18:00:52 UTC (rev 171282)
+++ trunk/Source/WebCore/page/EventHandler.h 2014-07-20 20:11:57 UTC (rev 171283)
</span><span class="lines">@@ -197,9 +197,9 @@
</span><span class="cx"> void defaultWheelEventHandler(Node*, WheelEvent*);
</span><span class="cx"> bool handlePasteGlobalSelection(const PlatformMouseEvent&);
</span><span class="cx">
</span><del>- void platformPrepareForWheelEvents(const PlatformWheelEvent& wheelEvent, const HitTestResult& result, Element*& wheelEventTarget, ContainerNode*& scrollableContainer, ScrollableArea*& scrollableArea, bool& isOverWidget);
</del><ins>+ void platformPrepareForWheelEvents(const PlatformWheelEvent&, const HitTestResult&, RefPtr<Element>& eventTarget, RefPtr<ContainerNode>& scrollableContainer, ScrollableArea*&, bool& isOverWidget);
</ins><span class="cx"> void platformRecordWheelEvent(const PlatformWheelEvent&);
</span><del>- bool platformCompleteWheelEvent(const PlatformWheelEvent&, Element* wheelEventTarget, ContainerNode* scrollableContainer, ScrollableArea*);
</del><ins>+ bool platformCompleteWheelEvent(const PlatformWheelEvent&, Element* eventTarget, ContainerNode* scrollableContainer, ScrollableArea*);
</ins><span class="cx"> bool platformCompletePlatformWidgetWheelEvent(const PlatformWheelEvent&, ContainerNode* scrollableContainer);
</span><span class="cx">
</span><span class="cx"> #if ENABLE(IOS_TOUCH_EVENTS) || ENABLE(IOS_GESTURE_EVENTS)
</span></span></pre></div>
<a id="trunkSourceWebCorepagemacEventHandlerMacmm"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/page/mac/EventHandlerMac.mm (171282 => 171283)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/page/mac/EventHandlerMac.mm 2014-07-20 18:00:52 UTC (rev 171282)
+++ trunk/Source/WebCore/page/mac/EventHandlerMac.mm 2014-07-20 20:11:57 UTC (rev 171283)
</span><span class="lines">@@ -815,7 +815,7 @@
</span><span class="cx"> return widget->platformWidget();
</span><span class="cx"> }
</span><span class="cx">
</span><del>-void EventHandler::platformPrepareForWheelEvents(const PlatformWheelEvent& wheelEvent, const HitTestResult& result, Element*& wheelEventTarget, ContainerNode*& scrollableContainer, ScrollableArea*& scrollableArea, bool& isOverWidget)
</del><ins>+void EventHandler::platformPrepareForWheelEvents(const PlatformWheelEvent& wheelEvent, const HitTestResult& result, RefPtr<Element>& wheelEventTarget, RefPtr<ContainerNode>& scrollableContainer, ScrollableArea*& scrollableArea, bool& isOverWidget)
</ins><span class="cx"> {
</span><span class="cx"> FrameView* view = m_frame.view();
</span><span class="cx">
</span><span class="lines">@@ -825,9 +825,9 @@
</span><span class="cx"> scrollableContainer = wheelEventTarget;
</span><span class="cx"> scrollableArea = view;
</span><span class="cx"> } else {
</span><del>- if (eventTargetIsPlatformWidget(wheelEventTarget)) {
</del><ins>+ if (eventTargetIsPlatformWidget(wheelEventTarget.get())) {
</ins><span class="cx"> scrollableContainer = wheelEventTarget;
</span><del>- scrollableArea = scrollViewForEventTarget(wheelEventTarget);
</del><ins>+ scrollableArea = scrollViewForEventTarget(wheelEventTarget.get());
</ins><span class="cx"> } else {
</span><span class="cx"> scrollableContainer = findEnclosingScrollableContainer(*wheelEventTarget);
</span><span class="cx"> if (scrollableContainer) {
</span><span class="lines">@@ -847,6 +847,7 @@
</span><span class="cx"> else
</span><span class="cx"> m_startedGestureAtScrollLimit = false;
</span><span class="cx"> m_latchedWheelEventElement = wheelEventTarget;
</span><ins>+ // FIXME: What prevents us from deleting this scrollable container while still holding a pointer to it?
</ins><span class="cx"> m_latchedScrollableContainer = scrollableContainer;
</span><span class="cx"> m_widgetIsLatched = result.isOverWidget();
</span><span class="cx"> isOverWidget = m_widgetIsLatched;
</span></span></pre>
</div>
</div>
</body>
</html>