Start and stop crash reporting outside of the loader lock.

Instead of using DllMain to start/stop crash reporting, it is now done by way of a specialization of a new ScopedInitializationManager template. Instances of this specialization are created on the stack in entrypoints to the DLL (for registration or to get a COM object). The lifetime of crash reporting is ordinarily bound to the lifetime of the ATL module. The exception to this is when the module is pinned, at which point crash reporting is also pinned.

This change removes the breakpad_handler_dll target (by reverting http://crrev.com/70898) since it is no longer needed.

BUG=163455
TEST=install chrome frame and notice that installation doesn't block for 1 minute while npchrome_frame.dll is registered.

Review URL: https://chromiumcodereview.appspot.com/12521002

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

13 files changed, 330 insertions, 69 deletions

diff --git a/chrome_frame/buggy_bho_handling.cc b/chrome_frame/buggy_bho_handling.cc
index 9d00528..fef41f4 100644
--- a/chrome_frame/buggy_bho_handling.cc
+++ b/chrome_frame/buggy_bho_handling.cc
@@ -11,6 +11,7 @@
 #include "base/win/scoped_comptr.h"
 #include "chrome_frame/exception_barrier.h"
 #include "chrome_frame/function_stub.h"
+#include "chrome_frame/pin_module.h"
 #include "chrome_frame/utils.h"
 #include "chrome_frame/vtable_patch_manager.h"
 
@@ -225,7 +226,7 @@ HRESULT BuggyBhoTls::PatchInvokeMethod(PROC* invoke) {
       hr = E_UNEXPECTED;
       FunctionStub::Destroy(stub);
     } else {
-      PinModule();  // No backing out now.
+      chrome_frame::PinModule();  // No backing out now.
       ::FlushInstructionCache(::GetCurrentProcess(), invoke, sizeof(PROC));
     }
   }
diff --git a/chrome_frame/chrome_frame.gyp b/chrome_frame/chrome_frame.gyp
index 797f731..ddf2fe2 100644
--- a/chrome_frame/chrome_frame.gyp
+++ b/chrome_frame/chrome_frame.gyp
@@ -122,6 +122,7 @@
         'chrome_launcher.h',
         'chrome_launcher_unittest.cc',
         'function_stub_unittest.cc',
+        'scoped_initialization_manager_unittest.cc',
         'test/chrome_tab_mocks.h',
         'test/chrome_frame_test_utils.h',
         'test/chrome_frame_test_utils.cc',
@@ -745,6 +746,8 @@
         'infobars/infobar_manager.cc',
         'metrics_service.cc',
         'metrics_service.h',
+        'pin_module.cc',
+        'pin_module.h',
         'policy_settings.cc',
         'policy_settings.h',
         'protocol_sink_wrap.cc',
@@ -897,6 +900,7 @@
         'chrome_frame_reporting.h',
         'chrome_tab.cc',
         'chrome_tab.def',
+        'scoped_initialization_manager.h',
         '<(SHARED_INTERMEDIATE_DIR)/chrome_frame/chrome_tab.h',
         '<(SHARED_INTERMEDIATE_DIR)/chrome_frame/npchrome_frame_dll_version.rc',
         # FIXME(slightlyoff): For chrome_tab.tlb. Giant hack until we can
