Give classes with virtual methods virtual protected destructors instead of implicit non-virtual public destructors.

Was originally:
Replace public nonvirtual destructors in classes with virtual members
with protected nonvirtual destructors where possible, and with
public virtual destructors where destruction of a derived class occurs.
(Excluding chrome/browser/...)

(Part 4 of http://www.gotw.ca/publications/mill18.htm
has a rationale for why public nonvirtual destructors in classes with
virtual members is dangerous.)

Patch by: Jacob Mandelson (jlmjln@gmail.com)
BUG=none
TEST=base_unittests & app_unittests

Review URL: http://codereview.chromium.org/200106

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@34633 0039d316-1c4b-4281-b951-d872f2087c98

22 files changed, 88 insertions, 22 deletions

diff --git a/app/animation.h b/app/animation.h
index e6877b6..bd38fb4 100644
--- a/app/animation.h
+++ b/app/animation.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2006-2008 The Chromium Authors. All rights reserved.
+// Copyright (c) 2009 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.
 // Inspired by NSAnimation
@@ -33,6 +33,9 @@ class AnimationDelegate {
   // Called when an animation has been canceled.
   virtual void AnimationCanceled(const Animation* animation) {
   }
+
+ protected:
+  virtual ~AnimationDelegate() {}
 };
 
 // Animation
diff --git a/app/table_model.h b/app/table_model.h
index c1e61b1..02d86dd 100644
--- a/app/table_model.h
+++ b/app/table_model.h
@@ -94,6 +94,8 @@ class TableModel {
   void ClearCollator();
 
  protected:
+  virtual ~TableModel() {}
+
   // Returns the collator used by CompareValues.
   icu::Collator* GetCollator();
 };
diff --git a/app/table_model_observer.h b/app/table_model_observer.h
index fcd50f4..cbf0d5d 100644
--- a/app/table_model_observer.h
+++ b/app/table_model_observer.h
@@ -20,6 +20,9 @@ class TableModelObserver {
 
   // Invoked when a range of items has been removed.
   virtual void OnItemsRemoved(int start, int length) = 0;
+
+ protected:
+  virtual ~TableModelObserver() {}
 };
 
 #endif  // APP_TABLE_MODEL_OBSERVER_H_
diff --git a/app/tree_model.h b/app/tree_model.h
index 882c82f..86d6424 100644
--- a/app/tree_model.h
+++ b/app/tree_model.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2008 The Chromium Authors. All rights reserved.
+// Copyright (c) 2009 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.
 
@@ -21,6 +21,9 @@ class TreeModelNode {
  public:
   // Returns the title for the node.
   virtual const std::wstring& GetTitle() const = 0;
+
+ protected:
+  virtual ~TreeModelNode() {}
 };
 
 // Observer for the TreeModel. Notified of significant events to the model.
@@ -45,6 +48,9 @@ class TreeModelObserver {
 
   // Notification that the contents of a node has changed.
   virtual void TreeNodeChanged(TreeModel* model, TreeModelNode* node) = 0;
+
+ protected:
+  virtual ~TreeModelObserver() {}
 };
 
 // TreeModel ------------------------------------------------------------------
@@ -86,6 +92,9 @@ class TreeModel {
   // default icon. The index is relative to the list of icons returned from
   // GetIcons.
   virtual int GetIconIndex(TreeModelNode* node) { return -1; }
+
+ protected:
+  virtual ~TreeModel() {}
 };
 
 #endif  // APP_TREE_MODEL_H_
diff --git a/base/file_descriptor_shuffle.h b/base/file_descriptor_shuffle.h
index 8ee77409..2592b80 100644
--- a/base/file_descriptor_shuffle.h
+++ b/base/file_descriptor_shuffle.h
@@ -37,6 +37,9 @@ class InjectionDelegate {
   virtual bool Move(int src, int dest) = 0;
   // Delete an element of the domain.
   virtual void Close(int fd) = 0;
+
+ protected:
+  virtual ~InjectionDelegate() {}
 };
 
 // An implementation of the InjectionDelegate interface using the file
diff --git a/base/task.h b/base/task.h
index 7c2f393..d71df20 100644
--- a/base/task.h
+++ b/base/task.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2006-2008 The Chromium Authors. All rights reserved.
+// Copyright (c) 2009 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.
 
