Provide a safety net for Windows hooks.

As mentioned in http://crbug.com/65655, we have seen cases where the IE process would crash in a Windows Hook called after static uninitialization. We are guessing that this occurs when the thread running the CEEE BHO got terminated without tearing down the BHO and thus we never unhooked.

This CL adds a module level set of hooks to be unhooked when the module gets terminated (before the static uninitialization).

BUG=65655
TEST=None

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

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

5 files changed, 58 insertions, 8 deletions

diff --git a/ceee/ie/common/ceee_module_util.h b/ceee/ie/common/ceee_module_util.h
index 3bef1e0..d7f8bf9 100644
--- a/ceee/ie/common/ceee_module_util.h
+++ b/ceee/ie/common/ceee_module_util.h
@@ -21,6 +21,13 @@ void Unlock();
 LONG LockModule();
 LONG UnlockModule();
 
+// Un/Register a hook to be unhooked by our termination safety net.
+// This is to protect ourselves against cases where the hook owner never
+// gets torn down (for whatever reason) and then the hook may be called when
+// we don't expect it to be.
+void RegisterHookForSafetyNet(HHOOK hook);
+void UnregisterHookForSafetyNet(HHOOK hook);
+
 class AutoLock {
  public:
   AutoLock() {
diff --git a/ceee/ie/plugin/bho/window_message_source.cc b/ceee/ie/plugin/bho/window_message_source.cc
index ec6b0ea..1ec5272 100644
--- a/ceee/ie/plugin/bho/window_message_source.cc
+++ b/ceee/ie/plugin/bho/window_message_source.cc
@@ -11,6 +11,7 @@
 #include "base/win_util.h"
 #include "ceee/common/window_utils.h"
 #include "ceee/common/windows_constants.h"
+#include "ceee/ie/common/ceee_module_util.h"
 
 WindowMessageSource::MessageSourceMap WindowMessageSource::message_source_map_;
 Lock WindowMessageSource::lock_;
@@ -37,6 +38,7 @@ bool WindowMessageSource::Initialize() {
     TearDown();
     return false;
   }
+  ceee_module_util::RegisterHookForSafetyNet(get_message_hook_);
 
   call_wnd_proc_ret_hook_ = ::SetWindowsHookEx(WH_CALLWNDPROCRET,
                                                CallWndProcRetHookProc, NULL,
@@ -45,16 +47,19 @@ bool WindowMessageSource::Initialize() {
     TearDown();
     return false;
   }
+  ceee_module_util::RegisterHookForSafetyNet(call_wnd_proc_ret_hook_);
   return true;
 }
 
 void WindowMessageSource::TearDown() {
   if (get_message_hook_ != NULL) {
+    ceee_module_util::UnregisterHookForSafetyNet(get_message_hook_);
     ::UnhookWindowsHookEx(get_message_hook_);
     get_message_hook_ = NULL;
   }
 
   if (call_wnd_proc_ret_hook_ != NULL) {
+    ceee_module_util::UnregisterHookForSafetyNet(call_wnd_proc_ret_hook_);
     ::UnhookWindowsHookEx(call_wnd_proc_ret_hook_);
     call_wnd_proc_ret_hook_ = NULL;
   }
diff --git a/ceee/ie/plugin/toolband/toolband_module.cc b/ceee/ie/plugin/toolband/toolband_module.cc
index 6e99c97..e1b068a 100644
--- a/ceee/ie/plugin/toolband/toolband_module.cc
+++ b/ceee/ie/plugin/toolband/toolband_module.cc
@@ -62,6 +62,13 @@ class ToolbandModule : public CAtlDllModuleT<ToolbandModule> {
     return module_initialized_;
   }
 
+  // Un/Register a hook to be unhooked by our termination safety net.
+  // This is to protect ourselves against cases where the hook owner never
+  // gets torn down (for whatever reason) and then the hook may be called when
+  // we don't expect it to be.
+  void RegisterHookForSafetyNet(HHOOK hook);
+  void UnregisterHookForSafetyNet(HHOOK hook);
+
   // We override reg/unregserver to register proxy/stubs.
   HRESULT DllRegisterServer();
   HRESULT DllUnregisterServer();
@@ -70,6 +77,9 @@ class ToolbandModule : public CAtlDllModuleT<ToolbandModule> {
   base::AtExitManager at_exit_;
   bool module_initialized_;
   bool crash_reporting_initialized_;
+  // Accumulate Hooks that may need to be freed before unloading DLL.
+  // Thread protected by m_csStaticDataInitAndTypeInfo.
+  std::set<HHOOK> hooks_;
 };
 
 ToolbandModule::ToolbandModule()
@@ -126,6 +136,16 @@ HRESULT ToolbandModule::DllGetClassObject(REFCLSID clsid, REFIID iid,
   return Super::DllGetClassObject(clsid, iid, object);
 }
 
+void ToolbandModule::RegisterHookForSafetyNet(HHOOK hook) {
+  CComCritSecLock<CComCriticalSection> lock(m_csStaticDataInitAndTypeInfo);
+  hooks_.insert(hook);
+}
+
+void ToolbandModule::UnregisterHookForSafetyNet(HHOOK hook) {
+  CComCritSecLock<CComCriticalSection> lock(m_csStaticDataInitAndTypeInfo);
+  hooks_.erase(hook);
+}
+
 HRESULT ToolbandModule::DllRegisterServer() {
   // No typelib registration.
   HRESULT hr = Super::DllRegisterServer();
@@ -173,6 +193,15 @@ void ToolbandModule::Init() {
 
 void ToolbandModule::Term() {
   CComCritSecLock<CComCriticalSection> lock(m_csStaticDataInitAndTypeInfo);
+  // We do this before checking if we were initialized because it is not
+  // related to initialization or not, but to un/registration from outsiders.
+  while (hooks_.size() > 0) {
+    std::set<HHOOK>::iterator it = hooks_.begin();
+    VLOG(1) << "Hook '" << *it << "' has fallen in our safety net.";
+    ::UnhookWindowsHookEx(*it);
+    hooks_.erase(it);
+  }
+
   if (!module_initialized_)
     return;
   if (crash_reporting_initialized_) {
@@ -201,6 +230,14 @@ LONG ceee_module_util::UnlockModule() {
   return module.Unlock();
 }
 
+void ceee_module_util::RegisterHookForSafetyNet(HHOOK hook) {
+  module.RegisterHookForSafetyNet(hook);
+}
+
+void ceee_module_util::UnregisterHookForSafetyNet(HHOOK hook) {
+  module.UnregisterHookForSafetyNet(hook);
+}
+
 // DLL Entry Point.
 extern "C" BOOL WINAPI DllMain(HINSTANCE instance, DWORD reason,
                                LPVOID reserved) {
diff --git a/ceee/ie/testing/ie_unittest_main.cc b/ceee/ie/testing/ie_unittest_main.cc
index 7727820..76ff071 100644
--- a/ceee/ie/testing/ie_unittest_main.cc
+++ b/ceee/ie/testing/ie_unittest_main.cc
@@ -24,10 +24,10 @@ LONG LockModule() {
 LONG UnlockModule() {
   return 0;
 }
-void Lock() {
-}
-void Unlock() {
-}
+void Lock() {}
+void Unlock() {}
+void RegisterHookForSafetyNet(HHOOK hook) {}
+void UnregisterHookForSafetyNet(HHOOK hook) {}
 
 }  // namespace ceee_module_util
 
diff --git a/ceee/ie/testing/mediumtest_ie_main.cc b/ceee/ie/testing/mediumtest_ie_main.cc
index 07c2755..d3d8786 100644
--- a/ceee/ie/testing/mediumtest_ie_main.cc
+++ b/ceee/ie/testing/mediumtest_ie_main.cc
@@ -23,10 +23,11 @@ LONG LockModule() {
 LONG UnlockModule() {
   return 0;
 }
-void Lock() {
-}
-void Unlock() {
-}
+
+void Lock() {}
+void Unlock() {}
+void RegisterHookForSafetyNet(HHOOK hook) {}
+void UnregisterHookForSafetyNet(HHOOK hook) {}
 
 }  // namespace ceee_module_util