Revert 68932 - Make members of Singleton<T> private and only visible to the singleton type. This enforces that the Singleton<T> pattern can only be used within classes which want singleton-ness.

As part of this CL I have also fixed up files which got missed in my previous CLs to use a GetInstance() method and use Singleton<T> from the source file.

There are a small number of places where I have also switched to LazyInstance as that was more appropriate for types used in a single source file.

BUG=65298
TEST=all existing tests should continue to pass.

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

TBR=satish@chromium.org
Review URL: http://codereview.chromium.org/5721005

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

5 files changed, 89 insertions, 160 deletions

diff --git a/base/i18n/number_formatting.cc b/base/i18n/number_formatting.cc
index df6af14..7a69294 100644
--- a/base/i18n/number_formatting.cc
+++ b/base/i18n/number_formatting.cc
@@ -6,8 +6,7 @@
 
 #include "base/format_macros.h"
 #include "base/logging.h"
-#include "base/lazy_instance.h"
-#include "base/scoped_ptr.h"
+#include "base/singleton.h"
 #include "base/string_util.h"
 #include "base/utf_string_conversions.h"
 #include "unicode/numfmt.h"
@@ -17,26 +16,25 @@ namespace base {
 
 namespace {
 
-struct NumberFormatWrapper {
-  NumberFormatWrapper() {
-    // There's no ICU call to destroy a NumberFormat object other than
-    // operator delete, so use the default Delete, which calls operator delete.
-    // This can cause problems if a different allocator is used by this file
-    // than by ICU.
+struct NumberFormatSingletonTraits
+    : public DefaultSingletonTraits<icu::NumberFormat> {
+  static icu::NumberFormat* New() {
     UErrorCode status = U_ZERO_ERROR;
-    number_format.reset(icu::NumberFormat::createInstance(status));
+    icu::NumberFormat* formatter = icu::NumberFormat::createInstance(status);
     DCHECK(U_SUCCESS(status));
+    return formatter;
   }
-
-  scoped_ptr<icu::NumberFormat> number_format;
+  // There's no ICU call to destroy a NumberFormat object other than
+  // operator delete, so use the default Delete, which calls operator delete.
+  // This can cause problems if a different allocator is used by this file than
+  // by ICU.
 };
 
 }  // namespace
 
-static LazyInstance<NumberFormatWrapper> g_number_format(LINKER_INITIALIZED);
-
 string16 FormatNumber(int64 number) {
-  icu::NumberFormat* number_format = g_number_format.Get().number_format.get();
+  icu::NumberFormat* number_format =
+      Singleton<icu::NumberFormat, NumberFormatSingletonTraits>::get();
 
   if (!number_format) {
     // As a fallback, just return the raw number in a string.
diff --git a/base/path_service.cc b/base/path_service.cc
index 56ce5fa..4381998 100644
--- a/base/path_service.cc
+++ b/base/path_service.cc
@@ -13,9 +13,9 @@
 #include "base/file_path.h"
 #include "base/file_util.h"
 #include "base/hash_tables.h"
-#include "base/lazy_instance.h"
 #include "base/lock.h"
 #include "base/logging.h"
+#include "base/singleton.h"
 
 namespace base {
   bool PathProvider(int key, FilePath* result);
@@ -118,10 +118,8 @@ struct PathData {
   }
 };
 
-static base::LazyInstance<PathData> g_path_data(base::LINKER_INITIALIZED);
-
 static PathData* GetPathData() {
-  return g_path_data.Pointer();
+  return Singleton<PathData>::get();
 }
 
 }  // namespace
diff --git a/base/singleton.h b/base/singleton.h
index 914f46c..8435c43 100644
--- a/base/singleton.h
+++ b/base/singleton.h
@@ -114,13 +114,6 @@ template <typename Type> intptr_t
 template <typename Type> base::subtle::Atomic32
     StaticMemorySingletonTraits<Type>::dead_ = 0;
 
-// This is a hack to work around a limitation where a template argument cannot
-// be declared as a friend directly. This is used in the below Singleton
-// template.
-template <typename T>
-struct FriendMaker {
-  typedef T FriendType;
-};
 
 // The Singleton<Type, Traits, DifferentiatingType> class manages a single
 // instance of Type which will be created on first use and will be destroyed at
@@ -131,36 +124,15 @@ struct FriendMaker {
 // singletons having the same memory allocation functions but serving a
 // different purpose. This is mainly used for Locks serving different purposes.
 //
-// Example usage:
-//
-// In your header:
-//   #include "base/singleton.h"
-//   class FooClass {
-//    public:
-//     static FooClass* GetInstance();  <-- See comment below on this.
-//     void Bar() { ... }
-//    private:
-//     FooClass() { ... }
-//     friend struct DefaultSingletonTraits<FooClass>;
-//
-//     DISALLOW_COPY_AND_ASSIGN(FooClass);
-//   };
-//
-// In your source file:
-//  FooClass* FooClass::GetInstance() {
-//    return Singleton<FooClass>::get();
-//  }
-//
-// And to call methods on FooClass:
-//   FooClass::GetInstance()->Bar();
-//
-// NOTE: It is important that FooClass::GetInstance() is not inlined in the
-// header, so that when source files from multiple targets include this header
-// they don't end up with different copies of the inlined code creating multiple
-// copies of the singleton.
+// Example usages: (none are preferred, they all result in the same code)
+//   1. FooClass* ptr = Singleton<FooClass>::get();
+//      ptr->Bar();
+//   2. Singleton<FooClass>()->Bar();
+//   3. Singleton<FooClass>::get()->Bar();
 //
 // Singleton<> has no non-static members and doesn't need to actually be
-// instantiated.
+// instantiated. It does no harm to instantiate it and use it as a class member
+// or at global level since it is acting as a POD type.
 //
 // This class is itself thread-safe. The underlying Type must of course be
 // thread-safe if you want to use it concurrently. Two parameters may be tuned
@@ -180,6 +152,20 @@ struct FriendMaker {
 // shouldn't be false unless absolutely necessary. Remember that the heap where
 // the object is allocated may be destroyed by the CRT anyway.
 //
+// If you want to ensure that your class can only exist as a singleton, make
+// its constructors private, and make DefaultSingletonTraits<> a friend:
+//
+//   #include "base/singleton.h"
+//   class FooClass {
+//    public:
+//     void Bar() { ... }
+//    private:
+//     FooClass() { ... }
+//     friend struct DefaultSingletonTraits<FooClass>;
+//
+//     DISALLOW_COPY_AND_ASSIGN(FooClass);
+//   };
+//
 // Caveats:
 // (a) Every call to get(), operator->() and operator*() incurs some overhead
 //     (16ns on my P4/2.8GHz) to check whether the object has already been
@@ -193,13 +179,7 @@ template <typename Type,
           typename Traits = DefaultSingletonTraits<Type>,
           typename DifferentiatingType = Type>
 class Singleton {
- private:
-#if defined(OS_WIN)
-  friend typename FriendMaker<Type>::FriendType;
-#else
-  friend class FriendMaker<Type>::FriendType;
-#endif
-
+ public:
   // This class is safe to be constructed and copy-constructed since it has no
   // member.
 
@@ -260,6 +240,16 @@ class Singleton {
     return reinterpret_cast<Type*>(value);
   }
 
+  // Shortcuts.
+  Type& operator*() {
+    return *get();
+  }
+
+  Type* operator->() {
+    return get();
+  }
+
+ private:
   // Adapter function for use with AtExit().  This should be called single
   // threaded, so don't use atomic operations.
   // Calling OnExit while singleton is in use by other threads is a mistake.
diff --git a/base/singleton_unittest.cc b/base/singleton_unittest.cc
index 288b89a..acb1247 100644
--- a/base/singleton_unittest.cc
+++ b/base/singleton_unittest.cc
@@ -16,127 +16,83 @@ template<typename Type>
 struct LockTrait : public DefaultSingletonTraits<Type> {
 };
 
-typedef void (*CallbackFunc)();
+struct Init5Trait : public DefaultSingletonTraits<int> {
+  static int* New() {
+    return new int(5);
+  }
+};
 
-class IntSingleton {
- public:
-  class DummyDifferentiatingClass {
-  };
-
-  struct Init5Trait : public DefaultSingletonTraits<IntSingleton> {
-    static IntSingleton* New() {
-      IntSingleton* instance = new IntSingleton();
-      instance->value_ = 5;
-      return instance;
-    }
-  };
+typedef void (*CallbackFunc)();
 
-  static IntSingleton* GetInstance() {
-    return Singleton<IntSingleton>::get();
-  }
-  static IntSingleton* GetInstanceWithDefaultTraits() {
-    return Singleton<IntSingleton,
-                     DefaultSingletonTraits<IntSingleton> >::get();
-  }
-  static IntSingleton* GetInstanceWithDifferentiatingClass() {
-    return Singleton<IntSingleton,
-                     DefaultSingletonTraits<IntSingleton>,
-                     DummyDifferentiatingClass>::get();
-  }
-  static IntSingleton* GetInstanceWithLockTrait() {
-    return Singleton<IntSingleton, LockTrait<IntSingleton> >::get();
+struct CallbackTrait : public DefaultSingletonTraits<CallbackFunc> {
+  static void Delete(CallbackFunc* p) {
+    if (*p)
+      (*p)();
+    DefaultSingletonTraits<CallbackFunc>::Delete(p);
   }
-  static IntSingleton* GetInstanceWithInit5Trait() {
-    return Singleton<IntSingleton, Init5Trait>::get();
+};
+
+struct StaticCallbackTrait : public StaticMemorySingletonTraits<CallbackFunc> {
+  static void Delete(CallbackFunc* p) {
+    if (*p)
+      (*p)();
+    StaticMemorySingletonTraits<CallbackFunc>::Delete(p);
   }
+};
+
 
-  int value_;
+struct NoLeakTrait : public CallbackTrait {
+};
+
+struct LeakTrait : public CallbackTrait {
+  static const bool kRegisterAtExit = false;
 };
 
 int* SingletonInt1() {
-  return &IntSingleton::GetInstance()->value_;
+  return Singleton<int>::get();
 }
 
 int* SingletonInt2() {
   // Force to use a different singleton than SingletonInt1.
-  return &IntSingleton::GetInstanceWithDefaultTraits()->value_;
+  return Singleton<int, DefaultSingletonTraits<int> >::get();
 }
 
+class DummyDifferentiatingClass {
+};
+
 int* SingletonInt3() {
   // Force to use a different singleton than SingletonInt1 and SingletonInt2.
-  return &IntSingleton::GetInstanceWithDifferentiatingClass()->value_;
+  // Note that any type can be used; int, float, std::wstring...
+  return Singleton<int, DefaultSingletonTraits<int>,
+                   DummyDifferentiatingClass>::get();
 }
 
 int* SingletonInt4() {
-  return &IntSingleton::GetInstanceWithLockTrait()->value_;
+  return Singleton<int, LockTrait<int> >::get();
 }
 
 int* SingletonInt5() {
-  return &IntSingleton::GetInstanceWithInit5Trait()->value_;
-}
-
-class CallbackSingleton {
- public:
-  struct CallbackTrait : public DefaultSingletonTraits<CallbackSingleton> {
-    static void Delete(CallbackSingleton* instance) {
-      if (instance->callback_)
-        (instance->callback_)();
-      DefaultSingletonTraits<CallbackSingleton>::Delete(instance);
-    }
-  };
-
-  struct NoLeakTrait : public CallbackTrait {
-  };
-
-  struct LeakTrait : public CallbackTrait {
-    static const bool kRegisterAtExit = false;
-  };
-
-  CallbackSingleton() : callback_(NULL) {
-  }
-
-  static CallbackSingleton* GetInstanceWithNoLeakTrait() {
-    return Singleton<CallbackSingleton, NoLeakTrait>::get();
-  }
-  static CallbackSingleton* GetInstanceWithLeakTrait() {
-    return Singleton<CallbackSingleton, LeakTrait>::get();
-  }
-  static CallbackSingleton* GetInstanceWithStaticTrait();
-
-  CallbackFunc callback_;
-};
-
-struct StaticCallbackTrait
-    : public StaticMemorySingletonTraits<CallbackSingleton> {
-  static void Delete(CallbackSingleton* instance) {
-    if (instance->callback_)
-      (instance->callback_)();
-    StaticMemorySingletonTraits<CallbackSingleton>::Delete(instance);
-  }
-};
-
-CallbackSingleton* CallbackSingleton::GetInstanceWithStaticTrait() {
-  return Singleton<CallbackSingleton, StaticCallbackTrait>::get();
+  return Singleton<int, Init5Trait>::get();
 }
 
 void SingletonNoLeak(CallbackFunc CallOnQuit) {
-  CallbackSingleton::GetInstanceWithNoLeakTrait()->callback_ = CallOnQuit;
+  *Singleton<CallbackFunc, NoLeakTrait>::get() = CallOnQuit;
 }
 
 void SingletonLeak(CallbackFunc CallOnQuit) {
-  CallbackSingleton::GetInstanceWithLeakTrait()->callback_ = CallOnQuit;
+  *Singleton<CallbackFunc, LeakTrait>::get() = CallOnQuit;
 }
 
 CallbackFunc* GetLeakySingleton() {
-  return &CallbackSingleton::GetInstanceWithLeakTrait()->callback_;
+  return Singleton<CallbackFunc, LeakTrait>::get();
 }
 
 void SingletonStatic(CallbackFunc CallOnQuit) {
-  CallbackSingleton::GetInstanceWithStaticTrait()->callback_ = CallOnQuit;
+  *Singleton<CallbackFunc, StaticCallbackTrait>::get() = CallOnQuit;
 }
 
 CallbackFunc* GetStaticSingleton() {
-  return &CallbackSingleton::GetInstanceWithStaticTrait()->callback_;
+  return Singleton<CallbackFunc, StaticCallbackTrait>::get();
 }
 
 }  // namespace
@@ -279,7 +235,7 @@ TEST_F(SingletonTest, Basic) {
     {
       // Resurrect the static singleton, and assert that it
       // still points to the same (static) memory.
-      StaticMemorySingletonTraits<CallbackSingleton>::Resurrect();
+      StaticMemorySingletonTraits<CallbackFunc>::Resurrect();
       EXPECT_EQ(GetStaticSingleton(), static_singleton);
     }
   }
diff --git a/base/worker_pool_mac.mm b/base/worker_pool_mac.mm
index 956cfb4..bbc7892 100644
--- a/base/worker_pool_mac.mm
+++ b/base/worker_pool_mac.mm
@@ -4,12 +4,11 @@
 
 #include "base/worker_pool_mac.h"
 
-#include "base/lazy_instance.h"
 #include "base/logging.h"
 #include "base/mac/scoped_nsautorelease_pool.h"
 #include "base/metrics/histogram.h"
-#include "base/scoped_nsobject.h"
 #include "base/scoped_ptr.h"
+#import "base/singleton_objc.h"
 #include "base/task.h"
 #include "base/third_party/dynamic_annotations/dynamic_annotations.h"
 #include "base/worker_pool_linux.h"
@@ -35,18 +34,6 @@ std::vector<id> outstanding_ops_;  // Outstanding operations at last check.
 size_t running_ = 0;               // Operations in |Run()|.
 size_t outstanding_ = 0;           // Operations posted but not completed.
 
-// We use a wrapper struct here for the NSOperationQueue so that the object
-// can be released when LazyInstance calls our destructor.
-struct NSOperationQueueWrapper {
-  NSOperationQueueWrapper() {
-    operation_queue.reset([[NSOperationQueue alloc] init]);
-  }
-  scoped_nsobject<NSOperationQueue> operation_queue;
-};
-
-static base::LazyInstance<NSOperationQueueWrapper> g_nsoperation_queue(
-    base::LINKER_INITIALIZED);
-
 }  // namespace
 
 namespace worker_pool_mac {
@@ -60,7 +47,7 @@ void SetUseLinuxWorkerPool(bool flag) {
 @implementation WorkerPoolObjC
 
 + (NSOperationQueue*)sharedOperationQueue {
-  return g_nsoperation_queue.Get().operation_queue.get();
+  return SingletonObjC<NSOperationQueue>::get();
 }
 
 @end  // @implementation WorkerPoolObjC