Issue 1201 - Enabling/disabling ABP everywhere applies only in the next opened tab

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 29 matching lines @@
 
 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;
@@ skipping 175 matching lines @@
     }
     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();
+      CPluginClientFactory::GetMimeFilterClientInstance();
       s_asyncWebBrowser2 = unknownSite;
       s_instances.insert(this);
     }
     s_criticalSectionLocal.Unlock();
 
     try
     {
       // Check if loaded as BHO
       auto webBrowser = GetBrowser();
       if (webBrowser)
@@ skipping 246 matching lines @@
     {
       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

[Eric, 2015/03/27 13:35:50]

I'm assuming there was a reason to have this statement in the code before. If
so, removing the call the 'GetMimeFilterClientInstance' is defective. We would
need a replacement, something like 'EnsureMimeFilterRegistered'.

[sergei, 2015/04/01 18:49:32]

It was indeed the issue here. We disable the plugin and deregister the
factories, but on the next request (JIC, we are inside
CPluginClass::OnBeforeNavigate2) we register them again, so it's removed for
good.

-    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(
@@ skipping 125 matching lines @@
   }
 }
 
 bool CPluginClass::InitObject(bool bBHO)
 {
   DEBUG_GENERAL("InitObject");
   CPluginSettings* settings = CPluginSettings::GetInstance();
 
   if (!settings->GetPluginEnabled())
   {
-    s_mimeFilter->Unregister();
+    CPluginClientFactory::ReleaseMimeFilterClientInstance();

[Oleksandr, 2015/03/27 08:02:29]

This fixes the issue. Just calling Unregister() here is not releasing m_spCFHTTP
and m_spCFHTTPS in CPluginMimeFilterClient, which apparently means our
Asynchronous Pluggable Protocols remain invoked. This renders s_mimeFilter
irrelevant, so removing it.

[sergei, 2015/04/01 18:49:32]

No, it does not mean that our APP handlers remain invoked. If we unregistered
them they are not called anymore, which is actually expected. According to the
debugger, the reference counter of the factories is equal to 1, so they are not
referenced by something else.

   }
 
   // Load theme module
   s_criticalSectionLocal.Lock();
   {
     if (!s_hUxtheme)
     {
       s_hUxtheme = ::GetModuleHandle(L"uxtheme.dll");
       if (s_hUxtheme)
       {
@@ skipping 438 matching lines @@
     {
       CPluginSettings* settings = CPluginSettings::GetInstance();
 
       settings->TogglePluginEnabled();
 
       // Enable / disable mime filter
       s_criticalSectionLocal.Lock();
       {
         if (settings->GetPluginEnabled())
         {
-          s_mimeFilter = CPluginClientFactory::GetMimeFilterClientInstance();
+          CPluginClientFactory::GetMimeFilterClientInstance();
         }
         else
         {
-          s_mimeFilter = NULL;
-
           CPluginClientFactory::ReleaseMimeFilterClientInstance();
         }
       }
       s_criticalSectionLocal.Unlock();
     }
     break;
   case ID_MENU_SETTINGS:
     {
       CComQIPtr<IWebBrowser2> browser = GetAsyncBrowser();
       if (browser)
@@ skipping 627 matching lines @@
           s_criticalSectionLocal.Unlock();
 
         }
       }
     }
 
     hTabWnd = ::GetWindow(hTabWnd, GW_HWNDNEXT);
   }
   return hTabWnd;
 }