Revert of Refactor thread_local.h's TLS Implementation to use ThreadL…

…ocalStorage (patchset chromium#2 id:60001 of https://codereview.chromium.org/1726203002/ )

Reason for revert:
Reverting to consider this leak in base_unittests and unit_tests

Leak_DefinitelyLost
8,192 bytes in 4 blocks are definitely lost in loss record 40,719 of 40,770
  operator new[](unsigned long) (m_replacemalloc/vg_replace_malloc.c:1144)
  (anonymous namespace)::ConstructTlsVector() (base/threading/thread_local_storage.cc:107)
  base::ThreadLocalStorage::StaticSlot::Set(void*) (base/threading/thread_local_storage.cc:246)
  base::ThreadLocalPointer<void>::Set(void*) (base/threading/thread_local.h:69)
50 new files were left in /tmp: Fix the tests to clean up themselves.
  base::ThreadLocalBoolean::Set(bool) (base/threading/thread_local.h:88)
  base::(anonymous namespace)::WorkerThread::ThreadMain() (base/threading/worker_pool_posix.cc:79)
  base::(anonymous namespace)::ThreadFunc(void*) (base/threading/platform_thread_posix.cc:68)
Suppression (error hash=#F6EC67ECF9CF9B28#):
  For more info on using suppressions see http://dev.chromium.org/developers/tree-sheriffs/sheriff-details-chromium/memory-sheriff#TOC-Suppressing-memory-reports
{
   <insert_a_suppression_name_here>
   Memcheck:Leak
   fun:_Zna*
   fun:_ZN12_GLOBAL__N_118ConstructTlsVectorEv
   fun:_ZN4base18ThreadLocalStorage10StaticSlot3SetEPv
   fun:_ZN4base18ThreadLocalPointerIvE3SetEPv
   fun:_ZN4base18ThreadLocalBoolean3SetEb
   fun:_ZN4base12_GLOBAL__N_112WorkerThread10ThreadMainEv
   fun:_ZN4base12_GLOBAL__N_110ThreadFuncEPv
}

Original issue's description:
> Refactor thread_local.h's TLS Implemetation to use ThreadLocalStorage
>
> thread_local_*'s implementation of TLS was redundant with
> thread_local_storage_*'s implementation.
>
> This change reduces the number of implementations by one.
>
> BUG=588824
>
> Committed: https://crrev.com/8f0b97ae15609cee6fe67d2b0b958ef37cffb1e0
> Cr-Commit-Position: refs/heads/master@{#377774}

TBR=brettw@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=588824

Review URL: https://codereview.chromium.org/1743693002

Cr-Commit-Position: refs/heads/master@{#377927}

base/BUILD.gn

@@ -797,10 +797,13 @@ component("base") {
     "threading/thread_id_name_manager.cc",
     "threading/thread_id_name_manager.h",
     "threading/thread_local.h",
+    "threading/thread_local_android.cc",
+    "threading/thread_local_posix.cc",
     "threading/thread_local_storage.cc",
     "threading/thread_local_storage.h",
     "threading/thread_local_storage_posix.cc",
     "threading/thread_local_storage_win.cc",
+    "threading/thread_local_win.cc",
     "threading/thread_restrictions.cc",
     "threading/thread_restrictions.h",
     "threading/watchdog.cc",

base/base.gypi

@@ -673,10 +673,13 @@
           'threading/thread_id_name_manager.cc',
           'threading/thread_id_name_manager.h',
           'threading/thread_local.h',
+          'threading/thread_local_android.cc',
+          'threading/thread_local_posix.cc',
           'threading/thread_local_storage.cc',
           'threading/thread_local_storage.h',
           'threading/thread_local_storage_posix.cc',
           'threading/thread_local_storage_win.cc',
+          'threading/thread_local_win.cc',
           'threading/thread_restrictions.cc',
           'threading/thread_restrictions.h',
           'threading/watchdog.cc',

base/threading/thread_local.h

@@ -2,34 +2,35 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-// WARNING: Thread local storage is a bit tricky to get right. Please make sure
-// that this is really the proper solution for what you're trying to achieve.
-// Don't prematurely optimize, most likely you can just use a Lock.
+// WARNING: Thread local storage is a bit tricky to get right.  Please make
+// sure that this is really the proper solution for what you're trying to
+// achieve.  Don't prematurely optimize, most likely you can just use a Lock.
 //
-// These classes implement a wrapper around ThreadLocalStorage::Slot. On
-// construction, they will allocate a TLS slot, and free the TLS slot on
-// destruction. No memory management (creation or destruction) is handled. This
-// means for uses of ThreadLocalPointer, you must correctly manage the memory
-// yourself, these classes will not destroy the pointer for you. There are no
-// at-thread-exit actions taken by these classes.
+// These classes implement a wrapper around the platform's TLS storage
+// mechanism.  On construction, they will allocate a TLS slot, and free the
+// TLS slot on destruction.  No memory management (creation or destruction) is
+// handled.  This means for uses of ThreadLocalPointer, you must correctly
+// manage the memory yourself, these classes will not destroy the pointer for
+// you.  There are no at-thread-exit actions taken by these classes.
 //
-// ThreadLocalPointer<Type> wraps a Type*. It performs no creation or
-// destruction, so memory management must be handled elsewhere. The first call
-// to Get() on a thread will return NULL. You can update the pointer with a call
-// to Set().
+// ThreadLocalPointer<Type> wraps a Type*.  It performs no creation or
+// destruction, so memory management must be handled elsewhere.  The first call
+// to Get() on a thread will return NULL.  You can update the pointer with a
+// call to Set().
 //
-// ThreadLocalBoolean wraps a bool. It will default to false if it has never
+// ThreadLocalBoolean wraps a bool.  It will default to false if it has never
 // been set otherwise with Set().
 //
-// Thread Safety: An instance of ThreadLocalStorage is completely thread safe
-// once it has been created. If you want to dynamically create an instance, you
-// must of course properly deal with safety and race conditions. This means a
-// function-level static initializer is generally inappropiate.
+// Thread Safety:  An instance of ThreadLocalStorage is completely thread safe
+// once it has been created.  If you want to dynamically create an instance,
+// you must of course properly deal with safety and race conditions.  This
+// means a function-level static initializer is generally inappropiate.
 //
-// In Android, the system TLS is limited.
+// In Android, the system TLS is limited, the implementation is backed with
+// ThreadLocalStorage.
 //
 // Example usage:
-//   // My class is logically attached to a single thread. We cache a pointer
+//   // My class is logically attached to a single thread.  We cache a pointer
 //   // on the thread it was created on, so we can implement current().
 //   MyClass::MyClass() {
 //     DCHECK(Singleton<ThreadLocalPointer<MyClass> >::get()->Get() == NULL);
@@ -50,42 +51,76 @@
 #ifndef BASE_THREADING_THREAD_LOCAL_H_
 #define BASE_THREADING_THREAD_LOCAL_H_
 
+#include "base/base_export.h"
 #include "base/macros.h"
 #include "base/threading/thread_local_storage.h"
+#include "build/build_config.h"
+
+#if defined(OS_POSIX)
+#include <pthread.h>
+#endif
 
 namespace base {
+namespace internal {
+
+// Helper functions that abstract the cross-platform APIs.  Do not use directly.
+struct BASE_EXPORT ThreadLocalPlatform {
+#if defined(OS_WIN)
+  typedef unsigned long SlotType;
+#elif defined(OS_ANDROID)
+  typedef ThreadLocalStorage::StaticSlot SlotType;
+#elif defined(OS_POSIX)
+  typedef pthread_key_t SlotType;
+#endif
+
+  static void AllocateSlot(SlotType* slot);
+  static void FreeSlot(SlotType slot);
+  static void* GetValueFromSlot(SlotType slot);
+  static void SetValueInSlot(SlotType slot, void* value);
+};
+
+}  // namespace internal
 
 template <typename Type>
 class ThreadLocalPointer {
  public:
-  ThreadLocalPointer() = default;
-  ~ThreadLocalPointer() = default;
+  ThreadLocalPointer() : slot_() {
+    internal::ThreadLocalPlatform::AllocateSlot(&slot_);
+  }
+
+  ~ThreadLocalPointer() {
+    internal::ThreadLocalPlatform::FreeSlot(slot_);
+  }
 
   Type* Get() {
-    return static_cast<Type*>(slot_.Get());
+    return static_cast<Type*>(
+        internal::ThreadLocalPlatform::GetValueFromSlot(slot_));
   }
 
   void Set(Type* ptr) {
-    slot_.Set(const_cast<void*>(static_cast<const void*>(ptr)));
+    internal::ThreadLocalPlatform::SetValueInSlot(
+        slot_, const_cast<void*>(static_cast<const void*>(ptr)));
   }
 
  private:
-  ThreadLocalStorage::Slot slot_;
+  typedef internal::ThreadLocalPlatform::SlotType SlotType;
+
+  SlotType slot_;
 
   DISALLOW_COPY_AND_ASSIGN(ThreadLocalPointer<Type>);
 };
 
 class ThreadLocalBoolean {
  public:
-  ThreadLocalBoolean() = default;
-  ~ThreadLocalBoolean() = default;
+  ThreadLocalBoolean() {}
+  ~ThreadLocalBoolean() {}
 
   bool Get() {
-    return tlp_.Get() != nullptr;
+    return tlp_.Get() != NULL;
   }
 
   void Set(bool val) {
-    tlp_.Set(val ? this : nullptr);
+    tlp_.Set(val ? this : NULL);
   }
 
  private:

base/threading/thread_local_android.cc

@@ -0,0 +1,31 @@
+// Copyright 2014 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.
+
+#include "base/threading/thread_local.h"
+
+namespace base {
+namespace internal {
+
+// static
+void ThreadLocalPlatform::AllocateSlot(SlotType* slot) {
+  slot->Initialize(nullptr);
+}
+
+// static
+void ThreadLocalPlatform::FreeSlot(SlotType slot) {
+  slot.Free();
+}
+
+// static
+void* ThreadLocalPlatform::GetValueFromSlot(SlotType slot) {
+  return slot.Get();
+}
+
+// static
+void ThreadLocalPlatform::SetValueInSlot(SlotType slot, void* value) {
+  slot.Set(value);
+}
+
+}  // namespace internal
+}  // namespace base

base/threading/thread_local_posix.cc

@@ -0,0 +1,43 @@
+// Copyright (c) 2011 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.
+
+#include "base/threading/thread_local.h"
+
+#include <pthread.h>
+
+#include "base/logging.h"
+#include "build/build_config.h"
+
+#if !defined(OS_ANDROID)
+
+namespace base {
+namespace internal {
+
+// static
+void ThreadLocalPlatform::AllocateSlot(SlotType* slot) {
+  int error = pthread_key_create(slot, NULL);
+  CHECK_EQ(error, 0);
+}
+
+// static
+void ThreadLocalPlatform::FreeSlot(SlotType slot) {
+  int error = pthread_key_delete(slot);
+  DCHECK_EQ(0, error);
+}
+
+// static
+void* ThreadLocalPlatform::GetValueFromSlot(SlotType slot) {
+  return pthread_getspecific(slot);
+}
+
+// static
+void ThreadLocalPlatform::SetValueInSlot(SlotType slot, void* value) {
+  int error = pthread_setspecific(slot, value);
+  DCHECK_EQ(error, 0);
+}
+
+}  // namespace internal
+}  // namespace base
+
+#endif  // !defined(OS_ANDROID)

base/threading/thread_local_storage.h

@@ -20,12 +20,9 @@ namespace base {
 
 namespace internal {
 
-// WARNING: You should *NOT* use this class directly.
-// PlatformThreadLocalStorage is a low-level abstraction of the OS's TLS
-// interface. Instead, you should use one of the following:
-// * ThreadLocalBoolean (from thread_local.h) for booleans.
-// * ThreadLocalPointer (from thread_local.h) for pointers.
-// * ThreadLocalStorage::StaticSlot/Slot for more direct control of the slot.
+// WARNING: You should *NOT* be using this class directly.
+// PlatformThreadLocalStorage is low-level abstraction to the OS's TLS
+// interface, you should instead be using ThreadLocalStorage::StaticSlot/Slot.
 class BASE_EXPORT PlatformThreadLocalStorage {
  public:
 

base/threading/thread_local_win.cc

@@ -0,0 +1,40 @@
+// Copyright (c) 2010 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.
+
+#include "base/threading/thread_local.h"
+
+#include <windows.h>
+
+#include "base/logging.h"
+
+namespace base {
+namespace internal {
+
+// static
+void ThreadLocalPlatform::AllocateSlot(SlotType* slot) {
+  *slot = TlsAlloc();
+  CHECK_NE(*slot, TLS_OUT_OF_INDEXES);
+}
+
+// static
+void ThreadLocalPlatform::FreeSlot(SlotType slot) {
+  if (!TlsFree(slot)) {
+    NOTREACHED() << "Failed to deallocate tls slot with TlsFree().";
+  }
+}
+
+// static
+void* ThreadLocalPlatform::GetValueFromSlot(SlotType slot) {
+  return TlsGetValue(slot);
+}
+
+// static
+void ThreadLocalPlatform::SetValueInSlot(SlotType slot, void* value) {
+  if (!TlsSetValue(slot, value)) {
+    LOG(FATAL) << "Failed to TlsSetValue().";
+  }
+}
+
+}  // namespace internal
+}  // namespace base