Search result page scrolls up before the correct page opens from google search

Anders Carlsson · Anders Carlsson · commit c092f72a27a4 · 2012-04-03T22:26:21.000Z
https://bugs.webkit.org/show_bug.cgi?id=81280 <rdar://problem/10973263> Reviewed by Sam Weinig. Make the requested scroll position part of the ScrollingTreeState so any changes being made are in sync with other important changes such as the scroll layer or contents size. * page/scrolling/ScrollingCoordinator.cpp: (WebCore::ScrollingCoordinator::frameViewRootLayerDidChange): Use coordinatesScrollingForFrameView instead of checking if the frame is the main frame. (WebCore::ScrollingCoordinator::requestScrollPositionUpdate): Call ScrollingTreeState::setRequestedScrollPosition. (WebCore::ScrollingCoordinator::setScrollLayer): Reset the requested scroll position. * page/scrolling/ScrollingTreeState.cpp: (WebCore::ScrollingTreeState::setRequestedScrollPosition): Keep track of the scroll position. * page/scrolling/mac/ScrollingTreeNodeMac.mm: (WebCore::ScrollingTreeNodeMac::update): Update the scroll position if it's changed. Canonical link: https://commits.webkit.org/100444@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@113096 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
@@ -1,3 +1,32 @@
+2012-03-15  Anders Carlsson  <andersca@apple.com>
+
+        Search result page scrolls up before the correct page opens from google search
+        https://bugs.webkit.org/show_bug.cgi?id=81280
+        <rdar://problem/10973263>
+
+        Reviewed by Sam Weinig.
+
+        Make the requested scroll position part of the ScrollingTreeState so any changes being made
+        are in sync with other important changes such as the scroll layer or contents size.
+
+        * page/scrolling/ScrollingCoordinator.cpp:
+        (WebCore::ScrollingCoordinator::frameViewRootLayerDidChange):
+        Use coordinatesScrollingForFrameView instead of checking if the frame is the main frame.
+
+        (WebCore::ScrollingCoordinator::requestScrollPositionUpdate):
+        Call ScrollingTreeState::setRequestedScrollPosition.
+
+        (WebCore::ScrollingCoordinator::setScrollLayer):
+        Reset the requested scroll position.
+
+        * page/scrolling/ScrollingTreeState.cpp:
+        (WebCore::ScrollingTreeState::setRequestedScrollPosition):
+        Keep track of the scroll position.
+
+        * page/scrolling/mac/ScrollingTreeNodeMac.mm:
+        (WebCore::ScrollingTreeNodeMac::update):
+        Update the scroll position if it's changed.
+
 2012-04-03  W. James MacLean  <wjmaclean@chromium.org>
 
         [chromium] Switch touchpad fling curve physics to absolute (not scaled) curve.
diff --git a/Source/WebCore/page/scrolling/ScrollingCoordinator.cpp b/Source/WebCore/page/scrolling/ScrollingCoordinator.cpp
@@ -216,7 +216,7 @@ void ScrollingCoordinator::frameViewRootLayerDidChange(FrameView* frameView)
     ASSERT(isMainThread());
     ASSERT(m_page);
 
-    if (frameView->frame() != m_page->mainFrame())
+    if (!coordinatesScrollingForFrameView(frameView))
         return;
 
     frameViewLayoutUpdated(frameView);
