Noissue - Remove dead code in CPluginClass::SetSite

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 76 matching lines @@
       return right - left;
     }
   };
 }
 
 CPluginClass::CPluginClass()
 {
   //Use this line to debug memory leaks
   //    _CrtDumpMemoryLeaks();
 
-  m_isAdviced = false;
-  m_nConnectionID = 0;
+  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);
@@ skipping 12 matching lines @@
 
 HRESULT CPluginClass::FinalConstruct()
 {
   return S_OK;
 }
 
 void CPluginClass::FinalRelease()
 {
   s_criticalSectionBrowser.Lock();
   {
     m_webBrowser2.Release();

[Eric, 2015/03/06 15:58:04]

Explicit release.

   }
   s_criticalSectionBrowser.Unlock();
 }
 
-
-// This method tries to get a 'connection point' from the stored browser, which can be
-// used to attach or detach from the stream of browser events
-CComPtr<IConnectionPoint> CPluginClass::GetConnectionPoint()
-{
-  CComQIPtr<IConnectionPointContainer, &IID_IConnectionPointContainer> pContainer(GetBrowser());
-  if (!pContainer)
-  {
-    return NULL;
-  }
-
-  CComPtr<IConnectionPoint> pPoint;
-  HRESULT hr = pContainer->FindConnectionPoint(DIID_DWebBrowserEvents2, &pPoint);
-  if (FAILED(hr))
-  {
-    DEBUG_ERROR_LOG(hr, PLUGIN_ERROR_SET_SITE, PLUGIN_ERROR_SET_SITE_FIND_CONNECTION_POINT, "Class::GetConnectionPoint - FindConnectionPoint")
-      return NULL;
-  }
-
-  return pPoint;
-}
-
 HWND CPluginClass::GetBrowserHWND() const
 {
   SHANDLE_PTR hBrowserWndHandle = NULL;
 
   CComQIPtr<IWebBrowser2> browser = GetBrowser();
   if (browser)
   {
     HRESULT hr = browser->get_HWND(&hBrowserWndHandle);
     if (FAILED(hr))
     {
@@ skipping 49 matching lines @@
   {
     url = m_tab->GetDocumentUrl();
   }
   return url;
 }
 
 DWORD WINAPI CPluginClass::StartInitObject(LPVOID thisPtr)
 {
   if (thisPtr == NULL)
     return 0;
-  if (!((CPluginClass*)thisPtr)->InitObject(true))
-  {
-    ((CPluginClass*)thisPtr)->Unadvice();
+  if (!((CPluginClass*)thisPtr)->InitObject())
+  {
+    ((CPluginClass*)thisPtr)->Unadvise();
   }
 
   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.

[sergei, 2015/03/09 10:23:50]

I'm not sure that we need this comment except the case when SetSite is not
called. Are you sure that 'SetSite(nullptr)' not called if the shutting down is
graceful, thus it's not a case of the crash or something like this? Would be
great if you can provide a link to the documentation.

[Eric, 2015/03/09 13:31:29]

I was just rewriting the comment, mostly, with the one change you picked up on;
see below. Given that the lifecycle of IE extensions is not well documented,
having some kind of comment here is certainly warranted.
Yes, mostly. I verified it last week. I haven't seen anything about this topic
in any MS documentation. Of particular interest, I haven't seen anything about a
requirement to call 'SetSite(nullptr)' before destroying a control instance. It
seems best to assume that it need not be.

I wrote some more detailed trace messages for this function that included values
of 'this' and the site pointer. Most of them were in matched pairs, but not all.
I was very reliably getting unmatched pairs. There was no obvious crashing
behavior, although I can't be completely sure there wasn't any internally; this
is why I said "mostly" above.

The consistency in unmatched pairs seems to be something related to never having
loaded a page or interrupting a page load by shutting down IE; I don't have
anything definitive there. I'm going to be looking into this issue further.

[Oleksandr, 2015/03/10 08:38:26]

I am pretty sure it _is_ called every time when IE is shutting down. It's that
sometimes the IE shutdown from the end user perspective doesn't really shutdown
the IE and the process keeps hanging. In that case it doesn't call
SetSite(nullptr). Do you have any test cases when it is not called?
Also, I think this comment merely reflects the documentation. I'd vote for
removing it or at least making it much shorter

[Oleksandr, 2015/03/10 08:42:04]

https://msdn.microsoft.com/en-us/library/bb250489(v=vs.85).aspx#basics
Could it have been the fact that the IE process did not exit?

  */

[sergei, 2015/03/30 11:26:23]

This comment

 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");
       }
 
       s_criticalSectionBrowser.Lock();
       {
         m_webBrowser2 = unknownSite;

[Eric, 2015/03/06 15:58:04]

Assigned to m_webBrowser2 here, declared as CComQIPtr<IWebBrowser2>

[sergei, 2015/03/06 16:07:19]

Here `AddRef` is called.

       }
       s_criticalSectionBrowser.Unlock();
 
       //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
       {
-        // Check if loaded as BHO
-        if (GetBrowser())

[sergei, 2015/03/09 10:23:50]

We don't need this comment now.

[Eric, 2015/03/09 13:31:29]

We don't need this particular comment, but we'll need another one in the next
patch set. We still need to check that the QI call on site pointer to
'IWebBrowser2' is not null. If it's not, we likely have some severe IE defect,
but regardless we need to handle that case gracefully.

[Eric, 2015/03/17 10:41:07]

Done.

+        auto webBrowser = GetBrowser();
+        if (webBrowser)
         {
           DEBUG_GENERAL("Loaded as BHO");
-          CComPtr<IConnectionPoint> pPoint = GetConnectionPoint();
-          if (pPoint)
+          HRESULT hr = DispEventAdvise(webBrowser);
+          if (SUCCEEDED(hr))
           {
-            HRESULT hr = pPoint->Advise((IDispatch*)this, &m_nConnectionID);
-            if (SUCCEEDED(hr))
+            m_isAdvised = true;
+            try
             {
-              m_isAdviced = 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");
-              }
+              std::thread startInitObjectThread(StartInitObject, this);
+              startInitObjectThread.detach(); // TODO: but actually we should wait for the thread in the dtr.
             }
-            else
+            catch (const std::system_error& ex)
             {
-              DEBUG_ERROR_LOG(hr, PLUGIN_ERROR_SET_SITE, PLUGIN_ERROR_SET_SITE_ADVICE, "Class::SetSite - Advice");
+              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");
+          }
         }
       }
       catch (const std::runtime_error& ex)
       {
         DEBUG_EXCEPTION(ex);
-        Unadvice();
+        Unadvise();
       }
     }
     else
     {
-      // Unadvice
-      Unadvice();
+      Unadvise();
 
       // Destroy window
       if (m_pWndProcStatus)
       {
         ::SetWindowLongPtr(m_hStatusBarWnd, GWLP_WNDPROC, (LPARAM)(WNDPROC)m_pWndProcStatus);
 
         m_pWndProcStatus = NULL;
       }
 
       if (m_hPaneWnd)
@@ skipping 22 matching lines @@
         {
           // TODO: Explicitly releasing a resource when a container becomes empty looks like a job better suited for shared_ptr
           CPluginClientFactory::ReleaseMimeFilterClientInstance();
         }
       }
       s_criticalSectionLocal.Unlock();
 
       // Release browser interface
       s_criticalSectionBrowser.Lock();
       {
         m_webBrowser2.Release();

[Eric, 2015/03/06 15:58:04]

Explicit release here.

[sergei, 2015/03/06 16:07:19]

So, since `AddRef` is called and `m_webBrowser2` is a smart pointer, there is no
inconsistency. After `m_webBrowser2.Release();` the following calls
`m_webBrowser2.Release();` are safe, they do nothing.

[Eric, 2015/03/06 16:27:22]

Calling 'Release' through 'CComPtr' apparently also zeros out the pointer as
well, contrary to all good design sense. I had to read the source code to see
this; it's certainly not an obvious behavior.

I am coming to despise ATL even more than I did before, which is quite saying
something.

       }
       s_criticalSectionBrowser.Unlock();
 
       DEBUG_GENERAL("================================================================================\nNEW TAB UI - END\n================================================================================")
 
       ::CoUninitialize();
     }
 
     IObjectWithSiteImpl<CPluginClass>::SetSite(unknownSite);
   }
