Issue #3432 - Manage COM events with a resource class

src/plugin/PluginClass.cpp

Index: src/plugin/PluginClass.cpp
===================================================================
--- a/src/plugin/PluginClass.cpp
+++ b/src/plugin/PluginClass.cpp
@@ -90,7 +90,8 @@
 }
 
 CPluginClass::CPluginClass()
-  : m_webBrowser2(nullptr)
+  : m_webBrowser2(nullptr),
+  detachedInitializationFailed(false)
 {
   DEBUG_GENERAL([this]() -> std::wstring
     {
@@ -102,7 +103,6 @@
   //Use this line to debug memory leaks
   //	_CrtDumpMemoryLeaks();
 
-  m_isAdvised = false;
   m_hTabWnd = NULL;
   m_hStatusBarWnd = NULL;
   m_hPaneWnd = NULL;
@@ -187,11 +187,11 @@
 {
   if (thisPtr == NULL)
     return 0;
-  if (!((CPluginClass*)thisPtr)->InitObject())
+  auto self = static_cast<CPluginClass*>(thisPtr);
+  if (!self->InitObject())
   {
-    ((CPluginClass*)thisPtr)->Unadvise();
+    self->detachedInitializationFailed = true;
   }
-
   return 0;
 }
 
@@ -218,7 +218,6 @@
       {
         DEBUG_ERROR_LOG(hr, PLUGIN_ERROR_SET_SITE, PLUGIN_ERROR_SET_SITE_COINIT, "Class::SetSite - CoInitialize");
       }
-
       /*
        * We were instantiated as a BHO, so our site is always of type IWebBrowser2.
        */
@@ -249,34 +248,22 @@
 
       try
       {
-        HRESULT hr = DispEventAdvise(m_webBrowser2);
-        if (SUCCEEDED(hr))
-        {
-          m_isAdvised = true;
-          try
-          {
-            std::thread startInitObjectThread(StartInitObject, this);
-            startInitObjectThread.detach(); // TODO: but actually we should wait for the thread in the dtr.
-          }
-          catch (const std::system_error& ex)
-          {
-            DEBUG_SYSTEM_EXCEPTION(ex, PLUGIN_ERROR_THREAD, PLUGIN_ERROR_MAIN_THREAD_CREATE_PROCESS, 
-              "Class::Thread - Failed to create StartInitObject thread");
-          }
-        }
-        else
-        {
-          DEBUG_ERROR_LOG(hr, PLUGIN_ERROR_SET_SITE, PLUGIN_ERROR_SET_SITE_ADVICE, "Class::SetSite - Advise");
-        }
+        std::thread startInitObjectThread(StartInitObject, this);
+        startInitObjectThread.detach(); // TODO: but actually we should wait for the thread in the dtr.
       }
-      catch (const std::runtime_error& ex)
+      catch (const std::system_error& ex)
       {
-        DEBUG_EXCEPTION(ex);
-        Unadvise();
+        detachedInitializationFailed = true;
+        DEBUG_SYSTEM_EXCEPTION(ex, PLUGIN_ERROR_THREAD, PLUGIN_ERROR_MAIN_THREAD_CREATE_PROCESS,
+          "Class::Thread - Failed to create StartInitObject thread");
       }
+
+      // Start events last to alleviate issues with deferred evaluation
+      browserEvents.Start(this, m_webBrowser2);
     }
     else
     {
+      browserEvents.Stop();
       DEBUG_GENERAL([this]() -> std::wstring
       {
         std::wstringstream ss;
@@ -285,7 +272,6 @@
         return ss.str();
       }());
 
-      Unadvise();
 
       // Destroy window
       if (m_pWndProcStatus)
@@ -465,6 +451,8 @@
 {
   try
   {
+    if (detachedInitializationFailed) return;
+
     ATL::CComQIPtr<IWebBrowser2> webBrowser = frameBrowserDisp;
     if (!webBrowser)
     {
@@ -509,6 +497,7 @@
 // Entry point
 void STDMETHODCALLTYPE CPluginClass::OnDownloadComplete()
 {
+  if (detachedInitializationFailed) return;
   try
   {
     if (!m_webBrowser2)
@@ -527,6 +516,7 @@
 // Entry point
 void STDMETHODCALLTYPE CPluginClass::OnDocumentComplete(IDispatch* frameBrowserDisp, VARIANT* /*urlOrPidl*/)
 {
+  if (detachedInitializationFailed) return;
   try
   {
     DEBUG_NAVI(L"Navi::Document Complete");
@@ -546,6 +536,7 @@
 // Entry point
 void STDMETHODCALLTYPE CPluginClass::OnWindowStateChanged(unsigned long flags, unsigned long validFlagsMask)
 {
+  if (detachedInitializationFailed) return;
   try
   {
     DEBUG_GENERAL(L"WindowStateChanged (check tab changed)");
@@ -576,6 +567,7 @@
 // Entry point
 void STDMETHODCALLTYPE CPluginClass::OnCommandStateChange(long /*command*/, VARIANT_BOOL /*enable*/)
 {
+  if (detachedInitializationFailed) return;
   try
   {
     if (m_hPaneWnd == NULL)
@@ -601,18 +593,6 @@
   }
 }
 
-// Entry point
-void STDMETHODCALLTYPE CPluginClass::OnOnQuit()
-{
-  try
-  {
-    Unadvise();
-  }
-  catch (...)
-  {
-  }
-}
-
 bool CPluginClass::InitObject()
 {
   DEBUG_GENERAL("InitObject - begin");
@@ -1001,6 +981,7 @@
 // Entry point
 STDMETHODIMP CPluginClass::QueryStatus(const GUID* pguidCmdGroup, ULONG cCmds, OLECMD prgCmds[], OLECMDTEXT* pCmdText)
 {
+  if (detachedInitializationFailed) return S_OK;
   try
   {
     if (cCmds == 0) return E_INVALIDARG;
@@ -1217,6 +1198,7 @@
 // Entry point
 STDMETHODIMP CPluginClass::Exec(const GUID*, DWORD nCmdID, DWORD, VARIANTARG*, VARIANTARG*)
 {
+  if (detachedInitializationFailed) return S_OK;
   try
   {
     HWND hBrowserWnd = GetBrowserHWND();
@@ -1642,29 +1624,6 @@
   }
 }
 
-
-void CPluginClass::Unadvise()
-{
-  if (!m_webBrowser2)
-  {
-    DEBUG_ERROR_LOG(0, 0, 0, "CPluginClass::Unadvise - Reached with m_webBrowser2 == nullptr");
-    return;
-  }
-  s_criticalSectionLocal.Lock();
-  {
-    if (m_isAdvised)
-    {
-      HRESULT hr = DispEventUnadvise(m_webBrowser2);
-      if (FAILED(hr))
-      {
-        DEBUG_ERROR_LOG(hr, PLUGIN_ERROR_SET_SITE, PLUGIN_ERROR_SET_SITE_UNADVISE, "Class::Unadvise - Unadvise");
-      }
-      m_isAdvised = false;
-    }
-  }
-  s_criticalSectionLocal.Unlock();
-}
-
 HICON CPluginClass::GetIcon(int type)
 {
   HICON icon = NULL;