Issue 5141 - Convert filter match to C++

compiled/bindings/main.cpp

Index: compiled/bindings/main.cpp
===================================================================
--- a/compiled/bindings/main.cpp
+++ b/compiled/bindings/main.cpp
@@ -29,16 +29,17 @@
 #include "../filter/ElemHideBase.h"
 #include "../filter/ElemHideFilter.h"
 #include "../filter/ElemHideException.h"
 #include "../filter/ElemHideEmulationFilter.h"
 #include "../subscription/Subscription.h"
 #include "../subscription/DownloadableSubscription.h"
 #include "../subscription/UserDefinedSubscription.h"
 #include "../storage/FilterStorage.h"
+#include "../filter/Matcher.h"
 #include "../FilterNotifier.h"
 
 int main()
 {
   try
   {
     class_<Filter>("Filter")
         .property("text", &Filter::GetText)
@@ -126,16 +127,39 @@
         .property("downloadStatus", &DownloadableSubscription::GetDownloadStatus, &DownloadableSubscription::SetDownloadStatus)
         .property("lastSuccess", &DownloadableSubscription::GetLastSuccess, &DownloadableSubscription::SetLastSuccess)
         .property("errors", &DownloadableSubscription::GetErrorCount, &DownloadableSubscription::SetErrorCount)
         .property("version", &DownloadableSubscription::GetDataRevision, &DownloadableSubscription::SetDataRevision)
         .property("requiredVersion", &DownloadableSubscription::GetRequiredVersion, &DownloadableSubscription::SetRequiredVersion)
         .property("downloadCount", &DownloadableSubscription::GetDownloadCount, &DownloadableSubscription::SetDownloadCount)
         .function("serialize", &DownloadableSubscription::Serialize);
 
+    class_<ReMatchResults>("ReMatchResults")
+        .function("push", &ReMatchResults::push);

[Wladimir Palant, 2017/10/09 08:39:47]

Why do we need to export what is clearly an internal class with an internal
method?

[sergei, 2017/10/09 15:27:51]

This class is used as a callback to obtain the results from JS implementation of
regular expressions. I expect it to be removed after complete switching to C++.

[hub, 2017/10/10 13:46:15]

The sole purpose of this class is to be able to get the results from a regexp
match. See in library.js. So it needs to have push() exported into JS.

+
+    class_<CombinedMatcher>("CombinedMatcher")
+        .class_function("create", &CombinedMatcher::Create)
+        .function("add", &CombinedMatcher::Add)
+        .function("remove", &CombinedMatcher::Remove)
+        .function("matchesAny", &CombinedMatcher::MatchesAny)
+        .function("clear", &CombinedMatcher::Clear)
+        .function("hasFilter", &CombinedMatcher::HasFilter)
+        .function("getKeywordForFilter", &CombinedMatcher::GetKeywordForFilter)
+        .function("findKeyword", &CombinedMatcher::FindKeyword);

[Wladimir Palant, 2017/10/09 08:39:47]

The three methods above shouldn't be exported, these are implementation details
now that JS code doesn't need to call.

In fact, the distinction between CombinedMatcher and Matcher was an
implementation details that shouldn't be exposed any more (mentioned in the
issue description).

[sergei, 2017/10/09 15:27:52]

I think they are currently exported for the sake of tests
(https://codereview.adblockplus.org/29556737/diff/29559667/compiled/bindings/m...).
I have no objection regarding removing the tests using them and not exposing
these methods but for present it seems they can be very useful to identify a
regression.

[Wladimir Palant, 2017/10/10 07:39:05]

Yes, that's probably it. We should remove/rewrite tests relying on internals
however.

[hub, 2017/10/10 13:46:15]

Note that currently these are test that exist in master as well.

Also notification.js currently explicitely instanciate a Matcher in addition to
using the defaultMatcher.

+
+    class_<Matcher>("Matcher")
+        .class_function("create", &Matcher::Create)
+        .function("add", &Matcher::Add)
+        .function("remove", &Matcher::Remove)
+        .function("matchesAny", &Matcher::MatchesAny)
+        .function("clear", &Matcher::Clear)
+        .function("hasFilter", &Matcher::HasFilter)
+        .function("getKeywordForFilter", &Matcher::GetKeywordForFilter)
+        .function("findKeyword", &Matcher::FindKeyword);
+
     singleton<FilterStorage>("FilterStorage", &FilterStorage::GetInstance)
         .property("subscriptionCount", &FilterStorage::GetSubscriptionCount)
         .function("subscriptionAt", &FilterStorage::SubscriptionAt)
         .function("indexOfSubscription", &FilterStorage::IndexOfSubscription)
         .function("getSubscriptionForFilter", &FilterStorage::GetSubscriptionForFilter)
         .function("addSubscription", &FilterStorage::AddSubscription)
         .function("removeSubscription", &FilterStorage::RemoveSubscription)
         .function("moveSubscription", &FilterStorage::MoveSubscription);