RESOLVED FIXED 100879
ScrollingStateNode::cloneAndResetNode() should not be virtual
https://bugs.webkit.org/show_bug.cgi?id=100879
Summary ScrollingStateNode::cloneAndResetNode() should not be virtual
Beth Dakin
Reported 2012-10-31 12:41:44 PDT
ScrollingStateNode::cloneAndResetNode() is currently pure virtual, and its implementation is 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.
Attachments
Patch (4.88 KB, patch)
2012-10-31 12:45 PDT, Beth Dakin
simon.fraser: review+
Beth Dakin
Comment 1 2012-10-31 12:45:09 PDT
Simon Fraser (smfr)
Comment 2 2012-10-31 12:47:26 PDT
Comment on attachment 171706 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=171706&action=review > Source/WebCore/page/scrolling/ScrollingStateNode.h:55 > + PassOwnPtr<ScrollingStateNode> cloneAndResetNode(); Maybe we should also change the name; the trailing 'node' is redundant, and suggests that it should take a node parameter. > Source/WebCore/page/scrolling/ScrollingStateScrollingNode.h:44 > + ScrollingStateScrollingNode(ScrollingStateScrollingNode*); Is this a copy constructor? Those normally take const Foo&
Beth Dakin
Comment 3 2012-10-31 12:54:50 PDT
(In reply to comment #2) > (From update of attachment 171706 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=171706&action=review > > > Source/WebCore/page/scrolling/ScrollingStateNode.h:55 > > + PassOwnPtr<ScrollingStateNode> cloneAndResetNode(); > > Maybe we should also change the name; the trailing 'node' is redundant, and suggests that it should take a node parameter. > Good idea. Changed. > > Source/WebCore/page/scrolling/ScrollingStateScrollingNode.h:44 > > + ScrollingStateScrollingNode(ScrollingStateScrollingNode*); > > Is this a copy constructor? Those normally take const Foo& It is a copy constructor. I will change this too.
Beth Dakin
Comment 4 2012-10-31 13:12:15 PDT
Note You need to log in before you can comment on or make changes to this bug.