Changeset 161194 in webkit

Timestamp:

Jan 1, 2014, 11:53:21 AM (12 years ago)

Author:

Simon Fraser

Message:

Updating the scrolling tree should use references to state nodes
​https://bugs.webkit.org/show_bug.cgi?id=126360

Reviewed by Anders Carlsson.

Change functions related to ScrollingTreeNode updating to take
const references to state nodes rather than pointers.

• page/scrolling/ScrollingStateNode.h:

(WebCore::ScrollingStateNode::hasChangedProperty):

• page/scrolling/ScrollingTree.cpp:

(WebCore::ScrollingTree::commitNewTreeState):
(WebCore::ScrollingTree::updateTreeFromStateNode): The node can be nil so
this continues to take a pointer.
(WebCore::ScrollingTree::removeDestroyedNodes):

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

(WebCore::ScrollingTreeNode::updateAfterChildren):

• page/scrolling/ScrollingTreeScrollingNode.cpp:

(WebCore::ScrollingTreeScrollingNode::updateBeforeChildren):

• page/scrolling/ScrollingTreeScrollingNode.h:
• page/scrolling/mac/ScrollingTreeFixedNode.h:
• page/scrolling/mac/ScrollingTreeFixedNode.mm:

(WebCore::ScrollingTreeFixedNode::updateBeforeChildren):

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

(WebCore::ScrollingTreeScrollingNodeMac::updateBeforeChildren):
(WebCore::ScrollingTreeScrollingNodeMac::updateAfterChildren):

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

(WebCore::ScrollingTreeStickyNode::updateBeforeChildren):

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• page/scrolling/ScrollingStateNode.h (modified) (1 diff)
• page/scrolling/ScrollingTree.cpp (modified) (5 diffs)
• page/scrolling/ScrollingTree.h (modified) (1 diff)
• page/scrolling/ScrollingTreeNode.h (modified) (1 diff)
• page/scrolling/ScrollingTreeScrollingNode.cpp (modified) (1 diff)
• page/scrolling/ScrollingTreeScrollingNode.h (modified) (1 diff)
• page/scrolling/mac/ScrollingTreeFixedNode.h (modified) (1 diff)
• page/scrolling/mac/ScrollingTreeFixedNode.mm (modified) (1 diff)
• page/scrolling/mac/ScrollingTreeScrollingNodeMac.h (modified) (1 diff)
• page/scrolling/mac/ScrollingTreeScrollingNodeMac.mm (modified) (2 diffs)
• page/scrolling/mac/ScrollingTreeStickyNode.h (modified) (1 diff)
• page/scrolling/mac/ScrollingTreeStickyNode.mm (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2014-01-01  Simon Fraser  <simon.fraser@apple.com>
+
+        Updating the scrolling tree should use references to state nodes
+        https://bugs.webkit.org/show_bug.cgi?id=126360
+
+        Reviewed by Anders Carlsson.
+
+        Change functions related to ScrollingTreeNode updating to take
+        const references to state nodes rather than pointers.
+
+        * page/scrolling/ScrollingStateNode.h:
+        (WebCore::ScrollingStateNode::hasChangedProperty):
+        * page/scrolling/ScrollingTree.cpp:
+        (WebCore::ScrollingTree::commitNewTreeState):
+        (WebCore::ScrollingTree::updateTreeFromStateNode): The node can be nil so
+        this continues to take a pointer.
+        (WebCore::ScrollingTree::removeDestroyedNodes):
+        * page/scrolling/ScrollingTree.h:
+        * page/scrolling/ScrollingTreeNode.h:
+        (WebCore::ScrollingTreeNode::updateAfterChildren):
+        * page/scrolling/ScrollingTreeScrollingNode.cpp:
+        (WebCore::ScrollingTreeScrollingNode::updateBeforeChildren):
+        * page/scrolling/ScrollingTreeScrollingNode.h:
+        * page/scrolling/mac/ScrollingTreeFixedNode.h:
+        * page/scrolling/mac/ScrollingTreeFixedNode.mm:
+        (WebCore::ScrollingTreeFixedNode::updateBeforeChildren):
+        * page/scrolling/mac/ScrollingTreeScrollingNodeMac.h:
+        * page/scrolling/mac/ScrollingTreeScrollingNodeMac.mm:
+        (WebCore::ScrollingTreeScrollingNodeMac::updateBeforeChildren):
+        (WebCore::ScrollingTreeScrollingNodeMac::updateAfterChildren):
+        * page/scrolling/mac/ScrollingTreeStickyNode.h:
+        * page/scrolling/mac/ScrollingTreeStickyNode.mm:
+        (WebCore::ScrollingTreeStickyNode::updateBeforeChildren):
+
 2014-01-01  Simon Fraser  <simon.fraser@apple.com>
 

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

@@ -63 +63 @@
 
     bool hasChangedProperties() const { return m_changedProperties; }
-    bool hasChangedProperty(unsigned propertyBit) { return m_changedProperties & (1 << propertyBit); }
+    bool hasChangedProperty(unsigned propertyBit) const { return m_changedProperties & (1 << propertyBit); }
     void resetChangedProperties() { m_changedProperties = 0; }
     void setPropertyChanged(unsigned propertyBit) { m_changedProperties |= (1 << propertyBit); }

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

@@ -147 +147 @@
     TemporaryChange<bool> changeHandlingProgrammaticScroll(m_isHandlingProgrammaticScroll, scrollRequestIsProgammatic);
 
-    removeDestroyedNodes(scrollingStateTree.get());
+    removeDestroyedNodes(*scrollingStateTree);
     updateTreeFromStateNode(rootNode);
 }
 