@@ -917,7 +921,7 @@
             '<(SHARED_INTERMEDIATE_DIR)/chrome_frame/chrome_frame_resources.rc',
           ],
           'dependencies': [
-            '../breakpad/breakpad.gyp:breakpad_handler_dll',
+            '../breakpad/breakpad.gyp:breakpad_handler',
             '../chrome/chrome.gyp:automation',
             # Installer
             '../chrome/chrome.gyp:installer_util',
diff --git a/chrome_frame/chrome_frame_reporting.cc b/chrome_frame/chrome_frame_reporting.cc
index f9fa490..0a0608a 100644
--- a/chrome_frame/chrome_frame_reporting.cc
+++ b/chrome_frame/chrome_frame_reporting.cc
@@ -4,15 +4,22 @@
 
 // Implementation of wrapper around common crash reporting.
 
+#include "chrome_frame/chrome_frame_reporting.h"
+
+#include "base/basictypes.h"
 #include "base/file_util.h"
 #include "base/file_version_info.h"
 #include "base/win/win_util.h"
 #include "chrome/installer/util/google_update_settings.h"
 #include "chrome/installer/util/install_util.h"
-#include "chrome_frame/chrome_frame_reporting.h"
+#include "chrome_frame/crash_reporting/crash_report.h"
 #include "chrome_frame/exception_barrier.h"
 #include "chrome_frame/utils.h"
 
+extern "C" IMAGE_DOS_HEADER __ImageBase;
+
+namespace {
+
 // Well known SID for the system principal.
 const wchar_t kSystemPrincipalSid[] = L"S-1-5-18";
 const wchar_t kChromePipeName[] = L"\\\\.\\pipe\\ChromeCrashServices";
@@ -45,15 +52,13 @@ google_breakpad::CustomClientInfo* GetCustomInfo(const wchar_t* dll_path) {
   return &custom_info;
 }
 
-extern "C" IMAGE_DOS_HEADER __ImageBase;
-
 bool InitializeCrashReporting() {
   // In headless mode we want crashes to be reported back.
   bool always_take_dump = IsHeadlessMode();
   // We want to use the Google Update crash reporting. We need to check if the
   // user allows it first.
   if (!always_take_dump && !GoogleUpdateSettings::GetCollectStatsConsent())
-      return true;
+    return true;
 
   // If we got here, we want to report crashes, so make sure all
   // ExceptionBarrierBase instances do so.
@@ -61,17 +66,16 @@ bool InitializeCrashReporting() {
 
   // Get the alternate dump directory. We use the temp path.
   base::FilePath temp_directory;
-  if (!file_util::GetTempDir(&temp_directory) || temp_directory.empty()) {
+  if (!file_util::GetTempDir(&temp_directory) || temp_directory.empty())
     return false;
-  }
 
   wchar_t dll_path[MAX_PATH * 2] = {0};
   GetModuleFileName(reinterpret_cast<HMODULE>(&__ImageBase), dll_path,
                     arraysize(dll_path));
 
   if (always_take_dump) {
-    return InitializeVectoredCrashReportingWithPipeName(true, kChromePipeName,
-      temp_directory.value(), GetCustomInfo(dll_path));
+    return InitializeVectoredCrashReportingWithPipeName(
+        true, kChromePipeName, temp_directory.value(), GetCustomInfo(dll_path));
   }
 
   // Build the pipe name. It can be either:
@@ -79,18 +83,31 @@ bool InitializeCrashReporting() {
   // Per-user install: "NamedPipe\GoogleCrashServices\<user SID>"
   std::wstring user_sid;
   if (InstallUtil::IsPerUserInstall(dll_path)) {
-    if (!base::win::GetUserSidString(&user_sid)) {
+    if (!base::win::GetUserSidString(&user_sid))
       return false;
-    }
   } else {
     user_sid = kSystemPrincipalSid;
   }
 
-  return InitializeVectoredCrashReporting(false, user_sid.c_str(),
-      temp_directory.value(), GetCustomInfo(dll_path));
+  return InitializeVectoredCrashReporting(
+      false, user_sid.c_str(), temp_directory.value(), GetCustomInfo(dll_path));
 }
 
 bool ShutdownCrashReporting() {
   ExceptionBarrierConfig::set_enabled(false);
   return ShutdownVectoredCrashReporting();
 }
+
+}  // namespace
+
+namespace chrome_frame {
+
+void CrashReportingTraits::Initialize() {
+  InitializeCrashReporting();
+}
+
+void CrashReportingTraits::Shutdown() {
+  ShutdownCrashReporting();
+}
+
+}  // namespace chrome_frame
diff --git a/chrome_frame/chrome_frame_reporting.h b/chrome_frame/chrome_frame_reporting.h
index 94bfbdc..94ee7f9 100644
--- a/chrome_frame/chrome_frame_reporting.h
+++ b/chrome_frame/chrome_frame_reporting.h
@@ -2,22 +2,27 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-// A wrapper around common crash reporting code to manage reporting for Chrome
-// Frame.
-
 #ifndef CHROME_FRAME_CHROME_FRAME_REPORTING_H_
 #define CHROME_FRAME_CHROME_FRAME_REPORTING_H_
 
-#include "chrome_frame/crash_reporting/crash_report.h"
+#include "chrome_frame/scoped_initialization_manager.h"
+
+namespace chrome_frame {
 
-extern const wchar_t kSystemPrincipalSid[];
+// A Traits class for a ScopedInitializationManager that starts/stops crash
+// reporting for npchrome_frame.dll.
+class CrashReportingTraits {
+ public:
+  static void Initialize();
+  static void Shutdown();
+};
 
-// Intialize crash reporting for Chrome Frame. Specific parameters here include
-// using the temp directory for dumps, determining if the install is system
-// wide or user specific, and customized client info.
-bool InitializeCrashReporting();
+// Manages crash reporting for the Chrome Frame dll. Crash reporting cannot be
+// reliably started or stopped when the loader lock is held, so DllMain cannot
+// be used to start/stop reporting. Rather, instances of this class are used in
+// each entrypoint into the dll.
+typedef ScopedInitializationManager<CrashReportingTraits> ScopedCrashReporting;
 
-// Shut down crash reporting for Chrome Frame.
-bool ShutdownCrashReporting();
+}  // namespace chrome_frame
 
 #endif  // CHROME_FRAME_CHROME_FRAME_REPORTING_H_
diff --git a/chrome_frame/chrome_tab.cc b/chrome_frame/chrome_tab.cc
index 8b25a05..cea60f8 100644
--- a/chrome_frame/chrome_tab.cc
+++ b/chrome_frame/chrome_tab.cc
@@ -11,6 +11,7 @@
 #include <objbase.h>
 
 #include "base/at_exit.h"
+#include "base/basictypes.h"
 #include "base/command_line.h"
 #include "base/file_util.h"
 #include "base/file_version_info.h"
@@ -40,6 +41,7 @@
 #include "chrome_frame/dll_redirector.h"
 #include "chrome_frame/exception_barrier.h"
 #include "chrome_frame/metrics_service.h"
+#include "chrome_frame/pin_module.h"
 #include "chrome_frame/resource.h"
 #include "chrome_frame/utils.h"
 #include "googleurl/src/url_util.h"
@@ -48,16 +50,6 @@
 using base::win::RegKey;
 
 namespace {
-// This function has the side effect of initializing an unprotected
-// vector pointer inside GoogleUrl. If this is called during DLL loading,
-// it has the effect of avoiding an initialization race on that pointer.
-// TODO(siggi): fix GoogleUrl.
-void InitGoogleUrl() {
-  static const char kDummyUrl[] = "http://www.google.com";
-
-  url_util::IsStandard(kDummyUrl,
-                       url_parse::MakeRange(0, arraysize(kDummyUrl)));
-}
 
 const wchar_t kInternetSettings[] =
     L"Software\\Microsoft\\Windows\\CurrentVersion\\Internet Settings";
@@ -83,7 +75,7 @@ const wchar_t kChromeFrameHelperWindowName[] =
     L"ChromeFrameHelperWindowName";
 
 // {0562BFC3-2550-45b4-BD8E-A310583D3A6F}
-static const GUID kChromeFrameProvider =
+const GUID kChromeFrameProvider =
     { 0x562bfc3, 0x2550, 0x45b4,
         { 0xbd, 0x8e, 0xa3, 0x10, 0x58, 0x3d, 0x3a, 0x6f } };
 
@@ -95,11 +87,22 @@ const wchar_t kChromeFramePrefix[] = L"chromeframe/";
 // See comments in DllGetClassObject.
 LPFNGETCLASSOBJECT g_dll_get_class_object_redir_ptr = NULL;
 
+// This function has the side effect of initializing an unprotected
+// vector pointer inside GoogleUrl. If this is called during DLL loading,
+// it has the effect of avoiding an initialization race on that pointer.
+// TODO(siggi): fix GoogleUrl.
+void InitGoogleUrl() {
+  static const char kDummyUrl[] = "http://www.google.com";
+
+  url_util::IsStandard(kDummyUrl,
+                       url_parse::MakeRange(0, arraysize(kDummyUrl)));
+}
+
 class ChromeTabModule : public CAtlDllModuleT<ChromeTabModule> {
  public:
   typedef CAtlDllModuleT<ChromeTabModule> ParentClass;
 
-  ChromeTabModule() : do_system_registration_(true) {}
+  ChromeTabModule() : do_system_registration_(true), crash_reporting_(NULL) {}
 
   DECLARE_LIBID(LIBID_ChromeTabLib)
   DECLARE_REGISTRY_APPID_RESOURCEID(IDR_CHROMETAB,
@@ -189,8 +192,42 @@ class ChromeTabModule : public CAtlDllModuleT<ChromeTabModule> {
     return hr;
   }
 
+  // The module is "locked" when an object takes a reference on it. The first
+  // time it is locked, take a reference on crash reporting to bind its lifetime
+  // to the module.
+  virtual LONG Lock() throw() {
+    LONG result = ParentClass::Lock();
+    if (result == 1) {
+      DCHECK_EQ(crash_reporting_,
+                static_cast<chrome_frame::ScopedCrashReporting*>(NULL));
+      crash_reporting_ = new chrome_frame::ScopedCrashReporting();
+    }
+    return result;
+  }
+
+  // The module is "unlocked" when an object that had a reference on it is
+  // destroyed. The last time it is unlocked, release the reference on crash
+  // reporting.
+  virtual LONG Unlock() throw() {
+    LONG result = ParentClass::Unlock();
+    if (!result) {
+      DCHECK_NE(crash_reporting_,
+                static_cast<chrome_frame::ScopedCrashReporting*>(NULL));
+      delete crash_reporting_;
+      crash_reporting_ = NULL;
+    }
+    return result;
+  }
+
   // See comments in AddCommonRGSReplacements
   bool do_system_registration_;
+
+ private:
+  // A scoper created when the module is initially locked and destroyed when it
+  // is finally unlocked. This is not a scoped_ptr since that could cause
+  // reporting to shut down at exit, which would lead to problems with the
+  // loader lock.
+  chrome_frame::ScopedCrashReporting* crash_reporting_;
 };
 
 ChromeTabModule _AtlModule;
@@ -548,6 +585,11 @@ HRESULT SetOrDeleteMimeHandlerKey(bool set, HKEY root_key) {
                                     HRESULT_FROM_WIN32(result2);
 }
 
+void OnPinModule() {
+  // Pin crash reporting by leaking a reference.
+  ignore_result(new chrome_frame::ScopedCrashReporting());
+}
+
 // Chrome Frame registration functions.
 //-----------------------------------------------------------------------------
 HRESULT RegisterSecuredMimeHandler(bool enable, bool is_system) {
@@ -829,7 +871,6 @@ extern "C" BOOL WINAPI DllMain(HINSTANCE instance,
 
     g_exit_manager = new base::AtExitManager();
     CommandLine::Init(0, NULL);
-    InitializeCrashReporting();
     logging::InitLogging(
         NULL,
         logging::LOG_ONLY_TO_SYSTEM_DEBUG_LOG,
@@ -862,6 +903,10 @@ extern "C" BOOL WINAPI DllMain(HINSTANCE instance,
     // can only get called once. For Chrome Frame, that is here.
     g_field_trial_list = new base::FieldTrialList(
         new metrics::SHA1EntropyProvider(MetricsService::GetClientID()));
+
+    // Set a callback so that crash reporting can be pinned when the module is
+    // pinned.
+    chrome_frame::SetPinModuleCallback(&OnPinModule);
   } else if (reason == DLL_PROCESS_DETACH) {
     delete g_field_trial_list;
     g_field_trial_list = NULL;
@@ -874,8 +919,6 @@ extern "C" BOOL WINAPI DllMain(HINSTANCE instance,
 
     delete g_exit_manager;
     g_exit_manager = NULL;
-
-    ShutdownCrashReporting();
   }
   return _AtlModule.DllMain(reason, reserved);
 }
@@ -887,6 +930,8 @@ STDAPI DllCanUnloadNow() {
 
 // Returns a class factory to create an object of the requested type
 STDAPI DllGetClassObject(REFCLSID rclsid, REFIID riid, LPVOID* ppv) {
+  chrome_frame::ScopedCrashReporting crash_reporting;
+
   // If we found another module present when we were loaded, then delegate to
   // that:
   if (g_dll_get_class_object_redir_ptr) {
@@ -904,6 +949,7 @@ STDAPI DllGetClassObject(REFCLSID rclsid, REFIID riid, LPVOID* ppv) {
 
 // DllRegisterServer - Adds entries to the system registry
 STDAPI DllRegisterServer() {
+  chrome_frame::ScopedCrashReporting crash_reporting;
   uint16 flags =  ACTIVEX | ACTIVEDOC | TYPELIB | GCF_PROTOCOL |
                   BHO_CLSID | BHO_REGISTRATION;
 
@@ -917,12 +963,14 @@ STDAPI DllRegisterServer() {
 
 // DllUnregisterServer - Removes entries from the system registry
 STDAPI DllUnregisterServer() {
+  chrome_frame::ScopedCrashReporting crash_reporting;
   HRESULT hr = CustomRegistration(ALL, false, true);
   return hr;
 }
 
 // DllRegisterUserServer - Adds entries to the HKCU hive in the registry.
 STDAPI DllRegisterUserServer() {
+  chrome_frame::ScopedCrashReporting crash_reporting;
   UINT flags =  ACTIVEX | ACTIVEDOC | TYPELIB | GCF_PROTOCOL |
                 BHO_CLSID | BHO_REGISTRATION;
 
@@ -936,6 +984,7 @@ STDAPI DllRegisterUserServer() {
 
 // DllUnregisterUserServer - Removes entries from the HKCU hive in the registry.
 STDAPI DllUnregisterUserServer() {
+  chrome_frame::ScopedCrashReporting crash_reporting;
   HRESULT hr = CustomRegistration(ALL, FALSE, false);
   return hr;
 }
diff --git a/chrome_frame/infobars/internal/subclassing_window_with_delegate.h b/chrome_frame/infobars/internal/subclassing_window_with_delegate.h
index 9bb6ab28..fc5f0ca 100644
--- a/chrome_frame/infobars/internal/subclassing_window_with_delegate.h
+++ b/chrome_frame/infobars/internal/subclassing_window_with_delegate.h
@@ -12,7 +12,7 @@
 #include "base/basictypes.h"
 #include "base/logging.h"
 #include "base/memory/scoped_ptr.h"
-#include "chrome_frame/utils.h"
+#include "chrome_frame/pin_module.h"
 
 // Implements behavior common to HostWindowManager and DisplacedWindowManager.
 template<typename T> class SubclassingWindowWithDelegate
@@ -53,7 +53,7 @@ template<typename T> class SubclassingWindowWithDelegate
 
     // Ensure we won't be unloaded while our window proc is attached to the tab
     // window.
-    PinModule();
+    chrome_frame::PinModule();
 
     delegate_.swap(new_delegate);
 
diff --git a/chrome_frame/pin_module.cc b/chrome_frame/pin_module.cc
new file mode 100644
index 0000000..b4cebbe
--- /dev/null
+++ b/chrome_frame/pin_module.cc
@@ -0,0 +1,49 @@
+// Copyright 2013 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 "chrome_frame/pin_module.h"
+
+#include <windows.h>
+
+#include "base/logging.h"
+#include "chrome_frame/utils.h"
+
+extern "C" IMAGE_DOS_HEADER __ImageBase;
+
+namespace chrome_frame {
+
+namespace {
+
+PinModuleCallbackFn g_pin_module_callback = NULL;
+
+}  // namespace
+
+void SetPinModuleCallback(PinModuleCallbackFn callback) {
+  g_pin_module_callback = callback;
+}
+
+void PinModule() {
+  static bool s_pinned = false;
+  if (!s_pinned && !IsUnpinnedMode()) {
+    wchar_t system_buffer[MAX_PATH];
+    HMODULE this_module = reinterpret_cast<HMODULE>(&__ImageBase);
+    system_buffer[0] = 0;
+    if (GetModuleFileName(this_module, system_buffer,
+                          arraysize(system_buffer)) != 0) {
+      HMODULE unused;
+      if (!GetModuleHandleEx(GET_MODULE_HANDLE_EX_FLAG_PIN, system_buffer,
+                             &unused)) {
+        DPLOG(FATAL) << "Failed to pin module " << system_buffer;
+      } else {
+        s_pinned = true;
+        if (g_pin_module_callback)
+          g_pin_module_callback();
+      }
+    } else {
+      DPLOG(FATAL) << "Could not get module path.";
+    }
+  }
+}
+
+}  // namespace chrome_frame
diff --git a/chrome_frame/pin_module.h b/chrome_frame/pin_module.h
new file mode 100644
index 0000000..8b4b4e8
--- /dev/null
+++ b/chrome_frame/pin_module.h
@@ -0,0 +1,21 @@
+// Copyright 2013 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.
+
+#ifndef CHROME_FRAME_PIN_MODULE_H_
+#define CHROME_FRAME_PIN_MODULE_H_
+
+namespace chrome_frame {
+
+typedef void (*PinModuleCallbackFn)(void);
+
+// Sets a callback function to be invoked when the module is pinned.
+void SetPinModuleCallback(PinModuleCallbackFn callback);
+
+// Utility function that prevents the current module from ever being unloaded.
+// Call if you make irreversible patches.
+void PinModule();
+
+}  // namespace chrome_frame
+
+#endif  // CHROME_FRAME_PIN_MODULE_H_
diff --git a/chrome_frame/scoped_initialization_manager.h b/chrome_frame/scoped_initialization_manager.h
new file mode 100644
index 0000000..0a22f4f
--- /dev/null
+++ b/chrome_frame/scoped_initialization_manager.h
@@ -0,0 +1,54 @@
+// Copyright 2013 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.
+
+#ifndef CHROME_FRAME_SCOPED_INITIALIZATION_MANAGER_H_
+#define CHROME_FRAME_SCOPED_INITIALIZATION_MANAGER_H_
+
+#include "base/basictypes.h"
+#include "base/lazy_instance.h"
+#include "base/synchronization/lock.h"
+
+namespace chrome_frame {
+
+// A class intended to be instantiated on the stack in a dyanmically loaded
+// shared object to initialize and shutdown the object's dependencies. |Traits|
+// must be a type with two public static void(void) functions named Initialize
+// and Shutdown. Traits::Initialize will be invoked when the first instance of
+// this class is created and Traits::Shutdown will be invoked when the last one
+// is destroyed.
+template<class Traits>
+class ScopedInitializationManager {
+ public:
+  ScopedInitializationManager() { AddRef(); }
+  ~ScopedInitializationManager() { Release(); }
+
+ private:
+  static void AddRef() {
+    base::AutoLock auto_lock(lock_.Get());
+    DCHECK_LT(ref_count_, kuint32max);
+    if (++ref_count_ == 1)
+      Traits::Initialize();
+  }
+
+  static void Release() {
+    base::AutoLock auto_lock(lock_.Get());
+    DCHECK_GT(ref_count_, 0U);
+    if (--ref_count_ == 0)
+      Traits::Shutdown();
+  }
+
+  static base::LazyInstance<base::Lock>::Leaky lock_;
+  static uint32 ref_count_;
+  DISALLOW_COPY_AND_ASSIGN(ScopedInitializationManager);
+};
+
+template<class Traits> base::LazyInstance<base::Lock>::Leaky
+    ScopedInitializationManager<Traits>::lock_ = LAZY_INSTANCE_INITIALIZER;
+
+template<class Traits> uint32
+    ScopedInitializationManager<Traits>::ref_count_ = 0;
+
+}  // namespace chrome_frame
+
+#endif  // CHROME_FRAME_SCOPED_INITIALIZATION_MANAGER_H_
diff --git a/chrome_frame/scoped_initialization_manager_unittest.cc b/chrome_frame/scoped_initialization_manager_unittest.cc
new file mode 100644
index 0000000..5c8c837
--- /dev/null
+++ b/chrome_frame/scoped_initialization_manager_unittest.cc
@@ -0,0 +1,86 @@
+// Copyright 2013 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 "chrome_frame/scoped_initialization_manager.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+using ::testing::InSequence;
+
+// Initialization traits that delegate to a registered delegate instance.
+class TestInitializationTraits {
+ public:
+  class Delegate {
+   public:
+    virtual void Initialize() = 0;
+    virtual void Shutdown() = 0;
+  };
+
+  static void SetDelegate(Delegate* delegate) {
+    delegate_ = delegate;
+  }
+
+  static void Initialize() {
+    ASSERT_TRUE(delegate_ != NULL);
+    delegate_->Initialize();
+  }
+
+  static void Shutdown() {
+    ASSERT_TRUE(delegate_ != NULL);
+    delegate_->Shutdown();
+  }
+
+ private:
+  static Delegate* delegate_;
+};
+
+TestInitializationTraits::Delegate* TestInitializationTraits::delegate_ = NULL;
+
+// A mock delegate for use in tests.
+class MockInitializationTraitsDelegate
+    : public TestInitializationTraits::Delegate {
+ public:
+  MOCK_METHOD0(Initialize, void());
+  MOCK_METHOD0(Shutdown, void());
+};
+
+// A test fixture that sets up a mock traits delegate for use in tests.
+class ScopedInitializationManagerTest : public ::testing::Test {
+ protected:
+  // A manager type that will invoke methods
+  typedef chrome_frame::ScopedInitializationManager<TestInitializationTraits>
+      TestScopedInitializationManager;
+
+  virtual void SetUp() OVERRIDE {
+    TestInitializationTraits::SetDelegate(&mock_traits_delegate_);
+  }
+  virtual void TearDown() OVERRIDE {
+    TestInitializationTraits::SetDelegate(NULL);
+  }
+
+  MockInitializationTraitsDelegate mock_traits_delegate_;
+};
+
+// Test that Initialize and Shutdown are called in order for the simple case.
+TEST_F(ScopedInitializationManagerTest, InitializeAndShutdown) {
+  {
+    InSequence dummy;
+    EXPECT_CALL(mock_traits_delegate_, Initialize());
+    EXPECT_CALL(mock_traits_delegate_, Shutdown());
+  }
+  TestScopedInitializationManager test_manager;
+}
+
+// Test that Initialize and Shutdown are called in order only once despite
+// multiple instances.
+TEST_F(ScopedInitializationManagerTest, InitializeAndShutdownOnlyOnce) {
+  {
+    InSequence dummy;
+    EXPECT_CALL(mock_traits_delegate_, Initialize());
+    EXPECT_CALL(mock_traits_delegate_, Shutdown());
+  }
+  TestScopedInitializationManager test_manager_1;
+  TestScopedInitializationManager test_manager_2;
+  TestScopedInitializationManager test_manager_3;
+}
diff --git a/chrome_frame/utils.cc b/chrome_frame/utils.cc
index 70abb32..7a70e28 100644
--- a/chrome_frame/utils.cc
+++ b/chrome_frame/utils.cc
@@ -1499,27 +1499,6 @@ bool CanNavigate(const GURL& url,
   return true;
 }
 
-void PinModule() {
-  static bool s_pinned = false;
-  if (!s_pinned && !IsUnpinnedMode()) {
-    wchar_t system_buffer[MAX_PATH];
-    HMODULE this_module = reinterpret_cast<HMODULE>(&__ImageBase);
-    system_buffer[0] = 0;
-    if (GetModuleFileName(this_module, system_buffer,
-                          arraysize(system_buffer)) != 0) {
-      HMODULE unused;
-      if (!GetModuleHandleEx(GET_MODULE_HANDLE_EX_FLAG_PIN, system_buffer,
-                             &unused)) {
-        DPLOG(FATAL) << "Failed to pin module " << system_buffer;
-      } else {
-        s_pinned = true;
-      }
-    } else {
-      DPLOG(FATAL) << "Could not get module path.";
-    }
-  }
-}
-
 void WaitWithMessageLoop(HANDLE* handles, int count, DWORD timeout) {
   base::Time now = base::Time::Now();
   base::Time wait_until = now + base::TimeDelta::FromMilliseconds(timeout);
diff --git a/chrome_frame/utils.h b/chrome_frame/utils.h
index 29084db..4341880 100644
--- a/chrome_frame/utils.h
+++ b/chrome_frame/utils.h
@@ -548,10 +548,6 @@ class NavigationConstraints;
 bool CanNavigate(const GURL& url,
                  NavigationConstraints* navigation_constraints);
 
-// Utility function that prevents the current module from ever being unloaded.
-// Call if you make irreversible patches.
-void PinModule();
-
 // Helper function to spin a message loop and dispatch messages while waiting
 // for a handle to be signaled.
 void WaitWithMessageLoop(HANDLE* handles, int count, DWORD timeout);
diff --git a/chrome_frame/vtable_patch_manager.cc b/chrome_frame/vtable_patch_manager.cc
index 30a8fe2..da1bbab 100644
--- a/chrome_frame/vtable_patch_manager.cc
+++ b/chrome_frame/vtable_patch_manager.cc
@@ -13,7 +13,7 @@
 #include "base/memory/scoped_ptr.h"
 #include "base/synchronization/lock.h"
 #include "chrome_frame/function_stub.h"
-#include "chrome_frame/utils.h"
+#include "chrome_frame/pin_module.h"
 
 namespace vtable_patch {
 
@@ -144,7 +144,7 @@ HRESULT PatchInterfaceMethods(void* unknown, MethodPatchInfo* patches) {
     } else {
       // Success, save the stub we created.
       it->stub_ = stub;
-      PinModule();
+      chrome_frame::PinModule();
     }
   }