Issue 119 - Switch to injecting CSS for element hiding

src/plugin/PluginClass.cpp

Index: src/plugin/PluginClass.cpp
diff --git a/src/plugin/PluginClass.cpp b/src/plugin/PluginClass.cpp
index 48d0e9cc5e02262aa75cbfbd8d1d80f7dec42717..83ff4f6ebe45ce881065eebbbc28f5d1fdd2c218 100644
--- a/src/plugin/PluginClass.cpp
+++ b/src/plugin/PluginClass.cpp
@@ -13,6 +13,7 @@
 #include "PluginUserSettings.h"
 #include "../shared/Utils.h"
 #include "../shared/Dictionary.h"
+#include "WebBrowserEventsListener.h"
 #include <thread>
 #include <array>
 
@@ -71,6 +72,7 @@ namespace AdblockPlus
 }
 
 CPluginClass::CPluginClass()
+  : m_data(std::make_shared<Data>())
 {
   //Use this line to debug memory leaks
   //	_CrtDumpMemoryLeaks();
@@ -85,17 +87,14 @@ CPluginClass::CPluginClass()
   m_isInitializedOk = false;
 
 
-  m_tab = new CPluginTab(this);
-
+  m_data->tab.reset(new CPluginTab(this));
   Dictionary::Create(GetBrowserLanguage());
 }
 
 CPluginClass::~CPluginClass()
 {
-  delete m_tab;
 }
 
-
 /////////////////////////////////////////////////////////////////////////////
 // Initialization
 
@@ -108,7 +107,7 @@ void CPluginClass::FinalRelease()
 {
   s_criticalSectionBrowser.Lock();
   {
-    m_webBrowser2.Release();
+    m_data.reset();
   }
   s_criticalSectionBrowser.Unlock();
 }
@@ -137,7 +136,7 @@ CComQIPtr<IWebBrowser2> CPluginClass::GetBrowser() const
 
   s_criticalSectionBrowser.Lock();
   {
-    browser = m_webBrowser2;
+    browser = m_data->webBrowser2;
   }
   s_criticalSectionBrowser.Unlock();
 
@@ -175,7 +174,7 @@ CString CPluginClass::GetBrowserUrl() const
   }
   else
   {
-    url = m_tab->GetDocumentUrl();
+    url = m_data->tab->GetDocumentUrl();
   }
 
   return url;
@@ -219,7 +218,7 @@ STDMETHODIMP CPluginClass::SetSite(IUnknown* unknownSite)
 
     s_criticalSectionBrowser.Lock();
     {
-      m_webBrowser2 = unknownSite;
+      m_data->webBrowser2 = ATL::CComQIPtr<IWebBrowser2>(unknownSite);
     }
     s_criticalSectionBrowser.Unlock();
 
