[webkit-changes] [149066] trunk/Source/WebCore

Wed, 24 Apr 2013 14:35:47 -0700

Title: [149066] trunk/Source/WebCore
Revision
149066
Author
bda...@apple.com
Date
2013-04-24 14:37:34 -0700 (Wed, 24 Apr 2013)

Log Message

Vertical overlay scrollbar in iframes fades in and out rapidly when you scroll in 
a circle
https://bugs.webkit.org/show_bug.cgi?id=115124
-and corresponding-
<rdar://problem/13168957>

Reviewed by Anders Carlsson.

With http://trac.webkit.org/changeset/119834 we started calling 
ScrollbarPainterController's contentAreaScrolled/contentAreaScrolledInDirection 
API on a zero-delay timer instead of calling it right away. This prevented some 
crashes that we saw whenever this was called during a layout. However, that delay, 
combined with the particulars of contentAreaScrolledInDirection cause this bug 
where sometimes the scrollbars in an iframe will fade out very noticeably when 
scrolling in a circle.

This change makes it so we will only use the zero-delay timer if the 
ScrollableArea is not currently handling a wheel event. If it IS handling a wheel 
event, then we will send the notifications to AppKit right away. I confirmed that 
this change did not reintroduce the old crashes. 

Keep track of whether we are currently handling a wheel event with the new member 
variable m_isHandlingWheelEvent.
* page/EventHandler.cpp:
(WebCore::EventHandler::EventHandler):
(WebCore::EventHandler::handleWheelEvent):
* page/EventHandler.h:
(WebCore::EventHandler::isHandlingWheelEvent):
(EventHandler):

To prevent layering violations, the ScrollableArea sub-classes will have to access 
this information from the EventHandler.
* page/FrameView.cpp:
(WebCore::FrameView::isHandlingWheelEvent):
* page/FrameView.h:
* platform/ScrollableArea.h:
(WebCore::ScrollableArea::isHandlingWheelEvent):
* rendering/RenderLayer.cpp:
(WebCore::RenderLayer::isHandlingWheelEvent):
* rendering/RenderLayer.h:
(RenderLayer):
* rendering/RenderListBox.cpp:
(WebCore::RenderListBox::isHandlingWheelEvent):
* rendering/RenderListBox.h:

sendContentAreaScrolledSoon() can be private. Also add new function 
sendContentAreaScrolled().
* platform/mac/ScrollAnimatorMac.h:
(ScrollAnimatorMac):

If the ScrollableArea is handling a wheel event, call 
sendContentAreaScrolled(), otherwise call sendContentAreaScrolledSoon()
* platform/mac/ScrollAnimatorMac.mm:
(WebCore::ScrollAnimatorMac::notifyContentAreaScrolled):
(WebCore::ScrollAnimatorMac::sendContentAreaScrolled):

Re-factored to use sendContentAreaScrolled()
(WebCore::ScrollAnimatorMac::sendContentAreaScrolledTimerFired):

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (149065 => 149066)


--- trunk/Source/WebCore/ChangeLog	2013-04-24 21:35:41 UTC (rev 149065)
+++ trunk/Source/WebCore/ChangeLog	2013-04-24 21:37:34 UTC (rev 149066)
@@ -1,3 +1,64 @@
+2013-04-24  Beth Dakin  <bda...@apple.com>
+
+        Vertical overlay scrollbar in iframes fades in and out rapidly when you scroll in 
+        a circle
+        https://bugs.webkit.org/show_bug.cgi?id=115124
+        -and corresponding-
+        <rdar://problem/13168957>
+
+        Reviewed by Anders Carlsson.
+
+        With http://trac.webkit.org/changeset/119834 we started calling 
+        ScrollbarPainterController's contentAreaScrolled/contentAreaScrolledInDirection 
+        API on a zero-delay timer instead of calling it right away. This prevented some 
+        crashes that we saw whenever this was called during a layout. However, that delay, 
+        combined with the particulars of contentAreaScrolledInDirection cause this bug 
+        where sometimes the scrollbars in an iframe will fade out very noticeably when 
+        scrolling in a circle.
+
+        This change makes it so we will only use the zero-delay timer if the 
+        ScrollableArea is not currently handling a wheel event. If it IS handling a wheel 
+        event, then we will send the notifications to AppKit right away. I confirmed that 
+        this change did not reintroduce the old crashes. 
+
+        Keep track of whether we are currently handling a wheel event with the new member 
+        variable m_isHandlingWheelEvent.
+        * page/EventHandler.cpp:
+        (WebCore::EventHandler::EventHandler):
+        (WebCore::EventHandler::handleWheelEvent):
+        * page/EventHandler.h:
+        (WebCore::EventHandler::isHandlingWheelEvent):
+        (EventHandler):
+
+        To prevent layering violations, the ScrollableArea sub-classes will have to access 
+        this information from the EventHandler.
+        * page/FrameView.cpp:
+        (WebCore::FrameView::isHandlingWheelEvent):
+        * page/FrameView.h:
+        * platform/ScrollableArea.h:
+        (WebCore::ScrollableArea::isHandlingWheelEvent):
+        * rendering/RenderLayer.cpp:
+        (WebCore::RenderLayer::isHandlingWheelEvent):
+        * rendering/RenderLayer.h:
+        (RenderLayer):
+        * rendering/RenderListBox.cpp:
+        (WebCore::RenderListBox::isHandlingWheelEvent):
+        * rendering/RenderListBox.h:
+
+        sendContentAreaScrolledSoon() can be private. Also add new function 
+        sendContentAreaScrolled().
+        * platform/mac/ScrollAnimatorMac.h:
+        (ScrollAnimatorMac):
+
+        If the ScrollableArea is handling a wheel event, call 
+        sendContentAreaScrolled(), otherwise call sendContentAreaScrolledSoon()
+        * platform/mac/ScrollAnimatorMac.mm:
+        (WebCore::ScrollAnimatorMac::notifyContentAreaScrolled):
+        (WebCore::ScrollAnimatorMac::sendContentAreaScrolled):
+
+        Re-factored to use sendContentAreaScrolled()
+        (WebCore::ScrollAnimatorMac::sendContentAreaScrolledTimerFired):
+
 2013-04-24  Ryosuke Niwa  <rn...@webkit.org>
 
         Fix a merge error in r149007 (was missing a null check added in r148777).

