Issue #1234 - Remove CString from ShouldBlock(), IsElementHidden() declarations

src/plugin/PluginFilter.cpp

 #include "PluginStdAfx.h"
 
 #include "PluginFilter.h"
 
 #if (defined PRODUCT_ADBLOCKPLUS)
 #include "PluginSettings.h"
 #include "PluginClient.h"
 #include "PluginClientFactory.h"
 #endif
 
@@ skipping 486 matching lines @@
           std::pair<CString, CFilterElementHide> pair = std::make_pair(filter->m_tag, *filter);
           m_elementHideTags.insert(pair);
         }
       }
     } while (separatorChar != '\0');
   }
 
   return true;
 }
 
-bool CPluginFilter::IsElementHidden(const std::wstring& tag_wstring, IHTMLElement* pEl, const std::wstring& domain, const std::wstring& indent) const

[Felix Dahlke, 2014/09/30 13:30:15]

Since "CString tag" is temporary and "const std::wstring& tag" is not, we should
IMO give the CString tag a different name rather than change the parameter name.
Same below.

[Eric, 2014/09/30 17:56:40]

I was trying to minimize the number of changed lines, but what it really does is
to require changing all those lines at the end of the process rather than the
beginning.

Done.

-{
-  CString tag = to_CString(tag_wstring);
+bool CPluginFilter::IsElementHidden(const std::wstring& tag, IHTMLElement* pEl, const std::wstring& domain, const std::wstring& indent) const
+{
+  CString tagCString = to_CString(tag);
 
   CString id;
   CComBSTR bstrId;
   if (SUCCEEDED(pEl->get_id(&bstrId)) && bstrId)
   {
     id = bstrId;
   }
 
   CString classNames;
   CComBSTR bstrClassNames;
   if (SUCCEEDED(pEl->get_className(&bstrClassNames)) && bstrClassNames)
   {
     classNames = bstrClassNames;
   }
 
   CriticalSection::Lock filterEngineLock(s_criticalSectionFilterMap);    
   {
     CString domainTest = to_CString(domain);
 
     // Search tag/id filters
     if (!id.IsEmpty())
     {
       std::pair<TFilterElementHideTagsNamed::const_iterator, TFilterElementHideTagsNamed::const_iterator> idItEnum =
-        m_elementHideTagsId.equal_range(std::make_pair(tag, id));
+        m_elementHideTagsId.equal_range(std::make_pair(tagCString, id));
       for (TFilterElementHideTagsNamed::const_iterator idIt = idItEnum.first; idIt != idItEnum.second; idIt ++)
       {
         if (idIt->second.IsMatchFilterElementHide(pEl))
         {
 #ifdef ENABLE_DEBUG_RESULT
           DEBUG_HIDE_EL(indent + "HideEl::Found (tag/id) filter:" + idIt->second.m_filterText)
-            CPluginDebug::DebugResultHiding(tag, "id:" + id, idIt->second.m_filterText);
+            CPluginDebug::DebugResultHiding(tagCString, "id:" + id, idIt->second.m_filterText);
 #endif
           return true;
         }
       }
 
       // Search general id
       idItEnum = m_elementHideTagsId.equal_range(std::make_pair("", id));
       for (TFilterElementHideTagsNamed::const_iterator idIt = idItEnum.first; idIt != idItEnum.second; idIt ++)
       {
         if (idIt->second.IsMatchFilterElementHide(pEl))
         {
 #ifdef ENABLE_DEBUG_RESULT
           DEBUG_HIDE_EL(indent + "HideEl::Found (?/id) filter:" + idIt->second.m_filterText)
-            CPluginDebug::DebugResultHiding(tag, "id:" + id, idIt->second.m_filterText);
+            CPluginDebug::DebugResultHiding(tagCString, "id:" + id, idIt->second.m_filterText);
 #endif
           return true;
         }
       }
     }
 
     // Search tag/className filters
     if (!classNames.IsEmpty())
     {
       int pos = 0;
       CString className = classNames.Tokenize(L" \t\n\r", pos);
       while (pos >= 0)
       {
         std::pair<TFilterElementHideTagsNamed::const_iterator, TFilterElementHideTagsNamed::const_iterator> classItEnum = 
-          m_elementHideTagsClass.equal_range(std::make_pair(tag, className));
+          m_elementHideTagsClass.equal_range(std::make_pair(tagCString, className));
 
         for (TFilterElementHideTagsNamed::const_iterator classIt = classItEnum.first; classIt != classItEnum.second; ++classIt)
         {
           if (classIt->second.IsMatchFilterElementHide(pEl))
           {
 #ifdef ENABLE_DEBUG_RESULT
             DEBUG_HIDE_EL(indent + "HideEl::Found (tag/class) filter:" + classIt->second.m_filterText)
-              CPluginDebug::DebugResultHiding(tag, "class:" + className, classIt->second.m_filterText);
+              CPluginDebug::DebugResultHiding(tagCString, "class:" + className, classIt->second.m_filterText);
 #endif
             return true;
           }
         }
 
         // Search general class name
         classItEnum = m_elementHideTagsClass.equal_range(std::make_pair("", className));
         for (TFilterElementHideTagsNamed::const_iterator classIt = classItEnum.first; classIt != classItEnum.second; ++ classIt)
         {
           if (classIt->second.IsMatchFilterElementHide(pEl))
           {
 #ifdef ENABLE_DEBUG_RESULT
             DEBUG_HIDE_EL(indent + "HideEl::Found (?/class) filter:" + classIt->second.m_filterText)
-              CPluginDebug::DebugResultHiding(tag, "class:" + className, classIt->second.m_filterText);
+              CPluginDebug::DebugResultHiding(tagCString, "class:" + className, classIt->second.m_filterText);
 #endif
             return true;
           }
         }
 
         // Next class name
         className = classNames.Tokenize(L" \t\n\r", pos);
       }
     }
 
     // Search tag filters
     std::pair<TFilterElementHideTags::const_iterator, TFilterElementHideTags::const_iterator> tagItEnum 
-      = m_elementHideTags.equal_range(tag);
+      = m_elementHideTags.equal_range(tagCString);
     for (TFilterElementHideTags::const_iterator tagIt = tagItEnum.first; tagIt != tagItEnum.second; ++ tagIt)
     {
       if (tagIt->second.IsMatchFilterElementHide(pEl))
       {
 #ifdef ENABLE_DEBUG_RESULT
         DEBUG_HIDE_EL(indent + "HideEl::Found (tag) filter:" + tagIt->second.m_filterText)
-          CPluginDebug::DebugResultHiding(tag, "-", tagIt->second.m_filterText);
+          CPluginDebug::DebugResultHiding(tagCString, "-", tagIt->second.m_filterText);
 #endif
         return true;
       }
     }
   }
 
   return false;
 }
 
 bool CPluginFilter::LoadHideFilters(std::vector<std::wstring> filters)
@@ skipping 52 matching lines @@
       }
       m_filterMapDefault[i].clear();
     }
 
     m_elementHideTags.clear();
     m_elementHideTagsId.clear();
     m_elementHideTagsClass.clear();
   }
 }
 
