Revert of Improve the ScopedHandle verifier. (patchset #1 of https://codereview.chromium.org/506013004/)

Reason for revert:
All windows perf bots (official builds) crashing.

http://build.chromium.org/p/chromium.perf/builders/Win%20Builder/builds/73062/steps/generate_telemetry_profiles/logs/stdio

	ChildEBP RetAddr
	0022f7c0 6a3ffa19 chrome_69c90000!base::debug::BreakDebugger+0x10
	0022f830 6a400622 chrome_69c90000!CheckIsChromeSxSProcess+0x26
	0022f834 6a400d52 chrome_69c90000!InstallUtil::IsChromeSxSProcess+0x16
	0022f83c 6a400c29 chrome_69c90000!BrowserDistribution::GetSpecificDistribution+0x3b
	0022f844 6a3fe166 chrome_69c90000!BrowserDistribution::GetDistribution+0x7
	0022f930 6a3fe11d chrome_69c90000!`anonymous namespace'::GetChromeChannelInternal+0x2c
	0022f948 6a092c6c chrome_69c90000!GoogleUpdateSettings::GetChromeChannel+0x21
	0022f9b4 6a08f70f chrome_69c90000!chrome::VersionInfo::GetChannel+0x60
	0022f9b8 6a08f65a chrome_69c90000!`anonymous namespace'::UseHooks+0x5
	0022f9dc 6a08eb5a chrome_69c90000!InstallCloseHandleHooks+0x15
	0022fa20 00fb7623 chrome_69c90000!ChromeMain+0x3e
	0022fab0 00fb7026 chrome!MainDllLoader::Launch+0x15f
	0022faf4 00fd936a chrome!wWinMain+0x5a
	0022fb40 766e338a chrome!__tmainCRTStartup+0xfd
	WARNING: Stack unwind information not available. Following frames may be wrong.
	0022fb4c 76f99f72 kernel32!BaseThreadInitThunk+0x12
	0022fb8c 76f99f45 ntdll!RtlInitializeExceptionChain+0x63
	0022fba4 00000000 ntdll!RtlInitializeExceptionChain+0x36

Original issue's description:
> Improve the ScopedHandle verifier.
>
> 1. Automate the selection of the proper channel to enable the verifier.
>    Now the code is enabled at runtime.
>
> 2. Switch to a hash_map to track handles.
>
> 3. Intercept CloseHandle to detect the code that is closing handles owned
>    by ScopedHandles. The initial implementation only covers chrome.exe/dll,
>    but the plan is to extend that in the future to all modules loaded in the
>    process.
>
> BUG=362176
> R=cpu@chromium.org
> R=sky@chromium.org
>
> See https://codereview.chromium.org/490043002/ for the actual review.
>
> TBR=cpu@chromium.org
> TBR=sky@chromium.org
>
> Committed: https://chromium.googlesource.com/chromium/src/+/c928d0383db43f2f4baf8f9b24ed7454bf7eda64

TBR=cpu@chromium.org,sky@chromium.org,rvargas@chromium.org
NOTREECHECKS=true
NOTRY=true
BUG=362176

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

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

2 files changed, 13 insertions, 105 deletions

diff --git a/base/win/scoped_handle.cc b/base/win/scoped_handle.cc
index fbc7d40..7b38369 100644
--- a/base/win/scoped_handle.cc
+++ b/base/win/scoped_handle.cc
@@ -4,12 +4,11 @@
 
 #include "base/win/scoped_handle.h"
 
-#include "base/containers/hash_tables.h"
+#include <map>
+
 #include "base/debug/alias.h"
-#include "base/hash.h"
 #include "base/lazy_instance.h"
-#include "base/logging.h"
-#include "base/synchronization/lock_impl.h"
+#include "base/synchronization/lock.h"
 #include "base/win/windows_version.h"
 
 namespace {
@@ -20,86 +19,23 @@ struct Info {
   const void* pc2;
   DWORD thread_id;
 };
-typedef base::hash_map<HANDLE, Info> HandleMap;
+typedef std::map<HANDLE, Info> HandleMap;
 
-typedef base::internal::LockImpl NativeLock;
 base::LazyInstance<HandleMap>::Leaky g_handle_map = LAZY_INSTANCE_INITIALIZER;
-base::LazyInstance<NativeLock>::Leaky g_lock = LAZY_INSTANCE_INITIALIZER;
-bool g_closing = false;
-bool g_verifier_enabled = false;
-
-bool CloseHandleWrapper(HANDLE handle) {
-  if (!::CloseHandle(handle))
-    CHECK(false);
-  return true;
-}
-
-// Simple automatic locking using a native critical section so it supports
-// recursive locking.
-class AutoNativeLock {
- public:
-  explicit AutoNativeLock(NativeLock& lock) : lock_(lock) {
-    lock_.Lock();
-  }
-
-  ~AutoNativeLock() {
-    lock_.Unlock();
-  }
-
- private:
-  NativeLock& lock_;
-  DISALLOW_COPY_AND_ASSIGN(AutoNativeLock);
-};
-
-inline size_t HashHandle(const HANDLE& handle) {
-  char buffer[sizeof(handle)];
-  memcpy(buffer, &handle, sizeof(handle));
-  return base::Hash(buffer, sizeof(buffer));
-}
+base::LazyInstance<base::Lock>::Leaky g_lock = LAZY_INSTANCE_INITIALIZER;
 
 }  // namespace
 
-namespace BASE_HASH_NAMESPACE {
-#if defined(COMPILER_MSVC)
-inline size_t hash_value(const HANDLE& handle) {
-  return HashHandle(handle);
-}
-#elif defined (COMPILER_GCC)
-template <>
-struct hash<HANDLE> {
-  size_t operator()(const HANDLE& handle) const {
-    return HashHandle(handle);
-  }
-};
-#endif
-}  // BASE_HASH_NAMESPACE
-
 namespace base {
 namespace win {
 
 // Static.
-bool HandleTraits::CloseHandle(HANDLE handle) {
-  if (!g_verifier_enabled)
-    return CloseHandleWrapper(handle);
-
-  AutoNativeLock lock(g_lock.Get());
-  g_closing = true;
-  CloseHandleWrapper(handle);
-  g_closing = false;
-
-  return true;
-}
-
-// Static.
 void VerifierTraits::StartTracking(HANDLE handle, const void* owner,
                                    const void* pc1, const void* pc2) {
-  if (!g_verifier_enabled)
-    return;
-
   // Grab the thread id before the lock.
   DWORD thread_id = GetCurrentThreadId();
 
-  AutoNativeLock lock(g_lock.Get());
+  AutoLock lock(g_lock.Get());
 
   Info handle_info = { owner, pc1, pc2, thread_id };
   std::pair<HANDLE, Info> item(handle, handle_info);
@@ -114,10 +50,7 @@ void VerifierTraits::StartTracking(HANDLE handle, const void* owner,
 // Static.
 void VerifierTraits::StopTracking(HANDLE handle, const void* owner,
                                   const void* pc1, const void* pc2) {
-  if (!g_verifier_enabled)
-    return;
-
-  AutoNativeLock lock(g_lock.Get());
+  AutoLock lock(g_lock.Get());
   HandleMap::iterator i = g_handle_map.Get().find(handle);
   if (i == g_handle_map.Get().end())
     CHECK(false);
@@ -131,23 +64,5 @@ void VerifierTraits::StopTracking(HANDLE handle, const void* owner,
   g_handle_map.Get().erase(i);
 }
 
-void EnableHandleVerifier() {
-  g_verifier_enabled = true;
-}
-
-void OnHandleBeingClosed(HANDLE handle) {
-  AutoNativeLock lock(g_lock.Get());
-  if (g_closing)
-    return;
-
-  HandleMap::iterator i = g_handle_map.Get().find(handle);
-  if (i == g_handle_map.Get().end())
-    return;
-
-  Info other = i->second;
-  debug::Alias(&other);
-  CHECK(false);
-}
-
 }  // namespace win
 }  // namespace base
diff --git a/base/win/scoped_handle.h b/base/win/scoped_handle.h
index e1e0f5d..00063af 100644
--- a/base/win/scoped_handle.h
+++ b/base/win/scoped_handle.h
@@ -101,7 +101,9 @@ class GenericScopedHandle {
       Verifier::StopTracking(handle_, this, BASE_WIN_GET_CALLER,
                              tracked_objects::GetProgramCounter());
 
-      Traits::CloseHandle(handle_);
+      if (!Traits::CloseHandle(handle_))
+        CHECK(false);
+
       handle_ = Traits::NullHandle();
     }
   }
@@ -118,7 +120,9 @@ class HandleTraits {
   typedef HANDLE Handle;
 
   // Closes the handle.
-  static bool BASE_EXPORT CloseHandle(HANDLE handle);
+  static bool CloseHandle(HANDLE handle) {
+    return ::CloseHandle(handle) != FALSE;
+  }
 
   // Returns true if the handle value is valid.
   static bool IsHandleValid(HANDLE handle) {
@@ -164,17 +168,6 @@ class BASE_EXPORT VerifierTraits {
 
 typedef GenericScopedHandle<HandleTraits, VerifierTraits> ScopedHandle;
 
-// This function should be called by the embedder to enable the use of
-// VerifierTraits at runtime. It has no effect if DummyVerifierTraits is used
-// for ScopedHandle.
-void BASE_EXPORT EnableHandleVerifier();
-
-// This should be called whenever the OS is closing a handle, if extended
-// verification of improper handle closing is desired. If |handle| is being
-// tracked by the handle verifier and ScopedHandle is not the one closing it,
-// a CHECK is generated.
-void BASE_EXPORT OnHandleBeingClosed(HANDLE handle);
-
 }  // namespace win
 }  // namespace base