Modified: trunk/Source/WebCore/page/EventHandler.cpp (149065 => 149066)


--- trunk/Source/WebCore/page/EventHandler.cpp	2013-04-24 21:35:41 UTC (rev 149065)
+++ trunk/Source/WebCore/page/EventHandler.cpp	2013-04-24 21:37:34 UTC (rev 149066)
@@ -348,6 +348,7 @@
     , m_baseEventType(PlatformEvent::NoType)
     , m_didStartDrag(false)
     , m_didLongPressInvokeContextMenu(false)
+    , m_isHandlingWheelEvent(false)
 #if ENABLE(CURSOR_VISIBILITY)
     , m_autoHideCursorTimer(this, &EventHandler::autoHideCursorTimerFired)
 #endif
@@ -2402,6 +2403,8 @@
     FrameView* view = m_frame->view();
     if (!view)
         return false;
+
+    m_isHandlingWheelEvent = true;
     setFrameWasScrolledByUser();
     LayoutPoint vPoint = view->windowToContents(e.position());
 
@@ -2447,21 +2450,24 @@
         
         if (isOverWidget && target && target->isWidget()) {
             Widget* widget = toRenderWidget(target)->widget();
-            if (widget && passWheelEventToWidget(e, widget))
+            if (widget && passWheelEventToWidget(e, widget)) {
+                m_isHandlingWheelEvent = false;
                 return true;
+            }
         }
 
-        if (node && !node->dispatchWheelEvent(event))
+        if (node && !node->dispatchWheelEvent(event)) {
+            m_isHandlingWheelEvent = false;
             return true;
+        }
     }
 
 
     // We do another check on the frame view because the event handler can run JS which results in the frame getting destroyed.
     view = m_frame->view();
-    if (!view)
-        return false;
-    
-    return view->wheelEvent(event);
+    bool didHandleEvent = view ? view->wheelEvent(event) : false;
+    m_isHandlingWheelEvent = false;
+    return didHandleEvent;
 }
 
 void EventHandler::defaultWheelEventHandler(Node* startNode, WheelEvent* wheelEvent)

Modified: trunk/Source/WebCore/page/EventHandler.h (149065 => 149066)


--- trunk/Source/WebCore/page/EventHandler.h	2013-04-24 21:35:41 UTC (rev 149065)
+++ trunk/Source/WebCore/page/EventHandler.h	2013-04-24 21:37:34 UTC (rev 149066)
@@ -257,6 +257,8 @@
     bool useHandCursor(Node*, bool isOverLink, bool shiftKey);
     void updateCursor();
 
+    bool isHandlingWheelEvent() const { return m_isHandlingWheelEvent; }
+
 private:
 #if ENABLE(DRAG_SUPPORT)
     static DragState& dragState();
@@ -494,6 +496,7 @@
     PlatformEvent::Type m_baseEventType;
     bool m_didStartDrag;
     bool m_didLongPressInvokeContextMenu;
+    bool m_isHandlingWheelEvent;
 
 #if ENABLE(CURSOR_VISIBILITY)
     Timer<EventHandler> m_autoHideCursorTimer;