@@ skipping 109 matching lines @@
       }
     }
     else
     {
       DEBUG_ERROR_LOG(hr, PLUGIN_ERROR_UI, PLUGIN_ERROR_UI_GET_STATUSBAR, "Class::Get statusbar state");
     }
   }
   DEBUG_GENERAL("ShowStatusBar end");
 }
 
-/*
- * #1163 This class is the implementation for method DISPID_BEFORENAVIGATE2 in CPluginClass::Invoke.
- * - It validates and convertes its own arguments, rather than unifying them in the Invoke body.
- * - It's declared void and not HRESULT, so DISPID_BEFORENAVIGATE2 can only return S_OK.
- */
-void CPluginClass::BeforeNavigate2(DISPPARAMS* pDispParams)
-{
-
-  if (pDispParams->cArgs < 7)
-  {
-    return;
-  }
-  //Register a mime filter if it's not registered yet
-  if (s_mimeFilter == NULL)
-  {
-    s_mimeFilter = CPluginClientFactory::GetMimeFilterClientInstance();
-  }
-
-  // Get the IWebBrowser2 interface
-  CComQIPtr<IWebBrowser2, &IID_IWebBrowser2> WebBrowser2Ptr;
-  VARTYPE vt = pDispParams->rgvarg[6].vt;
-  if (vt == VT_DISPATCH)
-  {
-    WebBrowser2Ptr = pDispParams->rgvarg[6].pdispVal;
-  }
-  else
-  {
-    // Wrong type, return.
-    return;
-  }
-
-  // Get the URL
-  std::wstring url;
-  const auto& arg = pDispParams->rgvarg[5];
-  vt = arg.vt;
-  if (vt == (VT_BYREF | VT_VARIANT) && arg.pvarVal->vt == VT_BSTR)
-  {
-    BSTR b = arg.pvarVal->bstrVal;
-    if (b) {
-      url = std::wstring(b, SysStringLen(b));
-      UnescapeUrl(url);
-    }
-  }
-  else
-  {
-    // Wrong type, return.
-    return;
-  }
-
-  // If webbrowser2 is equal to top level browser (as set in SetSite), we are navigating new page
-  CPluginClient* client = CPluginClient::GetInstance();
-  CString urlLegacy = ToCString(url);
-  if (urlLegacy.Find(L"javascript") == 0)
-  {
-  }
-  else if (GetBrowser().IsEqualObject(WebBrowser2Ptr))
-  {
-    m_tab->OnNavigate(url);
-
-    DEBUG_GENERAL(
+// Entry point
+void STDMETHODCALLTYPE CPluginClass::OnBeforeNavigate2(
+  IDispatch* frameBrowserDisp /**< [in] */,
+  VARIANT* urlVariant /**< [in] */,
+  VARIANT* /**< [in] Flags*/,
+  VARIANT* /**< [in] TargetFrameName*/,
+  VARIANT* /**< [in] PostData*/,
+  VARIANT* /**< [in] Headers*/,
+  VARIANT_BOOL* /**< [in, out] 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(url);
 #endif
-
-    UpdateStatusBar();
-  }
-  else
-  {
-    DEBUG_NAVI(L"Navi::Begin navigation url:" + url)
-    m_tab->CacheFrame(url);
-  }
-}
-
-/*
- * #1163 implements behavior for method DISPID_WINDOWSTATECHANGED in CPluginClass::Invoke
- * - should validate and convert arguments in Invoke, not here
- * - does not validate number of arguments before indexing into 'rgvarg'
- * - does not validate type of argument before using its value
- */
-STDMETHODIMP CPluginClass::OnTabChanged(DISPPARAMS* pDispParams, WORD wFlags)
-{
-  DEBUG_GENERAL("Tab changed");
-  bool newtabshown = pDispParams->rgvarg[1].intVal==3;
-  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("Tab change end");
-  return S_OK;
-}
-
-// This gets called whenever there's a browser event
-// ENTRY POINT
-STDMETHODIMP CPluginClass::Invoke(DISPID dispidMember, REFIID riid, LCID lcid, WORD wFlags, DISPPARAMS* pDispParams, VARIANT* pvarResult, EXCEPINFO* pExcepInfo, UINT* puArgErr)
+      UpdateStatusBar();
+    }
+    else
+    {
+      DEBUG_NAVI(L"Navi::Begin navigation url:" + url)
+      m_tab->CacheFrame(url);
+    }
+  }
+  catch (...)
+  {
+  }
+}
+
+// Entry point
+void STDMETHODCALLTYPE CPluginClass::OnDownloadComplete()
 {
   try
   {
-    WCHAR tmp[256];
-    wsprintf(tmp, L"Invoke: %d\n", dispidMember);
-    DEBUG_GENERAL(tmp);
-    switch (dispidMember)
-    {
-    case DISPID_WINDOWSTATECHANGED:
-      {
-        // #1163 should validate and convert arguments here
-        return OnTabChanged(pDispParams, wFlags);
-      }
-
-    case DISPID_HTMLDOCUMENTEVENTS2_ONBEFOREUPDATE:
-      break;
-
-    case DISPID_HTMLDOCUMENTEVENTS2_ONCLICK:
-      break;
-
-    case DISPID_EVMETH_ONLOAD:
-      DEBUG_NAVI("Navi::OnLoad")
-      break;
-
-    case DISPID_EVMETH_ONCHANGE:
-      break;
-
-    case DISPID_EVMETH_ONMOUSEDOWN:
-      break;
-
-    case DISPID_EVMETH_ONMOUSEENTER:
-      break;
-
-    case DISPID_IHTMLIMGELEMENT_START:
-      break;
-
-    case STDDISPID_XOBJ_ERRORUPDATE:
-      break;
-
-    case STDDISPID_XOBJ_ONPROPERTYCHANGE:
-      break;
-
-    case DISPID_READYSTATECHANGE:
-      DEBUG_NAVI("Navi::ReadyStateChange");
-      break;
-
-    case DISPID_BEFORENAVIGATE:
-      DEBUG_NAVI("Navi::BeforeNavigate");
-      break;
-
-    case DISPID_COMMANDSTATECHANGE:
-      if (m_hPaneWnd == NULL)
-      {
-        CreateStatusBarPane();
-      }
-      else
-      {
+    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*/)
+{
+  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)
+{
+  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();
+        }
+      }
+    }
+    notificationMessage.Hide();
+    DEBUG_GENERAL(L"WindowStateChanged (check tab changed) end");
+  }
+  catch (...)
+  {
+  }
+}
+
+// Entry point
+void STDMETHODCALLTYPE CPluginClass::OnCommandStateChange(long /*command*/, VARIANT_BOOL /*enable*/)
+{
+  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);
-          }
-        }      
-      }
-      break;
-
-    case DISPID_STATUSTEXTCHANGE:
-      break;
-
-    case DISPID_BEFORENAVIGATE2:
-      {
-        // #1163 should validate and convert parameters here
-        BeforeNavigate2(pDispParams);
-      }
-      break;
-
-    case DISPID_DOWNLOADBEGIN:
-      {
-        DEBUG_NAVI("Navi::Download Begin")
-      }
-      break;
-
-    case DISPID_DOWNLOADCOMPLETE:
-      {
-        DEBUG_NAVI("Navi::Download Complete");
-        CComQIPtr<IWebBrowser2> browser = GetBrowser();
-        if (browser)
-        {
-          m_tab->OnDownloadComplete(browser);
-        }
-      }
-      break;
-
-    case DISPID_DOCUMENTCOMPLETE:
-      {
-        DEBUG_NAVI("Navi::Document Complete");
-        CComQIPtr<IWebBrowser2> browser = GetBrowser();
-        if (browser && pDispParams->cArgs >= 2 && pDispParams->rgvarg[1].vt == VT_DISPATCH)
-        {
-          CComQIPtr<IWebBrowser2> pBrowser = pDispParams->rgvarg[1].pdispVal;
-          if (pBrowser)
-          {
-            CComBSTR bstrUrl;
-            if (SUCCEEDED(pBrowser->get_LocationURL(&bstrUrl)) && bstrUrl && ::SysStringLen(bstrUrl) > 0)
-            {
-              std::wstring url = std::wstring(bstrUrl, SysStringLen(bstrUrl));
-              UnescapeUrl(url);
-              m_tab->OnDocumentComplete(browser, url, browser.IsEqualObject(pBrowser));
-            }
-          }
-        }
-      }
-      break;
-
-    case DISPID_ONQUIT:
-    case DISPID_QUIT:
-      {
-        Unadvice();
-      }
-      break;
-
-    default:
-      {
-        CString did;
-        did.Format(L"DispId:%u", dispidMember);
-
-        DEBUG_NAVI(L"Navi::Default " + did)
-      }
-      /*
-       * Ordinarily a method not dispatched should return DISP_E_MEMBERNOTFOUND.
-       * As a conservative initial change, we leave it behaving as before,
-       *   which is to do nothing and return S_OK.
-       */
-      // do nothing
-      break;
-    }
-  }
-  catch(...)
-  {
-    DEBUG_GENERAL( "Caught unknown exception in CPluginClass::Invoke" );
-    return E_FAIL;
-  }
-  return S_OK;
-}
-
-bool CPluginClass::InitObject(bool bBHO)
+      {
+        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");
   CPluginSettings* settings = CPluginSettings::GetInstance();
 
   if (!settings->GetPluginEnabled())
   {
     s_mimeFilter->Unregister();
   }
 
   // Load theme module
@@ skipping 56 matching lines @@
 
     if (!GetAtomPaneClass())
     {
       DEBUG_ERROR_LOG(::GetLastError(), PLUGIN_ERROR_UI, PLUGIN_ERROR_UI_REGISTER_PANE_CLASS, "Class::InitObject - RegisterClassEx");
       return false;
     }
   }
 
   int ieVersion = AdblockPlus::IE::InstalledMajorVersion();
   // Create status pane
