Noissue - Remove a dead inheritance and some dead code

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 "PluginSettings.h"
 #include "AdblockPlusDomTraverser.h"
 #include "PluginClass.h"
 #include "PluginTabBase.h"
 #include "PluginUtil.h"
 #include "../shared/IE_version.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)
@@ skipping 260 matching lines @@
   {
     if (domain.empty() || domain != m_cacheDomain)
     {
       m_cacheFrames.clear();
       m_cacheDomain = domain;
     }
   }
   m_criticalSectionCache.Unlock();
 }
 
+/*
+ * TODO: Replace the management of the log queue in this thread within CPluginTabBase
+ *       with a version of the log queue that uses a single internal worker thread.
+ */

[Eric, 2015/02/18 19:20:25]

I put this comment into the change set because the present changes (all
worthwhile in their own right) are also a prerequisite for accomplishing this
TODO, which has a number of benefits:

-- Change from one thread per tab to a single thread for the log queue.

-- Use notify/wait within the log queue rather than polling in multiple threads.

-- Eliminate CPluginTabBase member variables m_isActivate and
m_continueThreadRunning, whose only purpose is to manage the loop at the end of
this thread-main function. In turn this allows elimination of two entry points,
OnActivate and OnUpdate, whose only behavior is to refresh m_isActivate.

-- Further localize all the debug/log/trace/reporting capabilities.

I'm willing to replace this TODO comment with an issue in Trac, but if we do
that, we should have at least a bit of discussion beforehand to decide the scope
of the task.

[Oleksandr, 2015/02/27 10:59:27]

I'd vote for an issue instead of a TODO. Though I don't see any immediate need
in doing this at all. Even if we create an issue I'd vote for it to be P4 or
even P5.

[Eric, 2015/02/27 16:47:47]

https://issues.adblockplus.org/ticket/2058
I don't know of any really pressing need, but it would make finding the right
thread in the debugger easier.

It will also make combining CPluginTabBase and CPluginTab go easier, because
there will be less to combine.

 void CPluginTabBase::ThreadProc()
 {
   // Force loading/creation of settings
   CPluginSettings* settings = CPluginSettings::GetInstance();
 
   settings->SetWorkingThreadId();
 
   CString threadInfo;
   threadInfo.Format(L"%d.%d", ::GetCurrentProcessId(), ::GetCurrentThreadId());
 
@@ skipping 15 matching lines @@
   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;
-      }

[Eric, 2015/02/18 19:20:25]

I don't know where this code came from originally, but how does one not notice a
constant condition that you've just set on the previous line?

[Oleksandr, 2015/02/27 10:59:27]

Let's just say there was a lot of optimization happening here. Also, let's not
ever mention this, ok? Ever. :D

[Eric, 2015/02/27 16:47:47]

I promise I won't bookmark this URL and pull it out whenever I'm particularly
frustrated with code quality.

Really. I promise.

+      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++;
   }
 }