Issue #3391 - Initialize themed drawing variables using DetachedInitializer

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 21 matching lines @@
 #include "../shared/Version.h"
 #include <thread>
 #include <array>
 
 #ifdef DEBUG_HIDE_EL
 DWORD profileTime = 0;
 #endif
 
 extern CComModule _Module;
 
-typedef HANDLE (WINAPI *OPENTHEMEDATA)(HWND, LPCWSTR);
-typedef HRESULT (WINAPI *DRAWTHEMEBACKGROUND)(HANDLE, HDC, INT, INT, LPRECT, LPRECT);
-typedef HRESULT (WINAPI *CLOSETHEMEDATA)(HANDLE);
+namespace
+{
+  typedef HANDLE (WINAPI *OPENTHEMEDATA)(HWND, LPCWSTR);
+  typedef HRESULT (WINAPI *DRAWTHEMEBACKGROUND)(HANDLE, HDC, INT, INT, LPRECT, LPRECT);
+  typedef HRESULT (WINAPI *CLOSETHEMEDATA)(HANDLE);
+
+  CLOSETHEMEDATA pfnClose = NULL;
+  DRAWTHEMEBACKGROUND pfnDrawThemeBackground = NULL;
+  OPENTHEMEDATA pfnOpenThemeData = NULL;
+}
 
 HICON CPluginClass::s_hIcons[ICON_MAX] = { NULL, NULL, NULL };
 DWORD CPluginClass::s_hIconTypes[ICON_MAX] = { IDI_ICON_DISABLED, IDI_ICON_ENABLED, IDI_ICON_DEACTIVATED };
 uint32_t iconHeight = 32;
 uint32_t iconWidth = 32;
 
 CPluginMimeFilterClient* CPluginClass::s_mimeFilter = NULL;
-
-CLOSETHEMEDATA pfnClose = NULL;
-DRAWTHEMEBACKGROUND pfnDrawThemeBackground = NULL;
-OPENTHEMEDATA pfnOpenThemeData = NULL;
-
 ATOM CPluginClass::s_atomPaneClass = NULL;
-HINSTANCE CPluginClass::s_hUxtheme = NULL;
 std::set<CPluginClass*> CPluginClass::s_instances;
 std::map<DWORD, CPluginClass*> CPluginClass::s_threadInstances;
-
 CComAutoCriticalSection CPluginClass::s_criticalSectionLocal;
 CComAutoCriticalSection CPluginClass::s_criticalSectionWindow;
+CComQIPtr<IWebBrowser2> CPluginClass::s_asyncWebBrowser2;
+CPluginClass::DetachedInitializerType CPluginClass::di;
 
-CComQIPtr<IWebBrowser2> CPluginClass::s_asyncWebBrowser2;
 
 /*
  * Without namespace declaration, the identifier "Rectangle" is ambiguous
  * See http://msdn.microsoft.com/en-us/library/windows/desktop/dd162898(v=vs.85).aspx
  */
 namespace AdblockPlus
 {
   /**
     * Replacement for ATL type CRect.
     */
@@ skipping 20 matching lines @@
   //    _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(this);
-
   Dictionary::Create(GetBrowserLanguage());
+  di.SpawnInitializer();
 }
 
 CPluginClass::~CPluginClass()
 {
   delete m_tab;
 }
 