@@ -666,6 +666,7 @@ template <typename ReturnValue>
 struct CallbackWithReturnValue {
   class Type {
    public:
+    virtual ~Type() {}
     virtual ReturnValue Run() = 0;
   };
 };
@@ -681,6 +682,9 @@ class CallbackWithReturnValueImpl
   virtual ReturnValue Run() {
     return (this->obj_->*(this->meth_))();
   }
+
+ protected:
+  virtual ~CallbackWithReturnValueImpl() {}
 };
 
 template <class T, typename ReturnValue>
diff --git a/base/waitable_event.h b/base/waitable_event.h
index 31aa085..8f5962f 100644
--- a/base/waitable_event.h
+++ b/base/waitable_event.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2006-2008 The Chromium Authors. All rights reserved.
+// Copyright (c) 2009 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.
 
@@ -121,6 +121,9 @@ class WaitableEvent {
     // pointers match then this function is called as a final check. See the
     // comments in ~Handle for why.
     virtual bool Compare(void* tag) = 0;
+
+   protected:
+    virtual ~Waiter() {}
   };
 
  private:
diff --git a/chrome/common/gtk_tree.h b/chrome/common/gtk_tree.h
index b0759f2..2b2f893 100644
--- a/chrome/common/gtk_tree.h
+++ b/chrome/common/gtk_tree.h
@@ -55,6 +55,9 @@ class TableAdapter : public TableModelObserver {
     // after clearing the list store.  Can be overriden by the delegate if it
     // needs to do extra initialization before the list store is populated.
     virtual void OnModelChanged() {}
+
+   protected:
+    virtual ~Delegate() {}
   };
 
   // |table_model| may be NULL.
diff --git a/chrome/common/worker_thread_ticker.h b/chrome/common/worker_thread_ticker.h
index bbc5380..bb39c3b 100644
--- a/chrome/common/worker_thread_ticker.h
+++ b/chrome/common/worker_thread_ticker.h
@@ -1,9 +1,9 @@
-// Copyright (c) 2006-2008 The Chromium Authors. All rights reserved.
+// Copyright (c) 2009 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 CHROME_COMMON_WORKER_THREAD_TICKER_H__
-#define CHROME_COMMON_WORKER_THREAD_TICKER_H__
+#ifndef CHROME_COMMON_WORKER_THREAD_TICKER_H_
+#define CHROME_COMMON_WORKER_THREAD_TICKER_H_
 
 #include <vector>
 
@@ -24,6 +24,9 @@ class WorkerThreadTicker {
    public:
     // Gets invoked when the timer period is up
     virtual void OnTick() = 0;
+
+   protected:
+    virtual ~Callback() {}
   };
 
   // tick_interval is the periodic interval in which to invoke the
@@ -84,4 +87,4 @@ class WorkerThreadTicker {
   DISALLOW_COPY_AND_ASSIGN(WorkerThreadTicker);
 };
 
-#endif  // CHROME_COMMON_WORKER_THREAD_TICKER_H__
+#endif  // CHROME_COMMON_WORKER_THREAD_TICKER_H_
diff --git a/chrome/common/x11_util.h b/chrome/common/x11_util.h
index aed7bad..3f2a92f 100644
--- a/chrome/common/x11_util.h
+++ b/chrome/common/x11_util.h
@@ -83,6 +83,9 @@ class EnumerateWindowsDelegate {
   // |xid| is the X Window ID of the enumerated window.  Return true to stop
   // further iteration.
   virtual bool ShouldStopIterating(XID xid) = 0;
+
+ protected:
+  virtual ~EnumerateWindowsDelegate() {}
 };
 
 // Enumerates all windows in the current display.  Will recurse into child
diff --git a/chrome/renderer/audio_message_filter.h b/chrome/renderer/audio_message_filter.h
index 9a21046..8d1a0b5 100644
--- a/chrome/renderer/audio_message_filter.h
+++ b/chrome/renderer/audio_message_filter.h
@@ -32,6 +32,9 @@ class AudioMessageFilter : public IPC::ChannelProxy::MessageFilter {
     // Called when notification of stream volume is received from the browser
     // process.
     virtual void OnVolume(double volume) = 0;
+
+   protected:
+    virtual ~Delegate() {}
   };
 
   explicit AudioMessageFilter(int32 route_id);
