Changeset 116830 in webkit

Timestamp:

May 11, 2012 5:56:13 PM (12 years ago)

Author:

andersca@apple.com

Message:

Comcast website displays bottom of page when loaded
​https://bugs.webkit.org/show_bug.cgi?id=86277
<rdar://problem/11426887>

Reviewed by Beth Dakin.

There were two bugs here. The first bug was that FrameView::setScrollPosition didn't end up calling into the scrolling coordinator
to update the scroll position. The second bug was that ScrollingTreeNodeMac::setScrollPosition didn't constrain the scroll position
to the edge of the page.

• page/FrameView.cpp:

(WebCore::FrameView::setScrollPosition):
Call requestScrollPositionUpdate.

• page/scrolling/ScrollingTree.cpp:
• page/scrolling/ScrollingTree.h:

Remove setMainFrameScrollPosition, it is not called by anyone.

• page/scrolling/mac/ScrollingTreeNodeMac.h:
• page/scrolling/mac/ScrollingTreeNodeMac.mm:

(WebCore::ScrollingTreeNodeMac::setScrollPosition):
Clamp to the page size and call setScrollPositionWithoutContentEdgeConstraints.

(WebCore::ScrollingTreeNodeMac::setScrollPositionWithoutContentEdgeConstraints):
Update the scroll layer position and call back to the main thread.

(WebCore::ScrollingTreeNodeMac::scrollBy):
Call setScrollPosition.

(WebCore::ScrollingTreeNodeMac::scrollByWithoutContentEdgeConstraints):
Call setScrollPositionWithoutContentEdgeConstraints.

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• page/FrameView.cpp (modified) (1 diff)
• page/scrolling/ScrollingTree.cpp (modified) (1 diff)
• page/scrolling/ScrollingTree.h (modified) (1 diff)
• page/scrolling/mac/ScrollingTreeNodeMac.h (modified) (2 diffs)
• page/scrolling/mac/ScrollingTreeNodeMac.mm (modified) (3 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2012-05-11  Anders Carlsson  <andersca@apple.com>
+
+        Comcast website displays bottom of page when loaded
+        https://bugs.webkit.org/show_bug.cgi?id=86277
+        <rdar://problem/11426887>
+
+        Reviewed by Beth Dakin.
+
+        There were two bugs here. The first bug was that FrameView::setScrollPosition didn't end up calling into the scrolling coordinator
+        to update the scroll position. The second bug was that ScrollingTreeNodeMac::setScrollPosition didn't constrain the scroll position
+        to the edge of the page.
+
+        * page/FrameView.cpp:
+        (WebCore::FrameView::setScrollPosition):
+        Call requestScrollPositionUpdate.
+
+        * page/scrolling/ScrollingTree.cpp:
+        * page/scrolling/ScrollingTree.h:
+        Remove setMainFrameScrollPosition, it is not called by anyone.
+
+        * page/scrolling/mac/ScrollingTreeNodeMac.h:
+        * page/scrolling/mac/ScrollingTreeNodeMac.mm:
+        (WebCore::ScrollingTreeNodeMac::setScrollPosition):
+        Clamp to the page size and call setScrollPositionWithoutContentEdgeConstraints.
+
+        (WebCore::ScrollingTreeNodeMac::setScrollPositionWithoutContentEdgeConstraints):
+        Update the scroll layer position and call back to the main thread.
+
+        (WebCore::ScrollingTreeNodeMac::scrollBy):
+        Call setScrollPosition.
+
+        (WebCore::ScrollingTreeNodeMac::scrollByWithoutContentEdgeConstraints):
+        Call setScrollPositionWithoutContentEdgeConstraints.
+
 2012-05-11  Gavin Barraclough  <barraclough@apple.com>
 

trunk/Source/WebCore/page/FrameView.cpp

@@ -1705 +1705 @@
     TemporaryChange<bool> changeInProgrammaticScroll(m_inProgrammaticScroll, true);
     m_maintainScrollPositionAnchor = 0;
+
+    if (requestScrollPositionUpdate(scrollPoint))
+        return;
+
     ScrollView::setScrollPosition(scrollPoint);
 }

trunk/Source/WebCore/page/scrolling/ScrollingTree.cpp