-  if (bBHO && ieVersion > 6 && !CreateStatusBarPane())
+  if (ieVersion > 6 && !CreateStatusBarPane())
   {
     return false;
   }
   
   s_criticalSectionLocal.Lock();
   int versionCompRes = CPluginClient::GetInstance()->CompareVersions(CPluginClient::GetInstance()->GetPref(L"currentVersion", L"0.0"), L"1.2");
 
   bool isFirstRun = CPluginClient::GetInstance()->IsFirstRun();
   CPluginClient::GetInstance()->SetPref(L"currentVersion", std::wstring(IEPLUGIN_VERSION));
   // This is the first time ABP was installed
@@ skipping 900 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 CPluginClass::Unadvice()
+void CPluginClass::Unadvise()
 {
   s_criticalSectionLocal.Lock();
   {
-    if (m_isAdviced)
-    {
-      CComPtr<IConnectionPoint> pPoint = GetConnectionPoint();
-      if (pPoint)
-      {
-        HRESULT hr = pPoint->Unadvise(m_nConnectionID);
-        if (FAILED(hr))
-        {
-          DEBUG_ERROR_LOG(hr, PLUGIN_ERROR_SET_SITE, PLUGIN_ERROR_SET_SITE_UNADVICE, "Class::Unadvice - Unadvise");
-        }
-      }
-
-      m_isAdviced = false;
+    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();
@@ skipping 79 matching lines @@
             break;
           }
           s_criticalSectionLocal.Unlock();
 
         }
       }
     }
 
     hTabWnd = ::GetWindow(hTabWnd, GW_HWNDNEXT);
   }
-
   return hTabWnd;
-
-}
+}