Rubber-banding is often not smooth on infinitely scrolling websites
authorbdakin@apple.com <bdakin@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 18 Oct 2013 19:11:08 +0000 (19:11 +0000)
committerbdakin@apple.com <bdakin@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 18 Oct 2013 19:11:08 +0000 (19:11 +0000)
https://bugs.webkit.org/show_bug.cgi?id=122985

Reviewed by Simon Fraser.

totalContentsSize is an important part of the calculation for
maximumScrollPosition(). This function is called repeatedly throughout the curve
of a rubber-band to determine the stretch amount. To keep the rubber-band
animation smooth, it should be allowed to finish its animation using the old
totalContentsSize. This patch does that by adding a new variable,
m_totalContentsSizeForRubberBand. This value should almost always be equivalent to
m_totalContentsSize. It will only vary if m_totalContentsSize has changed in the
middle of a rubber-band, and in that case, it will stay equivalent to the old
totalContentSize value until the rubber band animation finishes.

* page/scrolling/ScrollingTreeScrollingNode.cpp:
(WebCore::ScrollingTreeScrollingNode::updateBeforeChildren):
* page/scrolling/ScrollingTreeScrollingNode.h:
(WebCore::ScrollingTreeScrollingNode::totalContentsSizeForRubberBand):
(WebCore::ScrollingTreeScrollingNode::setTotalContentsSizeForRubberBand):
* page/scrolling/mac/ScrollingTreeScrollingNodeMac.mm:
(WebCore::ScrollingTreeScrollingNodeMac::stopSnapRubberbandTimer):
(WebCore::ScrollingTreeScrollingNodeMac::maximumScrollPosition):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@157642 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/page/scrolling/ScrollingTreeScrollingNode.cpp
Source/WebCore/page/scrolling/ScrollingTreeScrollingNode.h
Source/WebCore/page/scrolling/mac/ScrollingTreeScrollingNodeMac.mm

index 42d9d57..543dc6c 100644 (file)
@@ -1,3 +1,29 @@
+2013-10-18  Beth Dakin  <bdakin@apple.com>
+
+        Rubber-banding is often not smooth on infinitely scrolling websites
+        https://bugs.webkit.org/show_bug.cgi?id=122985
+
+        Reviewed by Simon Fraser.
+
+        totalContentsSize is an important part of the calculation for 
+        maximumScrollPosition(). This function is called repeatedly throughout the curve 
+        of a rubber-band to determine the stretch amount. To keep the rubber-band 
+        animation smooth, it should be allowed to finish its animation using the old 
+        totalContentsSize. This patch does that by adding a new variable, 
+        m_totalContentsSizeForRubberBand. This value should almost always be equivalent to 
+        m_totalContentsSize. It will only vary if m_totalContentsSize has changed in the 
+        middle of a rubber-band, and in that case, it will stay equivalent to the old 
+        totalContentSize value until the rubber band animation finishes.
+
+        * page/scrolling/ScrollingTreeScrollingNode.cpp:
+        (WebCore::ScrollingTreeScrollingNode::updateBeforeChildren):
+        * page/scrolling/ScrollingTreeScrollingNode.h:
+        (WebCore::ScrollingTreeScrollingNode::totalContentsSizeForRubberBand):
+        (WebCore::ScrollingTreeScrollingNode::setTotalContentsSizeForRubberBand):
+        * page/scrolling/mac/ScrollingTreeScrollingNodeMac.mm:
+        (WebCore::ScrollingTreeScrollingNodeMac::stopSnapRubberbandTimer):
+        (WebCore::ScrollingTreeScrollingNodeMac::maximumScrollPosition):
+
 2013-10-18  ChangSeok Oh  <changseok.oh@collabora.com>
 
         Unreviewed build fix for --no-svg option.
index b99534e..8da5b70 100644 (file)
@@ -29,6 +29,7 @@
 #if ENABLE(THREADED_SCROLLING)
 
 #include "ScrollingStateTree.h"
+#include "ScrollingTree.h"
 
 namespace WebCore {
 
@@ -58,8 +59,13 @@ void ScrollingTreeScrollingNode::updateBeforeChildren(ScrollingStateNode* stateN
     if (state->hasChangedProperty(ScrollingStateScrollingNode::ViewportRect))
         m_viewportRect = state->viewportRect();
 
-    if (state->hasChangedProperty(ScrollingStateScrollingNode::TotalContentsSize))
+    if (state->hasChangedProperty(ScrollingStateScrollingNode::TotalContentsSize)) {
+        if (scrollingTree()->isRubberBandInProgress())
+            m_totalContentsSizeForRubberBand = m_totalContentsSize;
+        else
+            m_totalContentsSizeForRubberBand = state->totalContentsSize();
         m_totalContentsSize = state->totalContentsSize();
+    }
 
     if (state->hasChangedProperty(ScrollingStateScrollingNode::FrameScaleFactor))
         m_frameScaleFactor = state->frameScaleFactor();
index ce7f167..eaf36cc 100644 (file)
@@ -61,6 +61,12 @@ protected:
     const IntRect& viewportRect() const { return m_viewportRect; }
     const IntSize& totalContentsSize() const { return m_totalContentsSize; }
 
+    // If the totalContentsSize changes in the middle of a rubber-band, we still want to use the old totalContentsSize for the sake of
+    // computing the stretchAmount(). Using the old value will keep the animation smooth. When there is no rubber-band in progress at
+    // all, m_totalContentsSizeForRubberBand should be equivalent to m_totalContentsSize.
+    const IntSize& totalContentsSizeForRubberBand() const { return m_totalContentsSizeForRubberBand; }
+    void setTotalContentsSizeForRubberBand(const IntSize& totalContentsSizeForRubberBand) { m_totalContentsSizeForRubberBand = totalContentsSizeForRubberBand; }
+
     float frameScaleFactor() const { return m_frameScaleFactor; }
 
     ScrollElasticity horizontalScrollElasticity() const { return m_horizontalScrollElasticity; }
@@ -79,6 +85,7 @@ protected:
 private:
     IntRect m_viewportRect;
     IntSize m_totalContentsSize;
+    IntSize m_totalContentsSizeForRubberBand;
     IntPoint m_scrollOrigin;
     
     float m_frameScaleFactor;
index 6244037..f63beed 100644 (file)
@@ -270,6 +270,9 @@ void ScrollingTreeScrollingNodeMac::stopSnapRubberbandTimer()
 
     scrollingTree()->setMainFrameIsRubberBanding(false);
 
+    // Since the rubberband timer has stopped, totalContentsSizeForRubberBand can be synchronized with totalContentsSize.
+    setTotalContentsSizeForRubberBand(totalContentsSize());
+
     CFRunLoopTimerInvalidate(m_snapRubberbandTimer.get());
     m_snapRubberbandTimer = nullptr;
 }
@@ -379,8 +382,8 @@ IntPoint ScrollingTreeScrollingNodeMac::minimumScrollPosition() const
 
 IntPoint ScrollingTreeScrollingNodeMac::maximumScrollPosition() const
 {
-    IntPoint position(totalContentsSize().width() - viewportRect().width(),
-                      totalContentsSize().height() - viewportRect().height());
+    IntPoint position(totalContentsSizeForRubberBand().width() - viewportRect().width(),
+                      totalContentsSizeForRubberBand().height() - viewportRect().height());
 
     position.clampNegativeToZero();