Issue 5143 - Convert ElemHideEmulation to C++

compiled/ElemHideEmulation.cpp

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-present 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 "ElemHide.h"
 #include "ElemHideEmulation.h"
 
-ElemHideEmulation* ElemHideEmulation::mInstance = new ElemHideEmulation();
-
 void ElemHideEmulation::Add(ElemHideBase& filter)
 {
   mFilters[filter.GetText()] = ElemHideBasePtr(&filter);
 }
 
 void ElemHideEmulation::Remove(ElemHideBase& filter)
 {
   mFilters.erase(filter.GetText());
 }
 
 void ElemHideEmulation::Clear()
 {
   mFilters.clear();
 }
 
-_ElemHideEmulation_FilterList* ElemHideEmulation::GetRulesForDomain(DependentString& domain)
+ElemHideEmulation_FilterList* ElemHideEmulation::GetRulesForDomain(const ElemHide& elemHide, DependentString& domain)
 {
-  auto result = new _ElemHideEmulation_FilterList;

[sergei, 2017/11/16 09:58:06]

I would prefer to have the type of `result` intrusive_ptr.

[hub, 2017/11/20 19:16:00]

Done.

-  for (auto filter: mFilters)
+  intrusive_ptr<ElemHideEmulation_FilterList> result(new ElemHideEmulation_FilterList());
+  for (const auto& entry: mFilters)
   {
     DependentString docDomain(domain);

[hub, 2017/11/02 04:11:13]

IsActiveOnDomain() will modify the DependentString passed to it, which makes
everything else fail. It is subtle. And IMHO it would be better if we made
IsActiveOnDomain() use an immutable domain string - (albeit it calls lowerCase()
on it which modifies the string in place)

[sergei, 2017/11/16 09:58:06]

agree.

-    if (!filter.is_deleted() &&

[hub, 2017/11/02 04:11:13]

IMHO we should change StringMap<> iterator so that the deleted entries are
skipped.

[sergei, 2017/11/16 09:58:06]

agree

-        filter.second->IsActiveOnDomain(docDomain, u""_str) &&
-        !ElemHide::GetInstance()->GetException(*filter.second, domain))
-      result->push_back(filter.second);
+    if (!(entry.is_deleted() || entry.is_invalid()) &&
+        entry.second->IsActiveOnDomain(docDomain) &&
+        !elemHide.GetException(*entry.second, domain))
+      result->push_back(entry.second);
   }
 
-  return result;
+  return result.release();
 }