@@ -276,10 +275,10 @@ STDMETHODIMP CPluginClass::SetSite(IUnknown* unknownSite)
           {
             s_criticalSectionBrowser.Lock();
             {
-              HRESULT hr = pServiceProvider->QueryService(IID_IWebBrowserApp, &m_webBrowser2);
+              HRESULT hr = pServiceProvider->QueryService(IID_IWebBrowserApp, &m_data->webBrowser2);
               if (SUCCEEDED(hr))
               {
-                if (m_webBrowser2)
+                if (m_data->webBrowser2)
                 {
                   InitObject(false);
                 }
@@ -349,7 +348,7 @@ STDMETHODIMP CPluginClass::SetSite(IUnknown* unknownSite)
     // Release browser interface
     s_criticalSectionBrowser.Lock();
     {
-      m_webBrowser2.Release();
+      m_data->webBrowser2.Release();
     }
     s_criticalSectionBrowser.Unlock();
 
@@ -496,6 +495,41 @@ void STDMETHODCALLTYPE CPluginClass::OnBeforeNavigate2(
   std::wstring url(V_BSTR(urlVariant), SysStringLen(V_BSTR(urlVariant)));
   UnescapeUrl(url);
 
+  {

[Oleksandr, 2015/02/04 20:59:57]

I would prefer for this to be a separate function instead. Like
RegisterWebBrowserEventsListener(Data* data). In fact this could be a function
in CPluginClientFactory.

[sergei, 2015/04/13 08:06:58]

I've extracted it into CPluginClass::EnsureWebBrowserConnected

[sergei, 2015/04/13 08:06:58]

I guess here and below instead of CPluginClientFactory CPluginClass is meant.

+    auto it = m_data->connectedWebBrowsersCache.find(webBrowser);
+    if (m_data->connectedWebBrowsersCache.end() == it)

[Oleksandr, 2015/02/04 20:59:57]

This is Yoda condition, IMO

[sergei, 2015/04/13 08:06:58]

fixed

+    {
+      ATL::CComObject<WebBrowserEventsListener>* listenerImpl = nullptr;
+      if (FAILED(ATL::CComObject<WebBrowserEventsListener>::CreateInstance(&listenerImpl)))
+      {
+        return;
+      }
+      ATL::CComPtr<IUnknown> listnerRefCounterGuard(listenerImpl->GetUnknown());

[Oleksandr, 2015/02/04 20:59:57]

missing 'e'? listenerRefCounterGuard

[sergei, 2015/04/13 08:06:58]

fixed

+      std::weak_ptr<Data> dataForCapturing = m_data;
+      auto onListenerDestroy = [webBrowser, dataForCapturing]

[Oleksandr, 2015/02/04 20:59:57]

If this whole block is moved to CPluginClientFactory, I would extract
onListenerDestroy and reloaded to separate functions too

[sergei, 2015/04/13 08:06:58]

I think it will complicate the code because it is more code and anonymous
functions without any algorithmic complexity which are not relevant without the
current function only confuse. However, they could be methods of Data as well as
some other methods of CPluginClass, but let's leave it for the refactoring when
we decide that we need it.

+      {
+        if (auto data = dataForCapturing.lock())
+        {
+          data->connectedWebBrowsersCache.erase(webBrowser);
+        }
+      };
+      if (FAILED(listenerImpl->Init(webBrowser, onListenerDestroy)))
+      {
+        return;
+      }
+      listenerImpl->reloaded = [webBrowser, dataForCapturing]
+      {
+        if (auto data = dataForCapturing.lock())
+        {
+          auto frameSrc = GetLocationUrl(*webBrowser);
+          UnescapeUrl(frameSrc);
+          data->tab->OnDocumentComplete(webBrowser, ToCString(frameSrc), data->webBrowser2.IsEqualObject(webBrowser));
+        }
+      };
+      m_data->connectedWebBrowsersCache.emplace(webBrowser, listenerImpl);
+    }
+  }
+
   //Register a mime filter if it's not registered yet
   if (s_mimeFilter == NULL)
   {
@@ -512,7 +546,7 @@ void STDMETHODCALLTYPE CPluginClass::OnBeforeNavigate2(
   }
   else if (GetBrowser().IsEqualObject(webBrowser))
   {
-    m_tab->OnNavigate(urlCString);
+    m_data->tab->OnNavigate(urlCString);
 
     DEBUG_GENERAL(L"================================================================================\nBegin main navigation url:" + urlCString + "\n================================================================================")
 
@@ -525,7 +559,7 @@ void STDMETHODCALLTYPE CPluginClass::OnBeforeNavigate2(
   else
   {
     DEBUG_NAVI(L"Navi::Begin navigation url:" + urlCString)
-    m_tab->CacheFrame(urlCString);
+    m_data->tab->CacheFrame(urlCString);
   }
 }
 
@@ -540,27 +574,12 @@ void STDMETHODCALLTYPE CPluginClass::OnDownloadComplete()
   ATL::CComPtr<IWebBrowser2> browser = GetBrowser();
   if (browser)
   {
-    m_tab->OnDownloadComplete(browser);
+    m_data->tab->OnDownloadComplete(browser);
   }
 }
 
 void STDMETHODCALLTYPE CPluginClass::OnDocumentComplete(IDispatch* frameBrowserDisp, VARIANT* /*urlOrPidl*/)
 {
-  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);
 }
 
 void STDMETHODCALLTYPE CPluginClass::OnWindowStateChanged(unsigned long flags, unsigned long validFlagsMask)
@@ -963,7 +982,7 @@ CPluginClass* CPluginClass::FindInstance(HWND hStatusBarWnd)
 
 CPluginTab* CPluginClass::GetTab()
 {
-  return m_tab;
+  return m_data->tab.get();
 }
 
 CPluginTab* CPluginClass::GetTab(DWORD dwThreadId)
@@ -975,7 +994,7 @@ CPluginTab* CPluginClass::GetTab(DWORD dwThreadId)
     std::map<DWORD,CPluginClass*>::const_iterator it = s_threadInstances.find(dwThreadId);
     if (it != s_threadInstances.end())
     {
-      tab = it->second->m_tab;
+      tab = it->second->m_data->tab.get();
     }
   }
   s_criticalSectionLocal.Unlock();
@@ -1210,7 +1229,7 @@ STDMETHODIMP CPluginClass::Exec(const GUID*, DWORD nCmdID, DWORD, VARIANTARG*, V
   }
 
   // Create menu
-  HMENU hMenu = CreatePluginMenu(m_tab->GetDocumentUrl());
+  HMENU hMenu = CreatePluginMenu(m_data->tab->GetDocumentUrl());
   if (!hMenu)
   {
     return E_FAIL;