GTTF: Add missing virtual destructors.

cc/render_pass_sink.h

 // Copyright 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef CC_RENDER_PASS_SINK_H_
 #define CC_RENDER_PASS_SINK_H_
 
 #include "base/memory/scoped_ptr.h"
 #include "cc/cc_export.h"
 
 namespace cc {
 
 class RenderPass;
 
 class CC_EXPORT RenderPassSink {
 public:
     virtual void appendRenderPass(scoped_ptr<RenderPass>) = 0;
+protected:

[jar (doing other things), 2013/01/28 17:02:58]

nit: Add white space line before line 18.  .... if we're following Chrome style

+    virtual ~RenderPassSink() { }

[jamesr, 2013/01/28 19:20:43]

not sure this one is really necessary. nobody takes ownership of a
RenderPassSink via the base class and the derived class doesn't need a virtual
destructor

[Paweł Hajdan Jr., 2013/01/28 19:55:49]

That's why it's protected. This seems to be a convention used when no one
deletes the object via pointer to the base type. This will allow enabling a
warning in gcc about missing virtual dtors (see bug), and that warning triggers
here (same thing about ownership also applies to most cases in this CL, since
most of them are protected and I think just one is public).

[jamesr, 2013/01/28 20:13:46]

Protected and non-virtual seems like the right choice here

[Paweł Hajdan Jr., 2013/01/29 09:40:22]

Alright, so we have agreed on protected. Good. :)

Now why non-virtual? The deriving class should have a virtual dtor anyway,
otherwise it's asking for trouble. You may still get "correct" code by making
sure you always invoke the right dtor by using the same pointer type as the
actual class allocated, but this seems pretty fragile. Refactoring, new derived
class, many things looking innocent can invalidate such assumptions.

A virtual dtor here forces any derived class to also have a virtual dtor. This
makes it much less error-prone. The class has a vtable already anyway.

Do you still think it's important to make this dtor non-virtual? If so, why? For
me the ease of misuse is a much bigger issue here, but I'm open for arguments
why a non-virtual dtor would be better. I can definitely learn more about C++.

[jar (doing other things), 2013/01/29 17:58:22]

Simple rule of thumb: If you have any virtual functions, you should have a
virtual destructor.  No one should argue (and I don't think anyone is).

On 2013/01/29 09:40:22, Paweł Hajdan Jr. wrote:

[jamesr, 2013/01/29 18:52:20]

Actually, I think this file is perfectly fine as-is.  What's the motivation for
changing it at all?

[Paweł Hajdan Jr., 2013/01/30 10:00:28]

Please see the bug and earlier comments. I removed it from this CL for now.

 };
 
 }
 #endif  // CC_RENDER_PASS_SINK_H_