RefCounted types should not have public destructors, base/ edition

BUG=123295
TEST=none


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

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

12 files changed, 62 insertions, 32 deletions

diff --git a/base/files/file_path_watcher.h b/base/files/file_path_watcher.h
index 3174a9e..3140628 100644
--- a/base/files/file_path_watcher.h
+++ b/base/files/file_path_watcher.h
@@ -33,11 +33,14 @@ class BASE_EXPORT FilePathWatcher {
   // corresponding FileWatcher object to prevent a reference cycle.
   class Delegate : public base::RefCountedThreadSafe<Delegate> {
    public:
-    virtual ~Delegate() {}
     virtual void OnFilePathChanged(const FilePath& path) = 0;
     // Called when platform specific code detected an error. The watcher will
     // not call OnFilePathChanged for future changes.
     virtual void OnFilePathError(const FilePath& path) {}
+
+   protected:
+    friend class base::RefCountedThreadSafe<Delegate>;
+    virtual ~Delegate() {}
   };
 
   // Used internally to encapsulate different members on different platforms.
@@ -57,6 +60,7 @@ class BASE_EXPORT FilePathWatcher {
     virtual void Cancel() = 0;
 
    protected:
+    friend class base::RefCountedThreadSafe<PlatformDelegate>;
     friend class FilePathWatcher;
 
     virtual ~PlatformDelegate();
@@ -84,8 +88,6 @@ class BASE_EXPORT FilePathWatcher {
     }
 
    private:
-    friend class base::RefCountedThreadSafe<PlatformDelegate>;
-
     scoped_refptr<base::MessageLoopProxy> message_loop_;
     bool cancelled_;
   };
diff --git a/base/files/file_path_watcher_browsertest.cc b/base/files/file_path_watcher_browsertest.cc
index 495f4d3..7886df8 100644
--- a/base/files/file_path_watcher_browsertest.cc
+++ b/base/files/file_path_watcher_browsertest.cc
@@ -65,6 +65,9 @@ class NotificationCollector
   }
 
  private:
