Issue 1793 - check whether the frame is whitelisted before injecting CSS

src/plugin/PluginTabBase.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 "PluginClient.h"
+#include "AdblockPlusClient.h"
+#include "PluginClientBase.h"
 #include "PluginSettings.h"
-#include "PluginTab.h"
 #include "AdblockPlusDomTraverser.h"
-#include "PluginClass.h"
 #include "PluginTabBase.h"
-#include "PluginUtil.h"
-#include "../shared/IE_version.h"
+#include "IeVersion.h"
 #include "../shared/Utils.h"
-#include <dispex.h>
 #include <Mshtmhst.h>
-
-int CPluginTabBase::s_dictionaryVersion = 0;
-int CPluginTabBase::s_settingsVersion = 1;
-int CPluginTabBase::s_filterVersion = 0;
-int CPluginTabBase::s_whitelistVersion = 0;
 
 CPluginTabBase::CPluginTabBase(CPluginClass* plugin)
   : m_plugin(plugin)
   , m_isActivated(false)
   , m_continueThreadRunning(true)
 {
   m_filter = std::auto_ptr<CPluginFilter>(new CPluginFilter());
   m_filter->hideFiltersLoadedEvent = CreateEvent(NULL, true, false, NULL);
 
   CPluginClient* client = CPluginClient::GetInstance();
   if (AdblockPlus::IE::InstalledMajorVersion() < 10)
   {
     m_isActivated = true;
   }
 
   try
   {
     m_thread = std::thread(&CPluginTabBase::ThreadProc, this);
   }
   catch (const std::system_error& ex)
   {
-    auto errDescription = std::string("Tab::Thread - Failed to create tab thread") +
-                ex.code().message() + ex.what();
-    DEBUG_ERROR_LOG(ex.code().value(), PLUGIN_ERROR_THREAD, PLUGIN_ERROR_TAB_THREAD_CREATE_PROCESS, errDescription.c_str());
+    DEBUG_SYSTEM_EXCEPTION(ex, PLUGIN_ERROR_THREAD, PLUGIN_ERROR_TAB_THREAD_CREATE_PROCESS,
+      "Tab::Thread - Failed to create tab thread");
   }
   m_traverser = new CPluginDomTraverser(static_cast<CPluginTab*>(this));
 }
 
 
 CPluginTabBase::~CPluginTabBase()
 {
   delete m_traverser;
   m_traverser = NULL;
   m_continueThreadRunning = false;
   if (m_thread.joinable()) {
     m_thread.join();
   }
 }
 
 void CPluginTabBase::OnActivate()
 {
   m_isActivated = true;
 }
 
 
 void CPluginTabBase::OnUpdate()
 {
   m_isActivated = true;
 }
 
 namespace
 {
+  // Entry Point
   void FilterLoader(CPluginTabBase* tabBase)
   {
-    tabBase->m_filter->LoadHideFilters(CPluginClient::GetInstance()->GetElementHidingSelectors(tabBase->GetDocumentDomain()));
-    SetEvent(tabBase->m_filter->hideFiltersLoadedEvent);
-  }
-}
-
-void CPluginTabBase::OnNavigate(const CString& url)
+    try
+    {
+      tabBase->m_filter->LoadHideFilters(CPluginClient::GetInstance()->GetElementHidingSelectors(tabBase->GetDocumentDomain()));
+      SetEvent(tabBase->m_filter->hideFiltersLoadedEvent);
+    }
+    catch (...)
+    {
+      // As a thread-main function, we truncate any C++ exception.
+    }
+  }
+}
+
+void CPluginTabBase::OnNavigate(const std::wstring& url)
 {
   SetDocumentUrl(url);
   ClearFrameCache(GetDocumentDomain());
   std::wstring domainString = GetDocumentDomain();
   ResetEvent(m_filter->hideFiltersLoadedEvent);
   try
   {
     std::thread filterLoaderThread(&FilterLoader, this);
     filterLoaderThread.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 start filter loader 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 start filter loader thread");
   }
   m_traverser->ClearCache();
 }
 