-void ScrollingTree::updateTreeFromStateNode(ScrollingStateNode* stateNode)
+void ScrollingTree::updateTreeFromStateNode(const ScrollingStateNode* stateNode)
 {
     if (!stateNode) {
@@ -166 +166 @@
     if (it != m_nodeMap.end()) {
         node = it->value;
-        node->updateBeforeChildren(stateNode);
+        node->updateBeforeChildren(*stateNode);
     } else {
         // If the node isn't found, it's either new and needs to be added to the tree, or there is a new ID for our
@@ -177 +177 @@
             m_rootNode = ScrollingTreeScrollingNode::create(*this, nodeID);
             m_nodeMap.set(nodeID, m_rootNode.get());
-            m_rootNode->updateBeforeChildren(stateNode);
+            m_rootNode->updateBeforeChildren(*stateNode);
             node = m_rootNode.get();
         } else {
@@ -202 +202 @@
                 parent->appendChild(newNode.release());
             }
-            node->updateBeforeChildren(stateNode);
+            node->updateBeforeChildren(*stateNode);
         }
     }
@@ -213 +213 @@
             updateTreeFromStateNode(stateNodeChildren->at(i).get());
     }
-    node->updateAfterChildren(stateNode);
-}
-
-void ScrollingTree::removeDestroyedNodes(ScrollingStateTree* stateTree)
-{
-    const Vector<ScrollingNodeID>& removedNodes = stateTree->removedNodes();
+    node->updateAfterChildren(*stateNode);
+}
+
+void ScrollingTree::removeDestroyedNodes(const ScrollingStateTree& stateTree)
+{
+    const Vector<ScrollingNodeID>& removedNodes = stateTree.removedNodes();
     size_t size = removedNodes.size();
     for (size_t i = 0; i < size; ++i) {

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

@@ -113 +113 @@
     explicit ScrollingTree(ScrollingCoordinator*);
 
-    void removeDestroyedNodes(ScrollingStateTree*);
-    void updateTreeFromStateNode(ScrollingStateNode*);
+    void removeDestroyedNodes(const ScrollingStateTree&);
+    void updateTreeFromStateNode(const ScrollingStateNode*);
 
     RefPtr<ScrollingCoordinator> m_scrollingCoordinator;

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

@@ -44 +44 @@
     virtual ~ScrollingTreeNode();
 
-    virtual void updateBeforeChildren(ScrollingStateNode*) = 0;
-    virtual void updateAfterChildren(ScrollingStateNode*) { }
+    virtual void updateBeforeChildren(const ScrollingStateNode&) = 0;
+    virtual void updateAfterChildren(const ScrollingStateNode&) { }
 
     virtual void parentScrollPositionDidChange(const IntRect& viewportRect, const FloatSize& cumulativeDelta) = 0;

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

@@ -48 +48 @@
 }
 
-void ScrollingTreeScrollingNode::updateBeforeChildren(ScrollingStateNode* stateNode)
+void ScrollingTreeScrollingNode::updateBeforeChildren(const ScrollingStateNode& stateNode)
 {
-    ScrollingStateScrollingNode* state = toScrollingStateScrollingNode(stateNode);
+    const ScrollingStateScrollingNode& state = toScrollingStateScrollingNode(stateNode);
 
-    if (state->hasChangedProperty(ScrollingStateScrollingNode::ViewportRect))
-        m_viewportRect = state->viewportRect();
+    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();
+            m_totalContentsSizeForRubberBand = state.totalContentsSize();
+        m_totalContentsSize = state.totalContentsSize();
     }
 
-    if (state->hasChangedProperty(ScrollingStateScrollingNode::ScrollOrigin))
-        m_scrollOrigin = state->scrollOrigin();
+    if (state.hasChangedProperty(ScrollingStateScrollingNode::ScrollOrigin))
+        m_scrollOrigin = state.scrollOrigin();
 
-    if (state->hasChangedProperty(ScrollingStateScrollingNode::ScrollableAreaParams))
-        m_scrollableAreaParameters = state->scrollableAreaParameters();
+    if (state.hasChangedProperty(ScrollingStateScrollingNode::ScrollableAreaParams))
+        m_scrollableAreaParameters = state.scrollableAreaParameters();
 
-    if (state->hasChangedProperty(ScrollingStateScrollingNode::FrameScaleFactor))
-        m_frameScaleFactor = state->frameScaleFactor();
+    if (state.hasChangedProperty(ScrollingStateScrollingNode::FrameScaleFactor))
+        m_frameScaleFactor = state.frameScaleFactor();
 
-    if (state->hasChangedProperty(ScrollingStateScrollingNode::ReasonsForSynchronousScrolling))
-        m_synchronousScrollingReasons = state->synchronousScrollingReasons();
+    if (state.hasChangedProperty(ScrollingStateScrollingNode::ReasonsForSynchronousScrolling))
+        m_synchronousScrollingReasons = state.synchronousScrollingReasons();
 
-    if (state->hasChangedProperty(ScrollingStateScrollingNode::HeaderHeight))
-        m_headerHeight = state->headerHeight();
+    if (state.hasChangedProperty(ScrollingStateScrollingNode::HeaderHeight))
+        m_headerHeight = state.headerHeight();
 
-    if (state->hasChangedProperty(ScrollingStateScrollingNode::FooterHeight))
-        m_footerHeight = state->footerHeight();
+    if (state.hasChangedProperty(ScrollingStateScrollingNode::FooterHeight))
+        m_footerHeight = state.footerHeight();
 
-    if (state->hasChangedProperty(ScrollingStateScrollingNode::BehaviorForFixedElements))
-        m_behaviorForFixed = state->scrollBehaviorForFixedElements();
+    if (state.hasChangedProperty(ScrollingStateScrollingNode::BehaviorForFixedElements))
+        m_behaviorForFixed = state.scrollBehaviorForFixedElements();
 }
 

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