-bool CPluginFilter::ShouldBlock(const std::wstring& src_wstring, int contentType, const std::wstring& domain_wstring, bool addDebug) const
-{
-  CString src = to_CString(src_wstring);
-  CString domain = to_CString(domain_wstring);
+bool CPluginFilter::ShouldBlock(const std::wstring& src, int contentType, const std::wstring& domain, bool addDebug) const
+{
+  CString srcCString = to_CString(src);
 
   // We should not block the empty string, so all filtering does not make sense
   // Therefore we just return
-  if (src.Trim().IsEmpty())
+  if (srcCString.Trim().IsEmpty())
   {
     return false;
   }
 
   CPluginSettings* settings = CPluginSettings::GetInstance();
 
   CString type;
   if (addDebug)
   {
     type = "OTHER";
 
     std::map<int,CString>::const_iterator it = m_contentMapText.find(contentType);
     if (it != m_contentMapText.end())
     {
       type = it->second;
     }
   }
 
   CPluginClient* client = CPluginClient::GetInstance();
-  if (client->Matches(std::wstring(src), std::wstring(type), std::wstring(domain)))
+  if (client->Matches(to_wstring(srcCString), to_wstring(type), domain))
   {
     if (addDebug)
     {
       DEBUG_FILTER("Filter::ShouldBlock " + type + " YES")
 
 #ifdef ENABLE_DEBUG_RESULT
-        CPluginDebug::DebugResultBlocking(type, src, domain);
+        CPluginDebug::DebugResultBlocking(type, srcCString, to_CString(domain));
 #endif
     }
     return true;
   }
   return false;
 }