Created attachment 44215 [details] Patch Inherits the following class from Noncopyable because it is instantiated by 'new' and no need to be copyable: class/struct name - instantiated at: WebCore/'location' class ContextMenuClient - (its child class) svg/graphics/SVGImage.cpp:232 class DragClient - (its child class) svg/graphics/SVGImage.cpp:237 class ChromeClient - (its child class) svg/graphics/SVGImage.cpp:243
style-queue ran check-webkit-style on attachment 44215 [details] without any errors.
Comment on attachment 44215 [details] Patch I'm not sure this is right. 69 class ChromeClient : public Noncopyable { Why should these Abstract Base Classes require that their implementors be non-copyable?
(In reply to comment #2) > I'm not sure this is right. > > 69 class ChromeClient : public Noncopyable { > > Why should these Abstract Base Classes require that their implementors be > non-copyable? They should not. I think itβs better to not add any base classes like Noncopyable or FastAllocBase to these abstract base classes.
(In reply to comment #2) > (From update of attachment 44215 [details]) > I'm not sure this is right. > > 69 class ChromeClient : public Noncopyable { > > Why should these Abstract Base Classes require that their implementors be > non-copyable? In this case I can inherit SVGImageChromeClient Noncopyable, then ChromeClient don't need to be inherited. At the case of DragClient the instantiation line is: static DragClient* dummyDragClient = new EmptyDragClient; DragClient is abstract and EmptyDragClient is abstract as well. At the case of ContextMenuClient: static ContextMenuClient* dummyContextMenuClient = new EmptyContextMenuClient; The problem is the same as at DragClient.
(In reply to comment #4) > At the case of DragClient the instantiation line is: > static DragClient* dummyDragClient = new EmptyDragClient; > > DragClient is abstract and EmptyDragClient is abstract as well. That's incorrect. EmptyDragClient is not abstract -- if it was, the new would not compile. > At the case of ContextMenuClient: > static ContextMenuClient* dummyContextMenuClient = new EmptyContextMenuClient; > > The problem is the same as at DragClient. Same comment.
Created attachment 44670 [details] Patch
You're right! Sorry, I was on the QtWebKit summit, so I can the updated patch now.
style-queue ran check-webkit-style on attachment 44670 [details] without any errors.
Comment on attachment 44670 [details] Patch Clearing flags on attachment: 44670 Committed r51989: <http://trac.webkit.org/changeset/51989>
All reviewed patches have been landed. Closing bug.