Modified: trunk/Source/WebCore/page/FrameView.cpp (149065 => 149066)


--- trunk/Source/WebCore/page/FrameView.cpp	2013-04-24 21:35:41 UTC (rev 149065)
+++ trunk/Source/WebCore/page/FrameView.cpp	2013-04-24 21:37:34 UTC (rev 149066)
@@ -1629,6 +1629,11 @@
     return m_frame ? m_frame->eventHandler()->lastKnownMousePosition() : IntPoint();
 }
 
+bool FrameView::isHandlingWheelEvent() const
+{
+    return m_frame ? m_frame->eventHandler()->isHandlingWheelEvent() : false;
+}
+
 bool FrameView::shouldSetCursor() const
 {
     Page* page = frame()->page();

Modified: trunk/Source/WebCore/page/FrameView.h (149065 => 149066)


--- trunk/Source/WebCore/page/FrameView.h	2013-04-24 21:35:41 UTC (rev 149065)
+++ trunk/Source/WebCore/page/FrameView.h	2013-04-24 21:37:34 UTC (rev 149066)
@@ -345,6 +345,7 @@
     static void setRepaintThrottlingDeferredRepaintDelayIncrementDuringLoading(double p);
 
     virtual IntPoint lastKnownMousePosition() const;
+    virtual bool isHandlingWheelEvent() const OVERRIDE;
     bool shouldSetCursor() const;
 
     virtual bool scrollbarsCanBeActive() const OVERRIDE;

Modified: trunk/Source/WebCore/platform/ScrollableArea.h (149065 => 149066)


--- trunk/Source/WebCore/platform/ScrollableArea.h	2013-04-24 21:35:41 UTC (rev 149065)
+++ trunk/Source/WebCore/platform/ScrollableArea.h	2013-04-24 21:37:34 UTC (rev 149066)
@@ -144,6 +144,7 @@
     virtual IntSize contentsSize() const = 0;
     virtual IntSize overhangAmount() const { return IntSize(); }
     virtual IntPoint lastKnownMousePosition() const { return IntPoint(); }
+    virtual bool isHandlingWheelEvent() const { return false; }
 
     virtual int headerHeight() const { return 0; }
     virtual int footerHeight() const { return 0; }

Modified: trunk/Source/WebCore/platform/mac/ScrollAnimatorMac.h (149065 => 149066)


--- trunk/Source/WebCore/platform/mac/ScrollAnimatorMac.h	2013-04-24 21:35:41 UTC (rev 149065)
+++ trunk/Source/WebCore/platform/mac/ScrollAnimatorMac.h	2013-04-24 21:37:34 UTC (rev 149066)
@@ -65,8 +65,6 @@
     void startScrollbarPaintTimer();
     void stopScrollbarPaintTimer();
 
-    void sendContentAreaScrolledSoon(const FloatSize& scrollDelta);
-
     void setVisibleScrollerThumbRect(const IntRect&);
 
 private:
@@ -124,6 +122,12 @@
 
     virtual void notifyContentAreaScrolled(const FloatSize& delta) OVERRIDE;
 
+    // sendContentAreaScrolledSoon() will do the same work that sendContentAreaScrolled() does except
+    // it does it after a zero-delay timer fires. This will prevent us from updating overlay scrollbar 
+    // information during layout.
+    void sendContentAreaScrolled(const FloatSize& scrollDelta);
+    void sendContentAreaScrolledSoon(const FloatSize& scrollDelta);
+
     FloatPoint adjustScrollPositionIfNecessary(const FloatPoint&) const;
 
     void immediateScrollTo(const FloatPoint&);

Modified: trunk/Source/WebCore/platform/mac/ScrollAnimatorMac.mm (149065 => 149066)


--- trunk/Source/WebCore/platform/mac/ScrollAnimatorMac.mm	2013-04-24 21:35:41 UTC (rev 149065)
+++ trunk/Source/WebCore/platform/mac/ScrollAnimatorMac.mm	2013-04-24 21:37:34 UTC (rev 149066)
@@ -983,7 +983,12 @@
     // This function is called when a page is going into the page cache, but the page 
     // isn't really scrolling in that case. We should only pass the message on to the
     // ScrollbarPainterController when we're really scrolling on an active page.
-    if (scrollableArea()->scrollbarsCanBeActive())
+    if (!scrollableArea()->scrollbarsCanBeActive())
+        return;
+
+    if (m_scrollableArea->isHandlingWheelEvent())
+        sendContentAreaScrolled(delta);
+    else
         sendContentAreaScrolledSoon(delta);
 }
 
@@ -1280,15 +1285,20 @@
         m_sendContentAreaScrolledTimer.startOneShot(0);
 }
 