+/*
+ * Because this initialization function does not throw, neither can EnsureInitialized().
+ */
+void CPluginClass::Initializer() // noexcept
+{
+  try
+  {
+    /*
+     * Initializes these static, anonymous-namespace variables
+     *   - pfnClose
+     *   - pfnDrawThemeBackground
+     *   - pfnOpenThemeData
+     */
+    HINSTANCE UxthemeModule = ::GetModuleHandle(L"uxtheme.dll");
+    if (!UxthemeModule)
+    {
+      DEBUG_ERROR_LOG(::GetLastError(), PLUGIN_ERROR_UI, PLUGIN_ERROR_UI_GET_UXTHEME, "CPluginClass::Initializer - GetModuleHandle(uxtheme.dll)");
+    }
+    else
+    {
+      pfnClose = (CLOSETHEMEDATA)::GetProcAddress(UxthemeModule, "CloseThemeData");
+      if (!pfnClose)
+      {
+        DEBUG_ERROR_LOG(::GetLastError(), PLUGIN_ERROR_UI, PLUGIN_ERROR_UI_GET_UXTHEME_CLOSE, "CPluginClass::Initializer - GetProcAddress(CloseThemeData)");
+      }
+      pfnDrawThemeBackground = (DRAWTHEMEBACKGROUND)::GetProcAddress(UxthemeModule, "DrawThemeBackground");
+      if (!pfnDrawThemeBackground)
+      {
+        DEBUG_ERROR_LOG(::GetLastError(), PLUGIN_ERROR_UI, PLUGIN_ERROR_UI_GET_UXTHEME_DRAW_BACKGROUND, "CPluginClass::Initializer - GetProcAddress(DrawThemeBackground)");
+      }
+      pfnOpenThemeData = (OPENTHEMEDATA)::GetProcAddress(UxthemeModule, "OpenThemeData");
+      if (!pfnOpenThemeData)
+      {
+        DEBUG_ERROR_LOG(::GetLastError(), PLUGIN_ERROR_UI, PLUGIN_ERROR_UI_GET_UXTHEME_OPEN, "CPluginClass::Initializer - GetProcAddress(pfnOpenThemeData)");
+      }
+    }
+  }
+  catch (...)
+  {
+    // Suppress all exceptions
+  }
+}
+
+
 HWND CPluginClass::GetBrowserHWND() const
 {
   if (!m_webBrowser2)
   {
     DEBUG_ERROR_LOG(0, 0, 0, "CPluginClass::GetBrowserHWND - Reached with m_webBrowser2 == nullptr");
     return nullptr;
   }
   SHANDLE_PTR hBrowserWndHandle = 0;
   HRESULT hr = m_webBrowser2->get_HWND(&hBrowserWndHandle);
   if (FAILED(hr))
@@ skipping 40 matching lines @@
   }
   return url;
 }
 
 DWORD WINAPI CPluginClass::StartInitObject(LPVOID thisPtr)
 {
   if (thisPtr == NULL)
     return 0;
   if (!((CPluginClass*)thisPtr)->InitObject())
   {
     ((CPluginClass*)thisPtr)->Unadvise();

[Eric, 2015/12/14 18:46:02]

This statement is the motivation to clean up detached initialization now. All
the explicit calls to Unadvise() are messy at best, and probably buggy.
Certainly they're an obstacle to get event registration and instance lifetime in
good working order.

   }
 
   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. 
  *
@@ skipping 41 matching lines @@
       try
       {
         DEBUG_GENERAL("Loaded as BHO");
         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.

[Eric, 2015/12/14 18:46:01]

This comment is sort-of-right; at least it expresses the right concern. We
should wait for a detached initialization thread at every point we rely upon its
side effects. That will generally include the destructor, but certainly isn't
limited to it. The function 'DetachedInitializer::EnsureInitialized()' does
exactly what this comment is concerned with. We won't need anything like this
comment after we've factored out 'InitObject()'.

           }
           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");
@@ skipping 338 matching lines @@
 bool CPluginClass::InitObject()
 {
   DEBUG_GENERAL("InitObject - begin");
   CPluginSettings* settings = CPluginSettings::GetInstance();
 
   if (!settings->GetPluginEnabled())
   {
     s_mimeFilter->Unregister();
   }
 
-  // Load theme module
-  s_criticalSectionLocal.Lock();
-  {
-    if (!s_hUxtheme)
-    {
-      s_hUxtheme = ::GetModuleHandle(L"uxtheme.dll");
-      if (s_hUxtheme)
-      {
-        pfnClose = (CLOSETHEMEDATA)::GetProcAddress(s_hUxtheme, "CloseThemeData");
-        if (!pfnClose)
-        {
-          DEBUG_ERROR_LOG(::GetLastError(), PLUGIN_ERROR_UI, PLUGIN_ERROR_UI_GET_UXTHEME_CLOSE, "Class::InitObject - GetProcAddress(CloseThemeData)");
-        }
-
-        pfnDrawThemeBackground = (DRAWTHEMEBACKGROUND)::GetProcAddress(s_hUxtheme, "DrawThemeBackground");
-        if (!pfnDrawThemeBackground)
-        {
-          DEBUG_ERROR_LOG(::GetLastError(), PLUGIN_ERROR_UI, PLUGIN_ERROR_UI_GET_UXTHEME_DRAW_BACKGROUND, "Class::InitObject - GetProcAddress(DrawThemeBackground)");
-        }
-
-        pfnOpenThemeData = (OPENTHEMEDATA)::GetProcAddress(s_hUxtheme, "OpenThemeData");
-        if (!pfnOpenThemeData)
-        {
-          DEBUG_ERROR_LOG(::GetLastError(), PLUGIN_ERROR_UI, PLUGIN_ERROR_UI_GET_UXTHEME_OPEN, "Class::InitObject - GetProcAddress(pfnOpenThemeData)");
-        }
-      }
-      else
-      {
-        DEBUG_ERROR_LOG(::GetLastError(), PLUGIN_ERROR_UI, PLUGIN_ERROR_UI_GET_UXTHEME, "Class::InitObject - GetModuleHandle(uxtheme.dll)");
-      }
-    }
-  }
-  s_criticalSectionLocal.Unlock();
-
   // Register pane class
   if (!GetAtomPaneClass())
   {
     WNDCLASSEX wcex;
 
     wcex.cbSize = sizeof(wcex);
     wcex.style = 0;
     wcex.lpfnWndProc = (WNDPROC)PaneWindowProc;
     wcex.cbClsExtra = 0;
     wcex.cbWndExtra = 0;
@@ skipping 252 matching lines @@
   {
     vFlags.intVal = navOpenInNewWindow;
     hr = GetAsyncBrowser()->Navigate(navigatePath, &vFlags, NULL, NULL, NULL);
   }
 
   if (FAILED(hr))
   {
     DEBUG_ERROR_LOG(hr, PLUGIN_ERROR_NAVIGATION, PLUGIN_ERROR_NAVIGATION_WELCOME, "Navigation::Welcome page failed")
   }
 }
-void CPluginClass::CloseTheme()
+
+void CPluginClass::UpdateTheme()
 {
+  EnsureInitialized(); // pfnClose, pfnOpenThemeData
   if (m_hTheme)
   {
     if (pfnClose)
     {
       pfnClose(m_hTheme);
     }
-
     m_hTheme = NULL;
   }
+  if (pfnOpenThemeData)
+  {
+    m_hTheme = pfnOpenThemeData(m_hPaneWnd, L"STATUS");
+  }
 }
 
-void CPluginClass::UpdateTheme()
-{
-  CloseTheme();
-
-  if (pfnOpenThemeData)
-  {
-    m_hTheme = pfnOpenThemeData(m_hPaneWnd, L"STATUS");
-    if (!m_hTheme)
-    {
-    }
-  }
-}
-
 
 CPluginClass* CPluginClass::FindInstance(HWND hStatusBarWnd)
 {
   CPluginClass* result = nullptr;
 
   s_criticalSectionLocal.Lock();
   {
     for (auto instance : s_instances)
     {
       if (instance->m_hStatusBarWnd == hStatusBarWnd)
@@ skipping 465 matching lines @@
 
         nDrawEdge = 3;
         rcClient.left += 3;
 
         ::DrawEdge(hDC, &rcClient, BDR_SUNKENOUTER, BF_RECT);
       }
       // Themed background drawing
       else
       {
         // Draw background
+        EnsureInitialized(); // pfnDrawThemeBackground
         if (pfnDrawThemeBackground)
         {
           AdblockPlus::Rectangle rc = rcClient;
           rc.right -= 2;
           pfnDrawThemeBackground(pClass->m_hTheme, hDC, 0, 0, &rc, NULL);
         }
 
         // Copy separator picture to left side
         int nHeight = rcClient.Height();
         int nWidth = rcClient.Width() - 2;
@@ skipping 207 matching lines @@
   s_criticalSectionLocal.Unlock();
 
   return icon;
 }
 
 ATOM CPluginClass::GetAtomPaneClass()
 {
   return s_atomPaneClass;
 }