- Revision
- 140571
- Author
- [email protected]
- Date
- 2013-01-23 12:17:01 -0800 (Wed, 23 Jan 2013)
Log Message
https://bugs.webkit.org/show_bug.cgi?id=107628
Sometimes scroll position is jerky during rubber-band, affects nytimes.com
-and corresponding-
<rdar://problem/12679549>
Reviewed by Simon Fraser.
The basic problem here is that isRubberBandInProgress() was only implemented for
main thread scrolling. So when we were actually scrolling on the scrolling thread,
that function would always return false regardless.
New ScrollableArea virtual function isRubberBandInProgress() will allow us to ask
the ScrollingCoordinator when the scrolling thread is scrolling, or the
ScrollAnimator otherwise.
* page/FrameView.cpp:
(WebCore::FrameView::isRubberBandInProgress):
* page/FrameView.h:
(FrameView):
* platform/ScrollableArea.h:
(WebCore::ScrollableArea::isRubberBandInProgress):
New ScrollingCoordinator function isRubberBandInProgress() always returns false
for non-Mac ports, and is overridden in ScrollingCoordinatorMac to consult the
ScrollingTree.
* page/scrolling/ScrollingCoordinator.h:
(WebCore::ScrollingCoordinator::isRubberBandInProgress):
* page/scrolling/mac/ScrollingCoordinatorMac.h:
(ScrollingCoordinatorMac):
* page/scrolling/mac/ScrollingCoordinatorMac.mm:
(WebCore::ScrollingCoordinatorMac::isRubberBandInProgress):
New variable m_mainFrameIsRubberBanding keeps track of whether there is currently
a rubber-band happening on the scrolling thread.
* page/scrolling/ScrollingTree.cpp:
(WebCore::ScrollingTree::ScrollingTree):
(WebCore::ScrollingTree::isRubberBandInProgress):
(WebCore::ScrollingTree::setMainFrameIsRubberBanding):
* page/scrolling/ScrollingTree.h:
(ScrollingTree):
(WebCore::ScrollingTree::rootNode):
Call setMainFrameIsRubberBanding() whenever the stretchAmount is calculated and
whenever we stop the rubber-band timer.
* page/scrolling/mac/ScrollingTreeScrollingNodeMac.mm:
(WebCore::ScrollingTreeScrollingNodeMac::stretchAmount):
(WebCore::ScrollingTreeScrollingNodeMac::stopSnapRubberbandTimer):
Consult FrameView for isRubberBandInProgress().
* platform/ScrollView.cpp:
(WebCore::ScrollView::updateScrollbars):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (140570 => 140571)
--- trunk/Source/WebCore/ChangeLog 2013-01-23 20:15:12 UTC (rev 140570)
+++ trunk/Source/WebCore/ChangeLog 2013-01-23 20:17:01 UTC (rev 140571)
@@ -1,3 +1,56 @@
+2013-01-23 Beth Dakin <[email protected]>
+
+ https://bugs.webkit.org/show_bug.cgi?id=107628
+ Sometimes scroll position is jerky during rubber-band, affects nytimes.com
+ -and corresponding-
+ <rdar://problem/12679549>
+
+ Reviewed by Simon Fraser.
+
+ The basic problem here is that isRubberBandInProgress() was only implemented for
+ main thread scrolling. So when we were actually scrolling on the scrolling thread,
+ that function would always return false regardless.
+
+ New ScrollableArea virtual function isRubberBandInProgress() will allow us to ask
+ the ScrollingCoordinator when the scrolling thread is scrolling, or the
+ ScrollAnimator otherwise.
+ * page/FrameView.cpp:
+ (WebCore::FrameView::isRubberBandInProgress):
+ * page/FrameView.h:
+ (FrameView):
+ * platform/ScrollableArea.h:
+ (WebCore::ScrollableArea::isRubberBandInProgress):
+
+ New ScrollingCoordinator function isRubberBandInProgress() always returns false
+ for non-Mac ports, and is overridden in ScrollingCoordinatorMac to consult the
+ ScrollingTree.
+ * page/scrolling/ScrollingCoordinator.h:
+ (WebCore::ScrollingCoordinator::isRubberBandInProgress):
+ * page/scrolling/mac/ScrollingCoordinatorMac.h:
+ (ScrollingCoordinatorMac):
+ * page/scrolling/mac/ScrollingCoordinatorMac.mm:
+ (WebCore::ScrollingCoordinatorMac::isRubberBandInProgress):
+
+ New variable m_mainFrameIsRubberBanding keeps track of whether there is currently
+ a rubber-band happening on the scrolling thread.
+ * page/scrolling/ScrollingTree.cpp:
+ (WebCore::ScrollingTree::ScrollingTree):
+ (WebCore::ScrollingTree::isRubberBandInProgress):
+ (WebCore::ScrollingTree::setMainFrameIsRubberBanding):
+ * page/scrolling/ScrollingTree.h:
+ (ScrollingTree):
+ (WebCore::ScrollingTree::rootNode):
+
+ Call setMainFrameIsRubberBanding() whenever the stretchAmount is calculated and
+ whenever we stop the rubber-band timer.
+ * page/scrolling/mac/ScrollingTreeScrollingNodeMac.mm:
+ (WebCore::ScrollingTreeScrollingNodeMac::stretchAmount):
+ (WebCore::ScrollingTreeScrollingNodeMac::stopSnapRubberbandTimer):
+
+ Consult FrameView for isRubberBandInProgress().
+ * platform/ScrollView.cpp:
+ (WebCore::ScrollView::updateScrollbars):
+
2013-01-23 Robert Hogan <[email protected]>
Abspos Inline block not positioned correctly in text-aligned container
Modified: trunk/Source/WebCore/page/FrameView.cpp (140570 => 140571)
--- trunk/Source/WebCore/page/FrameView.cpp 2013-01-23 20:15:12 UTC (rev 140570)
+++ trunk/Source/WebCore/page/FrameView.cpp 2013-01-23 20:17:01 UTC (rev 140571)
@@ -1907,6 +1907,28 @@
return page->chrome()->client()->shouldRubberBandInDirection(direction);
}
+bool FrameView::isRubberBandInProgress() const
+{
+ if (scrollbarsSuppressed())
+ return false;
+
+ // If the scrolling thread updates the scroll position for this FrameView, then we should return
+ // ScrollingCoordinator::isRubberBandInProgress().
+ if (Page* page = m_frame->page()) {
+ if (ScrollingCoordinator* scrollingCoordinator = page->scrollingCoordinator()) {
+ if (!scrollingCoordinator->shouldUpdateScrollLayerPositionOnMainThread())
+ return scrollingCoordinator->isRubberBandInProgress();
+ }
+ }
+
+ // If the main thread updates the scroll position for this FrameView, we should return
+ // ScrollAnimator::isRubberBandInProgress().
+ if (ScrollAnimator* scrollAnimator = existingScrollAnimator())
+ return scrollAnimator->isRubberBandInProgress();
+
+ return false;
+}
+
bool FrameView::requestScrollPositionUpdate(const IntPoint& position)
{
#if ENABLE(THREADED_SCROLLING)
Modified: trunk/Source/WebCore/page/FrameView.h (140570 => 140571)
--- trunk/Source/WebCore/page/FrameView.h 2013-01-23 20:15:12 UTC (rev 140570)
+++ trunk/Source/WebCore/page/FrameView.h 2013-01-23 20:17:01 UTC (rev 140571)
@@ -190,6 +190,7 @@
virtual void updateFixedElementsAfterScrolling();
virtual bool shouldRubberBandInDirection(ScrollDirection) const;
virtual bool requestScrollPositionUpdate(const IntPoint&) OVERRIDE;
+ virtual bool isRubberBandInProgress() const OVERRIDE;
// This is different than visibleContentRect() in that it ignores negative (or overly positive)
// offsets from rubber-banding, and it takes zooming into account.
Modified: trunk/Source/WebCore/page/scrolling/ScrollingCoordinator.h (140570 => 140571)
--- trunk/Source/WebCore/page/scrolling/ScrollingCoordinator.h 2013-01-23 20:15:12 UTC (rev 140570)
+++ trunk/Source/WebCore/page/scrolling/ScrollingCoordinator.h 2013-01-23 20:17:01 UTC (rev 140571)
@@ -126,6 +126,7 @@
virtual void updateScrollingNode(ScrollingNodeID, GraphicsLayer* /*scrollLayer*/, GraphicsLayer* /*counterScrollingLayer*/) { }
virtual void syncChildPositions(const LayoutRect&) { }
virtual String scrollingStateTreeAsText() const;
+ virtual bool isRubberBandInProgress() const { return false; }
// Generated a unique id for scroll layers.
ScrollingNodeID uniqueScrollLayerID();
Modified: trunk/Source/WebCore/page/scrolling/ScrollingTree.cpp (140570 => 140571)
--- trunk/Source/WebCore/page/scrolling/ScrollingTree.cpp 2013-01-23 20:15:12 UTC (rev 140570)
+++ trunk/Source/WebCore/page/scrolling/ScrollingTree.cpp 2013-01-23 20:17:01 UTC (rev 140571)
@@ -54,6 +54,7 @@
, m_canGoForward(false)
, m_mainFramePinnedToTheLeft(false)
, m_mainFramePinnedToTheRight(false)
+ , m_mainFrameIsRubberBanding(false)
, m_scrollingPerformanceLoggingEnabled(false)
, m_isHandlingProgrammaticScroll(false)
{
@@ -259,6 +260,20 @@
return m_canGoForward;
}
+bool ScrollingTree::isRubberBandInProgress()
+{
+ MutexLocker lock(m_mutex);
+
+ return m_mainFrameIsRubberBanding;
+}
+
+void ScrollingTree::setMainFrameIsRubberBanding(bool isRubberBanding)
+{
+ MutexLocker locker(m_mutex);
+
+ m_mainFrameIsRubberBanding = isRubberBanding;
+}
+
bool ScrollingTree::willWheelEventStartSwipeGesture(const PlatformWheelEvent& wheelEvent)
{
if (wheelEvent.phase() != PlatformWheelEventPhaseBegan)
Modified: trunk/Source/WebCore/page/scrolling/ScrollingTree.h (140570 => 140571)
--- trunk/Source/WebCore/page/scrolling/ScrollingTree.h 2013-01-23 20:15:12 UTC (rev 140570)
+++ trunk/Source/WebCore/page/scrolling/ScrollingTree.h 2013-01-23 20:17:01 UTC (rev 140571)
@@ -79,6 +79,9 @@
// Must be called from the scrolling thread. Handles the wheel event.
void handleWheelEvent(const PlatformWheelEvent&);
+ void setMainFrameIsRubberBanding(bool);
+ bool isRubberBandInProgress();
+
void invalidate();
void commitNewTreeState(PassOwnPtr<ScrollingStateTree>);
@@ -99,6 +102,8 @@
void setScrollingPerformanceLoggingEnabled(bool flag);
bool scrollingPerformanceLoggingEnabled();
+ ScrollingTreeScrollingNode* rootNode() const { return m_rootNode.get(); }
+
private:
explicit ScrollingTree(ScrollingCoordinator*);
@@ -121,6 +126,7 @@
bool m_canGoForward;
bool m_mainFramePinnedToTheLeft;
bool m_mainFramePinnedToTheRight;
+ bool m_mainFrameIsRubberBanding;
bool m_scrollingPerformanceLoggingEnabled;
Modified: trunk/Source/WebCore/page/scrolling/mac/ScrollingCoordinatorMac.h (140570 => 140571)
--- trunk/Source/WebCore/page/scrolling/mac/ScrollingCoordinatorMac.h 2013-01-23 20:15:12 UTC (rev 140570)
+++ trunk/Source/WebCore/page/scrolling/mac/ScrollingCoordinatorMac.h 2013-01-23 20:17:01 UTC (rev 140571)
@@ -75,6 +75,8 @@
virtual String scrollingStateTreeAsText() const OVERRIDE;
+ virtual bool isRubberBandInProgress() const OVERRIDE;
+
private:
// Return whether this scrolling coordinator can keep fixed position layers fixed to their
// containers while scrolling.
Modified: trunk/Source/WebCore/page/scrolling/mac/ScrollingCoordinatorMac.mm (140570 => 140571)
--- trunk/Source/WebCore/page/scrolling/mac/ScrollingCoordinatorMac.mm 2013-01-23 20:15:12 UTC (rev 140570)
+++ trunk/Source/WebCore/page/scrolling/mac/ScrollingCoordinatorMac.mm 2013-01-23 20:17:01 UTC (rev 140571)
@@ -88,6 +88,11 @@
return m_scrollingTree.get();
}
+bool ScrollingCoordinatorMac::isRubberBandInProgress() const
+{
+ return scrollingTree()->isRubberBandInProgress();
+}
+
void ScrollingCoordinatorMac::commitTreeStateIfNeeded()
{
if (!m_scrollingStateTree->hasChangedProperties())
Modified: trunk/Source/WebCore/page/scrolling/mac/ScrollingTreeScrollingNodeMac.mm (140570 => 140571)
--- trunk/Source/WebCore/page/scrolling/mac/ScrollingTreeScrollingNodeMac.mm 2013-01-23 20:15:12 UTC (rev 140570)
+++ trunk/Source/WebCore/page/scrolling/mac/ScrollingTreeScrollingNodeMac.mm 2013-01-23 20:17:01 UTC (rev 140571)
@@ -152,6 +152,13 @@
else if (scrollPosition().x() > maximumScrollPosition().x())
stretch.setWidth(scrollPosition().x() - maximumScrollPosition().x());
+ if (scrollingTree()->rootNode() == this) {
+ if (stretch.isZero())
+ scrollingTree()->setMainFrameIsRubberBanding(false);
+ else
+ scrollingTree()->setMainFrameIsRubberBanding(true);
+ }
+
return stretch;
}
@@ -236,6 +243,8 @@
if (!m_snapRubberbandTimer)
return;
+ scrollingTree()->setMainFrameIsRubberBanding(false);
+
CFRunLoopTimerInvalidate(m_snapRubberbandTimer.get());
m_snapRubberbandTimer = nullptr;
}
Modified: trunk/Source/WebCore/platform/ScrollView.cpp (140570 => 140571)
--- trunk/Source/WebCore/platform/ScrollView.cpp 2013-01-23 20:15:12 UTC (rev 140570)
+++ trunk/Source/WebCore/platform/ScrollView.cpp 2013-01-23 20:17:01 UTC (rev 140571)
@@ -612,12 +612,9 @@
}
IntPoint adjustedScrollPosition = IntPoint(desiredOffset);
+ if (!isRubberBandInProgress())
+ adjustedScrollPosition = adjustScrollPositionWithinRange(adjustedScrollPosition);
- if (ScrollAnimator* scrollAnimator = existingScrollAnimator()) {
- if (!scrollAnimator->isRubberBandInProgress())
- adjustedScrollPosition = adjustScrollPositionWithinRange(adjustedScrollPosition);
- }
-
if (adjustedScrollPosition != scrollPosition() || scrollOriginChanged()) {
ScrollableArea::scrollToOffsetWithoutAnimation(adjustedScrollPosition + IntSize(scrollOrigin().x(), scrollOrigin().y()));
resetScrollOriginChanged();
Modified: trunk/Source/WebCore/platform/ScrollableArea.h (140570 => 140571)
--- trunk/Source/WebCore/platform/ScrollableArea.h 2013-01-23 20:15:12 UTC (rev 140570)
+++ trunk/Source/WebCore/platform/ScrollableArea.h 2013-01-23 20:17:01 UTC (rev 140571)
@@ -158,6 +158,7 @@
virtual IntRect scrollableAreaBoundingBox() const = 0;
virtual bool shouldRubberBandInDirection(ScrollDirection) const { return true; }
+ virtual bool isRubberBandInProgress() const { return false; }
virtual bool scrollAnimatorEnabled() const { return false; }
