First patch in making destructors of refcounted objects private.

BUG=26749
Review URL: http://codereview.chromium.org/360042

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

24 files changed, 121 insertions, 38 deletions

diff --git a/app/gfx/font.h b/app/gfx/font.h
index 80a85ec..6be42ee 100644
--- a/app/gfx/font.h
+++ b/app/gfx/font.h
@@ -169,7 +169,6 @@ class Font {
              int ave_char_width,
              int style,
              int dlu_base_x);
-    ~HFontRef();
 
     // Accessors
     HFONT hfont() const { return hfont_; }
@@ -181,6 +180,10 @@ class Font {
     const std::wstring& font_name() const { return font_name_; }
 
    private:
+    friend class  base::RefCounted<HFontRef>;
+
+    ~HFontRef();
+
     const HFONT hfont_;
     const int height_;
     const int baseline_;
diff --git a/app/sql/connection.h b/app/sql/connection.h
index e87baa8..e7c2a2d 100644
--- a/app/sql/connection.h
+++ b/app/sql/connection.h
@@ -76,7 +76,6 @@ class Connection;
 // corruption, low-level IO errors or locking violations.
 class ErrorDelegate : public base::RefCounted<ErrorDelegate> {
  public:
-  virtual ~ErrorDelegate() {}
   // |error| is an sqlite result code as seen in sqlite\preprocessed\sqlite3.h
   // |connection| is db connection where the error happened and |stmt| is
   // our best guess at the statement that triggered the error.  Do not store
@@ -89,6 +88,11 @@ class ErrorDelegate : public base::RefCounted<ErrorDelegate> {
   // re-tried then returning SQLITE_OK is appropiate; otherwise is recomended
   // that you return the original |error| or the appropiae error code.
   virtual int OnError(int error, Connection* connection, Statement* stmt) = 0;
+
+ protected:
+  friend class base::RefCounted<ErrorDelegate>;
+
+  virtual ~ErrorDelegate() {}
 };
 
 class Connection {
@@ -286,7 +290,6 @@ class Connection {
     // Default constructor initializes to an invalid statement.
     StatementRef();
     StatementRef(Connection* connection, sqlite3_stmt* stmt);
-    ~StatementRef();
 
     // When true, the statement can be used.
     bool is_valid() const { return !!stmt_; }
@@ -304,6 +307,10 @@ class Connection {
     void Close();
 
    private:
+    friend class base::RefCounted<StatementRef>;
+
+    ~StatementRef();
+
     Connection* connection_;
     sqlite3_stmt* stmt_;
 
diff --git a/base/directory_watcher.h b/base/directory_watcher.h
index 2d5b901..207d6d2 100644
--- a/base/directory_watcher.h
+++ b/base/directory_watcher.h
@@ -46,9 +46,13 @@ class DirectoryWatcher {
   // Used internally to encapsulate different members on different platforms.
   class PlatformDelegate : public base::RefCounted<PlatformDelegate> {
    public:
-    virtual ~PlatformDelegate() {}
     virtual bool Watch(const FilePath& path, Delegate* delegate,
                        MessageLoop* backend_loop, bool recursive) = 0;
+
+   protected:
+    friend class base::RefCounted<PlatformDelegate>;
+
+    virtual ~PlatformDelegate() {}
   };
 
  private:
diff --git a/base/directory_watcher_win.cc b/base/directory_watcher_win.cc
index d97fca4c..e318d4b 100644
--- a/base/directory_watcher_win.cc
+++ b/base/directory_watcher_win.cc
@@ -15,7 +15,6 @@ class DirectoryWatcherImpl : public DirectoryWatcher::PlatformDelegate,
                              public base::ObjectWatcher::Delegate {
  public:
   DirectoryWatcherImpl() : delegate_(NULL), handle_(INVALID_HANDLE_VALUE) {}
-  virtual ~DirectoryWatcherImpl();
 
   virtual bool Watch(const FilePath& path, DirectoryWatcher::Delegate* delegate,
                      MessageLoop* backend_loop, bool recursive);
@@ -24,6 +23,8 @@ class DirectoryWatcherImpl : public DirectoryWatcher::PlatformDelegate,
   virtual void OnObjectSignaled(HANDLE object);
 
  private:
+  virtual ~DirectoryWatcherImpl();
+
   // Delegate to notify upon changes.
   DirectoryWatcher::Delegate* delegate_;
   // Path we're watching (passed to delegate).
diff --git a/base/field_trial.h b/base/field_trial.h
index 0779981..1248e83 100644
--- a/base/field_trial.h
+++ b/base/field_trial.h
@@ -116,6 +116,10 @@ class FieldTrial : public base::RefCounted<FieldTrial> {
                               const std::string& trial_name);
 
  private:
+  friend class base::RefCounted<FieldTrial>;
+
+  ~FieldTrial() {}
+
   // The name of the field trial, as can be found via the FieldTrialList.
   // This is empty of the trial is not in the experiment.
   const std::string name_;
diff --git a/base/message_loop_unittest.cc b/base/message_loop_unittest.cc
index d0fafe3..16d1c1a 100644
--- a/base/message_loop_unittest.cc
+++ b/base/message_loop_unittest.cc
@@ -67,6 +67,10 @@ class Foo : public base::RefCounted<Foo> {
   const std::string& result() const { return result_; }
 
  private:
+  friend class base::RefCounted<Foo>;
+
+  ~Foo() {}
+
   int test_count_;
   std::string result_;
 };
@@ -76,6 +80,11 @@ class QuitMsgLoop : public base::RefCounted<QuitMsgLoop> {
   void QuitNow() {
     MessageLoop::current()->Quit();
   }
+
+ private:
+  friend class base::RefCounted<QuitMsgLoop>;
+
+  ~QuitMsgLoop() {}
 };
 
 void RunTest_PostTask(MessageLoop::Type message_loop_type) {
diff --git a/base/message_pump_glib_unittest.cc b/base/message_pump_glib_unittest.cc
index e3a04e8..d742055 100644
--- a/base/message_pump_glib_unittest.cc
+++ b/base/message_pump_glib_unittest.cc
@@ -330,6 +330,10 @@ class ConcurrentHelper : public base::RefCounted<ConcurrentHelper>  {
   int task_count() const { return task_count_; }
 
  private:
+  friend class base::RefCounted<ConcurrentHelper>;
+
+  ~ConcurrentHelper() {}
+
   static const int kStartingEventCount = 20;
   static const int kStartingTaskCount = 20;
 
@@ -455,6 +459,10 @@ class GLibLoopRunner : public base::RefCounted<GLibLoopRunner> {
   }
 
  private:
+  friend class base::RefCounted<GLibLoopRunner>;
+
+  ~GLibLoopRunner() {}
+
   bool quit_;
 };
 
diff --git a/base/ref_counted.h b/base/ref_counted.h
index ea532a4..f2e4327 100644
--- a/base/ref_counted.h
+++ b/base/ref_counted.h
@@ -71,8 +71,13 @@ class RefCountedThreadSafeBase {
 //
 //   class MyFoo : public base::RefCounted<MyFoo> {
 //    ...
+//    private:
+//     friend class base::RefCounted<MyFoo>;
+//     ~MyFoo();
 //   };
 //
+// You should always make your destructor private, to avoid any code deleting
+// the object accidently while there are references to it.
 template <class T>
 class RefCounted : public subtle::RefCountedBase {
  public:
@@ -115,10 +120,10 @@ struct DefaultRefCountedThreadSafeTraits {
 //    ...
 //   };
 //
-// If you're using the default trait, then you may choose to add compile time
+// If you're using the default trait, then you should add compile time
 // asserts that no one else is deleting your object.  i.e.
 //    private:
-//     friend struct base::RefCountedThreadSafe<MyFoo>;
+//     friend class base::RefCountedThreadSafe<MyFoo>;
 //     ~MyFoo();
 template <class T, typename Traits = DefaultRefCountedThreadSafeTraits<T> >
 class RefCountedThreadSafe : public subtle::RefCountedThreadSafeBase {
diff --git a/base/ref_counted_memory.h b/base/ref_counted_memory.h
index 30d2917..eae7984 100644
--- a/base/ref_counted_memory.h
+++ b/base/ref_counted_memory.h
@@ -15,16 +15,19 @@
 // A generic interface to memory. This object is reference counted because one
 // of its two subclasses own the data they carry, and we need to have
 // heterogeneous containers of these two types of memory.
-class RefCountedMemory : public base::RefCountedThreadSafe< RefCountedMemory > {
+class RefCountedMemory : public base::RefCountedThreadSafe<RefCountedMemory> {
  public:
-  virtual ~RefCountedMemory() {}
-
   // Retrieves a pointer to the beginning of the data we point to. If the data
   // is empty, this will return NULL.
   virtual const unsigned char* front() const = 0;
 
   // Size of the memory pointed to.
   virtual size_t size() const = 0;
+
+ protected:
+  friend class base::RefCountedThreadSafe<RefCountedMemory>;
+
+  virtual ~RefCountedMemory() {}
 };
 
 // An implementation of RefCountedMemory, where the ref counting does not
diff --git a/base/ref_counted_unittest.cc b/base/ref_counted_unittest.cc
index ba392d6..f2739fce 100644
--- a/base/ref_counted_unittest.cc
+++ b/base/ref_counted_unittest.cc
@@ -8,6 +8,9 @@
 namespace {
 
 class SelfAssign : public base::RefCounted<SelfAssign> {
+  friend class base::RefCounted<SelfAssign>;
+
+  ~SelfAssign() {}
 };
 
 class CheckDerivedMemberAccess : public scoped_refptr<SelfAssign> {
diff --git a/base/stack_container_unittest.cc b/base/stack_container_unittest.cc
index 3085d0f..dd741bb 100644
--- a/base/stack_container_unittest.cc
+++ b/base/stack_container_unittest.cc
@@ -16,10 +16,14 @@ class Dummy : public base::RefCounted<Dummy> {
   Dummy(int* alive) : alive_(alive) {
     ++*alive_;
   }
+
+ private:
+  friend class base::RefCounted<Dummy>;
+
   ~Dummy() {
     --*alive_;
   }
- private:
+
   int* const alive_;
 };
 
diff --git a/ipc/file_descriptor_set_posix.h b/ipc/file_descriptor_set_posix.h
index c3d26ba8..7653eb6 100644
--- a/ipc/file_descriptor_set_posix.h
+++ b/ipc/file_descriptor_set_posix.h
@@ -19,7 +19,6 @@
 class FileDescriptorSet : public base::RefCountedThreadSafe<FileDescriptorSet> {
  public:
   FileDescriptorSet();
-  ~FileDescriptorSet();
 
   // This is the maximum number of descriptors per message. We need to know this
   // because the control message kernel interface has to be given a buffer which
@@ -90,6 +89,10 @@ class FileDescriptorSet : public base::RefCountedThreadSafe<FileDescriptorSet> {
   // ---------------------------------------------------------------------------
 
  private:
+  friend class base::RefCountedThreadSafe<FileDescriptorSet>;
+
+  ~FileDescriptorSet();
+
   // A vector of descriptors and close flags. If this message is sent, then
   // these descriptors are sent as control data. After sending, any descriptors
   // with a true flag are closed. If this message has been received, then these
diff --git a/ipc/ipc_channel_proxy.cc b/ipc/ipc_channel_proxy.cc
index 4d13b2b8..cbe8cc2 100644
--- a/ipc/ipc_channel_proxy.cc
+++ b/ipc/ipc_channel_proxy.cc
@@ -10,12 +10,6 @@
 
 namespace IPC {
 
-// static
-void ChannelProxy::MessageFilterTraits::Destruct(
-    ChannelProxy::MessageFilter* filter) {
-  filter->OnDestruct();
-}
-
 //------------------------------------------------------------------------------
 
 // This task ensures the message is deleted if the task is deleted without
diff --git a/ipc/ipc_channel_proxy.h b/ipc/ipc_channel_proxy.h
index 3369f50..aee6dae 100644
--- a/ipc/ipc_channel_proxy.h
+++ b/ipc/ipc_channel_proxy.h
@@ -47,10 +47,7 @@ class SendTask;
 class ChannelProxy : public Message::Sender {
  public:
 
-  class MessageFilter;
-  struct MessageFilterTraits {
-    static void Destruct(MessageFilter* filter);
-  };
+  struct MessageFilterTraits;
 
   // A class that receives messages on the thread where the IPC channel is
   // running.  It can choose to prevent the default action for an IPC message.
@@ -95,6 +92,12 @@ class ChannelProxy : public Message::Sender {
     }
   };
 
+  struct MessageFilterTraits {
+    static void Destruct(MessageFilter* filter) {
+      filter->OnDestruct();
+    }
+  };
+
   // Initializes a channel proxy.  The channel_id and mode parameters are
   // passed directly to the underlying IPC::Channel.  The listener is called on
   // the thread that creates the ChannelProxy.  The filter's OnMessageReceived
@@ -158,7 +161,6 @@ class ChannelProxy : public Message::Sender {
    public:
     Context(Channel::Listener* listener, MessageFilter* filter,
             MessageLoop* ipc_thread);
-    virtual ~Context() { }
     void ClearIPCMessageLoop() { ipc_message_loop_ = NULL; }
     MessageLoop* ipc_message_loop() const { return ipc_message_loop_; }
     const std::string& channel_id() const { return channel_id_; }
@@ -167,6 +169,9 @@ class ChannelProxy : public Message::Sender {
     void OnDispatchMessage(const Message& message);
 
    protected:
+    friend class base::RefCountedThreadSafe<Context>;
+    virtual ~Context() { }
+
     // IPC::Channel::Listener methods:
     virtual void OnMessageReceived(const Message& message);
     virtual void OnChannelConnected(int32 peer_pid);
@@ -191,6 +196,7 @@ class ChannelProxy : public Message::Sender {
    private:
     friend class ChannelProxy;
     friend class SendTask;
+
     // Create the Channel
     void CreateChannel(const std::string& id, const Channel::Mode& mode);
 
diff --git a/ipc/ipc_sync_channel.cc b/ipc/ipc_sync_channel.cc
index 784b835..d34ea6f 100644
--- a/ipc/ipc_sync_channel.cc
+++ b/ipc/ipc_sync_channel.cc
@@ -51,9 +51,6 @@ class SyncChannel::ReceivedSyncMsgQueue :
     return rv;
   }
 
-  ~ReceivedSyncMsgQueue() {
-  }
-
   // Called on IPC thread when a synchronous message or reply arrives.
   void QueueMessage(const Message& msg, SyncChannel::SyncContext* context) {
     bool was_task_pending;
@@ -157,6 +154,8 @@ class SyncChannel::ReceivedSyncMsgQueue :
   }
 
  private:
+  friend class base::RefCountedThreadSafe<ReceivedSyncMsgQueue>;
+
   // See the comment in SyncChannel::SyncChannel for why this event is created
   // as manual reset.
   ReceivedSyncMsgQueue() :
@@ -167,6 +166,8 @@ class SyncChannel::ReceivedSyncMsgQueue :
       top_send_done_watcher_(NULL) {
   }
 
+  ~ReceivedSyncMsgQueue() {}
+
   // Holds information about a queued synchronous message or reply.
   struct QueuedMessage {
     QueuedMessage(Message* m, SyncContext* c) : message(m), context(c) { }
diff --git a/ipc/ipc_sync_channel.h b/ipc/ipc_sync_channel.h
index 1c7360d..cde293a58 100644
--- a/ipc/ipc_sync_channel.h
+++ b/ipc/ipc_sync_channel.h
@@ -61,8 +61,6 @@ class SyncChannel : public ChannelProxy,
                 MessageLoop* ipc_thread,
                 base::WaitableEvent* shutdown_event);
 
-    ~SyncContext();
-
     // Adds information about an outgoing sync message to the context so that
     // we know how to deserialize the reply.
     void Push(IPC::SyncMessage* sync_msg);
@@ -97,10 +95,11 @@ class SyncChannel : public ChannelProxy,
     }
 
    private:
+    ~SyncContext();
     // IPC::ChannelProxy methods that we override.
 
     // Called on the listener thread.
-   virtual void Clear();
+    virtual void Clear();
 
     // Called on the IPC thread.
     virtual void OnMessageReceived(const Message& msg);
diff --git a/media/audio/linux/alsa_output.h b/media/audio/linux/alsa_output.h
index 58d9ea0..8aa0af3 100644
--- a/media/audio/linux/alsa_output.h
+++ b/media/audio/linux/alsa_output.h
@@ -75,7 +75,6 @@ class AlsaPcmOutputStream :
                       AlsaWrapper* wrapper,
                       AudioManagerLinux* manager,
                       MessageLoop* message_loop);
-  virtual ~AlsaPcmOutputStream();
 
   // Implementation of AudioOutputStream.
   virtual bool Open(size_t packet_size);
@@ -86,6 +85,7 @@ class AlsaPcmOutputStream :
   virtual void GetVolume(double* volume);
 
  private:
+  friend class base::RefCountedThreadSafe<AlsaPcmOutputStream>;
   friend class AlsaPcmOutputStreamTest;
   FRIEND_TEST(AlsaPcmOutputStreamTest, AutoSelectDevice_DeviceSelect);
   FRIEND_TEST(AlsaPcmOutputStreamTest, AutoSelectDevice_FallbackDevices);
@@ -106,6 +106,8 @@ class AlsaPcmOutputStream :
   FRIEND_TEST(AlsaPcmOutputStreamTest, WritePacket_StopStream);
   FRIEND_TEST(AlsaPcmOutputStreamTest, WritePacket_WriteFails);
 
+  virtual ~AlsaPcmOutputStream();
+
   // In-memory buffer to hold sound samples before pushing to the sound device.
   // TODO(ajwong): There are now about 3 buffer/packet implementations. Factor
   // them out.
diff --git a/media/base/buffers.h b/media/base/buffers.h
index 27ad130..6102ea9 100644
--- a/media/base/buffers.h
+++ b/media/base/buffers.h
@@ -100,6 +100,9 @@ class Buffer : public StreamSample {
 
   // If there's no data in this buffer, it represents end of stream.
   virtual bool IsEndOfStream() const { return GetData() == NULL; }
+
+ protected:
+  virtual ~Buffer() {}
 };
 
 
@@ -114,6 +117,9 @@ class WritableBuffer : public Buffer  {
 
   // Returns the size of the underlying buffer.
   virtual size_t GetBufferSize() const = 0;
+
+ protected:
+  virtual ~WritableBuffer() {}
 };
 
 
diff --git a/media/base/factory.h b/media/base/factory.h
index 3ccfcb1..ade8990 100644
--- a/media/base/factory.h
+++ b/media/base/factory.h
@@ -93,6 +93,8 @@ class FilterFactoryCollection : public FilterFactory {
   }
 
  private:
+  ~FilterFactoryCollection() {}
+
   typedef std::vector< scoped_refptr<FilterFactory> > FactoryVector;
   FactoryVector factories_;
 
@@ -123,6 +125,8 @@ class FilterFactoryImpl0 : public FilterFactory {
   }
 
  private:
+  ~FilterFactoryImpl0() {}
+
   DISALLOW_COPY_AND_ASSIGN(FilterFactoryImpl0);
 };
 
@@ -152,6 +156,8 @@ class FilterFactoryImpl1 : public FilterFactory {
   }
 
  private:
+   ~FilterFactoryImpl1() {}
+
   A const a_;
   DISALLOW_COPY_AND_ASSIGN(FilterFactoryImpl1);
 };
@@ -173,6 +179,8 @@ class FilterFactoryImpl2 : public FilterFactory {
   }
 
  private:
+  ~FilterFactoryImpl2() {}
+
   A const a_;
   B const b_;
 
@@ -214,6 +222,8 @@ class InstanceFilterFactory : public FilterFactory {
   }
 
  private:
+  ~InstanceFilterFactory() {}
+
   scoped_refptr<Filter> filter_;
   bool create_called_;
 
diff --git a/media/filters/ffmpeg_demuxer.h b/media/filters/ffmpeg_demuxer.h
index 7ab9449..f6af38c 100644
--- a/media/filters/ffmpeg_demuxer.h
+++ b/media/filters/ffmpeg_demuxer.h
@@ -54,8 +54,6 @@ class FFmpegDemuxerStream : public DemuxerStream, public AVStreamProvider {
   // inside |stream|.
   FFmpegDemuxerStream(FFmpegDemuxer* demuxer, AVStream* stream);
 
-  virtual ~FFmpegDemuxerStream();
-
   // Returns true is this stream has pending reads, false otherwise.
   //
   // Safe to call on any thread.
@@ -85,6 +83,8 @@ class FFmpegDemuxerStream : public DemuxerStream, public AVStreamProvider {
   virtual void* QueryInterface(const char* interface_id);
 
  private:
+  virtual ~FFmpegDemuxerStream();
+
   // Carries out enqueuing a pending read on the demuxer thread.
   void ReadTask(Callback1<Buffer*>::Type* read_callback);
 
diff --git a/printing/printed_document.h b/printing/printed_document.h
index 6e20d33..c7d806c 100644
--- a/printing/printed_document.h
+++ b/printing/printed_document.h
@@ -39,7 +39,6 @@ class PrintedDocument : public base::RefCountedThreadSafe<PrintedDocument> {
   PrintedDocument(const PrintSettings& settings,
                   PrintedPagesSource* source,
                   int cookie);
-  ~PrintedDocument();
 
   // Sets a page's data. 0-based. Takes metafile ownership.
   // Note: locks for a short amount of time.
@@ -106,6 +105,10 @@ class PrintedDocument : public base::RefCountedThreadSafe<PrintedDocument> {
   static const std::wstring& debug_dump_path();
 
  private:
+  friend class base::RefCountedThreadSafe<PrintedDocument>;
+
+  ~PrintedDocument();
+
   // Array of data for each print previewed page.
   typedef std::map<int, scoped_refptr<PrintedPage> > PrintedPages;
 
diff --git a/printing/printed_page.h b/printing/printed_page.h
index 3c954ac..101be9a 100644
--- a/printing/printed_page.h
+++ b/printing/printed_page.h
@@ -24,7 +24,6 @@ class PrintedPage : public base::RefCountedThreadSafe<PrintedPage> {
   PrintedPage(int page_number,
               NativeMetafile* native_metafile,
               const gfx::Size& page_size);
-  ~PrintedPage();
 
   // Getters
   int page_number() const { return page_number_; }
@@ -32,6 +31,10 @@ class PrintedPage : public base::RefCountedThreadSafe<PrintedPage> {
   const gfx::Size& page_size() const { return page_size_; }
 
  private:
+  friend class base::RefCountedThreadSafe<PrintedPage>;
+
+  ~PrintedPage();
+
   // Page number inside the printed document.
   const int page_number_;
 
diff --git a/skia/ext/bitmap_platform_device_linux.cc b/skia/ext/bitmap_platform_device_linux.cc
index a74ec83..b53b1b3 100644
--- a/skia/ext/bitmap_platform_device_linux.cc
+++ b/skia/ext/bitmap_platform_device_linux.cc
@@ -55,10 +55,6 @@ class BitmapPlatformDevice::BitmapPlatformDeviceData
   void SetMatrixClip(const SkMatrix& transform, const SkRegion& region);
 
  protected:
-  friend class base::RefCounted<BitmapPlatformDeviceData>;
-
-  ~BitmapPlatformDeviceData();
-
   void LoadConfig();
 
   // The Cairo surface inside this DC.
@@ -82,6 +78,11 @@ class BitmapPlatformDevice::BitmapPlatformDeviceData
   BitmapPlatformDeviceData(const BitmapPlatformDeviceData&);
   BitmapPlatformDeviceData& operator=(
       const BitmapPlatformDeviceData&);
+
+ private:
+  friend class base::RefCounted<BitmapPlatformDeviceData>;
+
+  ~BitmapPlatformDeviceData();
 };
 
 BitmapPlatformDevice::BitmapPlatformDeviceData::BitmapPlatformDeviceData(
diff --git a/views/widget/aero_tooltip_manager.h b/views/widget/aero_tooltip_manager.h
index c81991c..dda3376 100644
--- a/views/widget/aero_tooltip_manager.h
+++ b/views/widget/aero_tooltip_manager.h
@@ -45,6 +45,10 @@ class AeroTooltipManager : public TooltipManagerWin {
     void Execute();
 
    private:
+    friend class base::RefCounted<InitialTimer>;
+
+    ~InitialTimer() {}
+
     AeroTooltipManager* manager_;
   };