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
+ * Copyright (C) 2006-2016 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
  * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
  * GNU General Public License for more details.
  *
  * You should have received a copy of the GNU General Public License
  * along with Adblock Plus.  If not, see <http://www.gnu.org/licenses/>.
  */
 
 #include "PluginStdAfx.h"
 
 #include "PluginClass.h"
 #include "PluginSettings.h"
 #include "PluginSystem.h"
 #include "PluginFilter.h"
 #include "PluginMimeFilterClient.h"
 #include "AdblockPlusClient.h"
 #include "PluginClientBase.h"
 #include "PluginClientFactory.h"
 #include "PluginUtil.h"
 #include "../shared/Utils.h"
 #include "../shared/Dictionary.h"
 #include "IeVersion.h"
 #include "../shared/Version.h"
+#include "Instances.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);
 
 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;
+
+namespace
+{
+  /**
+   * A synchronized map whose key is always the current thread ID.
+   */
+  class CurrentThreadMap
+    : public SyncMap<DWORD, CPluginClass*, nullptr>
+  {
+    typedef SyncMap<DWORD, CPluginClass*, nullptr> Base;
+
+  public:
+    bool AddIfAbsent(CPluginClass* p)
+    {
+      return Base::AddIfAbsent(::GetCurrentThreadId(), p);
+    }
+
+    bool RemoveAndCheck()
+    {
+      return Base::RemoveIfPresent(::GetCurrentThreadId());
+    }
+
+    CPluginClass* Locate()
+    {
+      return Base::Locate(::GetCurrentThreadId());
+    }
+  };
+
+  /**
+   * Map from thread ID's to CPluginClass instances
+   */
+  CurrentThreadMap threadMap;
+}
 
 /*
  * 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.
     */
   class Rectangle
     : public RECT
   {
   public:
-    int Height() const
-    {
-      return bottom - top;
-    }
-
-    int Width() const
-    {
-      return right - left;
+    unsigned long Height() const
+    {
+      if (bottom < top)
+      {
+        throw std::runtime_error("invariant violation: rectangle bottom < top");
+      }
+      return static_cast<unsigned long>(bottom - top);
+    }
+
+    unsigned long Width() const
+    {
+      if (right < left)
+      {
+        throw std::runtime_error("invariant violation: rectangle right < left");
+      }
+      return static_cast<unsigned long>(right - left);
     }
   };
 }
 
 CPluginClass::CPluginClass()
   : 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_hTabWnd = NULL;
   m_hStatusBarWnd = NULL;
   m_hPaneWnd = NULL;
   m_nPaneWidth = 0;
   m_pWndProcStatus = NULL;
   m_hTheme = NULL;
   m_isInitializedOk = false;
-
-
   m_tab = new CPluginTab();
-
   Dictionary::Create(GetBrowserLanguage());
 }
 
 CPluginClass::~CPluginClass()
 {
   DEBUG_GENERAL([this]() -> std::wstring
     {
       std::wstring s = L"CPluginClass::<destructor>, this = ";
       s += ToHexLiteral(this);
       return s;
@@ skipping 36 matching lines @@
 
   return browser;
 }
 
 std::wstring CPluginClass::GetBrowserUrl() const
 {
   std::wstring url;
   if (m_webBrowser2)
   {
     CComBSTR bstrURL;
-    if (SUCCEEDED(m_webBrowser2->get_LocationURL(&bstrURL)) && bstrURL)
-    {
-      url = std::wstring(bstrURL, SysStringLen(bstrURL));
+    if (SUCCEEDED(m_webBrowser2->get_LocationURL(&bstrURL)))
+    {
+      url = ToWstring(bstrURL);
     }
   }
   else
   {
-    DEBUG_ERROR_LOG(0, 0, 0, "CPluginClass::GetBrowserUrl - Reached with m_webBrowser2 == nullptr");
+    DEBUG_GENERAL(L"CPluginClass::GetBrowserUrl - Reached with m_webBrowser2 == nullptr (probable invariant violation)");
+  }
+  if (url.empty())
+  {
     url = m_tab->GetDocumentUrl();
   }
   return url;
 }
 
 DWORD WINAPI CPluginClass::StartInitObject(LPVOID thisPtr)
 {
   if (thisPtr == NULL)
     return 0;
   auto self = static_cast<CPluginClass*>(thisPtr);
@@ skipping 36 matching lines @@
         throw std::logic_error("CPluginClass::SetSite - Unable to convert site pointer to IWebBrowser2*");
       }
       DEBUG_GENERAL([this]() -> std::wstring
         {
           std::wstringstream ss;
           ss << L"CPluginClass::SetSite, this = " << ToHexLiteral(this);
           ss << L", browser = " << ToHexLiteral(m_webBrowser2);
           return ss.str();
         }());
 
+      /*
+       * Add ourselves to the thread map.
+       */
+      if (!threadMap.AddIfAbsent(this))
+      {
+        throw std::logic_error("CPluginClass::SetSite - May not overwrite existing entry in thread map");
+      }
+
       //register the mimefilter
       //and only mimefilter
       //on some few computers the mimefilter does not get properly registered when it is done on another thread
       s_criticalSectionLocal.Lock();
       {
         // 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
       {
         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)
       {
         detachedInitializationFailed = true;
         DEBUG_SYSTEM_EXCEPTION(ex, PLUGIN_ERROR_THREAD, PLUGIN_ERROR_MAIN_THREAD_CREATE_PROCESS,
           "Class::Thread - Failed to create StartInitObject thread");
       }
 
-      // Start event last, after everything is ready to go

[sergei, 2016/01/06 08:41:57]

This comment is confusing because not everything is ready yet, just a couple of
lines above we are starting some asynchronous initialization. It's clear that
before binding with event listeners we need to prepare other members, but
apparently not everything is ready to go yet.

IMO, it would be better without this comment.

[Eric, 2016/01/07 16:24:47]

Reworded.

+      // 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";
@@ skipping 15 matching lines @@
         m_hPaneWnd = NULL;
       }
 
       m_hTabWnd = NULL;
       m_hStatusBarWnd = NULL;
 
       // Remove instance from the list, shutdown threads
       HANDLE hMainThread = NULL;
       HANDLE hTabThread = NULL;
 
+      if (!threadMap.RemoveAndCheck())
+      {
+        throw std::logic_error("CPluginClass::SetSite - Missing entry in thread map");
+      }
+
       s_criticalSectionLocal.Lock();
       {
         s_instances.erase(this);
-
-        std::map<DWORD,CPluginClass*>::iterator it = s_threadInstances.find(::GetCurrentThreadId());
-        if (it != s_threadInstances.end())
-        {
-          s_threadInstances.erase(it);
-        }
         if (s_instances.empty())
         {
           // TODO: Explicitly releasing a resource when a container becomes empty looks like a job better suited for shared_ptr
           CPluginClientFactory::ReleaseMimeFilterClientInstance();
         }
       }
       s_criticalSectionLocal.Unlock();
 
       m_webBrowser2 = nullptr;
 
@@ skipping 128 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;

[sergei, 2016/01/06 08:41:57]

Basically all these checks in each method are indicating that something is wrong
with the design. It would be better to bind event listeners only after finishing
with the prerequisites.

[sergei, 2016/01/06 08:41:57]

it would be better to use some method `bool IsInitialized() const`, it's a bit
confusing that we are checking only result of "detached" initialization.

[Eric, 2016/01/07 16:24:47]

Something is definitely wrong with the design. It's not something, however,
that's going to be quick to change. The present change set is only incremental
progress.

The biggest single problem is that 'CPluginClass' is too much of a god class.
The current detached initialization thread is working with what should be
several different classes. Synchronization for the completion of initialization
should be isolated into these classes. Until that's (partially) accomplished,
the real issues won't be clear. 

In addition, the lack of transactional (atomic) registration already causes race
conditions we should be dealing with. See #3433 for that aspect.
That's something that may not be strictly possible. (And it might not even be
desirable.) Regardless, it's premature to talk about it at any great length
right now. The code is in too poor a shape to make that a productive
conversation yet.

[Eric, 2016/01/07 16:24:48]

I don't disagree in principle, but that change is premature. I think that
particular function may not even be what we want, not exactly. The flag
"detachedInitializationFailed" is really a proxy for something more like
"IsPluginDisabledOnTechnicalGrounds()". We're not ready to write such a function
yet. There are issue around transient vs. permanent failures, for example, that
we're nowhere close to resolving.

We have something like four different kinds of initialization for
'CPluginClass'. They are not all working well yet. We're going to have to live
with some rather more explicit checking on this issue until we've got all the
pieces in place. It's not going to happen instantly.

Anticipating this class of concerns, I created #3433 a few weeks ago on this
topic. Addressing that issue is going to be multi-commit process, I'm sure.

[sergei, 2016/02/04 13:45:49]

Acknowledged.

 
     ATL::CComQIPtr<IWebBrowser2> webBrowser = frameBrowserDisp;
     if (!webBrowser)
     {
       return;
     }
     if (!urlVariant || urlVariant->vt != VT_BSTR)
     {
       return;
     }
-    std::wstring url(urlVariant->bstrVal, SysStringLen(urlVariant->bstrVal));
+    std::wstring url = ToWstring(urlVariant->bstrVal);
 
     // If webbrowser2 is equal to top level browser (as set in SetSite), we are
     // navigating new page
     CPluginClient* client = CPluginClient::GetInstance();
     if (url.find(L"javascript") == 0)
     {
     }
     else if (IsRootBrowser(webBrowser))
     {
       m_tab->OnNavigate(url);
@@ skipping 61 matching lines @@
 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())
-      {
-        s_threadInstances[::GetCurrentThreadId()] = this;
-        if (!m_isInitializedOk)
-        {
-          m_isInitializedOk = true;
-          InitObject();
-          UpdateStatusBar();
-        }
+      if (!m_isInitializedOk)
+      {
+        m_isInitializedOk = true;
+        InitObject();
+        UpdateStatusBar();
       }
     }
     notificationMessage.Hide();
     DEBUG_GENERAL(L"WindowStateChanged (check tab changed) end");
   }
   catch (...)
   {
   }
 }
 
@@ skipping 387 matching lines @@
   s_criticalSectionLocal.Unlock();
 
   return result;
 }
 
 CPluginTab* CPluginClass::GetTab()
 {
   return m_tab;
 }
 
-CPluginTab* CPluginClass::GetTab(DWORD dwThreadId)
-{
-  CPluginTab* tab = NULL;
-
-  s_criticalSectionLocal.Lock();
-  {
-    std::map<DWORD,CPluginClass*>::const_iterator it = s_threadInstances.find(dwThreadId);
-    if (it != s_threadInstances.end())
-    {
-      tab = it->second->m_tab;
-    }
-  }
-  s_criticalSectionLocal.Unlock();
-
-  return tab;
-}
-
-
+CPluginTab* CPluginClass::GetTabForCurrentThread()
+{
+  auto p = threadMap.Locate();
+  return p ? p->m_tab : nullptr;
+}
+
+// Entry point
 STDMETHODIMP CPluginClass::QueryStatus(const GUID* pguidCmdGroup, ULONG cCmds, OLECMD prgCmds[], OLECMDTEXT* pCmdText)
 {
-  if (detachedInitializationFailed) return;
-  if (cCmds == 0) return E_INVALIDARG;
-  if (prgCmds == 0) return E_POINTER;
-
-  prgCmds[0].cmdf = OLECMDF_ENABLED;
-
+  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");
+    return E_FAIL;
+  }
   return S_OK;
 }
 
 HMENU CPluginClass::CreatePluginMenu(const std::wstring& url)
 {
   DEBUG_GENERAL("CreatePluginMenu");
   HINSTANCE hInstance = _AtlBaseModule.GetModuleInstance();
 
   HMENU hMenu = ::LoadMenu(hInstance, MAKEINTRESOURCE(IDR_MENU1));
 
@@ skipping 129 matching lines @@
 
   MENUITEMINFOW miiSep = {};
   miiSep.cbSize = sizeof(miiSep);
   miiSep.fMask = MIIM_TYPE | MIIM_FTYPE;
   miiSep.fType = MFT_SEPARATOR;
 
   CPluginClient* client = CPluginClient::GetInstance();
   CPluginSettings* settings = CPluginSettings::GetInstance();
   {
     ctext = dictionary->Lookup("menu", "menu-disable-on-site");
-    // Is domain in white list?
     ReplaceString(ctext, L"?1?", client->GetHostFromUrl(url));
-    if (client->IsWhitelistedUrl(GetTab()->GetDocumentUrl()))
-    {
+    /*
+     * The display state of the "disable on this site" menu item depends upon tab content
+     */
+    if (!GetTab()->IsPossibleToDisableOnSite())
+    {
+      // Since we can't disable the present content,
+      // it makes no sense to offer the user an option to block it.
+      fmii.fState = MFS_UNCHECKED | MFS_DISABLED;
+    }
+    else if (client->IsWhitelistedUrl(GetTab()->GetDocumentUrl()))
+    {
+      // Domain is in white list, indicated by a check mark
       fmii.fState = MFS_CHECKED | MFS_ENABLED;
     }
     else
     {
       fmii.fState = MFS_UNCHECKED | MFS_ENABLED;
     }
     fmii.fMask = MIIM_STRING | MIIM_STATE;
     fmii.dwTypeData = const_cast<LPWSTR>(ctext.c_str());
     fmii.cch = static_cast<UINT>(ctext.size());
 
@@ skipping 28 matching lines @@
   ctext = dictionary->Lookup("menu", "menu-settings");
   fmii.fMask  = MIIM_STATE | MIIM_STRING;
   fmii.fState = MFS_ENABLED;
   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;
-  HWND hBrowserWnd = GetBrowserHWND();
-  if (!hBrowserWnd)
-  {
-    return E_FAIL;
-  }
-
-  // Create menu
-  HMENU hMenu = CreatePluginMenu(m_tab->GetDocumentUrl());
-  if (!hMenu)
-  {
-    return E_FAIL;
-  }
-
-  // Check if button in toolbar was pressed
-  int nIDCommand = -1;
-  BOOL bRightAlign = FALSE;
-
-  POINT pt;
-  GetCursorPos(&pt);
-
-  HWND hWndToolBar = ::WindowFromPoint(pt);
-
-  DWORD nProcessId;
-  ::GetWindowThreadProcessId(hWndToolBar, &nProcessId);
-
-  if (hWndToolBar && ::GetCurrentProcessId() == nProcessId)
-  {
-    ::ScreenToClient(hWndToolBar, &pt);
-    int nButton = (int)::SendMessage(hWndToolBar, TB_HITTEST, 0, (LPARAM)&pt);
-
-    if (nButton > 0)
-    {
-      TBBUTTON pTBBtn = {};
-
-      if (SendMessage(hWndToolBar, TB_GETBUTTON, nButton, (LPARAM)&pTBBtn))
-      {
-        RECT rcButton;
-        nIDCommand = pTBBtn.idCommand;
-
-        if (SendMessage(hWndToolBar, TB_GETRECT, nIDCommand, (LPARAM)&rcButton))
-        {
-          pt.x = rcButton.left;
-          pt.y = rcButton.bottom;
-          ClientToScreen(hWndToolBar, &pt);
-
-          RECT rcWorkArea;
-          SystemParametersInfo(SPI_GETWORKAREA, 0, (LPVOID)&rcWorkArea, 0);
-          if (rcWorkArea.right - pt.x < 150)
+  if (detachedInitializationFailed) return S_OK;
+  try
+  {
+    HWND hBrowserWnd = GetBrowserHWND();
+    if (!hBrowserWnd)
+    {
+      return E_FAIL;
+    }
+
+    // Create menu
+    HMENU hMenu = CreatePluginMenu(m_tab->GetDocumentUrl());
+    if (!hMenu)
+    {
+      return E_FAIL;
+    }
+
+    // Check if button in toolbar was pressed
+    int nIDCommand = -1;
+    BOOL bRightAlign = FALSE;
+
+    POINT pt;
+    GetCursorPos(&pt);
+
+    HWND hWndToolBar = ::WindowFromPoint(pt);
+
+    DWORD nProcessId;
+    ::GetWindowThreadProcessId(hWndToolBar, &nProcessId);
+
+    if (hWndToolBar && ::GetCurrentProcessId() == nProcessId)
+    {
+      ::ScreenToClient(hWndToolBar, &pt);
+      int nButton = (int)::SendMessage(hWndToolBar, TB_HITTEST, 0, (LPARAM)&pt);
+
+      if (nButton > 0)
+      {
+        TBBUTTON pTBBtn = {};
+
+        if (SendMessage(hWndToolBar, TB_GETBUTTON, nButton, (LPARAM)&pTBBtn))
+        {
+          RECT rcButton;
+          nIDCommand = pTBBtn.idCommand;
+
+          if (SendMessage(hWndToolBar, TB_GETRECT, nIDCommand, (LPARAM)&rcButton))
           {
-            bRightAlign = TRUE;
-            pt.x = rcButton.right;
+            pt.x = rcButton.left;
             pt.y = rcButton.bottom;
             ClientToScreen(hWndToolBar, &pt);
+
+            RECT rcWorkArea;
+            SystemParametersInfo(SPI_GETWORKAREA, 0, (LPVOID)&rcWorkArea, 0);
+            if (rcWorkArea.right - pt.x < 150)
+            {
+              bRightAlign = TRUE;
+              pt.x = rcButton.right;
+              pt.y = rcButton.bottom;
+              ClientToScreen(hWndToolBar, &pt);
+            }
           }
         }
       }
+      else
+      {
+        GetCursorPos(&pt);
+      }
+    }
+
+    // Display menu
+    UINT nFlags = 0;
+    if (bRightAlign)
+    {
+      nFlags |= TPM_RIGHTALIGN;
     }
     else
     {
-      GetCursorPos(&pt);
-    }
-  }
-
-  // Display menu
-  UINT nFlags = 0;
-  if (bRightAlign)
-  {
-    nFlags |= TPM_RIGHTALIGN;
-  }
-  else
-  {
-    nFlags |= TPM_LEFTALIGN;
-  }
-
-  DisplayPluginMenu(hMenu, nIDCommand, pt, nFlags);
+      nFlags |= TPM_LEFTALIGN;
+    }
+
+    DisplayPluginMenu(hMenu, nIDCommand, pt, nFlags);
+  }
+  catch (...)
+  {
+    // Suppress exception, log only
+    DEBUG_GENERAL(L"CPluginClass::Exec - exception");
+    return E_FAIL;
+  }
 
   return S_OK;
 }
 
-/////////////////////////////////////////////////////////////////////////////
-// Window procedures
-
+// Entry point
 LRESULT CALLBACK CPluginClass::NewStatusProc(HWND hWnd, UINT message, WPARAM wParam, LPARAM lParam)
 {
-  // Find tab
-  CPluginClass *pClass = FindInstance(hWnd);
-  if (!pClass)
-  {
-    return DefWindowProc(hWnd, message, wParam, lParam);
-  }
-
-  // Process message
-  switch (message)
-  {
-  case SB_SIMPLE:
-    {
-      ShowWindow(pClass->m_hPaneWnd, !wParam);
+  CPluginClass *pClass;
+  try
+  {
+    // Find tab
+    pClass = FindInstance(hWnd);
+    if (!pClass)
+    {
+      /*
+       * Race condition if reached.
+       * We did not unhook the window procedure for the status bar when the last BHO instance using it terminated.
+       * The next best thing is to call the system default window function.
+       */
+      return DefWindowProc(hWnd, message, wParam, lParam);
+    }
+
+    // Process message
+    switch (message)
+    {
+    case SB_SIMPLE:
+      {
+        ShowWindow(pClass->m_hPaneWnd, !wParam);
+        break;
+      }
+
+    case WM_SYSCOLORCHANGE:
+      {
+        pClass->UpdateTheme();
+        break;
+      }
+
+    case SB_SETPARTS:
+      {
+        if (!lParam || !wParam || wParam > 30 || !IsWindow(pClass->m_hPaneWnd))
+        {
+          return CallWindowProc(pClass->m_pWndProcStatus, hWnd, message, wParam, lParam);
+        }
+
+        WPARAM nParts = wParam;
+        if (STATUSBAR_PANE_NUMBER >= nParts)
+        {
+          return CallWindowProc(pClass->m_pWndProcStatus, hWnd, message, wParam, lParam);
+        }
+
+        HLOCAL hLocal = LocalAlloc(LHND, sizeof(int) * (nParts + 1));
+        LPINT lpParts = (LPINT)LocalLock(hLocal);
+        memcpy(lpParts, (void*)lParam, wParam*sizeof(int));
+
+        for (unsigned i = 0; i < STATUSBAR_PANE_NUMBER; i++)
+        {
+          lpParts[i] -= pClass->m_nPaneWidth;
+        }
+        LRESULT hRet = CallWindowProc(pClass->m_pWndProcStatus, hWnd, message, wParam, (LPARAM)lpParts);
+
+        AdblockPlus::Rectangle rcPane;
+        ::SendMessage(hWnd, SB_GETRECT, STATUSBAR_PANE_NUMBER, (LPARAM)&rcPane);
+
+        AdblockPlus::Rectangle rcClient;
+        ::GetClientRect(hWnd, &rcClient);
+
+        ::MoveWindow(
+          pClass->m_hPaneWnd,
+          lpParts[STATUSBAR_PANE_NUMBER] - pClass->m_nPaneWidth,
+          0,
+          pClass->m_nPaneWidth,
+          rcClient.Height(),
+          TRUE);
+
+        ::LocalFree(hLocal);
+        return hRet;
+      }
+
+    default:
       break;
     }
-
-  case WM_SYSCOLORCHANGE:
-    {
-      pClass->UpdateTheme();
-      break;
-    }
-
-  case SB_SETPARTS:
-    {
-      if (!lParam || !wParam || wParam > 30 || !IsWindow(pClass->m_hPaneWnd))
-      {
-        return CallWindowProc(pClass->m_pWndProcStatus, hWnd, message, wParam, lParam);
-      }
-
-      WPARAM nParts = wParam;
-      if (STATUSBAR_PANE_NUMBER >= nParts)
-      {
-        return CallWindowProc(pClass->m_pWndProcStatus, hWnd, message, wParam, lParam);
-      }
-
-      HLOCAL hLocal = LocalAlloc(LHND, sizeof(int) * (nParts+1));
-      LPINT lpParts = (LPINT)LocalLock(hLocal);
-      memcpy(lpParts, (void*)lParam, wParam*sizeof(int));
-
-      for (unsigned i = 0; i < STATUSBAR_PANE_NUMBER; i++)
-      {
-        lpParts[i] -= pClass->m_nPaneWidth;
-      }
-      LRESULT hRet = CallWindowProc(pClass->m_pWndProcStatus, hWnd, message, wParam, (LPARAM)lpParts);
-
-      AdblockPlus::Rectangle rcPane;
-      ::SendMessage(hWnd, SB_GETRECT, STATUSBAR_PANE_NUMBER, (LPARAM)&rcPane);
-
-      AdblockPlus::Rectangle rcClient;
-      ::GetClientRect(hWnd, &rcClient);
-
-      ::MoveWindow(
-        pClass->m_hPaneWnd,
-        lpParts[STATUSBAR_PANE_NUMBER] - pClass->m_nPaneWidth,
-        0,
-        pClass->m_nPaneWidth,
-        rcClient.Height(),
-        TRUE);
-
-      ::LocalFree(hLocal);
-
-
-      return hRet;
-    }
-
-  default:
-    break;
-  }
-
-  LRESULT result = CallWindowProc(pClass->m_pWndProcStatus, hWnd, message, wParam, lParam);
-
-
-  return result;
-
+  }
+  catch (...)
+  {
+    // Suppress exception. Fall through to default handler.
+    DEBUG_GENERAL(L"CPluginClass::NewStatusProc - exception");
+  }
+  return ::CallWindowProc(pClass->m_pWndProcStatus, hWnd, message, wParam, lParam);
 }
 
 
 HICON CPluginClass::GetStatusBarIcon(const std::wstring& url)
 {
   // use the disable icon as defualt, if the client doesn't exists
   HICON hIcon = GetIcon(ICON_PLUGIN_DEACTIVATED);
 
-  CPluginTab* tab = GetTab(::GetCurrentThreadId());
+  CPluginTab* tab = GetTabForCurrentThread();
   if (tab)
   {
     CPluginClient* client = CPluginClient::GetInstance();
     if (CPluginSettings::GetInstance()->IsPluginEnabled())
     {
       if (client->IsWhitelistedUrl(url))
       {
         hIcon = GetIcon(ICON_PLUGIN_DISABLED);
       }
       else
       {
         CPluginSettings* settings = CPluginSettings::GetInstance();
         hIcon = GetIcon(ICON_PLUGIN_ENABLED);
       }
     }
   }
   return hIcon;
 }
 
-
+// Entry point
 LRESULT CALLBACK CPluginClass::PaneWindowProc(HWND hWnd, UINT message, WPARAM wParam, LPARAM lParam)
 {
-  // Find tab
-  CPluginClass *pClass = FindInstance(GetParent(hWnd));
-  if (!pClass)
-  {
-    return ::DefWindowProc(hWnd, message, wParam, lParam);
-  }
-
-  // Process message
-  switch (message)
-  {
-
-  case WM_SETCURSOR:
-    {
-      ::SetCursor(::LoadCursor(NULL, IDC_ARROW));
-      return TRUE;
-    }
-  case WM_PAINT:
-    {
-      PAINTSTRUCT ps;
-      HDC hDC = ::BeginPaint(hWnd, &ps);
-
-      AdblockPlus::Rectangle rcClient;
-      ::GetClientRect(hWnd, &rcClient);
-
-      int nDrawEdge = 0;
-
-      // Old Windows background drawing
-      if (pClass->m_hTheme == NULL)
-      {
-        ::FillRect(hDC, &rcClient, (HBRUSH)(COLOR_BTNFACE + 1));
-        ::DrawEdge(hDC, &rcClient, BDR_RAISEDINNER, BF_LEFT);
-
-        nDrawEdge = 3;
-        rcClient.left += 3;
-
-        ::DrawEdge(hDC, &rcClient, BDR_SUNKENOUTER, BF_RECT);
-      }
-      // Themed background drawing
-      else
-      {
-        // Draw background
-        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;
-
-        for (int i = 0; i < 2; i++)
-        {
-          for (int j = 0; j < nHeight; j++)
+  try
+  {
+    // Find tab
+    CPluginClass *pClass = FindInstance(GetParent(hWnd));
+    if (!pClass)
+    {
+      return ::DefWindowProc(hWnd, message, wParam, lParam);
+    }
+
+    // Process message
+    switch (message)
+    {
+    case WM_SETCURSOR:
+      {
+        ::SetCursor(::LoadCursor(NULL, IDC_ARROW));
+        return TRUE;
+      }
+    case WM_PAINT:
+      {
+        PAINTSTRUCT ps;
+        HDC hDC = ::BeginPaint(hWnd, &ps);
+
+        AdblockPlus::Rectangle rcClient;
+        ::GetClientRect(hWnd, &rcClient);
+
+        int nDrawEdge = 0;
+
+        // Old Windows background drawing
+        if (pClass->m_hTheme == NULL)
+        {
+          ::FillRect(hDC, &rcClient, (HBRUSH)(COLOR_BTNFACE + 1));
+          ::DrawEdge(hDC, &rcClient, BDR_RAISEDINNER, BF_LEFT);
+
+          nDrawEdge = 3;
+          rcClient.left += 3;
+
+          ::DrawEdge(hDC, &rcClient, BDR_SUNKENOUTER, BF_RECT);
+        }
+        // Themed background drawing
+        else
+        {
+          // Draw background
+          if (pfnDrawThemeBackground)
           {
-            COLORREF clr = ::GetPixel(hDC, i + nWidth, j);
-
-            // Ignore black boxes (if source is obscured by other windows)
-            if (clr != -1 && (GetRValue(clr) > 8 || GetGValue(clr) > 8 || GetBValue(clr) > 8))
+            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;
+
+          for (int i = 0; i < 2; i++)
+          {
+            for (int j = 0; j < nHeight; j++)
             {
-              ::SetPixel(hDC, i, j, clr);
+              COLORREF clr = ::GetPixel(hDC, i + nWidth, j);
+
+              // Ignore black boxes (if source is obscured by other windows)
+              if (clr != -1 && (GetRValue(clr) > 8 || GetGValue(clr) > 8 || GetBValue(clr) > 8))
+              {
+                ::SetPixel(hDC, i, j, clr);
+              }
             }
           }
         }
-      }
-
-      // Draw icon
-      if (CPluginClient::GetInstance())
-      {
-        HICON hIcon = GetStatusBarIcon(pClass->GetTab()->GetDocumentUrl());
-
-        int offx = nDrawEdge;
-        if (hIcon)
-        {
-          //Get the RECT for the leftmost pane (the status text pane)
+
+        // Draw icon
+        if (CPluginClient::GetInstance())
+        {
+          HICON hIcon = GetStatusBarIcon(pClass->GetTab()->GetDocumentUrl());
+
+          int offx = nDrawEdge;
+          if (hIcon)
+          {
+            //Get the RECT for the leftmost pane (the status text pane)
+            RECT rect;
+            BOOL rectRes = ::SendMessage(pClass->m_hStatusBarWnd, SB_GETRECT, 0, (LPARAM)&rect);
+            ::DrawIconEx(hDC, 0, rect.bottom - rect.top - iconHeight, hIcon, iconWidth, iconHeight, NULL, NULL, DI_NORMAL);
+            offx += iconWidth;
+          }
+#ifdef _DEBUG
+          // Display version
+          HFONT hFont = (HFONT)::SendMessage(pClass->m_hStatusBarWnd, WM_GETFONT, 0, 0);
+          HGDIOBJ hOldFont = ::SelectObject(hDC, hFont);
+
+          AdblockPlus::Rectangle rcText = rcClient;
+          rcText.left += offx;
+          ::SetBkMode(hDC, TRANSPARENT);
+          ::DrawTextW(hDC, IEPLUGIN_VERSION, -1, &rcText, DT_WORD_ELLIPSIS | DT_LEFT | DT_SINGLELINE | DT_VCENTER);
+
+          ::SelectObject(hDC, hOldFont);
+#endif // _DEBUG
+        }
+
+        // Done!
+        EndPaint(hWnd, &ps);
+
+        return 0;
+      }
+
+    case WM_LBUTTONUP:
+    case WM_RBUTTONUP:
+      {
+        std::wstring url = pClass->GetBrowserUrl();
+        if (url != pClass->GetTab()->GetDocumentUrl())
+        {
+          pClass->GetTab()->SetDocumentUrl(url);
+        }
+
+        // Create menu
+        HMENU hMenu = pClass->CreatePluginMenu(url);
+        if (!hMenu)
+        {
+          return 0;
+        }
+
+        // Display menu
+        POINT pt;
+        ::GetCursorPos(&pt);
+
+        RECT rc;
+        ::GetWindowRect(hWnd, &rc);
+
+        if (rc.left >= 0 && rc.top >= 0)
+        {
+          pt.x = rc.left;
+          pt.y = rc.top;
+        }
+
+        pClass->DisplayPluginMenu(hMenu, -1, pt, TPM_LEFTALIGN | TPM_BOTTOMALIGN);
+        break;
+      }
+    case WM_DESTROY:
+      break;
+    case SC_CLOSE:
+      break;
+
+    case WM_UPDATEUISTATE:
+      {
+        CPluginTab* tab = GetTabForCurrentThread();
+        if (tab)
+        {
+          tab->OnActivate();
           RECT rect;
-          BOOL rectRes = ::SendMessage(pClass->m_hStatusBarWnd, SB_GETRECT, 0, (LPARAM)&rect);
-          ::DrawIconEx(hDC, 0, rect.bottom - rect.top - iconHeight, hIcon, iconWidth, iconHeight, NULL, NULL, DI_NORMAL);
-          offx += iconWidth;
-        }
-#ifdef _DEBUG
-        // Display version
-        HFONT hFont = (HFONT)::SendMessage(pClass->m_hStatusBarWnd, WM_GETFONT, 0, 0);
-        HGDIOBJ hOldFont = ::SelectObject(hDC,hFont);
-
-        AdblockPlus::Rectangle rcText = rcClient;
-        rcText.left += offx;
-        ::SetBkMode(hDC, TRANSPARENT);
-        ::DrawTextW(hDC, IEPLUGIN_VERSION, -1, &rcText, DT_WORD_ELLIPSIS|DT_LEFT|DT_SINGLELINE|DT_VCENTER);
-
-        ::SelectObject(hDC, hOldFont);
-#endif // _DEBUG
-      }
-
-      // Done!
-      EndPaint(hWnd, &ps);
-
-      return 0;
-    }
-
-  case WM_LBUTTONUP:
-  case WM_RBUTTONUP:
-    {
-      std::wstring url = pClass->GetBrowserUrl();
-      if (url != pClass->GetTab()->GetDocumentUrl())
-      {
-        pClass->GetTab()->SetDocumentUrl(url);
-      }
-
-      // Create menu
-      HMENU hMenu = pClass->CreatePluginMenu(url);
-      if (!hMenu)
-      {
-        return 0;
-      }
-
-      // Display menu
-      POINT pt;
-      ::GetCursorPos(&pt);
-
-      RECT rc;
-      ::GetWindowRect(hWnd, &rc);
-
-      if (rc.left >= 0 && rc.top >= 0)
-      {
-        pt.x = rc.left;
-        pt.y = rc.top;
-      }
-
-      pClass->DisplayPluginMenu(hMenu, -1, pt, TPM_LEFTALIGN|TPM_BOTTOMALIGN);
-    }
-    break;
-  case WM_DESTROY:
-    break;
-  case SC_CLOSE:
-    break;
-
-  case WM_UPDATEUISTATE:
-    {
-      CPluginTab* tab = GetTab(::GetCurrentThreadId());
-      if (tab)
-      {
-        tab->OnActivate();
+          GetWindowRect(pClass->m_hPaneWnd, &rect);
+          pClass->notificationMessage.MoveToCenter(rect);
+        }
+        if (LOWORD(wParam) == UIS_CLEAR)
+        {
+          pClass->notificationMessage.Hide();
+        }
+        break;
+      }
+    case WM_WINDOWPOSCHANGING:
+      {
         RECT rect;
         GetWindowRect(pClass->m_hPaneWnd, &rect);
-        pClass->notificationMessage.Move(rect.left + (rect.right - rect.left) / 2, rect.top + (rect.bottom - rect.top) / 2); 
-      }
-      if (LOWORD(wParam) == UIS_CLEAR)
-      {
-        pClass->notificationMessage.Hide();
-      }
-    }
-    break;
-  case WM_WINDOWPOSCHANGING:
-    {
-      RECT rect;
-      GetWindowRect(pClass->m_hPaneWnd, &rect);
-      if (pClass->notificationMessage.IsVisible())
-      {
-        pClass->notificationMessage.Move(rect.left + (rect.right - rect.left) / 2, rect.top + (rect.bottom - rect.top) / 2); 
-      }
-    }
-    break;
-  case WM_WINDOWPOSCHANGED:
-    {
-      WINDOWPOS* wndPos = reinterpret_cast<WINDOWPOS*>(lParam);
-      if (wndPos->flags & SWP_HIDEWINDOW)
-      {
-        pClass->notificationMessage.Hide();
-      }
-    }
-    break;
-  case WM_ALREADY_UP_TO_DATE:
-    {
-      Dictionary* dictionary = Dictionary::GetInstance();
-      std::wstring upToDateText = dictionary->Lookup("updater", "update-already-up-to-date-text");
-      std::wstring upToDateTitle = dictionary->Lookup("updater", "update-already-up-to-date-title");
-      pClass->notificationMessage.SetTextAndIcon(upToDateText, upToDateTitle, TTI_INFO);
-    }
-    break;
-  case WM_UPDATE_CHECK_ERROR:
-    {
-      Dictionary* dictionary = Dictionary::GetInstance();
-      std::wstring errorText = dictionary->Lookup("updater", "update-error-text");
-      std::wstring errorTitle = dictionary->Lookup("updater", "update-error-title");
-      pClass->notificationMessage.SetTextAndIcon(errorText, errorText, TTI_ERROR);
-    }
-    break;
-  case WM_DOWNLOADING_UPDATE:
-    {
-      Dictionary* dictionary = Dictionary::GetInstance();
-      std::wstring downloadingText = dictionary->Lookup("updater", "downloading-update-text");
-      std::wstring downloadingTitle = dictionary->Lookup("updater", "downloading-update-title");
-      pClass->notificationMessage.SetTextAndIcon(downloadingText, downloadingTitle, TTI_INFO);
-    }
-    break;
-  }
-
-  return DefWindowProc(hWnd, message, wParam, lParam);
+        if (pClass->notificationMessage.IsVisible())
+        {
+          pClass->notificationMessage.MoveToCenter(rect);
+        }
+        break;
+      }
+    case WM_WINDOWPOSCHANGED:
+      {
+        WINDOWPOS* wndPos = reinterpret_cast<WINDOWPOS*>(lParam);
+        if (wndPos->flags & SWP_HIDEWINDOW)
+        {
+          pClass->notificationMessage.Hide();
+        }
+        break;
+      }
+    case WM_ALREADY_UP_TO_DATE:
+      {
+        Dictionary* dictionary = Dictionary::GetInstance();
+        std::wstring upToDateText = dictionary->Lookup("updater", "update-already-up-to-date-text");
+        std::wstring upToDateTitle = dictionary->Lookup("updater", "update-already-up-to-date-title");
+        pClass->notificationMessage.SetTextAndIcon(upToDateText, upToDateTitle, TTI_INFO);
+        break;
+      }
+    case WM_UPDATE_CHECK_ERROR:
+      {
+        Dictionary* dictionary = Dictionary::GetInstance();
+        std::wstring errorText = dictionary->Lookup("updater", "update-error-text");
+        std::wstring errorTitle = dictionary->Lookup("updater", "update-error-title");
+        pClass->notificationMessage.SetTextAndIcon(errorText, errorTitle, TTI_ERROR);
+        break;
+      }
+    case WM_DOWNLOADING_UPDATE:
+      {
+        Dictionary* dictionary = Dictionary::GetInstance();
+        std::wstring downloadingText = dictionary->Lookup("updater", "downloading-update-text");
+        std::wstring downloadingTitle = dictionary->Lookup("updater", "downloading-update-title");
+        pClass->notificationMessage.SetTextAndIcon(downloadingText, downloadingTitle, TTI_INFO);
+        break;
+      }
+    }
+  }
+  catch (...)
+  {
+    // Suppress exception. Fall through to default handler.
+    DEBUG_GENERAL(L"CPluginClass::PaneWindowProc - exception");
+  }
+  return ::DefWindowProc(hWnd, message, wParam, lParam);
 }
 
 
 void CPluginClass::UpdateStatusBar()
 {
   DEBUG_GENERAL("*** Updating statusbar")
   if (m_hPaneWnd == NULL)
   {
     CreateStatusBarPane();
   }
@@ skipping 25 matching lines @@
   s_criticalSectionLocal.Unlock();
 
   return icon;
 }
 
 ATOM CPluginClass::GetAtomPaneClass()
 {
   return s_atomPaneClass;
 }