Issue 5142 - Convert Element Hiding to C++

compiled/ElemHide.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"
+
+OwnedString ElemHide_SelectorList::SelectorAt(size_t idx) const
+{
+  return mSelectors[idx]->GetSelector();
+}
+
+const String& ElemHide_SelectorList::FilterKeyAt(size_t idx) const
+{
+  return mSelectors[idx]->GetText();
+}
+
+void ElemHide::Clear()
+{
+  mFilters.clear();
+  mExceptions.clear();
+  mFiltersByDomain.clear();
+  mKnownExceptions.clear();
+}
+
+ActiveFilter::DomainMap ElemHide::defaultDomains;

[sergei, 2018/01/16 16:43:59]

It should be just
namespace
{
  const ActiveFilter::DomainMap defaultDomains = {{ActiveFilter::DEFAULT_DOMAIN,
true}};
}

[hub, 2018/01/19 02:11:03]

Done.

[sergei, 2018/01/22 15:40:07]

Do we really need it not in the anonymous namespace?

[hub, 2018/01/22 16:59:15]

Indeed we don't.

Done

+
+void ElemHide::AddToFiltersByDomain(ElemHideBase& filter)
+{
+  auto domains = filter.GetDomains();
+  if (!domains)
+    domains = &defaultDomains;
+
+  DependentString text(filter.GetText());
+  for (auto& domain : *domains)

[sergei, 2018/01/16 16:43:59]

it should be `const auto&`.

[hub, 2018/01/19 02:11:04]

Done.

+  {
+    auto& filters = mFiltersByDomain[domain.first];
+    if (domain.second)
+      filters[text] = ElemHideBasePtr(&filter);
+    else
+    {
+      auto iter = filters.find(text);
+      if (iter != filters.end())
+        filters.erase(iter);
+    }
+  }
+}
+
+void ElemHide::Add(ElemHideBase& filter)
+{
+  // we must ensure we have the right class.
+  // This is an error, but we might get Invalid filters.
+  if (!filter.As<ElemHideBase>())
+    return;
+
+  DependentString text(filter.GetText());
+  if (filter.mType == Filter::Type::ELEMHIDEEXCEPTION)

[sergei, 2018/01/16 18:05:27]

what about
if (auto exceptionFilter = filter.As<ElemHideException>())
{
  use exceptionFilter here.
}
and below too.

Can it happen that Filter::Type::ELEMHIDEEXCEPTION is just a mask and a simple
comparison is not enough?

[hub, 2018/01/19 02:11:03]

Done.
Filter::Type::ELEMHIDEEXCEPTION is not a mask, but Filter::As<> should be
enough.

+  {
+    if (mKnownExceptions.find(text))
+      return;
+
+    auto selector = filter.GetSelector();
+    mExceptions[selector].emplace_back(filter.As<ElemHideException>());
+
+    // Selector is not longer unconditional
+    mUnconditionalSelectors.erase(text);
+    mUnconditionalSelectorsCache.reset();
+    mKnownExceptions.insert(text);
+  }
+  else
+  {
+    if (mFilters.find(text))
+      return;
+
+    mFilters[text] = &filter;
+    if (!((filter.GetDomains() && filter.GetDomains()->size()) ||
+          mExceptions.find(filter.GetSelector())))
+    {
+      // The new filter's selector is unconditionally applied to all domains
+      mUnconditionalSelectors.insert(text);
+      mUnconditionalSelectorsCache.reset();
+    }
+    else
+      AddToFiltersByDomain(filter);
+  }
+}
+
+void ElemHide::Remove(ElemHideBase& filter)
+{
+  DependentString text(filter.GetText());
+
+  if (filter.mType == Filter::Type::ELEMHIDEEXCEPTION)
+  {
+    // never seen the exception.
+    if (!mKnownExceptions.find(text))
+      return;
+
+    auto selector = filter.GetSelector();
+    auto& list = mExceptions[selector];
+    auto iter = std::find(
+      list.begin(), list.end(),
+      ElemHideExceptionPtr(filter.As<ElemHideException>()));
+    if (iter != list.end())
+      list.erase(iter);
+    mKnownExceptions.erase(text);
+  }
+  else
+  {
+    if (!mFilters.find(text))
+      return;
+
+    if (mUnconditionalSelectors.find(text))
+    {
+      mUnconditionalSelectors.erase(text);
+      mUnconditionalSelectorsCache.reset();
+    }
+    else
+    {
+      auto domains = filter.GetDomains();
+      for (auto domain : *domains)

[sergei, 2018/01/16 18:05:27]

Can domains be nullptr?

[hub, 2018/01/19 02:11:03]

Anything can happen. I check it elsewhere too.

Done

+      {
+        auto& list = mFiltersByDomain[domain.first];
+        list.erase(text);
+      }
+    }
+
+    mFilters.erase(text);
+  }
+}
+
+ElemHideException* ElemHide::GetException(const ElemHideBase& filter,
+                                          DependentString& docDomain) const
+{
+  auto exception = mExceptions.find(filter.GetSelector());
+  if (!exception)
+    return nullptr;
+
+  auto& list = exception->second;
+  for (auto iter = list.rbegin(); iter != list.rend(); iter++)
+  {
+    DependentString domain(docDomain);
+    if ((*iter)->IsActiveOnDomain(domain, DependentString()))
+    {
+      ElemHideExceptionPtr filter(*iter);
+      return filter.release();
+    }
+  }
+
+  return nullptr;
+}
+
+ElemHide_SelectorList* ElemHide::GetUnconditionalSelectors() const
+{
+  if (!mUnconditionalSelectorsCache)
+  {
+    mUnconditionalSelectorsCache =
+      intrusive_ptr<ElemHide_SelectorList>(new ElemHide_SelectorList(), false);
+    annotate_address(mUnconditionalSelectorsCache.get(), "ElemHide_SelectorList");
+    for (auto unconditional : mUnconditionalSelectors)
+    {
+      auto entry = mFilters.find(unconditional.first);
+      if (entry)
+        mUnconditionalSelectorsCache->push_back(entry->second);
+    }
+  }
+  return intrusive_ptr<ElemHide_SelectorList>(mUnconditionalSelectorsCache).release();
+}
+
+ElemHide_SelectorList* ElemHide::GetSelectorsForDomain(const String& domain,
+  Criteria criteria) const
+{
+  intrusive_ptr<ElemHide_SelectorList> selectors(new ElemHide_SelectorList());
+  annotate_address(selectors.get(), "ElemHide_SelectorList");
+
+  if (criteria < NO_UNCONDITIONAL)
+  {
+    auto selector = GetUnconditionalSelectors();

[sergei, 2018/01/16 16:43:59]

what about using of intrusive_ptr?

[hub, 2018/01/19 02:11:03]

Done.

+    selectors->append(*selector);
+    selector->ReleaseRef();
+  }
+
+  bool specificOnly = criteria >= SPECIFIC_ONLY;
+  StringSet seenFilters;
+  DependentString docDomain(domain);
+
+  DependentString currentDomain(domain);
+  while (true)
+  {
+    if (specificOnly && currentDomain.empty())
+      break;
+
+    auto filters = mFiltersByDomain.find(currentDomain);
+    if (filters)
+    {
+      for (auto& entry : filters->second)
+      {
+        if (entry.first.is_invalid() || entry.first.is_deleted())

[sergei, 2018/01/16 16:43:59]

I have added https://issues.adblockplus.org/ticket/6280 and
https://issues.adblockplus.org/ticket/6281

[hub, 2018/01/19 02:11:04]

Acknowledged.

+          continue;
+
+        if (seenFilters.find(entry.first))
+          continue;
+        seenFilters.insert(entry.first);
+
+        auto filter = entry.second;
+        if (filter && !GetException(*filter, docDomain))
+          selectors->push_back(filter);
+      }
+    }
+
+    if (currentDomain.empty())
+      break;
+
+    auto nextDot = currentDomain.find(u'.');
+    currentDomain = nextDot == String::npos ?
+      u""_str : DependentString(currentDomain, nextDot + 1);
+  }
+
+  return selectors.release();
+}