Bug 164636 – Warning added in r208542

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Alejandro G. Castro]

We have to call the base class explicetely in the copy constructors.

[Alejandro G. Castro]

Created attachment 294485 [details]
Patch

[youenn fablet]

Comment on attachment 294485 [details]
Patch

Change is fine I guess but it seems a bit strange to have a copy constructor of a refcounted class.

It might be better to remove the copy constructor here.
Instead, we could have a create() that would take the four parameters (source, track, and the booleans) with default values if needed.
Then clone would use create() instead of the copy constructor.

[Alexey Proskuryakov]

I agree, this code smells wrong. When is the copy constructor used?

[Eric Carlson]

(In reply to comment #2)
> Comment on attachment 294485 [details]
> Patch
> 
> Change is fine I guess but it seems a bit strange to have a copy constructor
> of a refcounted class.
> 
> It might be better to remove the copy constructor here.
> Instead, we could have a create() that would take the four parameters
> (source, track, and the booleans) with default values if needed.
> Then clone would use create() instead of the copy constructor.

I agree, Youenn's suggestion makes much more sense.

[Alejandro G. Castro]

*** Bug 164688 has been marked as a duplicate of this bug. ***

[Alejandro G. Castro]

(In reply to comment #4)
> (In reply to comment #2)
> > Comment on attachment 294485 [details]
> > Patch
> > 
> > Change is fine I guess but it seems a bit strange to have a copy constructor
> > of a refcounted class.
> > 
> > It might be better to remove the copy constructor here.
> > Instead, we could have a create() that would take the four parameters
> > (source, track, and the booleans) with default values if needed.
> > Then clone would use create() instead of the copy constructor.
> 
> I agree, Youenn's suggestion makes much more sense.

Right, as ap was saying I'm not sure if this is even used, just fixing the refactoring problem added, I seemed fishy that is why I wanted the review for fixing the warning. I'll check it in more detail.

Thanks for the review.

[Alejandro G. Castro]

Created attachment 294832 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 294832 [details]
Patch

Clearing flags on attachment: 294832

Committed r208786: <http://trac.webkit.org/changeset/208786>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.