@@ -238,7 +238,14 @@ bool ScrollingCoordinator::requestScrollPositionUpdate(FrameView* frameView, con
     // since FrameView expects scroll position updates to happen synchronously.
     updateMainFrameScrollPosition(scrollPosition);
 
-    ScrollingThread::dispatch(bind(&ScrollingTree::setMainFrameScrollPosition, m_scrollingTree.get(), scrollPosition));
+    if (frameView->frame()->document()->inPageCache()) {
+        // If this frame view's document is being put into the page cache, we don't want to update our
+        // main frame scroll position.
+        return true;
+    }
+
+    m_scrollingTreeState->setRequestedScrollPosition(scrollPosition);
+    scheduleTreeStateCommit();
     return true;
 #else
     UNUSED_PARAM(scrollPosition);
diff --git a/Source/WebCore/page/scrolling/ScrollingTreeState.cpp b/Source/WebCore/page/scrolling/ScrollingTreeState.cpp
@@ -131,6 +131,12 @@ void ScrollingTreeState::setHasEnabledVerticalScrollbar(bool hasEnabledVerticalS
     m_changedProperties |= HasEnabledVerticalScrollbar;
 }
 
+void ScrollingTreeState::setRequestedScrollPosition(const IntPoint& requestedScrollPosition)
+{
+    m_requestedScrollPosition = requestedScrollPosition;
+    m_changedProperties |= RequestedScrollPosition;
+}
+
 PassOwnPtr<ScrollingTreeState> ScrollingTreeState::commit()
 {
     OwnPtr<ScrollingTreeState> treeState = adoptPtr(new ScrollingTreeState(*this));
diff --git a/Source/WebCore/page/scrolling/ScrollingTreeState.h b/Source/WebCore/page/scrolling/ScrollingTreeState.h
@@ -60,6 +60,7 @@ class ScrollingTreeState {
         HasEnabledHorizontalScrollbar = 1 << 7,
         HasEnabledVerticalScrollbar = 1 << 8,
         ScrollLayer = 1 << 9,
+        RequestedScrollPosition = 1 << 10,
     };
 
     bool hasChangedProperties() const { return m_changedProperties; }
@@ -95,6 +96,9 @@ class ScrollingTreeState {
     PlatformLayer* platformScrollLayer() const;
     void setScrollLayer(const GraphicsLayer*);
 
+    const IntPoint& requestedScrollPosition() const { return m_requestedScrollPosition; }
+    void setRequestedScrollPosition(const IntPoint&);
+
     // Copies the current tree state and clears the changed properties mask in the original.
     PassOwnPtr<ScrollingTreeState> commit();
 
@@ -118,6 +122,8 @@ class ScrollingTreeState {
     bool m_hasEnabledHorizontalScrollbar;
     bool m_hasEnabledVerticalScrollbar;
 
+    IntPoint m_requestedScrollPosition;
+
 #if PLATFORM(MAC)
     RetainPtr<PlatformLayer> m_platformScrollLayer;
 #endif
diff --git a/Source/WebCore/page/scrolling/mac/ScrollingTreeNodeMac.mm b/Source/WebCore/page/scrolling/mac/ScrollingTreeNodeMac.mm
@@ -58,6 +58,9 @@
     if (state->changedProperties() & ScrollingTreeState::ScrollLayer)
         m_scrollLayer = state->platformScrollLayer();
 
+    if (state->changedProperties() & ScrollingTreeState::RequestedScrollPosition)
+        setScrollPosition(state->requestedScrollPosition());
+
     if (state->changedProperties() & (ScrollingTreeState::ScrollLayer | ScrollingTreeState::ContentsSize | ScrollingTreeState::ViewportRect))
         updateMainFramePinState(scrollPosition());
 

Original file line number	Diff line number	Diff line change
`@@ -131,6 +131,12 @@ void ScrollingTreeState::setHasEnabledVerticalScrollbar(bool hasEnabledVerticalS`
`131`	`131`	`m_changedProperties \|= HasEnabledVerticalScrollbar;`
`132`	`132`	`}`
`133`	`133`
	`134`	`+void ScrollingTreeState::setRequestedScrollPosition(const IntPoint& requestedScrollPosition)`
	`135`	`+{`
	`136`	`+ m_requestedScrollPosition = requestedScrollPosition;`
	`137`	`+ m_changedProperties \|= RequestedScrollPosition;`
	`138`	`+}`
	`139`	`+`
`134`	`140`	`PassOwnPtr<ScrollingTreeState> ScrollingTreeState::commit()`
`135`	`141`	`{`
`136`	`142`	`OwnPtr<ScrollingTreeState> treeState = adoptPtr(new ScrollingTreeState(*this));`