WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
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+
Details
Formatted Diff
Diff
View All
Add attachment
proposed patch, testcase, etc.
Beth Dakin
Comment 1
2012-10-31 12:45:09 PDT
Created
attachment 171706
[details]
Patch
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
http://trac.webkit.org/changeset/133067
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug