Issue 1672 - Subscribe and listen COM events using ATL::IDispEventImpl and BEGIN_SINK_MAP

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
+ * 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 "PluginClient.h"
 #include "PluginClientFactory.h"
 #include "PluginMutex.h"
 #include "sddl.h"
 #include "PluginUtil.h"
 #include "PluginUserSettings.h"
 #include "../shared/Utils.h"
 #include "../shared/Dictionary.h"
 #include "../shared/IE_version.h"
 #include <thread>
 #include <array>
 
 #ifdef DEBUG_HIDE_EL
 DWORD profileTime = 0;
 #endif
 
 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;
@@ skipping 111 matching lines @@
 
   s_criticalSectionLocal.Lock();
   {
     browser = s_asyncWebBrowser2;
   }
   s_criticalSectionLocal.Unlock();
 
   return browser;
 }
 
-CString CPluginClass::GetBrowserUrl() const
-{
-  CString url;
-
+std::wstring CPluginClass::GetBrowserUrl() const
+{
+  std::wstring url;
   CComQIPtr<IWebBrowser2> browser = GetBrowser();
   if (browser)
   {
     CComBSTR bstrURL;
-
-    if (SUCCEEDED(browser->get_LocationURL(&bstrURL)))
-    {
-      url = bstrURL;
-      CPluginClient::UnescapeUrl(url);
+    if (SUCCEEDED(browser->get_LocationURL(&bstrURL)) && bstrURL)
+    {
+      url = std::wstring(bstrURL, SysStringLen(bstrURL));
+      UnescapeUrl(url);
     }
   }
   else
   {
     url = m_tab->GetDocumentUrl();
   }
-
   return url;
 }
 
 DWORD WINAPI CPluginClass::StartInitObject(LPVOID thisPtr)
 {
   if (thisPtr == NULL)
     return 0;
   if (!((CPluginClass*)thisPtr)->InitObject(true))
   {
     ((CPluginClass*)thisPtr)->Unadvise();
   }
 
   return 0;
 }
-
-
 
 // This gets called when a new browser window is created (which also triggers the
 // creation of this object). The pointer passed in should be to a IWebBrowser2
 // interface that represents the browser for the window.
 // it is also called when a tab is closed, this unknownSite will be null
 // so we should handle that it is called this way several times during a session
 STDMETHODIMP CPluginClass::SetSite(IUnknown* unknownSite)
 {
   CPluginSettings* settings = CPluginSettings::GetInstance();
 
@@ skipping 29 matching lines @@
     }
     s_criticalSectionLocal.Unlock();
 
     try
     {
       // Check if loaded as BHO
       auto webBrowser = GetBrowser();
       if (webBrowser)
       {
         DEBUG_GENERAL("Loaded as BHO");
         HRESULT hr = DispEventAdvise(webBrowser);

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

The Advise/Unadvise life cycle belong in a constructor/destructor pair.

Right now setting up the advice relationship happens in loose code here and in a
separate function elsewhere. It would be clearer to localize these to the same
place in the code.

[sergei, 2015/01/13 11:49:12]

It does not belong to constructor/destructor pair.
I see, it definitely makes sense to have a pair Advise/Unadvise which works with
m_isAdvised and makes necessary calls. But I would like to do it as another
issue may be even GFB.

We have the method `Unadvise` to avoid code duplication because it's called from
several places but "advising" is called only from one place here, so it should
be fine to have it as is from the simplification point of view. As well as it's
always better to have smaller commits.

[Eric, 2015/01/13 18:44:30]

It absolutely should be paired.

As it's written now, there's a race condition between calling DispEventAdvise()
and setting 'm_isAdvised'. In what should be the destructor, there's a
semaphore-protected block that ensures that DispEventUnadvise() is called
atomically with resetting 'm_isAdvised' (which is defective--see comment--but
whatever). There should be a similar semaphore-protected block for
DispEventAdvise().

So this pair of calls is managing a kind of event-connection resource, and
because it's a resource, it falls under the purview of RAII. Unlike with
connection points, there's no explicit handle or object associated with the
resource, but it's still a resource. The member 'm_isAdvised' acts as the proxy
for that resource.

Such a resource class should have two members:
  -- a browser pointer, to avoid calling the (de facto) global GetBrowser() in
the destructor.
  -- the semaphore used to protect the block. Per #1467, we can use the standard
library for this.

The class does not need 'm_isAdvised', because it would be redundant. Such a
member would always be true during the existence of the instance.

The member variable in CPluginClass can be unique_ptr<>, which will
automatically call the destructor. Presence of the allocated instance represents
presence of the resource.

         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)
           {
-            auto errDescription = std::string("Class::Thread - Failed to create StartInitObject thread, ") +
-              ex.code().message() + ex.what();
-            DEBUG_ERROR_LOG(ex.code().value(), PLUGIN_ERROR_THREAD, PLUGIN_ERROR_MAIN_THREAD_CREATE_PROCESS, errDescription.c_str());
+            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 - Advice");
+          DEBUG_ERROR_LOG(hr, PLUGIN_ERROR_SET_SITE, PLUGIN_ERROR_SET_SITE_ADVICE, "Class::SetSite - Advise");
         }
       }
       else // Check if loaded as toolbar handler
       {
         DEBUG_GENERAL("Loaded as toolbar handler");
         CComPtr<IServiceProvider> pServiceProvider;
 
         HRESULT hr = unknownSite->QueryInterface(&pServiceProvider);
         if (SUCCEEDED(hr))
         {
@@ skipping 16 matching lines @@
             }
             s_criticalSectionBrowser.Unlock();
           }
         }
         else
         {
           DEBUG_ERROR_LOG(hr, PLUGIN_ERROR_SET_SITE, PLUGIN_ERROR_SET_SITE_QUERY_SERVICE_PROVIDER, "Class::SetSite - QueryInterface (service provider)");
         }
       }
     }
-    catch (std::runtime_error e)
-    {
-      DEBUG_ERROR(e.what());
+    catch (const std::runtime_error& ex)
+    {
+      DEBUG_EXCEPTION(ex);
       Unadvise();
     }
   }
   else
   {
-    // Unadvise

[Oleksandr, 2015/01/10 23:27:24]

I guess we can loose the captain obvious comment here.

[sergei, 2015/01/13 11:49:12]

removed

     Unadvise();
 
     // Destroy window
     if (m_pWndProcStatus)
     {
       ::SetWindowLongPtr(m_hStatusBarWnd, GWLP_WNDPROC, (LPARAM)(WNDPROC)m_pWndProcStatus);
 
       m_pWndProcStatus = NULL;
     }
 
@@ skipping 147 matching lines @@
         }
       }
     }
     else
     {
       DEBUG_ERROR_LOG(hr, PLUGIN_ERROR_UI, PLUGIN_ERROR_UI_GET_STATUSBAR, "Class::Get statusbar state");
     }
   }
   DEBUG_GENERAL("ShowStatusBar end");
 }
 

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

// Entry point

We need to annotate all the entry points with a consistent textual marker for
auditing. Unfortunately the function signature isn't consistent. 

+// Entry point
 void STDMETHODCALLTYPE CPluginClass::OnBeforeNavigate2(
-                        /* [in] */ IDispatch* frameBrowserDisp,
-                        /* [in] */ VARIANT* urlVariant,
-                        /* [in] */ VARIANT* /*Flags*/,
-                        /* [in] */ VARIANT* /*TargetFrameName*/,
-                        /* [in] */ VARIANT* /*PostData*/,
-                        /* [in] */ VARIANT* /*Headers*/,
-                        /* [in, out] */ VARIANT_BOOL* /*Cancel*/)
-{

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

This patch set changes OnBeforeNavigate2 into an entry point. That means it
needs a 'try' statement and a catch-all block.

-  ATL::CComQIPtr<IWebBrowser2> webBrowser = frameBrowserDisp;
-  if (!webBrowser)
-  {
-    return;
-  }
-
-  if (!urlVariant || urlVariant->vt != VT_BSTR)
-  {
-    return;
-  }
-  std::wstring url(urlVariant->bstrVal, SysStringLen(urlVariant->bstrVal));
-  UnescapeUrl(url);
-
-  //Register a mime filter if it's not registered yet
-  if (s_mimeFilter == NULL)
-  {
-    s_mimeFilter = CPluginClientFactory::GetMimeFilterClientInstance();
-  }
-
-  CString urlCString = ToCString(url);
-
-  // If webbrowser2 is equal to top level browser (as set in SetSite), we are navigating new page
-  CPluginClient* client = CPluginClient::GetInstance();
-
-  if (urlCString.Find(L"javascript") == 0)
-  {
-  }
-  else if (GetBrowser().IsEqualObject(webBrowser))
-  {
-    m_tab->OnNavigate(urlCString);
-
-    DEBUG_GENERAL(L"================================================================================\nBegin main navigation url:" + urlCString + "\n================================================================================")
+  /* [in] */ IDispatch* frameBrowserDisp,
+  /* [in] */ VARIANT* urlVariant,
+  /* [in] */ VARIANT* /*Flags*/,
+  /* [in] */ VARIANT* /*TargetFrameName*/,
+  /* [in] */ VARIANT* /*PostData*/,
+  /* [in] */ VARIANT* /*Headers*/,
+  /* [in, out] */ VARIANT_BOOL* /*Cancel*/)
+{
+  try
+  {
+    ATL::CComQIPtr<IWebBrowser2> webBrowser = frameBrowserDisp;
+    if (!webBrowser)
+    {
+      return;
+    }
+    if (!urlVariant || urlVariant->vt != VT_BSTR)
+    {
+      return;
+    }
+    std::wstring url(urlVariant->bstrVal, SysStringLen(urlVariant->bstrVal));
+    UnescapeUrl(url);
+
+    //Register a mime filter if it's not registered yet
+    if (s_mimeFilter == nullptr)
+    {
+      s_mimeFilter = CPluginClientFactory::GetMimeFilterClientInstance();
+    }
+    // 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 (GetBrowser().IsEqualObject(webBrowser))
+    {
+      m_tab->OnNavigate(url);
+      DEBUG_GENERAL(
+      L"================================================================================\n"
+      L"Begin main navigation url:" + url + L"\n"
+      L"================================================================================")
 
 #ifdef ENABLE_DEBUG_RESULT
-      CPluginDebug::DebugResultDomain(urlCString);
+      CPluginDebug::DebugResultDomain(url);
 #endif
-
-    UpdateStatusBar();
-  }
-  else
-  {
-    DEBUG_NAVI(L"Navi::Begin navigation url:" + urlCString)
-    m_tab->CacheFrame(urlCString);
-  }
-}
-

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

// Entry point

-void STDMETHODCALLTYPE CPluginClass::OnDownloadBegin()
-{
-  DEBUG_NAVI("Navi::Download Begin")
-}
-

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

// Entry point

+      UpdateStatusBar();
+    }
+    else
+    {
+      DEBUG_NAVI(L"Navi::Begin navigation url:" + url)
+      m_tab->CacheFrame(url);
+    }
+  }
+  catch (...)
+  {
+  }
+}
+
+// Entry point
 void STDMETHODCALLTYPE CPluginClass::OnDownloadComplete()
 {

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

try-statement and catch-all

-  DEBUG_NAVI("Navi::Download Complete")
-  ATL::CComPtr<IWebBrowser2> browser = GetBrowser();
-  if (browser)
-  {
-    m_tab->OnDownloadComplete(browser);
-  }
-}
-

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

// Entry point

+  try
+  {
+    DEBUG_NAVI(L"Navi::Download Complete")
+    ATL::CComPtr<IWebBrowser2> browser = GetBrowser();
+    if (browser)
+    {
+      m_tab->OnDownloadComplete(browser);
+    }
+  }
+  catch (...)
+  {
+  }
+}
+
+// Entry point
 void STDMETHODCALLTYPE CPluginClass::OnDocumentComplete(IDispatch* frameBrowserDisp, VARIANT* /*urlOrPidl*/)
 {

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

try-statement and catch-all

-  ATL::CComQIPtr<IWebBrowser2> webBrowser2 = frameBrowserDisp;
-  if (!webBrowser2)
-  {
-    return;
-  }
-  ATL::CString frameSrc;
-  ATL::CComBSTR locationUrl;
-  if (FAILED(webBrowser2->get_LocationURL(&locationUrl)) && !!locationUrl)
-  {
-    return;
-  }
-  frameSrc = locationUrl;
-  CPluginClient::UnescapeUrl(frameSrc);
-  bool isRootPageBrowser = GetBrowser().IsEqualObject(webBrowser2);
-  m_tab->OnDocumentComplete(webBrowser2, frameSrc, isRootPageBrowser);
-}
-

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

// Entry point

+  try
+  {
+    DEBUG_NAVI(L"Navi::Document Complete");
+    ATL::CComQIPtr<IWebBrowser2> webBrowser2 = frameBrowserDisp;
+    if (!webBrowser2)
+    {
+      return;
+    }
+    std::wstring frameSrc = GetLocationUrl(*webBrowser2);
+    UnescapeUrl(frameSrc);
+    bool isRootPageBrowser = GetBrowser().IsEqualObject(webBrowser2);
+    m_tab->OnDocumentComplete(webBrowser2, frameSrc, isRootPageBrowser);
+  }
+  catch (...)
+  {
+  }
+}
+
+// Entry point
 void STDMETHODCALLTYPE CPluginClass::OnWindowStateChanged(unsigned long flags, unsigned long validFlagsMask)
 {

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

try-statement and catch-all

-  DEBUG_GENERAL("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;
-        if (!InitObject(true))
-        {
-          //                                    Unadvise();
-        }

[Oleksandr, 2015/01/10 23:27:24]

This doesn't seem to be rebased to the current tip. (rev. 746)

-        UpdateStatusBar();
-      }
-    }
-  }
-  notificationMessage.Hide();
-  DEBUG_GENERAL("Tab change end");
-}
-

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

// Entry point

+  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(true);
+          UpdateStatusBar();
+        }
+      }
+    }
+    notificationMessage.Hide();
+    DEBUG_GENERAL(L"WindowStateChanged (check tab changed) end");
+  }
+  catch (...)
+  {
+  }
+}
+
+// Entry point
 void STDMETHODCALLTYPE CPluginClass::OnCommandStateChange(long /*command*/, VARIANT_BOOL /*enable*/)
 {

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

try-statement and catch-all

-  if (m_hPaneWnd == NULL)
-  {
-    CreateStatusBarPane();
-  }
-  else
-  {
-    if (AdblockPlus::IE::InstalledMajorVersion() > 6)
-    {
-      RECT rect;
-      BOOL rectRes = GetClientRect(m_hStatusBarWnd, &rect);
-      if (rectRes == TRUE)
-      {
-        MoveWindow(m_hPaneWnd, rect.right - 200, 0, m_nPaneWidth, rect.bottom - rect.top, TRUE);
-      }
-    }
+  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(bool bBHO)
 {
   DEBUG_GENERAL("InitObject");
   CPluginSettings* settings = CPluginSettings::GetInstance();
 
   if (!settings->GetPluginEnabled())
   {
@@ skipping 204 matching lines @@
     DEBUG_ERROR_LOG(0, PLUGIN_ERROR_UI, PLUGIN_ERROR_UI_NO_STATUSBAR_WIN, "Class::CreateStatusBarPane - No status bar")
     return true;
   }
 
   // Calculate pane height
   AdblockPlus::Rectangle rcStatusBar;
   ::GetClientRect(hWndStatusBar, &rcStatusBar);
 
   if (rcStatusBar.Height() > 0)
   {
+    if (rcStatusBar.Height() < iconWidth)
+    { 
+      iconWidth = 19;
+      iconHeight = 19;
+    }
+
 #ifdef _DEBUG
     m_nPaneWidth = 70;
 #else
-    m_nPaneWidth = min(rcStatusBar.Height(), 22);
+    m_nPaneWidth = min(rcStatusBar.Height(), iconWidth);
 #endif
   }
   else
   {
 #ifdef _DEBUG
     m_nPaneWidth = 70;
 #else
-    m_nPaneWidth = 22;
+    m_nPaneWidth = iconWidth;
 #endif
   }
   // Create pane window
   HWND hWndNewPane = ::CreateWindowEx(
     NULL,
     MAKEINTATOM(GetAtomPaneClass()),
     L"",
     WS_CHILD | WS_VISIBLE | WS_CLIPSIBLINGS | WS_CLIPCHILDREN,
-    rcStatusBar.Width() - 500,0,m_nPaneWidth,rcStatusBar.Height(),
+    rcStatusBar.Width() - 500, 0, m_nPaneWidth, rcStatusBar.Height(),
     hWndStatusBar,
     (HMENU)3671,
     _Module.m_hInst,
     NULL);
 
   if (!hWndNewPane)
   {
     DEBUG_ERROR_LOG(::GetLastError(), PLUGIN_ERROR_UI, PLUGIN_ERROR_UI_CREATE_STATUSBAR_PANE, "Class::CreateStatusBarPane - CreateWindowEx")
     return false;
   }
@@ skipping 122 matching lines @@
 STDMETHODIMP CPluginClass::QueryStatus(const GUID* pguidCmdGroup, ULONG cCmds, OLECMD prgCmds[], OLECMDTEXT* pCmdText)
 {
   if (cCmds == 0) return E_INVALIDARG;
   if (prgCmds == 0) return E_POINTER;
 
   prgCmds[0].cmdf = OLECMDF_ENABLED;
 
   return S_OK;
 }
 
-HMENU CPluginClass::CreatePluginMenu(const CString& url)
+HMENU CPluginClass::CreatePluginMenu(const std::wstring& url)
 {
   DEBUG_GENERAL("CreatePluginMenu");
   HINSTANCE hInstance = _AtlBaseModule.GetModuleInstance();
 
   HMENU hMenu = ::LoadMenu(hInstance, MAKEINTRESOURCE(IDR_MENU1));
 
   HMENU hMenuTrackPopup = GetSubMenu(hMenu, 0);
 
   SetMenuBar(hMenuTrackPopup, url);
 
@@ skipping 87 matching lines @@
           {
             DEBUG_ERROR_LOG(hr, PLUGIN_ERROR_NAVIGATION, PLUGIN_ERROR_NAVIGATION_SETTINGS, "Navigation::Failed")
           }
         }
       }
       break;
     }
   case ID_MENU_DISABLE_ON_SITE:
     {
       CPluginSettings* settings = CPluginSettings::GetInstance();
-      CString urlString = GetTab()->GetDocumentUrl();
-      if (client->IsWhitelistedUrl(to_wstring(urlString)))
-      {
-        settings->RemoveWhiteListedDomain(to_CString(client->GetHostFromUrl(to_wstring(urlString))));
+      std::wstring urlString = GetTab()->GetDocumentUrl();
+      if (client->IsWhitelistedUrl(urlString))
+      {
+        settings->RemoveWhiteListedDomain(ToCString(client->GetHostFromUrl(urlString)));
       }
       else
       {
-        settings->AddWhiteListedDomain(to_CString(client->GetHostFromUrl(to_wstring(urlString))));
+        settings->AddWhiteListedDomain(ToCString(client->GetHostFromUrl(urlString)));
       }
       GetBrowser()->Refresh();
     }
   default:
     break;
   }
 
   // Invalidate and redraw the control
   UpdateStatusBar();
 }
 
 
-bool CPluginClass::SetMenuBar(HMENU hMenu, const CString& url)
+bool CPluginClass::SetMenuBar(HMENU hMenu, const std::wstring& url)
 {
   DEBUG_GENERAL("SetMenuBar");
 
   std::wstring ctext;
   Dictionary* dictionary = Dictionary::GetInstance();
 
   MENUITEMINFOW fmii = {};
   fmii.cbSize = sizeof(fmii);
 
   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(to_wstring(url)));
-    if (client->IsWhitelistedUrl(to_wstring(GetTab()->GetDocumentUrl())))
+    ReplaceString(ctext, L"?1?", client->GetHostFromUrl(url));
+    if (client->IsWhitelistedUrl(GetTab()->GetDocumentUrl()))
     {
       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 193 matching lines @@
   }
 
   LRESULT result = CallWindowProc(pClass->m_pWndProcStatus, hWnd, message, wParam, lParam);
 
 
   return result;
 
 }
 
 
-HICON CPluginClass::GetStatusBarIcon(const CString& url)
+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());
   if (tab)
   {
     CPluginClient* client = CPluginClient::GetInstance();
     if (CPluginSettings::GetInstance()->IsPluginEnabled())
     {
-      if (client->IsWhitelistedUrl(ToWstring(url)))
+      if (client->IsWhitelistedUrl(url))
       {
         hIcon = GetIcon(ICON_PLUGIN_DISABLED);
       }
       else
       {
         CPluginSettings* settings = CPluginSettings::GetInstance();
         hIcon = GetIcon(ICON_PLUGIN_ENABLED);
       }
     }
   }
@@ skipping 68 matching lines @@
             }
           }
         }
       }
 
       // Draw icon
       if (CPluginClient::GetInstance())
       {
         HICON hIcon = GetStatusBarIcon(pClass->GetTab()->GetDocumentUrl());
 
-        int offx = (rcClient.Height() - 16)/2 + nDrawEdge;
+        int offx = nDrawEdge;
         if (hIcon)
         {
-          ::DrawIconEx(hDC, offx, (rcClient.Height() - 16)/2 + 2, hIcon, 16, 16, NULL, NULL, DI_NORMAL);
-          offx += 22;
+          //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:
     {
-      CString strURL = pClass->GetBrowserUrl();
-      if (strURL != pClass->GetTab()->GetDocumentUrl())
-      {
-        pClass->GetTab()->SetDocumentUrl(strURL);
+      std::wstring url = pClass->GetBrowserUrl();
+      if (url != pClass->GetTab()->GetDocumentUrl())
+      {
+        pClass->GetTab()->SetDocumentUrl(url);
       }
 
       // Create menu
-      HMENU hMenu = pClass->CreatePluginMenu(strURL);
+      HMENU hMenu = pClass->CreatePluginMenu(url);
       if (!hMenu)
       {
         return 0;
       }
 
       // Display menu
       POINT pt;
       ::GetCursorPos(&pt);
 
       RECT rc;
@@ skipping 85 matching lines @@
   {
     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 STDMETHODCALLTYPE CPluginClass::Unadvise()

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

If no OnQuit function is provided, this is an entry point.

+void CPluginClass::Unadvise()
 {
   s_criticalSectionLocal.Lock();
   {
     if (m_isAdvised)
     {
       HRESULT hr = DispEventUnadvise(GetBrowser());
       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])
     {
-      s_hIcons[type] = ::LoadIcon(_Module.m_hInst, MAKEINTRESOURCE(s_hIconTypes[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")
+        DEBUG_ERROR_LOG(::GetLastError(), PLUGIN_ERROR_UI, PLUGIN_ERROR_UI_LOAD_ICON, "Class::GetIcon - LoadIcon");
       }
     }
 
     icon = s_hIcons[type];
   }
   s_criticalSectionLocal.Unlock();
 
   return icon;
 }
 
@@ skipping 60 matching lines @@
             break;
           }
           s_criticalSectionLocal.Unlock();
 
         }
       }
     }
 
     hTabWnd = ::GetWindow(hTabWnd, GW_HWNDNEXT);
   }
-
   return hTabWnd;
-
-}
+}