-void ScrollAnimatorMac::sendContentAreaScrolledTimerFired(Timer<ScrollAnimatorMac>*)
+void ScrollAnimatorMac::sendContentAreaScrolled(const FloatSize& delta)
 {
-    if (supportsContentAreaScrolledInDirection()) {
-        [m_scrollbarPainterController.get() contentAreaScrolledInDirection:NSMakePoint(m_contentAreaScrolledTimerScrollDelta.width(), m_contentAreaScrolledTimerScrollDelta.height())];
-        m_contentAreaScrolledTimerScrollDelta = FloatSize();
-    } else
+    if (supportsContentAreaScrolledInDirection())
+        [m_scrollbarPainterController.get() contentAreaScrolledInDirection:NSMakePoint(delta.width(), delta.height())];
+    else
         [m_scrollbarPainterController.get() contentAreaScrolled];
 }
 
+void ScrollAnimatorMac::sendContentAreaScrolledTimerFired(Timer<ScrollAnimatorMac>*)
+{
+    sendContentAreaScrolled(m_contentAreaScrolledTimerScrollDelta);
+    m_contentAreaScrolledTimerScrollDelta = FloatSize();
+}
+
 void ScrollAnimatorMac::setVisibleScrollerThumbRect(const IntRect& scrollerThumb)
 {
     IntRect rectInViewCoordinates = scrollerThumb;

Modified: trunk/Source/WebCore/rendering/RenderLayer.cpp (149065 => 149066)


--- trunk/Source/WebCore/rendering/RenderLayer.cpp	2013-04-24 21:35:41 UTC (rev 149065)
+++ trunk/Source/WebCore/rendering/RenderLayer.cpp	2013-04-24 21:37:34 UTC (rev 149066)
@@ -2751,6 +2751,11 @@
     return renderer()->frame() ? renderer()->frame()->eventHandler()->lastKnownMousePosition() : IntPoint();
 }
 
+bool RenderLayer::isHandlingWheelEvent() const
+{
+    return renderer()->frame() ? renderer()->frame()->eventHandler()->isHandlingWheelEvent() : false;
+}
+
 IntRect RenderLayer::rectForHorizontalScrollbar(const IntRect& borderBoxRect) const
 {
     if (!m_hBar)

Modified: trunk/Source/WebCore/rendering/RenderLayer.h (149065 => 149066)


--- trunk/Source/WebCore/rendering/RenderLayer.h	2013-04-24 21:35:41 UTC (rev 149065)
+++ trunk/Source/WebCore/rendering/RenderLayer.h	2013-04-24 21:37:34 UTC (rev 149066)
@@ -1022,6 +1022,7 @@
     virtual IntSize contentsSize() const;
     virtual IntSize overhangAmount() const;
     virtual IntPoint lastKnownMousePosition() const;
+    virtual bool isHandlingWheelEvent() const OVERRIDE;
     virtual bool shouldSuspendScrollAnimations() const;
     virtual bool scrollbarsCanBeActive() const;
     virtual IntRect scrollableAreaBoundingBox() const OVERRIDE;

Modified: trunk/Source/WebCore/rendering/RenderListBox.cpp (149065 => 149066)


--- trunk/Source/WebCore/rendering/RenderListBox.cpp	2013-04-24 21:35:41 UTC (rev 149065)
+++ trunk/Source/WebCore/rendering/RenderListBox.cpp	2013-04-24 21:37:34 UTC (rev 149066)
@@ -815,6 +815,14 @@
     return view->frameView()->lastKnownMousePosition();
 }
 
+bool RenderListBox::isHandlingWheelEvent() const
+{
+    RenderView* view = this->view();
+    if (!view)
+        return false;
+    return view->frameView()->isHandlingWheelEvent();
+}
+
 bool RenderListBox::shouldSuspendScrollAnimations() const
 {
     RenderView* view = this->view();

Modified: trunk/Source/WebCore/rendering/RenderListBox.h (149065 => 149066)


--- trunk/Source/WebCore/rendering/RenderListBox.h	2013-04-24 21:35:41 UTC (rev 149065)
+++ trunk/Source/WebCore/rendering/RenderListBox.h	2013-04-24 21:37:34 UTC (rev 149066)
@@ -119,6 +119,7 @@
     virtual int visibleHeight() const OVERRIDE;
     virtual int visibleWidth() const OVERRIDE;
     virtual IntPoint lastKnownMousePosition() const OVERRIDE;
+    virtual bool isHandlingWheelEvent() const OVERRIDE;
     virtual bool shouldSuspendScrollAnimations() const OVERRIDE;
     virtual bool scrollbarsCanBeActive() const OVERRIDE;
 

_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to