@@ -46 +46 @@
     virtual ~ScrollingTreeScrollingNode();
 
-    virtual void updateBeforeChildren(ScrollingStateNode*) OVERRIDE;
+    virtual void updateBeforeChildren(const ScrollingStateNode&) OVERRIDE;
 
     // FIXME: We should implement this when we support ScrollingTreeScrollingNodes as children.

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

@@ -48 +48 @@
     ScrollingTreeFixedNode(ScrollingTree&, ScrollingNodeID);
 
-    virtual void updateBeforeChildren(ScrollingStateNode*) OVERRIDE;
+    virtual void updateBeforeChildren(const ScrollingStateNode&) OVERRIDE;
     virtual void parentScrollPositionDidChange(const IntRect& viewportRect, const FloatSize& cumulativeDelta) OVERRIDE;
 

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

@@ -48 +48 @@
 }
 
-void ScrollingTreeFixedNode::updateBeforeChildren(ScrollingStateNode* stateNode)
+void ScrollingTreeFixedNode::updateBeforeChildren(const ScrollingStateNode& stateNode)
 {
-    ScrollingStateFixedNode* fixedStateNode = toScrollingStateFixedNode(stateNode);
+    const ScrollingStateFixedNode& fixedStateNode = toScrollingStateFixedNode(stateNode);
 
-    if (fixedStateNode->hasChangedProperty(ScrollingStateNode::ScrollLayer))
-        m_layer = fixedStateNode->platformScrollLayer();
+    if (fixedStateNode.hasChangedProperty(ScrollingStateNode::ScrollLayer))
+        m_layer = fixedStateNode.platformScrollLayer();
 
-    if (stateNode->hasChangedProperty(ScrollingStateFixedNode::ViewportConstraints))
-        m_constraints = fixedStateNode->viewportConstraints();
+    if (stateNode.hasChangedProperty(ScrollingStateFixedNode::ViewportConstraints))
+        m_constraints = fixedStateNode.viewportConstraints();
 }
 

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

