https://bugs.webkit.org/show_bug.cgi?id=100879
authorbdakin@apple.com <bdakin@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 31 Oct 2012 20:11:00 +0000 (20:11 +0000)
committerbdakin@apple.com <bdakin@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 31 Oct 2012 20:11:00 +0000 (20:11 +0000)
ScrollingStateNode::cloneAndResetNode() should not be virtual

Reviewed by Simon Fraser.

cloneAndResetNode() is currently pure virtual and implemented only
in ScrollingStateScrollingNode. However, all of the work that it
does at this time is stuff that a generic ScrollingStateNode could
do. We should move this implementation to the base class so that it
does not need to be duplicated for future node types.

This patch also re-names cloneAndResetNode() to cloneAndReset()
and correspondingly re-names cloneAndResetChildNodes() to
cloneAndResetChildren().

Finally the patch also changes the copy constructors of both of these
classes to take a const reference instead of a pointer.

* page/scrolling/ScrollingStateNode.cpp:
(WebCore::ScrollingStateNode::ScrollingStateNode):
(WebCore::ScrollingStateNode::cloneAndReset):
(WebCore):
(WebCore::ScrollingStateNode::cloneAndResetChildren):
* page/scrolling/ScrollingStateNode.h:
(ScrollingStateNode):
* page/scrolling/ScrollingStateScrollingNode.cpp:
(WebCore::ScrollingStateScrollingNode::ScrollingStateScrollingNode):
(WebCore):
* page/scrolling/ScrollingStateScrollingNode.h:
(ScrollingStateScrollingNode):
* page/scrolling/ScrollingStateTree.cpp:
(WebCore::ScrollingStateTree::commit):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@133067 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/page/scrolling/ScrollingStateNode.cpp
Source/WebCore/page/scrolling/ScrollingStateNode.h
Source/WebCore/page/scrolling/ScrollingStateScrollingNode.cpp
Source/WebCore/page/scrolling/ScrollingStateScrollingNode.h
Source/WebCore/page/scrolling/ScrollingStateTree.cpp

index a287130..b8a6f04 100644 (file)
@@ -1,3 +1,38 @@
+2012-10-31  Beth Dakin  <bdakin@apple.com>
+
+        https://bugs.webkit.org/show_bug.cgi?id=100879
+        ScrollingStateNode::cloneAndResetNode() should not be virtual
+
+        Reviewed by Simon Fraser.
+
+        cloneAndResetNode() is currently pure virtual and implemented only 
+        in ScrollingStateScrollingNode. However, all of the work that it 
+        does at this time is stuff that a generic ScrollingStateNode could 
+        do. We should move this implementation to the base class so that it 
+        does not need to be duplicated for future node types.
+
+        This patch also re-names cloneAndResetNode() to cloneAndReset()
+        and correspondingly re-names cloneAndResetChildNodes() to 
+        cloneAndResetChildren(). 
+
+        Finally the patch also changes the copy constructors of both of these 
+        classes to take a const reference instead of a pointer.
+
+        * page/scrolling/ScrollingStateNode.cpp:
+        (WebCore::ScrollingStateNode::ScrollingStateNode):
+        (WebCore::ScrollingStateNode::cloneAndReset):
+        (WebCore):
+        (WebCore::ScrollingStateNode::cloneAndResetChildren):
+        * page/scrolling/ScrollingStateNode.h:
+        (ScrollingStateNode):
+        * page/scrolling/ScrollingStateScrollingNode.cpp:
+        (WebCore::ScrollingStateScrollingNode::ScrollingStateScrollingNode):
+        (WebCore):
+        * page/scrolling/ScrollingStateScrollingNode.h:
+        (ScrollingStateScrollingNode):
+        * page/scrolling/ScrollingStateTree.cpp:
+        (WebCore::ScrollingStateTree::commit):
+
 2012-10-31  Tom Sepez  <tsepez@chromium.org>
         
         Malformed X-XSS-Protection headers not reported.