diff --git a/chrome/test/automation/tab_proxy.h b/chrome/test/automation/tab_proxy.h
index 61ab571..b811b70 100644
--- a/chrome/test/automation/tab_proxy.h
+++ b/chrome/test/automation/tab_proxy.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2006-2008 The Chromium Authors. All rights reserved.
+// Copyright (c) 2009 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.
 
@@ -44,6 +44,9 @@ class TabProxy : public AutomationResourceProxy {
   class TabProxyDelegate {
    public:
     virtual void OnMessageReceived(TabProxy* tab, const IPC::Message& msg) {}
+
+   protected:
+    virtual ~TabProxyDelegate() {}
   };
 
   TabProxy(AutomationMessageSender* sender,
diff --git a/ipc/ipc_logging.h b/ipc/ipc_logging.h
index ad0559b..271b2a1 100644
--- a/ipc/ipc_logging.h
+++ b/ipc/ipc_logging.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2006-2008 The Chromium Authors. All rights reserved.
+// Copyright (c) 2009 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.
 
@@ -29,6 +29,9 @@ class Logging {
   class Consumer {
    public:
     virtual void Log(const LogData& data) = 0;
+
+   protected:
+    virtual ~Consumer() {}
   };
 
   void SetConsumer(Consumer* consumer);
diff --git a/ipc/ipc_sync_message.h b/ipc/ipc_sync_message.h
index 37c6c71..48e9f99 100644
--- a/ipc/ipc_sync_message.h
+++ b/ipc/ipc_sync_message.h
@@ -1,9 +1,9 @@
-// Copyright (c) 2006-2008 The Chromium Authors. All rights reserved.
+// Copyright (c) 2009 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 IPC_IPC_SYNC_MESSAGE_H__
-#define IPC_IPC_SYNC_MESSAGE_H__
+#ifndef IPC_IPC_SYNC_MESSAGE_H_
+#define IPC_IPC_SYNC_MESSAGE_H_
 
 #if defined(OS_WIN)
 #include <windows.h>
@@ -84,6 +84,7 @@ class SyncMessage : public Message {
 // Used to deserialize parameters from a reply to a synchronous message
 class MessageReplyDeserializer {
  public:
+  virtual ~MessageReplyDeserializer() {}
   bool SerializeOutputParameters(const Message& msg);
  private:
   // Derived classes need to implement this, using the given iterator (which
@@ -93,4 +94,4 @@ class MessageReplyDeserializer {
 
 }  // namespace IPC
 
-#endif  // IPC_IPC_SYNC_MESSAGE_H__
+#endif  // IPC_IPC_SYNC_MESSAGE_H_
diff --git a/media/base/clock.h b/media/base/clock.h
index 93af4a9..f586b63 100644
--- a/media/base/clock.h
+++ b/media/base/clock.h
@@ -40,6 +40,9 @@ class Clock {
 
   // Returns the current elapsed media time.
   virtual base::TimeDelta Elapsed() const = 0;
+
+ protected:
+  virtual ~Clock() {}
 };
 
 }  // namespace media
diff --git a/net/base/directory_lister.h b/net/base/directory_lister.h
index 609a087..541741e 100644
--- a/net/base/directory_lister.h
+++ b/net/base/directory_lister.h
@@ -33,6 +33,9 @@ class DirectoryLister : public base::RefCountedThreadSafe<DirectoryLister>,
     virtual void OnListFile(
         const file_util::FileEnumerator::FindInfo& data) = 0;
     virtual void OnListDone(int error) = 0;
+
+   protected:
+    virtual ~DirectoryListerDelegate() {}
   };
 
   DirectoryLister(const FilePath& dir, DirectoryListerDelegate* delegate);
diff --git a/net/socket/client_socket_pool_base.h b/net/socket/client_socket_pool_base.h
index cfad0a0..897ffec 100644
--- a/net/socket/client_socket_pool_base.h
+++ b/net/socket/client_socket_pool_base.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2006-2008 The Chromium Authors. All rights reserved.
+// Copyright (c) 2009 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.
 //
@@ -451,7 +451,7 @@ class ClientSocketPoolBase {
           unused_idle_socket_timeout, used_idle_socket_timeout,
           new ConnectJobFactoryAdaptor(connect_job_factory))) {}
 
-  ~ClientSocketPoolBase() {}
+  virtual ~ClientSocketPoolBase() {}
 
   // These member functions simply forward to ClientSocketPoolBaseHelper.
 
diff --git a/net/tools/fetch/http_listen_socket.h b/net/tools/fetch/http_listen_socket.h
index bb6d383..73f09f6 100644
--- a/net/tools/fetch/http_listen_socket.h
+++ b/net/tools/fetch/http_listen_socket.h
@@ -19,6 +19,9 @@ class HttpListenSocket : public ListenSocket,
    public:
     virtual void OnRequest(HttpListenSocket* connection,
                            HttpServerRequestInfo* info) = 0;
+
+   protected:
+    virtual ~Delegate() {}
   };
 
   static HttpListenSocket* Listen(const std::string& ip, int port,
diff --git a/net/url_request/url_request_unittest.h b/net/url_request/url_request_unittest.h
index b4a17ca..649aa37 100644
--- a/net/url_request/url_request_unittest.h
+++ b/net/url_request/url_request_unittest.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2006-2008 The Chromium Authors. All rights reserved.
+// Copyright (c) 2009 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.
 
@@ -240,9 +240,9 @@ class TestDelegate : public URLRequest::Delegate {
 // that can provide various responses useful for testing.
 class BaseTestServer : public base::RefCounted<BaseTestServer> {
  protected:
-  BaseTestServer() { }
+  BaseTestServer() {}
   BaseTestServer(int connection_attempts, int connection_timeout)
-      : launcher_(connection_attempts, connection_timeout) { }
+      : launcher_(connection_attempts, connection_timeout) {}
 
  public:
   void set_forking(bool forking) {
diff --git a/views/accelerator.h b/views/accelerator.h
index a8d4c16..82be336 100644
--- a/views/accelerator.h
+++ b/views/accelerator.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2006-2008 The Chromium Authors. All rights reserved.
+// Copyright (c) 2009 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.
 
@@ -32,6 +32,8 @@ class Accelerator : public menus::Accelerator {
       modifiers_ |= Event::EF_ALT_DOWN;
   }
 
+  virtual ~Accelerator() {}
+
   bool IsShiftDown() const {
     return (modifiers_ & Event::EF_SHIFT_DOWN) == Event::EF_SHIFT_DOWN;
   }
@@ -54,6 +56,9 @@ class AcceleratorTarget {
  public:
   // This method should return true if the accelerator was processed.
   virtual bool AcceleratorPressed(const Accelerator& accelerator) = 0;
+
+ protected:
+  virtual ~AcceleratorTarget() {}
 };
 }
 
diff --git a/views/view.h b/views/view.h
index ef806ea..13a85ee 100644
--- a/views/view.h
+++ b/views/view.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2006-2008 The Chromium Authors. All rights reserved.
+// Copyright (c) 2009 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.
 
@@ -68,6 +68,9 @@ class ContextMenuController {
                                int x,
                                int y,
                                bool is_mouse_gesture) = 0;
+
+ protected:
+  virtual ~ContextMenuController() {}
 };
 
 // DragController is responsible for writing drag data for a view, as well as
@@ -94,6 +97,9 @@ class DragController {
                             int press_y,
                             int x,
                             int y) = 0;
+
+ protected:
+  virtual ~DragController() {}
 };
 
 /////////////////////////////////////////////////////////////////////////////
diff --git a/webkit/glue/devtools/devtools_rpc.h b/webkit/glue/devtools/devtools_rpc.h
index 2a2ef12..3e7a1b2 100644
--- a/webkit/glue/devtools/devtools_rpc.h
+++ b/webkit/glue/devtools/devtools_rpc.h
@@ -273,7 +273,7 @@ class Class : public Noncopyable {\
   Class() { \
     class_name = #Class; \
   } \
-  ~Class() {} \
+  virtual ~Class() {} \
   \
   STRUCT( \
       TOOLS_RPC_API_METHOD0, \