Diff
Modified: trunk/Source/WebCore/ChangeLog (171282 => 171283)
--- 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)
@@ -1,3 +1,25 @@
+2014-07-20 Darin Adler <[email protected]>
+
+ 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.
+
2014-07-20 Simon Fraser <[email protected]>
Print layerIDs in GraphicsLayer dumps
Modified: trunk/Source/WebCore/page/EventHandler.cpp (171282 => 171283)
--- 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)
@@ -2505,14 +2505,17 @@
}
#if !PLATFORM(GTK)
+
bool EventHandler::shouldTurnVerticalTicksIntoHorizontal(const HitTestResult&, const PlatformWheelEvent&) const
{
return false;
}
+
#endif
#if !PLATFORM(MAC)
-void EventHandler::platformPrepareForWheelEvents(const PlatformWheelEvent&, const HitTestResult&, Element*&, ContainerNode*&, ScrollableArea*&, bool&)
+
+void EventHandler::platformPrepareForWheelEvents(const PlatformWheelEvent&, const HitTestResult&, RefPtr<Element>&, RefPtr<ContainerNode>&, ScrollableArea*&, bool&)
{
}
@@ -2535,15 +2538,15 @@
{
return true;
}
+
#endif
-bool EventHandler::handleWheelEvent(const PlatformWheelEvent& e)
+bool EventHandler::handleWheelEvent(const PlatformWheelEvent& event)
{
Document* document = m_frame.document();
-
if (!document->renderView())
return false;
-
+
RefPtr<FrameView> protector(m_frame.view());
FrameView* view = m_frame.view();
@@ -2552,63 +2555,56 @@
m_isHandlingWheelEvent = true;
setFrameWasScrolledByUser();
- LayoutPoint vPoint = view->windowToContents(e.position());
HitTestRequest request(HitTestRequest::ReadOnly | HitTestRequest::DisallowShadowContent);
- HitTestResult result(vPoint);
+ HitTestResult result(view->windowToContents(event.position()));
document->renderView()->hitTest(request, result);
- Element* element = result.innerElement();
-
+ RefPtr<Element> element = result.innerElement();
+ RefPtr<ContainerNode> scrollableContainer;
+ ScrollableArea* scrollableArea = nullptr;
bool isOverWidget = result.isOverWidget();
+ platformPrepareForWheelEvents(event, result, element, scrollableContainer, scrollableArea, isOverWidget);
- ContainerNode* scrollableContainer = nullptr;
- ScrollableArea* scrollableArea = nullptr;
- platformPrepareForWheelEvents(e, result, element, scrollableContainer, scrollableArea, isOverWidget);
-
#if PLATFORM(COCOA)
- if (e.phase() == PlatformWheelEventPhaseNone && e.momentumPhase() == PlatformWheelEventPhaseNone) {
-#else
- if (!e.useLatchedEventElement()) {
+ if (event.phase() == PlatformWheelEventPhaseNone && event.momentumPhase() == PlatformWheelEventPhaseNone)
#endif
+ {
m_latchedWheelEventElement = nullptr;
m_previousWheelScrolledElement = nullptr;
}
// FIXME: It should not be necessary to do this mutation here.
- // Instead, the handlers should know convert vertical scrolls
- // appropriately.
- PlatformWheelEvent event = e;
- if (m_baseEventType == PlatformEvent::NoType && shouldTurnVerticalTicksIntoHorizontal(result, e))
- event = event.copyTurningVerticalTicksIntoHorizontalTicks();
+ // Instead, the handlers should know convert vertical scrolls appropriately.
+ PlatformWheelEvent adjustedEvent = event;
+ if (m_baseEventType == PlatformEvent::NoType && shouldTurnVerticalTicksIntoHorizontal(result, event))
+ adjustedEvent = event.copyTurningVerticalTicksIntoHorizontalTicks();
- platformRecordWheelEvent(event);
+ platformRecordWheelEvent(adjustedEvent);
if (element) {
- // 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;
+ 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());
+ }
}
}
- if (!element->dispatchWheelEvent(event)) {
+ if (!element->dispatchWheelEvent(adjustedEvent)) {
m_isHandlingWheelEvent = false;
-
if (scrollableArea && scrollableArea->isScrolledProgrammatically()) {
- // Web developer is controlling scrolling. Don't attempt to latch ourselves:
+ // Web developer is controlling scrolling, so don't attempt to latch.
clearLatchedState();
scrollableArea->setScrolledProgrammatically(false);
}
-
return true;
}
}
@@ -2616,7 +2612,7 @@
if (scrollableArea)
scrollableArea->setScrolledProgrammatically(false);
- return platformCompleteWheelEvent(e, element, scrollableContainer, scrollableArea);
+ return platformCompleteWheelEvent(event, element.get(), scrollableContainer.get(), scrollableArea);
}
void EventHandler::clearLatchedState()
Modified: trunk/Source/WebCore/page/EventHandler.h (171282 => 171283)
--- 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)
@@ -197,9 +197,9 @@
void defaultWheelEventHandler(Node*, WheelEvent*);
bool handlePasteGlobalSelection(const PlatformMouseEvent&);
- void platformPrepareForWheelEvents(const PlatformWheelEvent& wheelEvent, const HitTestResult& result, Element*& wheelEventTarget, ContainerNode*& scrollableContainer, ScrollableArea*& scrollableArea, bool& isOverWidget);
+ void platformPrepareForWheelEvents(const PlatformWheelEvent&, const HitTestResult&, RefPtr<Element>& eventTarget, RefPtr<ContainerNode>& scrollableContainer, ScrollableArea*&, bool& isOverWidget);
void platformRecordWheelEvent(const PlatformWheelEvent&);
- bool platformCompleteWheelEvent(const PlatformWheelEvent&, Element* wheelEventTarget, ContainerNode* scrollableContainer, ScrollableArea*);
+ bool platformCompleteWheelEvent(const PlatformWheelEvent&, Element* eventTarget, ContainerNode* scrollableContainer, ScrollableArea*);
bool platformCompletePlatformWidgetWheelEvent(const PlatformWheelEvent&, ContainerNode* scrollableContainer);
#if ENABLE(IOS_TOUCH_EVENTS) || ENABLE(IOS_GESTURE_EVENTS)
Modified: trunk/Source/WebCore/page/mac/EventHandlerMac.mm (171282 => 171283)
--- 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)
@@ -815,7 +815,7 @@
return widget->platformWidget();
}
-void EventHandler::platformPrepareForWheelEvents(const PlatformWheelEvent& wheelEvent, const HitTestResult& result, Element*& wheelEventTarget, ContainerNode*& scrollableContainer, ScrollableArea*& scrollableArea, bool& isOverWidget)
+void EventHandler::platformPrepareForWheelEvents(const PlatformWheelEvent& wheelEvent, const HitTestResult& result, RefPtr<Element>& wheelEventTarget, RefPtr<ContainerNode>& scrollableContainer, ScrollableArea*& scrollableArea, bool& isOverWidget)
{
FrameView* view = m_frame.view();
@@ -825,9 +825,9 @@
scrollableContainer = wheelEventTarget;
scrollableArea = view;
} else {
- if (eventTargetIsPlatformWidget(wheelEventTarget)) {
+ if (eventTargetIsPlatformWidget(wheelEventTarget.get())) {
scrollableContainer = wheelEventTarget;
- scrollableArea = scrollViewForEventTarget(wheelEventTarget);
+ scrollableArea = scrollViewForEventTarget(wheelEventTarget.get());
} else {
scrollableContainer = findEnclosingScrollableContainer(*wheelEventTarget);
if (scrollableContainer) {
@@ -847,6 +847,7 @@
else
m_startedGestureAtScrollLimit = false;
m_latchedWheelEventElement = wheelEventTarget;
+ // FIXME: What prevents us from deleting this scrollable container while still holding a pointer to it?
m_latchedScrollableContainer = scrollableContainer;
m_widgetIsLatched = result.isOverWidget();
isOverWidget = m_widgetIsLatched;