index 8c1a383..7d242c2 100644 (file)
@@ -43,27 +43,39 @@ ScrollingStateNode::ScrollingStateNode(ScrollingStateTree* scrollingStateTree, S
 // This copy constructor is used for cloning nodes in the tree, and it doesn't make sense
 // to clone the relationship pointers, so don't copy that information from the original
 // node.
-ScrollingStateNode::ScrollingStateNode(ScrollingStateNode* stateNode)
+ScrollingStateNode::ScrollingStateNode(const ScrollingStateNode& stateNode)
     : m_scrollingStateTree(0)
-    , m_nodeID(stateNode->scrollingNodeID())
+    , m_nodeID(stateNode.scrollingNodeID())
     , m_parent(0)
-    , m_scrollLayerDidChange(stateNode->scrollLayerDidChange())
+    , m_scrollLayerDidChange(stateNode.scrollLayerDidChange())
 {
-    setScrollLayer(stateNode->platformScrollLayer());
+    setScrollLayer(stateNode.platformScrollLayer());
 }
 
 ScrollingStateNode::~ScrollingStateNode()
 {
 }
 
-void ScrollingStateNode::cloneAndResetChildNodes(ScrollingStateNode* clone)
+PassOwnPtr<ScrollingStateNode> ScrollingStateNode::cloneAndReset()
+{
+    OwnPtr<ScrollingStateScrollingNode> clone = adoptPtr(new ScrollingStateScrollingNode(*toScrollingStateScrollingNode(this)));
+
+    // Now that this node is cloned, reset our change properties.
+    setScrollLayerDidChange(false);
+    resetChangedProperties();
+
+    cloneAndResetChildren(clone.get());
+    return clone.release();
+}
+
+void ScrollingStateNode::cloneAndResetChildren(ScrollingStateNode* clone)
 {
     if (!m_children)
         return;
 
     size_t size = m_children->size();
     for (size_t i = 0; i < size; ++i)
-        clone->appendChild(m_children->at(i)->cloneAndResetNode());
+        clone->appendChild(m_children->at(i)->cloneAndReset());
 }
 
 void ScrollingStateNode::appendChild(PassOwnPtr<ScrollingStateNode> childNode)
index eb8c389..4a2b329 100644 (file)
@@ -44,16 +44,14 @@ class GraphicsLayer;
 class ScrollingStateTree;
 
 class ScrollingStateNode {
-    WTF_MAKE_NONCOPYABLE(ScrollingStateNode);
-
 public:
     ScrollingStateNode(ScrollingStateTree*, ScrollingNodeID);
     virtual ~ScrollingStateNode();
 
     virtual bool isScrollingStateScrollingNode() { return false; }
 
-    virtual PassOwnPtr<ScrollingStateNode> cloneAndResetNode() = 0;
-    void cloneAndResetChildNodes(ScrollingStateNode*);
+    PassOwnPtr<ScrollingStateNode> cloneAndReset();
+    void cloneAndResetChildren(ScrollingStateNode*);
 
     virtual bool hasChangedProperties() const = 0;
     virtual unsigned changedProperties() const = 0;
@@ -82,7 +80,7 @@ public:
     void removeChild(ScrollingStateNode*);
 
 protected:
-    ScrollingStateNode(ScrollingStateNode*);
+    ScrollingStateNode(const ScrollingStateNode&);
 
     ScrollingStateTree* m_scrollingStateTree;
 
index ba35830..cc1e898 100644 (file)
@@ -53,23 +53,23 @@ ScrollingStateScrollingNode::ScrollingStateScrollingNode(ScrollingStateTree* sta
 {
 }
 
-ScrollingStateScrollingNode::ScrollingStateScrollingNode(ScrollingStateScrollingNode* stateNode)
+ScrollingStateScrollingNode::ScrollingStateScrollingNode(const ScrollingStateScrollingNode& stateNode)
     : ScrollingStateNode(stateNode)
-    , m_changedProperties(stateNode->changedProperties())
-    , m_viewportRect(stateNode->viewportRect())
-    , m_contentsSize(stateNode->contentsSize())
-    , m_nonFastScrollableRegion(stateNode->nonFastScrollableRegion())
-    , m_wheelEventHandlerCount(stateNode->wheelEventHandlerCount())
-    , m_shouldUpdateScrollLayerPositionOnMainThread(stateNode->shouldUpdateScrollLayerPositionOnMainThread())
-    , m_horizontalScrollElasticity(stateNode->horizontalScrollElasticity())
-    , m_verticalScrollElasticity(stateNode->verticalScrollElasticity())
-    , m_hasEnabledHorizontalScrollbar(stateNode->hasEnabledHorizontalScrollbar())
-    , m_hasEnabledVerticalScrollbar(stateNode->hasEnabledVerticalScrollbar())
-    , m_requestedScrollPositionRepresentsProgrammaticScroll(stateNode->requestedScrollPositionRepresentsProgrammaticScroll())
-    , m_horizontalScrollbarMode(stateNode->horizontalScrollbarMode())
-    , m_verticalScrollbarMode(stateNode->verticalScrollbarMode())
-    , m_requestedScrollPosition(stateNode->requestedScrollPosition())
-    , m_scrollOrigin(stateNode->scrollOrigin())
+    , m_changedProperties(stateNode.changedProperties())
+    , m_viewportRect(stateNode.viewportRect())
+    , m_contentsSize(stateNode.contentsSize())
+    , m_nonFastScrollableRegion(stateNode.nonFastScrollableRegion())
+    , m_wheelEventHandlerCount(stateNode.wheelEventHandlerCount())
+    , m_shouldUpdateScrollLayerPositionOnMainThread(stateNode.shouldUpdateScrollLayerPositionOnMainThread())
+    , m_horizontalScrollElasticity(stateNode.horizontalScrollElasticity())
+    , m_verticalScrollElasticity(stateNode.verticalScrollElasticity())
+    , m_hasEnabledHorizontalScrollbar(stateNode.hasEnabledHorizontalScrollbar())
+    , m_hasEnabledVerticalScrollbar(stateNode.hasEnabledVerticalScrollbar())
+    , m_requestedScrollPositionRepresentsProgrammaticScroll(stateNode.requestedScrollPositionRepresentsProgrammaticScroll())
+    , m_horizontalScrollbarMode(stateNode.horizontalScrollbarMode())
+    , m_verticalScrollbarMode(stateNode.verticalScrollbarMode())
+    , m_requestedScrollPosition(stateNode.requestedScrollPosition())
+    , m_scrollOrigin(stateNode.scrollOrigin())
 {
 }
 
@@ -77,18 +77,6 @@ ScrollingStateScrollingNode::~ScrollingStateScrollingNode()
 {
 }
 
-PassOwnPtr<ScrollingStateNode> ScrollingStateScrollingNode::cloneAndResetNode()
-{
-    OwnPtr<ScrollingStateScrollingNode> clone = adoptPtr(new ScrollingStateScrollingNode(this));
-
-    // Now that this node is cloned, reset our change properties.
-    setScrollLayerDidChange(false);
-    resetChangedProperties();
-
-    cloneAndResetChildNodes(clone.get());
-    return clone.release();
-}
-
 void ScrollingStateScrollingNode::setViewportRect(const IntRect& viewportRect)
 {
     if (m_viewportRect == viewportRect)
index 714d926..1e1931a 100644 (file)
@@ -40,6 +40,8 @@ namespace WebCore {
 class ScrollingStateScrollingNode : public ScrollingStateNode {
 public:
     static PassOwnPtr<ScrollingStateScrollingNode> create(ScrollingStateTree*, ScrollingNodeID);
+
+    ScrollingStateScrollingNode(const ScrollingStateScrollingNode&);
     virtual ~ScrollingStateScrollingNode();
 
     enum ChangedProperty {
@@ -60,8 +62,6 @@ public:
 
     virtual bool isScrollingStateScrollingNode() OVERRIDE { return true; }
 
-    virtual PassOwnPtr<ScrollingStateNode> cloneAndResetNode() OVERRIDE;
-
     virtual bool hasChangedProperties() const OVERRIDE { return m_changedProperties; }
     virtual unsigned changedProperties() const OVERRIDE { return m_changedProperties; }
     virtual void resetChangedProperties() OVERRIDE { m_changedProperties = 0; }
@@ -109,7 +109,6 @@ public:
 
 private:
     ScrollingStateScrollingNode(ScrollingStateTree*, ScrollingNodeID);
-    ScrollingStateScrollingNode(ScrollingStateScrollingNode*);
 
     unsigned m_changedProperties;
 
index 84bc5db..4e3b538 100644 (file)
@@ -49,7 +49,7 @@ PassOwnPtr<ScrollingStateTree> ScrollingStateTree::commit()
 {
     // This function clones and resets the current state tree, but leaves the tree structure intact. 
     OwnPtr<ScrollingStateTree> treeStateClone = ScrollingStateTree::create();
-    treeStateClone->setRootStateNode(static_pointer_cast<ScrollingStateScrollingNode>(m_rootStateNode->cloneAndResetNode()));
+    treeStateClone->setRootStateNode(static_pointer_cast<ScrollingStateScrollingNode>(m_rootStateNode->cloneAndReset()));
 
     // Copy the IDs of the nodes that have been removed since the last commit into the clone.
     treeStateClone->m_nodesRemovedSinceLastCommit.swap(m_nodesRemovedSinceLastCommit);