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)

[sergei, 2018/01/25 16:04:00]

Just a note: it should accept elemhide now

[hub, 2018/01/25 19:05:27]

It is already done. I just didn't update the review.

+ElemHideEmulation_FilterList* ElemHideEmulation::GetRulesForDomain(const ElemHide& elemHide, DependentString& domain)
 {
-  intrusive_ptr<ElemHideEmulation_FilterList> result(new ElemHideEmulation_FilterList);

[sergei, 2018/01/25 16:04:01]

could you please add () to the constructor call?

[hub, 2018/01/25 19:05:27]

Done.

-  for (auto filter: mFilters)

[sergei, 2018/01/25 16:04:01]

perhaps it would be better to call it filterEntry because it's not exactly a
filter.

[hub, 2018/01/25 19:05:27]

entry like elsewhere.

Done.

+  intrusive_ptr<ElemHideEmulation_FilterList> result(new ElemHideEmulation_FilterList());
+  for (const auto& entry: mFilters)
   {
     DependentString docDomain(domain);
-    if (!filter.is_deleted() &&
-        filter.second->IsActiveOnDomain(docDomain, u""_str) &&

[sergei, 2018/01/25 16:04:01]

what do you think about having Dereferenceable iterators? E.g. *filterEntry
could be equal to filterEntry.second. Of course it's beyond the scope of the
current issue.

[hub, 2018/01/25 19:05:27]

filterEntry isn't an iterator, but the entry in the hash map. I don't think it
brings anything but confusion to do that. It is not in line with std:: either.

-        !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.release();
 }