Issue #3432 - Manage COM events with a resource class

src/plugin/PluginClass.cpp

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-2015 Eyeo GmbH
  *
  * Adblock Plus is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 3 as
  * published by the Free Software Foundation.
  *
  * Adblock Plus is distributed in the hope that it will be useful,
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
@@ skipping 72 matching lines @@
     }
 
     int Width() const
     {
       return right - left;
     }
   };
 }
 
 CPluginClass::CPluginClass()
-  : m_webBrowser2(nullptr)
+  : m_webBrowser2(nullptr),
+  detachedInitializationFailed(false)
 {
   DEBUG_GENERAL([this]() -> std::wstring
     {
       std::wstring s = L"CPluginClass::<constructor>, this = ";
       s += ToHexLiteral(this);
       return s;
     }());
 
   //Use this line to debug memory leaks
   //    _CrtDumpMemoryLeaks();
 
-  m_isAdvised = false;
   m_hTabWnd = NULL;
   m_hStatusBarWnd = NULL;
   m_hPaneWnd = NULL;
   m_nPaneWidth = 0;
   m_pWndProcStatus = NULL;
   m_hTheme = NULL;
   m_isInitializedOk = false;
 
 
   m_tab = new CPluginTab();
@@ skipping 64 matching lines @@
     DEBUG_ERROR_LOG(0, 0, 0, "CPluginClass::GetBrowserUrl - Reached with m_webBrowser2 == nullptr");
     url = m_tab->GetDocumentUrl();
   }
   return url;
 }
 
 DWORD WINAPI CPluginClass::StartInitObject(LPVOID thisPtr)
 {
   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;
 }
 
 /*
  * IE calls this when it creates a new browser window or tab, immediately after it also 
  * creates the object. The argument 'unknownSite' in is the OLE "site" of the object, 
  * which is an IWebBrowser2 interface associated with the window/tab. 
  *
  * IE also ordinarily calls this again when its window/tab is closed, in which case
  * 'unknownSite' will be null. Extraordinarily, this is sometimes _not_ called when IE
  * is shutting down. Thus 'SetSite(nullptr)' has some similarities with a destructor,
  * but it is not a proper substitute for one.
  */
 STDMETHODIMP CPluginClass::SetSite(IUnknown* unknownSite)
 {
   try
   {
     if (unknownSite)
     {
       DEBUG_GENERAL(L"================================================================================\nNEW TAB UI\n================================================================================");
 
       HRESULT hr = ::CoInitialize(NULL);
       if (FAILED(hr))
       {
         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.
        */
       m_webBrowser2 = ATL::CComQIPtr<IWebBrowser2>(unknownSite);
       if (!m_webBrowser2)
       {
         throw std::logic_error("CPluginClass::SetSite - Unable to convert site pointer to IWebBrowser2*");
       }
       DEBUG_GENERAL([this]() -> std::wstring
         {
@@ skipping 10 matching lines @@
       {
         // Always register on startup, then check if we need to unregister in a separate thread
         s_mimeFilter = CPluginClientFactory::GetMimeFilterClientInstance();
         s_asyncWebBrowser2 = unknownSite;
         s_instances.insert(this);
       }
       s_criticalSectionLocal.Unlock();
 
       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;
         ss << L"CPluginClass::SetSite, this = " << ToHexLiteral(this);
         ss << L", browser = nullptr";
         return ss.str();
       }());
 
-      Unadvise();
 
       // Destroy window
       if (m_pWndProcStatus)
       {
         ::SetWindowLongPtr(m_hStatusBarWnd, GWLP_WNDPROC, (LPARAM)(WNDPROC)m_pWndProcStatus);
 
         m_pWndProcStatus = NULL;
       }
 
       if (m_hPaneWnd)
@@ skipping 159 matching lines @@
   IDispatch* frameBrowserDisp /**< [in] */,
   VARIANT* urlVariant /**< [in] */,
   VARIANT* /**< [in] Flags*/,
   VARIANT* /**< [in] TargetFrameName*/,
   VARIANT* /**< [in] PostData*/,
   VARIANT* /**< [in] Headers*/,
   VARIANT_BOOL* /**< [in, out] Cancel*/)
 {
   try
   {
+    if (detachedInitializationFailed) return;
+
     ATL::CComQIPtr<IWebBrowser2> webBrowser = frameBrowserDisp;
     if (!webBrowser)
     {
       return;
     }
     if (!urlVariant || urlVariant->vt != VT_BSTR)
     {
       return;
     }
     std::wstring url(urlVariant->bstrVal, SysStringLen(urlVariant->bstrVal));
@@ skipping 24 matching lines @@
     }
   }
   catch (...)
   {
   }
 }
 
 // Entry point
 void STDMETHODCALLTYPE CPluginClass::OnDownloadComplete()
 {
+  if (detachedInitializationFailed) return;
   try
   {
     if (!m_webBrowser2)
     {
       DEBUG_ERROR_LOG(0, 0, 0, "CPluginClass::OnDownloadComplete - Reached with m_webBrowser2 == nullptr");
       return;
     }
     DEBUG_NAVI(L"Navi::Download Complete")
     m_tab->OnDownloadComplete(m_webBrowser2);
   }
   catch (...)
   {
   }
 }
 
 // Entry point
 void STDMETHODCALLTYPE CPluginClass::OnDocumentComplete(IDispatch* frameBrowserDisp, VARIANT* /*urlOrPidl*/)
 {
+  if (detachedInitializationFailed) return;
   try
   {
     DEBUG_NAVI(L"Navi::Document Complete");
     ATL::CComQIPtr<IWebBrowser2> webBrowser2 = frameBrowserDisp;
     if (!webBrowser2)
     {
       return;
     }
     std::wstring frameSrc = GetLocationUrl(*webBrowser2);
     m_tab->OnDocumentComplete(webBrowser2, frameSrc, IsRootBrowser(webBrowser2));
   }
   catch (...)
   {
   }
 }
 
 // Entry point
 void STDMETHODCALLTYPE CPluginClass::OnWindowStateChanged(unsigned long flags, unsigned long validFlagsMask)
 {
+  if (detachedInitializationFailed) return;
   try
   {
     DEBUG_GENERAL(L"WindowStateChanged (check tab changed)");
     bool newtabshown = validFlagsMask == (OLECMDIDF_WINDOWSTATE_USERVISIBLE | OLECMDIDF_WINDOWSTATE_ENABLED)
       && flags == (OLECMDIDF_WINDOWSTATE_USERVISIBLE | OLECMDIDF_WINDOWSTATE_ENABLED);
     if (newtabshown)
     {
       std::map<DWORD,CPluginClass*>::const_iterator it = s_threadInstances.find(GetCurrentThreadId());
       if (it == s_threadInstances.end())
       {
@@ skipping 10 matching lines @@
     DEBUG_GENERAL(L"WindowStateChanged (check tab changed) end");
   }
   catch (...)
   {
   }
 }
 
 // Entry point
 void STDMETHODCALLTYPE CPluginClass::OnCommandStateChange(long /*command*/, VARIANT_BOOL /*enable*/)
 {
+  if (detachedInitializationFailed) return;
   try
   {
     if (m_hPaneWnd == NULL)
     {
       CreateStatusBarPane();
     }
     else
     {
       if (AdblockPlus::IE::InstalledMajorVersion() > 6)
       {
         RECT rect;
         //Get the RECT for the leftmost pane (the status text pane)
         BOOL rectRes = ::SendMessage(m_hStatusBarWnd, SB_GETRECT, 0, (LPARAM)&rect);
         if (rectRes == TRUE)
         {
           MoveWindow(m_hPaneWnd, rect.right - m_nPaneWidth, 0, m_nPaneWidth, rect.bottom - rect.top, TRUE);
         }
       }
     }
   }
   catch (...)
   {
   }
 }
 
-// Entry point
-void STDMETHODCALLTYPE CPluginClass::OnOnQuit()
-{
-  try
-  {
-    Unadvise();
-  }
-  catch (...)
-  {
-  }
-}
-
 bool CPluginClass::InitObject()
 {
   DEBUG_GENERAL("InitObject - begin");
   CPluginSettings* settings = CPluginSettings::GetInstance();
 
   if (!settings->GetPluginEnabled())
   {
     s_mimeFilter->Unregister();
   }
 
@@ skipping 368 matching lines @@
     }
   }
   s_criticalSectionLocal.Unlock();
 
   return tab;
 }
 
 // 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;
     if (prgCmds == 0) return E_POINTER;
 
     prgCmds[0].cmdf = OLECMDF_ENABLED;
   }
   catch (...)
   {
     DEBUG_GENERAL(L"CPluginClass::QueryStatus - exception");
@@ skipping 196 matching lines @@
   fmii.dwTypeData = const_cast<LPWSTR>(ctext.c_str());
   fmii.cch = static_cast<UINT>(ctext.size());
   ::SetMenuItemInfoW(hMenu, ID_MENU_SETTINGS, FALSE, &fmii);
 
   return true;
 }
 
 // Entry point
 STDMETHODIMP CPluginClass::Exec(const GUID*, DWORD nCmdID, DWORD, VARIANTARG*, VARIANTARG*)
 {
+  if (detachedInitializationFailed) return S_OK;
   try
   {
     HWND hBrowserWnd = GetBrowserHWND();
     if (!hBrowserWnd)
     {
       return E_FAIL;
     }
 
     // Create menu
     HMENU hMenu = CreatePluginMenu(m_tab->GetDocumentUrl());
@@ skipping 405 matching lines @@
   if (m_hPaneWnd == NULL)
   {
     CreateStatusBarPane();
   }
   if ((m_hPaneWnd != NULL) && !::InvalidateRect(m_hPaneWnd, NULL, FALSE))
   {
       DEBUG_ERROR_LOG(::GetLastError(), PLUGIN_ERROR_UI, PLUGIN_ERROR_UI_INVALIDATE_STATUSBAR, "Class::Invalidate statusbar");
   }
 }
 
-
-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;
 
   s_criticalSectionLocal.Lock();
   {
     if (!s_hIcons[type])
     {
       std::wstring imageToLoad = L"#";
       imageToLoad += std::to_wstring(s_hIconTypes[type]);
       s_hIcons[type] = (HICON)::LoadImage(_Module.m_hInst, imageToLoad.c_str(), IMAGE_ICON, iconWidth, iconHeight, LR_SHARED);
       if (!s_hIcons[type])
       {
         DEBUG_ERROR_LOG(::GetLastError(), PLUGIN_ERROR_UI, PLUGIN_ERROR_UI_LOAD_ICON, "Class::GetIcon - LoadIcon");
       }
     }
 
     icon = s_hIcons[type];
   }
   s_criticalSectionLocal.Unlock();
 
   return icon;
 }
 
 ATOM CPluginClass::GetAtomPaneClass()
 {
   return s_atomPaneClass;
 }