Changeset 130989 in webkit

Timestamp:

Oct 10, 2012 6:24:51 PM (12 years ago)

Author:

Beth Dakin

Message:

​https://bugs.webkit.org/show_bug.cgi?id=98968
REGRESSION: Unable to scroll with trackpad on some websites after
r130783

Reviewed by Simon Fraser.

This bug seems to reproduce mostly on web pages that require login.
The re-routing causes extra churn of the RenderLayerBacking, and we
end up destroying and re-creating the backing and therefore the
ScrollingStateNodes, and when the timing is just wrong we get into a
state where the ScrollingTree has a set of incorrect information.
This patch fixes the bug by making sure the ScrollingStateNodes keep
the ScrollingTree in synch with their re-set data.

Tell the ScrollingStateTree that the root layer has changed.

• page/scrolling/ScrollingCoordinator.cpp:

(WebCore::ScrollingCoordinator::frameViewRootLayerDidChange):

New virtual function will set every property as having changed.

• page/scrolling/ScrollingStateNode.h:

(WebCore::ScrollingStateNode::setHasChangedProperties):

• page/scrolling/ScrollingStateScrollingNode.cpp:

(WebCore::ScrollingStateScrollingNode::setHasChangedProperties):
(WebCore):

• page/scrolling/ScrollingStateScrollingNode.h:

When the root layer changes, make sure we update the ScrollingTree
appropriately by indicating that every property could have changed.

• page/scrolling/ScrollingStateTree.cpp:

(WebCore::ScrollingStateTree::rootLayerDidChange):
(WebCore):

• page/scrolling/ScrollingStateTree.h:

(ScrollingStateTree):

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• page/scrolling/ScrollingCoordinator.cpp (modified) (1 diff)
• page/scrolling/ScrollingStateNode.h (modified) (1 diff)
• page/scrolling/ScrollingStateScrollingNode.cpp (modified) (1 diff)
• page/scrolling/ScrollingStateScrollingNode.h (modified) (2 diffs)
• page/scrolling/ScrollingStateTree.cpp (modified) (1 diff)
• page/scrolling/ScrollingStateTree.h (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2012-10-10  Beth Dakin  <bdakin@apple.com>
+
+        https://bugs.webkit.org/show_bug.cgi?id=98968
+        REGRESSION: Unable to scroll with trackpad on some websites after 
+        r130783
+
+        Reviewed by Simon Fraser.
+
+        This bug seems to reproduce mostly on web pages that require login. 
+        The re-routing causes extra churn of the RenderLayerBacking, and we 
+        end up destroying and re-creating the backing and therefore the 
+        ScrollingStateNodes, and when the timing is just wrong we get into a 
+        state where the ScrollingTree has a set of incorrect information. 
+        This patch fixes the bug by making sure the ScrollingStateNodes keep 
+        the ScrollingTree in synch with their re-set data.
+
+        Tell the ScrollingStateTree that the root layer has changed.
+        * page/scrolling/ScrollingCoordinator.cpp:
+        (WebCore::ScrollingCoordinator::frameViewRootLayerDidChange):
+
+        New virtual function will set every property as having changed.
+        * page/scrolling/ScrollingStateNode.h:
+        (WebCore::ScrollingStateNode::setHasChangedProperties):
+        * page/scrolling/ScrollingStateScrollingNode.cpp:
+        (WebCore::ScrollingStateScrollingNode::setHasChangedProperties):
+        (WebCore):
+        * page/scrolling/ScrollingStateScrollingNode.h:
+
+        When the root layer changes, make sure we update the ScrollingTree 
+        appropriately by indicating that every property could have changed.
+        * page/scrolling/ScrollingStateTree.cpp:
+        (WebCore::ScrollingStateTree::rootLayerDidChange):
+        (WebCore):
+        * page/scrolling/ScrollingStateTree.h:
+        (ScrollingStateTree):
+
 2012-10-10  MORITA Hajime  <morrita@google.com>
 

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

@@ -252 +252 @@
     ensureRootStateNodeForFrameView(frameView);
     ASSERT(m_scrollingStateTree->rootStateNode());
+    m_scrollingStateTree->rootLayerDidChange();
 #endif
 

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

@@ -57 +57 @@
     virtual unsigned changedProperties() const = 0;
     virtual void resetChangedProperties() = 0;
+    virtual void setHasChangedProperties() { setScrollLayerDidChange(true); }
 
     PlatformLayer* platformScrollLayer() const;

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

@@ -78 +78 @@
 }
 
+void ScrollingStateScrollingNode::setHasChangedProperties()
+{
+    m_changedProperties = All;
+    ScrollingStateNode::setHasChangedProperties();
+}
+
 PassOwnPtr<ScrollingStateNode> ScrollingStateScrollingNode::cloneAndResetNode()
 {

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

@@ -57 +57 @@
         ScrollOrigin = 1 << 11,
         RequestedScrollPosition = 1 << 12,
+        All = (1 << 13) - 1 // This will need to be updated if we add or remove anything the ChangedProperties.
     };
 
@@ -66 +67 @@
     virtual unsigned changedProperties() const OVERRIDE { return m_changedProperties; }
     virtual void resetChangedProperties() OVERRIDE { m_changedProperties = 0; }
+    virtual void setHasChangedProperties();
 
     const IntRect& viewportRect() const { return m_viewportRect; }

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

@@ -69 +69 @@
 }
 
+void ScrollingStateTree::rootLayerDidChange()
+{
+    // If the root layer has changed, then destroyed and re-created the root state node. That means that the
+    // cached properties in ScrollingStateScrollingNode are no longer reflective of the properties we have
+    // cached over in the ScrollingTree. To resolve this, we will mark all of the properties as having changed
+    // so that the ScrollingTree will be in synch with the state tree.
+    setHasChangedProperties(true);
+    rootStateNode()->setHasChangedProperties();
+}
+
 } // namespace WebCore
 

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

@@ -63 +63 @@
     void removeNode(ScrollingStateNode*);
 
+    void rootLayerDidChange();
+
     void setHasChangedProperties(bool changedProperties) { m_hasChangedProperties = changedProperties; }
     bool hasChangedProperties() const { return m_hasChangedProperties; }