@@ -45 +45 @@
 private:
     // ScrollingTreeNode member functions.
-    virtual void updateBeforeChildren(ScrollingStateNode*) OVERRIDE;
-    virtual void updateAfterChildren(ScrollingStateNode*) OVERRIDE;
+    virtual void updateBeforeChildren(const ScrollingStateNode&) OVERRIDE;
+    virtual void updateAfterChildren(const ScrollingStateNode&) OVERRIDE;
     virtual void handleWheelEvent(const PlatformWheelEvent&) OVERRIDE;
 

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

@@ -71 +71 @@
 }
 
-void ScrollingTreeScrollingNodeMac::updateBeforeChildren(ScrollingStateNode* stateNode)
+void ScrollingTreeScrollingNodeMac::updateBeforeChildren(const ScrollingStateNode& stateNode)
 {
     ScrollingTreeScrollingNode::updateBeforeChildren(stateNode);
-    ScrollingStateScrollingNode* scrollingStateNode = toScrollingStateScrollingNode(stateNode);
-
-    if (scrollingStateNode->hasChangedProperty(ScrollingStateNode::ScrollLayer))
-        m_scrollLayer = scrollingStateNode->platformScrollLayer();
-
-    if (scrollingStateNode->hasChangedProperty(ScrollingStateScrollingNode::CounterScrollingLayer))
-        m_counterScrollingLayer = scrollingStateNode->counterScrollingPlatformLayer();
-
-    if (scrollingStateNode->hasChangedProperty(ScrollingStateScrollingNode::HeaderLayer))
-        m_headerLayer = scrollingStateNode->headerPlatformLayer();
-
-    if (scrollingStateNode->hasChangedProperty(ScrollingStateScrollingNode::FooterLayer))
-        m_footerLayer = scrollingStateNode->footerPlatformLayer();
-
-    if (scrollingStateNode->hasChangedProperty(ScrollingStateScrollingNode::PainterForScrollbar)) {
-        m_verticalScrollbarPainter = scrollingStateNode->verticalScrollbarPainter();
-        m_horizontalScrollbarPainter = scrollingStateNode->horizontalScrollbarPainter();
-    }
-
-    if (scrollingStateNode->hasChangedProperty(ScrollingStateScrollingNode::ReasonsForSynchronousScrolling)) {
+    const auto& scrollingStateNode = toScrollingStateScrollingNode(stateNode);
+
+    if (scrollingStateNode.hasChangedProperty(ScrollingStateNode::ScrollLayer))
+        m_scrollLayer = scrollingStateNode.platformScrollLayer();
+
+    if (scrollingStateNode.hasChangedProperty(ScrollingStateScrollingNode::CounterScrollingLayer))
+        m_counterScrollingLayer = scrollingStateNode.counterScrollingPlatformLayer();
+
+    if (scrollingStateNode.hasChangedProperty(ScrollingStateScrollingNode::HeaderLayer))
+        m_headerLayer = scrollingStateNode.headerPlatformLayer();
+
+    if (scrollingStateNode.hasChangedProperty(ScrollingStateScrollingNode::FooterLayer))
+        m_footerLayer = scrollingStateNode.footerPlatformLayer();
+
+    if (scrollingStateNode.hasChangedProperty(ScrollingStateScrollingNode::PainterForScrollbar)) {
+        m_verticalScrollbarPainter = scrollingStateNode.verticalScrollbarPainter();
+        m_horizontalScrollbarPainter = scrollingStateNode.horizontalScrollbarPainter();
+    }
+
+    if (scrollingStateNode.hasChangedProperty(ScrollingStateScrollingNode::ReasonsForSynchronousScrolling)) {
         if (shouldUpdateScrollLayerPositionSynchronously()) {
             // We're transitioning to the slow "update scroll layer position on the main thread" mode.
             // Initialize the probable main thread scroll position with the current scroll layer position.
-            if (scrollingStateNode->hasChangedProperty(ScrollingStateScrollingNode::RequestedScrollPosition))
-                m_probableMainThreadScrollPosition = scrollingStateNode->requestedScrollPosition();
+            if (scrollingStateNode.hasChangedProperty(ScrollingStateScrollingNode::RequestedScrollPosition))
+                m_probableMainThreadScrollPosition = scrollingStateNode.requestedScrollPosition();
             else {
                 CGPoint scrollLayerPosition = m_scrollLayer.get().position;
@@ -109 +109 @@
     }
 
-    if (scrollingStateNode->hasChangedProperty(ScrollingStateScrollingNode::WheelEventHandlerCount)) {
+    if (scrollingStateNode.hasChangedProperty(ScrollingStateScrollingNode::WheelEventHandlerCount)) {
         if (scrollingTree().scrollingPerformanceLoggingEnabled())
-            logWheelEventHandlerCountChanged(scrollingStateNode->wheelEventHandlerCount());
-    }
-}
-
-void ScrollingTreeScrollingNodeMac::updateAfterChildren(ScrollingStateNode* stateNode)
+            logWheelEventHandlerCountChanged(scrollingStateNode.wheelEventHandlerCount());
+    }
+}
+
+void ScrollingTreeScrollingNodeMac::updateAfterChildren(const ScrollingStateNode& stateNode)
 {
     ScrollingTreeScrollingNode::updateAfterChildren(stateNode);
 
-    ScrollingStateScrollingNode* scrollingStateNode = toScrollingStateScrollingNode(stateNode);
+    const auto& scrollingStateNode = toScrollingStateScrollingNode(stateNode);
 
     // Update the scroll position after child nodes have been updated, because they need to have updated their constraints before any scrolling happens.
-    if (scrollingStateNode->hasChangedProperty(ScrollingStateScrollingNode::RequestedScrollPosition))
-        setScrollPosition(scrollingStateNode->requestedScrollPosition());
-
-    if (scrollingStateNode->hasChangedProperty(ScrollingStateNode::ScrollLayer) || scrollingStateNode->hasChangedProperty(ScrollingStateScrollingNode::TotalContentsSize) || scrollingStateNode->hasChangedProperty(ScrollingStateScrollingNode::ViewportRect))
+    if (scrollingStateNode.hasChangedProperty(ScrollingStateScrollingNode::RequestedScrollPosition))
+        setScrollPosition(scrollingStateNode.requestedScrollPosition());
+
+    if (scrollingStateNode.hasChangedProperty(ScrollingStateNode::ScrollLayer) || scrollingStateNode.hasChangedProperty(ScrollingStateScrollingNode::TotalContentsSize) || scrollingStateNode.hasChangedProperty(ScrollingStateScrollingNode::ViewportRect))
         updateMainFramePinState(scrollPosition());
 }

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

@@ -48 +48 @@
     ScrollingTreeStickyNode(ScrollingTree&, ScrollingNodeID);
 
-    virtual void updateBeforeChildren(ScrollingStateNode*) OVERRIDE;
+    virtual void updateBeforeChildren(const ScrollingStateNode&) OVERRIDE;
     virtual void parentScrollPositionDidChange(const IntRect& viewportRect, const FloatSize& cumulativeDelta) OVERRIDE;
 

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

@@ -48 +48 @@
 }
 
-void ScrollingTreeStickyNode::updateBeforeChildren(ScrollingStateNode* stateNode)
+void ScrollingTreeStickyNode::updateBeforeChildren(const ScrollingStateNode& stateNode)
 {
-    ScrollingStateStickyNode* stickyStateNode = toScrollingStateStickyNode(stateNode);
+    const ScrollingStateStickyNode& stickyStateNode = toScrollingStateStickyNode(stateNode);
 
-    if (stickyStateNode->hasChangedProperty(ScrollingStateNode::ScrollLayer))
-        m_layer = stickyStateNode->platformScrollLayer();
+    if (stickyStateNode.hasChangedProperty(ScrollingStateNode::ScrollLayer))
+        m_layer = stickyStateNode.platformScrollLayer();
 
-    if (stateNode->hasChangedProperty(ScrollingStateStickyNode::ViewportConstraints))
-        m_constraints = stickyStateNode->viewportConstraints();
+    if (stateNode.hasChangedProperty(ScrollingStateStickyNode::ViewportConstraints))
+        m_constraints = stickyStateNode.viewportConstraints();
 }