+namespace
+{
+  /**
+   * Determine if the HTML file is one of ours.
+   * The criterion is that it appear in the "html/templates" folder within our installation.
+   *
+   * Warning: This function may fail if the argument is not a "file://" URL.
+   * This is occasionally the case in circumstances yet to be characterized.
+   */
+  bool IsOurHtmlFile(const std::wstring& url)
+  {
+    // Declared static because the value is derived from an installation directory, which won't change during run-time.
+    static auto dir = FileUrl(HtmlFolderPath());
+
+    DEBUG_GENERAL([&]() -> std::wstring {
+      std::wstring log = L"InjectABP. Current URL: ";
+      log += url;
+      log += L", template directory URL: ";
+      log += dir;
+      return log;
+    }());
+
+    /*
+     * The length check here is defensive, in case the document URL is truncated for some reason.
+     */
+    if (url.length() < 5)
+    {
+      // We can't match ".html" at the end of the URL if it's too short.
+      return false;
+    }
+    auto urlCstr = url.c_str();
+    // Check the prefix to match our directory
+    // Check the suffix to be an HTML file
+    return (_wcsnicmp(urlCstr, dir.c_str(), dir.length()) == 0) &&
+      (_wcsnicmp(urlCstr + url.length() - 5, L".html", 5) == 0);
+  }
+}
+
 void CPluginTabBase::InjectABP(IWebBrowser2* browser)
 {
   CriticalSection::Lock lock(m_csInject);
-  CString url = GetDocumentUrl();
-  CString log;
-  log.Format(L"InjectABP. Current URL: %s, settings URL: %s", url, UserSettingsFileUrl().c_str());
-  DEBUG_GENERAL(log);
-  if (!(0 == url.CompareNoCase(CString(UserSettingsFileUrl().c_str())) ||
-      0 == url.CompareNoCase(CString(FirstRunPageFileUrl().c_str()))))
-  {
-    DEBUG_GENERAL(L"Not injecting");
+  auto url = GetDocumentUrl();
+  if (!IsOurHtmlFile(url))
+  {
+    DEBUG_GENERAL(L"InjectABP. Not injecting");
     return;
   }
-  DEBUG_GENERAL(L"Going to inject");
+  DEBUG_GENERAL(L"InjectABP. Injecting");
   CComPtr<IDispatch> pDocDispatch;
   browser->get_Document(&pDocDispatch);
   CComQIPtr<IHTMLDocument2> pDoc2 = pDocDispatch;
   if (!pDoc2)
   {
     DEBUG_ERROR_LOG(0, PLUGIN_ERROR_CREATE_SETTINGS_JAVASCRIPT, PLUGIN_ERROR_CREATE_SETTINGS_JAVASCRIPT_INVOKE, "CPluginTabBase::InjectABP - Failed to QI document");
     return;
   }
-
   CComPtr<IHTMLWindow2> pWnd2;
   pDoc2->get_parentWindow(&pWnd2);
   if (!pWnd2)
   {
     DEBUG_ERROR_LOG(0, PLUGIN_ERROR_CREATE_SETTINGS_JAVASCRIPT, PLUGIN_ERROR_CREATE_SETTINGS_JAVASCRIPT_INVOKE, "CPluginTabBase::InjectABP - Failed to get parent window");
     return;
   }
   CComQIPtr<IDispatchEx> pWndEx = pWnd2;
   if (!pWndEx)
   {
@@ skipping 21 matching lines @@
   hr = pWndEx->Invoke(dispid, IID_NULL, LOCALE_USER_DEFAULT, DISPATCH_PROPERTYPUT | DISPATCH_PROPERTYPUTREF, &params, 0, 0, 0);
   DEBUG_GENERAL("Invoke");
   if (FAILED(hr))
   {
     DEBUG_ERROR_LOG(hr, PLUGIN_ERROR_CREATE_SETTINGS_JAVASCRIPT, PLUGIN_ERROR_CREATE_SETTINGS_JAVASCRIPT_INVOKE, "CPluginTabBase::InjectABP - Failed to create Settings in JavaScript");
   }
 }
 
 namespace
 {
-  void InjectABPCSS(IHTMLDocument2& htmlDocument2, const std::vector<std::wstring>& hideFilters)

[Eric, 2015/01/13 19:52:52]

We're not distinguishing between fatal errors and simple non-support here. If IE
9 is required, then simply check for it at the start and be done with it. Then
if the QI calls fail (for example, when converting from IHTMLStyleSheet to
IHTMLStyleSheet4) it can be properly designated as a fatal error by throwing an
exception.

I don't see anything that should fail here, assuming version prerequisites are
met. Thus the 'void' return type is appropriate. What's not correct, though, is
for failures to return rather than to throw. Alternatively, and it's not one I
like, is to declare the function 'bool' and return success/failure. Since this
is brand new code, there's no good reason not to do it right and move forward
(void/throw) rather than remain mired in the past (bool/return).

Overall, this function would be far easier to read with two modifications. (1)
Declare all the [out] argument variables immediately before use. (2) Add a blank
line separating each individual operation and its error handing.

[Oleksandr, 2015/03/13 10:00:27]

On 2015/01/13 19:52:52, Eric wrote:
What's not correct, though, is
I guess this code made into this codereview by accident, since InjectABPCSS is
not actually used anywhere. But still I disagree with the void/throw. In this
case in an unlikely scenario of anything going wrong it's better to just ignore
the error and keep doing what's next in the code rather than dropping everything
and unwinding the stack. This method should not throw, IMO.

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

I agree this function should not be in this code review.

I also think that throwing would hurt more than help here, so far let's discuss
it in http://codereview.adblockplus.org/6567422169448448/.

-  {
-    ATL::CComPtr<IHTMLElement> stylePureHtmlElement;
-    ATL::CComQIPtr<IHTMLStyleElement> styleHtmlElement;
-    if (FAILED(htmlDocument2.createElement(ATL::CComBSTR(L"style"), &stylePureHtmlElement)))
-    {
-      DEBUG_GENERAL(L"Cannot create style element");
-      return;
-    }
-    if (!(styleHtmlElement = stylePureHtmlElement))
-    {
-      DEBUG_GENERAL(L"Cannot obtain IHTMLStyleElement from IHTMLElement");
-      return;
-    }
-    if (FAILED(styleHtmlElement->put_type(ATL::CComBSTR("text/css"))))
-    {
-      DEBUG_GENERAL(L"Cannot set type text/css");
-      return;
-    }
-    ATL::CComQIPtr<IHTMLStyleSheet> styleSheet;
-    // IHTMLStyleElement2 is availabe starting from IE9, Vista
-    ATL::CComQIPtr<IHTMLStyleElement2> styleHtmlElement2 = styleHtmlElement;
-    if (!styleHtmlElement2)
-    {
-      DEBUG_GENERAL(L"Cannot obtain IHTMLStyleElement2 from IHTMLStyleElement");
-      return;
-    }
-    if (FAILED(styleHtmlElement2->get_sheet(&styleSheet)) || !styleSheet)
-    {
-      DEBUG_GENERAL(L"Cannot obtain IHTMLStyleSheet");
-      return;
-    }
-    // IHTMLStyleSheet4 is availabe starting from IE9, Vista
-    ATL::CComQIPtr<IHTMLStyleSheet4> styleSheet4 = styleSheet;
-    if (!styleSheet4)
-    {
-      DEBUG_GENERAL(L"Cannot obtain IHTMLStyleSheet4");
-      return;
-    }
-    long newIndex = 0;
-    std::wstring cssValue = L"{ display: none !important; }";
-    for (const auto& selector : hideFilters)
-    {
-      auto cssRule = selector + cssValue;
-      ATL::CComBSTR selector(cssRule.size(), cssRule.c_str());
-      if (SUCCEEDED(styleSheet4->insertRule(selector, newIndex, &newIndex)))
-      {
-        ++newIndex;
-      }
-      else
-      {
-        DEBUG_GENERAL(ToCString(L"Cannot add rule for selector " + cssRule));
-      }
-    }
-
-    ATL::CComQIPtr<IHTMLDOMNode> styleNode = stylePureHtmlElement;
-    if (!styleNode)
-    {
-      DEBUG_GENERAL(L"Cannot obtain IHTMLDOMNode from stylePureHtmlElement");
-      return;
-    }
-    ATL::CComPtr<IHTMLElement> headHtmlElement;
-    // IHTMLDocument7 is availabe starting from IE9, Vista
-    ATL::CComQIPtr<IHTMLDocument7> htmlDocument7 = &htmlDocument2;
-    if (!htmlDocument7)
-    {
-      DEBUG_GENERAL(L"Cannot obtain IHTMLDocument7 from htmlDocument2");
-      return;
-    }
-    if (FAILED(htmlDocument7->get_head(&headHtmlElement)))
-    {
-      DEBUG_GENERAL(L"Cannot obtain head from pDoc7");
-      return;
-    }
-    ATL::CComQIPtr<IHTMLDOMNode> headNode = headHtmlElement;
-    if (!headNode)
-    {
-      DEBUG_GENERAL(L"Cannot obtain headNode from headHtmlElement");
-      return;
-    }
-    if (FAILED(headNode->appendChild(styleNode, nullptr)))
-    {
-      DEBUG_GENERAL(L"Cannot append blocking style");
-    }
+  ATL::CComPtr<IWebBrowser2> GetParent(IWebBrowser2& browser)
+  {
+    ATL::CComPtr<IDispatch> parentDispatch;
+    if (FAILED(browser.get_Parent(&parentDispatch)) || !parentDispatch)
+    {
+      return nullptr;
+    }
+    // The InternetExplorer application always returns a pointer to itself.
+    // https://msdn.microsoft.com/en-us/library/aa752136(v=vs.85).aspx
+    if (parentDispatch.IsEqualObject(&browser))
+    {
+      return nullptr;
+    }
+    ATL::CComQIPtr<IServiceProvider> parentDocumentServiceProvider = parentDispatch;
+    if (!parentDocumentServiceProvider)
+    {
+      return nullptr;
+    }
+    ATL::CComPtr<IWebBrowser2> parentBrowser;
+    if (FAILED(parentDocumentServiceProvider->QueryService(SID_SWebBrowserApp, &parentBrowser)))
+    {
+      return nullptr;
+    }
+    return parentBrowser;
+  }
+
+  bool IsFrameWhiteListed(ATL::CComPtr<IWebBrowser2> frame)
+  {
+    if (!frame)
+    {
+      return false;
+    }
+    auto url = GetLocationUrl(*frame);
+    std::vector<std::string> frameHierarchy;
+    while(frame = GetParent(*frame))
+    {
+      frameHierarchy.push_back(ToUtf8String(GetLocationUrl(*frame)));
+    }
+    CPluginClient* client = CPluginClient::GetInstance();
+    return client->IsWhitelistedUrl(url, frameHierarchy)
+        || client->IsElemhideWhitelistedOnDomain(url, frameHierarchy);
   }
 }
 
 void CPluginTabBase::OnDownloadComplete(IWebBrowser2* browser)
 {
   CPluginClient* client = CPluginClient::GetInstance();
-  std::wstring url = to_wstring(GetDocumentUrl());
+  std::wstring url = GetDocumentUrl();
   if (!client->IsWhitelistedUrl(url) && !client->IsElemhideWhitelistedOnDomain(url))
   {
     m_traverser->TraverseDocument(browser, GetDocumentDomain(), GetDocumentUrl());
   }
   InjectABP(browser);
 }
 
-ATL::CComPtr<IWebBrowser2> GetParent(const ATL::CComPtr<IWebBrowser2>& browser)

[Eric, 2015/01/13 19:52:52]

Since you're not copying the CComPtr and incurring an Add()/Release() pair, you
can just pass this argument as "IWebBrowser2*".

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

fixed to use `IWebBrowser2&`.

-{
-  ATL::CComPtr<IWebBrowser2> retValue;
-  ATL::CComPtr<IDispatch> browserParentDispatch;
-  if (FAILED(browser->get_Parent(&browserParentDispatch)) || !browserParentDispatch)
-  {
-    return retValue;

[Eric, 2015/01/13 19:52:52]

"return nullptr" is much clearer here.

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

fixed

-  }
-  // The InternetExplorer application always returns a pointer to itself.
-  if (browserParentDispatch.IsEqualObject(browser))
-  {
-    return retValue;

[Eric, 2015/01/13 19:52:52]

As before "return nullptr"

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

fixed

-  }
-  ATL::CComQIPtr<IServiceProvider> parentDocumentServiceProvider = browserParentDispatch;
-  if (!parentDocumentServiceProvider)
-  {
-    return retValue;

[Eric, 2015/01/13 19:52:52]

return nullptr

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

fixed

-  }

[Eric, 2015/01/13 19:52:52]

This is the right place to declare 'retValue'.

As a rule, it's best to declare variables that receive [out] arguments
immediately before they're used, if only to ensure that operator& doesn't fail.

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

fixed

-  if (FAILED(parentDocumentServiceProvider->QueryService(IID_IWebBrowserApp, &retValue)) || !retValue)

[Oleksandr, 2015/03/13 10:00:27]

Why is this querying for IWebBrowserApp and not IWebBrowser2? Output expects
IWebBrowser2.

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

fixed

-  {
-    // error

[Eric, 2015/01/13 19:52:52]

You've got to do something here, since a failed COM call leaves an [out]
argument in an undefined state.

Best would be to throw an exception.

[Oleksandr, 2015/03/13 10:00:27]

return nullptr would be better IMO

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

I don't think we should expect undefined state of out-parameter, it will
complicate the things to unusable state, check the rules of COM world
https://msdn.microsoft.com/en-us/library/windows/desktop/ms686638(v=vs.85).aspx.

-  }
-  return retValue;
-}
-
-bool IsFrameWhiteListed(ATL::CComPtr<IWebBrowser2> frame)
-{
-  if (!frame)
-  {
-    return false;
-  }
-  CPluginClient* client = CPluginClient::GetInstance();
-  if (!client)
-  {
-    return false;

[Eric, 2015/01/13 19:52:52]

This is really a pretty serious error.

Personally, I'd prefer that GetInstance() throw in case of error (and thus never
return nullptr), but that's perhaps too drastic a change for the moment. At
least throw here.

-  }
-  auto url = GetLocationUrl(*frame);
-  std::vector<std::string> frameHierarchy;
-  for(frame = GetParent(frame); frame; frame = GetParent(frame))
-  {
-    frameHierarchy.push_back(ToUtf8String(GetLocationUrl(*frame)));
-  }
-  return client->IsWhitelistedUrl(url, frameHierarchy)
-      || client->IsElemhideWhitelistedOnDomain(url, frameHierarchy);
-}
-
-void CPluginTabBase::OnDocumentComplete(IWebBrowser2* browser, const CString& url, bool isDocumentBrowser)
-{
-  CString documentUrl = GetDocumentUrl();
+void CPluginTabBase::OnDocumentComplete(IWebBrowser2* browser, const std::wstring& url, bool isDocumentBrowser)
+{
+  std::wstring documentUrl = GetDocumentUrl();
 
   if (isDocumentBrowser)
   {
     if (url != documentUrl)
     {
       SetDocumentUrl(url);
     }
     InjectABP(browser);
   }
-  if (url.Left(6) != "res://")
+  CString urlLegacy = ToCString(url);
+  if (urlLegacy.Left(6) != "res://")
   {
     // Get document
     CComPtr<IDispatch> pDocDispatch;
     HRESULT hr = browser->get_Document(&pDocDispatch);
     if (FAILED(hr) || !pDocDispatch)
     {
       return;
     }
 
     CComQIPtr<IHTMLDocument2> pDoc = pDocDispatch;
@@ skipping 26 matching lines @@
 
   m_criticalSection.Lock();
   {
     domain = m_documentDomain;
   }
   m_criticalSection.Unlock();
 
   return domain;
 }
 
-void CPluginTabBase::SetDocumentUrl(const CString& url)
+void CPluginTabBase::SetDocumentUrl(const std::wstring& url)
 {
   m_criticalSection.Lock();
   {
     m_documentUrl = url;
-    m_documentDomain = CAdblockPlusClient::GetInstance()->GetHostFromUrl(to_wstring(url));
+    m_documentDomain = CAdblockPlusClient::GetInstance()->GetHostFromUrl(url);
   }
   m_criticalSection.Unlock();
 }
 
-CString CPluginTabBase::GetDocumentUrl()
-{
-  CString url;
+std::wstring CPluginTabBase::GetDocumentUrl()
+{
+  std::wstring url;
 
   m_criticalSection.Lock();
   {
     url = m_documentUrl;
   }
   m_criticalSection.Unlock();
 
   return url;
 }
 
 
 // ============================================================================
 // Frame caching
 // ============================================================================
-bool CPluginTabBase::IsFrameCached(const CString& url)
+bool CPluginTabBase::IsFrameCached(const std::wstring& url)
 {
   bool isFrame;
 
   m_criticalSectionCache.Lock();
   {
     isFrame = m_cacheFrames.find(url) != m_cacheFrames.end();
   }
   m_criticalSectionCache.Unlock();
 
   return isFrame;
 }
 
-void CPluginTabBase::CacheFrame(const CString& url)
+void CPluginTabBase::CacheFrame(const std::wstring& url)
 {
   m_criticalSectionCache.Lock();
   {
     m_cacheFrames.insert(url);
   }
   m_criticalSectionCache.Unlock();
 }
 
 void CPluginTabBase::ClearFrameCache(const std::wstring& domain)
 {
   m_criticalSectionCache.Lock();
   {
     if (domain.empty() || domain != m_cacheDomain)
     {
       m_cacheFrames.clear();
       m_cacheDomain = domain;
     }
   }
   m_criticalSectionCache.Unlock();
 }
 
 void CPluginTabBase::ThreadProc()
 {
   // Force loading/creation of settings
-  CPluginSettings* settings = CPluginSettings::GetInstance();
-
-  settings->SetWorkingThreadId();
-
-  CString threadInfo;
-  threadInfo.Format(L"%d.%d", ::GetCurrentProcessId(), ::GetCurrentThreadId());
-
-  CString debugText;
-
-  debugText += L"================================================================================";
-  debugText += L"\nTAB THREAD " + threadInfo;
-  debugText += L"\n================================================================================";
-
-  DEBUG_GENERAL(debugText)
+  CPluginSettings::GetInstance()->SetWorkingThreadId();
+
+  std::string message =
+    "================================================================================\n"
+    "TAB THREAD process=";
+  message += std::to_string(::GetCurrentProcessId());
+  message + " thread=";
+  message += std::to_string(::GetCurrentThreadId());
+  message +=
+    "\n"
+    "================================================================================";
+  DEBUG_GENERAL(message);
 
   // --------------------------------------------------------------------
   // Tab loop
   // --------------------------------------------------------------------
 
   DWORD loopCount = 0;
   DWORD tabLoopIteration = 1;
 
   while (this->m_continueThreadRunning)
   {
 #ifdef ENABLE_DEBUG_THREAD
     CStringA sTabLoopIteration;
     sTabLoopIteration.Format("%u", tabLoopIteration);
 
     DEBUG_THREAD("--------------------------------------------------------------------------------")
       DEBUG_THREAD("Loop iteration " + sTabLoopIteration);
     DEBUG_THREAD("--------------------------------------------------------------------------------")
 #endif
-      if (this->m_isActivated)
-      {
-        bool isChanged = false;
-
-        if (isChanged)
-        {
-          this->m_plugin->UpdateStatusBar();
-        }
-
-        this->m_isActivated = false;
-      }
+      this->m_isActivated = false;
 
       // --------------------------------------------------------------------
       // End loop
       // --------------------------------------------------------------------
 
       // Sleep loop
       while (this->m_continueThreadRunning && !this->m_isActivated && (++loopCount % (TIMER_THREAD_SLEEP_TAB_LOOP / 50)) != 0)
       {
         // Post async plugin error
         CPluginError pluginError;
-        if (CPluginClient::PopFirstPluginError(pluginError))
+        if (LogQueue::PopFirstPluginError(pluginError))
         {
-          CPluginClient::LogPluginError(pluginError.GetErrorCode(), pluginError.GetErrorId(), pluginError.GetErrorSubid(), pluginError.GetErrorDescription(), true, pluginError.GetProcessId(), pluginError.GetThreadId());
+          LogQueue::LogPluginError(pluginError.GetErrorCode(), pluginError.GetErrorId(), pluginError.GetErrorSubid(), pluginError.GetErrorDescription(), true, pluginError.GetProcessId(), pluginError.GetThreadId());
         }
 
         // Non-hanging sleep
         Sleep(50);
       }
 
       tabLoopIteration++;
   }
 }