@@ -98 +98 @@
 }
 
-void ScrollingTree::setMainFrameScrollPosition(const IntPoint& scrollPosition)
-{
-    ASSERT(ScrollingThread::isCurrentThread());
-
-    m_rootNode->setScrollPosition(scrollPosition);
-}
-
 static void derefScrollingCoordinator(ScrollingCoordinator* scrollingCoordinator)
 {

trunk/Source/WebCore/page/scrolling/ScrollingTree.h

@@ -76 +76 @@
     void handleWheelEvent(const PlatformWheelEvent&);
 
-    void setMainFrameScrollPosition(const IntPoint&);
-
     void invalidate();
     void commitNewTreeState(PassOwnPtr<ScrollingTreeState>);

trunk/Source/WebCore/page/scrolling/mac/ScrollingTreeNodeMac.h

@@ -46 +46 @@
     virtual void update(ScrollingTreeState*) OVERRIDE;
     virtual void handleWheelEvent(const PlatformWheelEvent&) OVERRIDE;
-    virtual void setScrollPosition(const IntPoint&) OVERRIDE;
 
     // ScrollElasticityController member functions.
@@ -63 +62 @@
 
     IntPoint scrollPosition() const;
+    void setScrollPosition(const IntPoint&);
+    void setScrollPositionWithoutContentEdgeConstraints(const IntPoint&);
+
     void setScrollLayerPosition(const IntPoint&);
 

trunk/Source/WebCore/page/scrolling/mac/ScrollingTreeNodeMac.mm

@@ -86 +86 @@
 }
 
-void ScrollingTreeNodeMac::setScrollPosition(const IntPoint& scrollPosition)
-{
-    updateMainFramePinState(scrollPosition);
-
-    if (shouldUpdateScrollLayerPositionOnMainThread()) {
-        m_probableMainThreadScrollPosition = scrollPosition;
-        scrollingTree()->updateMainFrameScrollPositionAndScrollLayerPosition(scrollPosition);
-        return;
-    }
-
-    setScrollLayerPosition(scrollPosition);
-    scrollingTree()->updateMainFrameScrollPosition(scrollPosition);
-}
-
 bool ScrollingTreeNodeMac::allowsHorizontalStretching()
 {
@@ -241 +227 @@
 }
 
+void ScrollingTreeNodeMac::setScrollPosition(const IntPoint& scrollPosition)
+{
+    IntPoint newScrollPosition = scrollPosition;
+    newScrollPosition = newScrollPosition.shrunkTo(maximumScrollPosition());
+    newScrollPosition = newScrollPosition.expandedTo(minimumScrollPosition());
+
+    setScrollPositionWithoutContentEdgeConstraints(newScrollPosition);
+}
+
+void ScrollingTreeNodeMac::setScrollPositionWithoutContentEdgeConstraints(const IntPoint& scrollPosition)
+{
+    updateMainFramePinState(scrollPosition);
+
+    if (shouldUpdateScrollLayerPositionOnMainThread()) {
+        m_probableMainThreadScrollPosition = scrollPosition;
+        scrollingTree()->updateMainFrameScrollPositionAndScrollLayerPosition(scrollPosition);
+        return;
+    }
+
+    setScrollLayerPosition(scrollPosition);
+    scrollingTree()->updateMainFrameScrollPosition(scrollPosition);
+}
+
 void ScrollingTreeNodeMac::setScrollLayerPosition(const IntPoint& position)
 {
@@ -264 +273 @@
 void ScrollingTreeNodeMac::scrollBy(const IntSize& offset)
 {
-    IntPoint newScrollPosition = scrollPosition() + offset;
-    newScrollPosition = newScrollPosition.shrunkTo(maximumScrollPosition());
-    newScrollPosition = newScrollPosition.expandedTo(minimumScrollPosition());
-
-    setScrollPosition(newScrollPosition);
+    setScrollPosition(scrollPosition() + offset);
 }
 
 void ScrollingTreeNodeMac::scrollByWithoutContentEdgeConstraints(const IntSize& offset)
 {
-    setScrollPosition(scrollPosition() + offset);
+    setScrollPositionWithoutContentEdgeConstraints(scrollPosition() + offset);
 }