+  friend class base::RefCountedThreadSafe<NotificationCollector>;
+  ~NotificationCollector() {}
+
   void RecordChange(TestDelegate* delegate) {
     ASSERT_TRUE(loop_->BelongsToCurrentThread());
     ASSERT_TRUE(delegates_.count(delegate));
@@ -107,6 +110,9 @@ class TestDelegate : public FilePathWatcher::Delegate {
     ADD_FAILURE() << "Error " << path.value();
   }
 
+ protected:
+  virtual ~TestDelegate() {}
+
  private:
   scoped_refptr<NotificationCollector> collector_;
 
@@ -249,6 +255,9 @@ class Deleter : public FilePathWatcher::Delegate {
 
   scoped_ptr<FilePathWatcher> watcher_;
   MessageLoop* loop_;
+
+ private:
+  virtual ~Deleter() {}
 };
 
 // Verify that deleting a watcher during the callback doesn't crash.
diff --git a/base/files/file_path_watcher_kqueue.cc b/base/files/file_path_watcher_kqueue.cc
index d4ddb7d..29c104a 100644
--- a/base/files/file_path_watcher_kqueue.cc
+++ b/base/files/file_path_watcher_kqueue.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Copyright (c) 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.
 
@@ -54,7 +54,6 @@ class FilePathWatcherImpl : public FilePathWatcher::PlatformDelegate,
                             public MessageLoop::DestructionObserver {
  public:
   FilePathWatcherImpl() : kqueue_(-1) {}
-  virtual ~FilePathWatcherImpl() {}
 
   // MessageLoopForIO::Watcher overrides.
   virtual void OnFileCanReadWithoutBlocking(int fd) OVERRIDE;
@@ -68,6 +67,9 @@ class FilePathWatcherImpl : public FilePathWatcher::PlatformDelegate,
                      FilePathWatcher::Delegate* delegate) OVERRIDE;
   virtual void Cancel() OVERRIDE;
 
+ protected:
+  virtual ~FilePathWatcherImpl() {}
+
  private:
   class EventData {
    public:
diff --git a/base/files/file_path_watcher_linux.cc b/base/files/file_path_watcher_linux.cc
index 72c8610c..4bdfe74 100644
--- a/base/files/file_path_watcher_linux.cc
+++ b/base/files/file_path_watcher_linux.cc
@@ -110,9 +110,10 @@ class FilePathWatcherImpl : public FilePathWatcher::PlatformDelegate,
   // cleanup thread, in case it quits before Cancel() is called.
   virtual void WillDestroyCurrentMessageLoop() OVERRIDE;
 
- private:
+ protected:
   virtual ~FilePathWatcherImpl() {}
 
+ private:
   // Cleans up and stops observing the |message_loop_| thread.
   void CancelOnMessageLoopThread() OVERRIDE;
 
diff --git a/base/files/file_path_watcher_mac.cc b/base/files/file_path_watcher_mac.cc
index 07a4dd2..b678f8b 100644
--- a/base/files/file_path_watcher_mac.cc
+++ b/base/files/file_path_watcher_mac.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Copyright (c) 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.
 
@@ -46,7 +46,6 @@ class FilePathWatcherImpl : public FilePathWatcher::PlatformDelegate,
                             public MessageLoop::DestructionObserver {
  public:
   FilePathWatcherImpl() : kqueue_(-1) {}
-  virtual ~FilePathWatcherImpl() {}
 
   // MessageLoopForIO::Watcher overrides.
   virtual void OnFileCanReadWithoutBlocking(int fd) OVERRIDE;
@@ -60,6 +59,9 @@ class FilePathWatcherImpl : public FilePathWatcher::PlatformDelegate,
                      FilePathWatcher::Delegate* delegate) OVERRIDE;
   virtual void Cancel() OVERRIDE;
 
+ protected:
+  virtual ~FilePathWatcherImpl() {}
+
  private:
   class EventData {
    public:
diff --git a/base/files/file_path_watcher_stub.cc b/base/files/file_path_watcher_stub.cc
index 6c49101..d91c1a7 100644
--- a/base/files/file_path_watcher_stub.cc
+++ b/base/files/file_path_watcher_stub.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Copyright (c) 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.
 
@@ -22,6 +22,9 @@ class FilePathWatcherImpl : public FilePathWatcher::PlatformDelegate {
   virtual void Cancel() OVERRIDE {}
 
   virtual void CancelOnMessageLoopThread() OVERRIDE {}
+
+ protected:
+  virtual ~FilePathWatcherImpl() {}
 };
 
 }  // namespace
diff --git a/base/memory/ref_counted_memory.cc b/base/memory/ref_counted_memory.cc
index f8e6c66..59d141d 100644
--- a/base/memory/ref_counted_memory.cc
+++ b/base/memory/ref_counted_memory.cc
@@ -6,11 +6,9 @@
 
 #include "base/logging.h"
 
-RefCountedMemory::RefCountedMemory() {
-}
+RefCountedMemory::RefCountedMemory() {}
 
-RefCountedMemory::~RefCountedMemory() {
-}
+RefCountedMemory::~RefCountedMemory() {}
 
 namespace base {
 
@@ -22,8 +20,9 @@ size_t RefCountedStaticMemory::size() const {
   return length_;
 }
 
-RefCountedBytes::RefCountedBytes() {
-}
+RefCountedStaticMemory::~RefCountedStaticMemory() {}
+
+RefCountedBytes::RefCountedBytes() {}
 
 RefCountedBytes::RefCountedBytes(const std::vector<unsigned char>& initializer)
     : data_(initializer) {
@@ -46,8 +45,7 @@ size_t RefCountedBytes::size() const {
   return data_.size();
 }
 
-RefCountedBytes::~RefCountedBytes() {
-}
+RefCountedBytes::~RefCountedBytes() {}
 
 RefCountedString::RefCountedString() {}
 
diff --git a/base/memory/ref_counted_memory.h b/base/memory/ref_counted_memory.h
index 1434a90..c9170f5 100644
--- a/base/memory/ref_counted_memory.h
+++ b/base/memory/ref_counted_memory.h
@@ -51,6 +51,8 @@ class BASE_EXPORT RefCountedStaticMemory : public RefCountedMemory {
   virtual size_t size() const OVERRIDE;
 
  private:
+  virtual ~RefCountedStaticMemory();
+
   const unsigned char* data_;
   size_t length_;
 
@@ -79,7 +81,6 @@ class BASE_EXPORT RefCountedBytes : public RefCountedMemory {
   std::vector<unsigned char>& data() { return data_; }
 
  private:
-  friend class base::RefCountedThreadSafe<RefCountedBytes>;
   virtual ~RefCountedBytes();
 
   std::vector<unsigned char> data_;
@@ -106,7 +107,6 @@ class BASE_EXPORT RefCountedString : public RefCountedMemory {
   std::string& data() { return data_; }
 
  private:
-  friend class base::RefCountedThreadSafe<RefCountedString>;
   virtual ~RefCountedString();
 
   std::string data_;
diff --git a/base/memory/ref_counted_unittest.cc b/base/memory/ref_counted_unittest.cc
index 93b93c4..8ddd5be 100644
--- a/base/memory/ref_counted_unittest.cc
+++ b/base/memory/ref_counted_unittest.cc
@@ -27,7 +27,6 @@ class ScopedRefPtrToSelf : public base::RefCounted<ScopedRefPtrToSelf> {
   ScopedRefPtrToSelf()
       : ALLOW_THIS_IN_INITIALIZER_LIST(self_ptr_(this)) {
   }
-  ~ScopedRefPtrToSelf() { was_destroyed_ = true; }
 
   static bool was_destroyed() { return was_destroyed_; }
 
@@ -35,6 +34,7 @@ class ScopedRefPtrToSelf : public base::RefCounted<ScopedRefPtrToSelf> {
 
  private:
   friend class base::RefCounted<ScopedRefPtrToSelf>;
+  ~ScopedRefPtrToSelf() { was_destroyed_ = true; }
 
   static bool was_destroyed_;
 
diff --git a/base/message_loop_unittest.cc b/base/message_loop_unittest.cc
index d1e921c..15f825b 100644
--- a/base/message_loop_unittest.cc
+++ b/base/message_loop_unittest.cc
@@ -397,6 +397,11 @@ class RecordDeletionProbe : public base::RefCounted<RecordDeletionProbe> {
   RecordDeletionProbe(RecordDeletionProbe* post_on_delete, bool* was_deleted)
       : post_on_delete_(post_on_delete), was_deleted_(was_deleted) {
   }
+  void Run() {}
+
+ private:
+  friend class base::RefCounted<RecordDeletionProbe>;
+
   ~RecordDeletionProbe() {
     *was_deleted_ = true;
     if (post_on_delete_)
@@ -404,8 +409,7 @@ class RecordDeletionProbe : public base::RefCounted<RecordDeletionProbe> {
           FROM_HERE,
           base::Bind(&RecordDeletionProbe::Run, post_on_delete_.get()));
   }
-  void Run() {}
- private:
+
   scoped_refptr<RecordDeletionProbe> post_on_delete_;
   bool* was_deleted_;
 };
@@ -1624,15 +1628,18 @@ class DestructionObserverProbe :
       : task_destroyed_(task_destroyed),
         destruction_observer_called_(destruction_observer_called) {
   }
-  virtual ~DestructionObserverProbe() {
-    EXPECT_FALSE(*destruction_observer_called_);
-    *task_destroyed_ = true;
-  }
   virtual void Run() {
     // This task should never run.
     ADD_FAILURE();
   }
  private:
+  friend class base::RefCounted<DestructionObserverProbe>;
+
+  virtual ~DestructionObserverProbe() {
+    EXPECT_FALSE(*destruction_observer_called_);
+    *task_destroyed_ = true;
+  }
+
   bool* task_destroyed_;
   bool* destruction_observer_called_;
 };
diff --git a/base/threading/sequenced_worker_pool_unittest.cc b/base/threading/sequenced_worker_pool_unittest.cc
index 42cf859..54ca01d 100644
--- a/base/threading/sequenced_worker_pool_unittest.cc
+++ b/base/threading/sequenced_worker_pool_unittest.cc
@@ -37,8 +37,7 @@ const size_t kNumWorkerThreads = 3;
 // provides a way to unblock a certain number of them.
 class ThreadBlocker {
  public:
-  ThreadBlocker() : lock_(), cond_var_(&lock_), unblock_counter_(0) {
-  }
+  ThreadBlocker() : lock_(), cond_var_(&lock_), unblock_counter_(0) {}
 
   void Block() {
     {
@@ -79,6 +78,7 @@ class TestTracker : public base::RefCountedThreadSafe<TestTracker> {
   void FastTask(int id) {
     SignalWorkerDone(id);
   }
+
   void SlowTask(int id) {
     base::PlatformThread::Sleep(base::TimeDelta::FromSeconds(1));
     SignalWorkerDone(id);
@@ -128,6 +128,9 @@ class TestTracker : public base::RefCountedThreadSafe<TestTracker> {
   }
 
  private:
+  friend class base::RefCountedThreadSafe<TestTracker>;
+  ~TestTracker() {}
+
   void SignalWorkerDone(int id) {
     {
       base::AutoLock lock(lock_);
@@ -152,7 +155,8 @@ class SequencedWorkerPoolTest : public testing::Test {
  public:
   SequencedWorkerPoolTest()
       : pool_owner_(kNumWorkerThreads, "test"),
-        tracker_(new TestTracker) {}
+        tracker_(new TestTracker) {
+  }
 
   virtual ~SequencedWorkerPoolTest() {}
 
diff --git a/base/threading/worker_pool_unittest.cc b/base/threading/worker_pool_unittest.cc
index 4be06d3..0452335 100644
--- a/base/threading/worker_pool_unittest.cc
+++ b/base/threading/worker_pool_unittest.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Copyright (c) 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.
 
@@ -24,8 +24,7 @@ namespace {
 class PostTaskAndReplyTester
     : public base::RefCountedThreadSafe<PostTaskAndReplyTester> {
  public:
-  PostTaskAndReplyTester() : finished_(false), test_event_(false, false) {
-  }
+  PostTaskAndReplyTester() : finished_(false), test_event_(false, false) {}
 
   void RunTest() {
     ASSERT_TRUE(thread_checker_.CalledOnValidThread());
@@ -55,6 +54,9 @@ class PostTaskAndReplyTester
   }
 
  private:
+  friend class base::RefCountedThreadSafe<PostTaskAndReplyTester>;
+  ~PostTaskAndReplyTester() {}
+
   bool finished_;
   WaitableEvent test_event_;