From 877d55dcd3e01d1db858f154a1f369e76b2ebaf2 Mon Sep 17 00:00:00 2001 From: "jam@chromium.org" Date: Thu, 5 Nov 2009 21:53:08 +0000 Subject: 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 --- base/directory_watcher.h | 6 +++++- base/directory_watcher_win.cc | 3 ++- base/field_trial.h | 4 ++++ base/message_loop_unittest.cc | 9 +++++++++ base/message_pump_glib_unittest.cc | 8 ++++++++ base/ref_counted.h | 9 +++++++-- base/ref_counted_memory.h | 9 ++++++--- base/ref_counted_unittest.cc | 3 +++ base/stack_container_unittest.cc | 6 +++++- 9 files changed, 49 insertions(+), 8 deletions(-) (limited to 'base') 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 { public: - virtual ~PlatformDelegate() {} virtual bool Watch(const FilePath& path, Delegate* delegate, MessageLoop* backend_loop, bool recursive) = 0; + + protected: + friend class base::RefCounted; + + 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 { const std::string& trial_name); private: + friend class base::RefCounted; + + ~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 { const std::string& result() const { return result_; } private: + friend class base::RefCounted; + + ~Foo() {} + int test_count_; std::string result_; }; @@ -76,6 +80,11 @@ class QuitMsgLoop : public base::RefCounted { void QuitNow() { MessageLoop::current()->Quit(); } + + private: + friend class base::RefCounted; + + ~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 { int task_count() const { return task_count_; } private: + friend class base::RefCounted; + + ~ConcurrentHelper() {} + static const int kStartingEventCount = 20; static const int kStartingTaskCount = 20; @@ -455,6 +459,10 @@ class GLibLoopRunner : public base::RefCounted { } private: + friend class base::RefCounted; + + ~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 { // ... +// private: +// friend class base::RefCounted; +// ~MyFoo(); // }; // +// You should always make your destructor private, to avoid any code deleting +// the object accidently while there are references to it. template 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; +// friend class base::RefCountedThreadSafe; // ~MyFoo(); template > 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 { 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; + + 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 { + friend class base::RefCounted; + + ~SelfAssign() {} }; class CheckDerivedMemberAccess : public scoped_refptr { 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(int* alive) : alive_(alive) { ++*alive_; } + + private: + friend class base::RefCounted; + ~Dummy() { --*alive_; } - private: + int* const alive_; }; -- cgit v1.1