Issue 5556 - Update to use libadblockplus revision hg:566f64c8a2a8

libadblockplus-android/jni/JniFilterEngine.cpp

Index: libadblockplus-android/jni/JniFilterEngine.cpp
diff --git a/libadblockplus-android/jni/JniFilterEngine.cpp b/libadblockplus-android/jni/JniFilterEngine.cpp
index 3f6c0d4870391cabf213397131cf11c53a34153c..6d6d582c0f38e5162db51431710286084895e90f 100644
--- a/libadblockplus-android/jni/JniFilterEngine.cpp
+++ b/libadblockplus-android/jni/JniFilterEngine.cpp
@@ -19,7 +19,7 @@
 #include "Utils.h"
 #include "JniCallbacks.h"
 #include <thread>
-#include "JniJsEngine.h"
+#include "JniPlatform.h"
 
 static jobject SubscriptionsToArrayList(JNIEnv* env, std::vector<AdblockPlus::Subscription>&& subscriptions)
 {
@@ -48,63 +48,12 @@ static AdblockPlus::FilterEngine::ContentType ConvertContentType(JNIEnv *env,
 
 namespace
 {
-  struct JniFilterEngine
-  {
-    AdblockPlus::ITimer* timer;
-    AdblockPlus::FilterEnginePtr filterEngine;
-  };
-
   AdblockPlus::FilterEngine& GetFilterEngineRef(jlong ptr)

[anton, 2017/09/06 06:21:27]

rename to `platformPtr`

[sergei, 2017/09/08 09:45:01]

renamed to jniPlatformPtr

   {
-    return *JniLongToTypePtr<JniFilterEngine>(ptr)->filterEngine;
+    return JniLongToTypePtr<JniPlatform>(ptr)->platform->GetFilterEngine();
   }
 }
 
-static jlong JNICALL JniCtor(JNIEnv* env, jclass clazz, jlong jniJsEnginePtr, jobject jIsSubscriptionDownloadAllowedCallback)
-{
-  try
-  {
-    auto jniJsEngine = JniLongToTypePtr<JniJsEngine>(jniJsEnginePtr);
-    auto jsEngine = jniJsEngine->jsEngine;
-    auto jniFilterEngine = new JniFilterEngine();
-    jniFilterEngine->timer = jniJsEngine->timer;
-
-    if (jIsSubscriptionDownloadAllowedCallback)
-    {
-      AdblockPlus::FilterEngine::CreationParameters creationParameters;
-      auto callback = std::make_shared<JniIsAllowedConnectionTypeCallback>(env, jIsSubscriptionDownloadAllowedCallback);
-
-      creationParameters.isSubscriptionDownloadAllowedCallback =
-        [callback](const std::string* allowedConnectionTypeArg, const std::function<void(bool)>& doneCallback)
-      {
-        std::shared_ptr<std::string> allowedConnectionType;
-        if (allowedConnectionTypeArg)
-        {
-          allowedConnectionType = std::make_shared<std::string>(*allowedConnectionTypeArg);
-        }
-        std::thread([callback, allowedConnectionType, doneCallback]
-        {
-          doneCallback(callback->Callback(allowedConnectionType.get()));
-        }).detach();
-      };
-
-      jniFilterEngine->filterEngine = AdblockPlus::FilterEngine::Create(jsEngine, creationParameters);
-    }
-    else
-    {
-      jniFilterEngine->filterEngine = AdblockPlus::FilterEngine::Create(jsEngine);
-    }
-
-    return JniPtrToLong(jniFilterEngine);
-  }
-  CATCH_THROW_AND_RETURN(env, 0)
-}
-
-static void JNICALL JniDtor(JNIEnv* env, jclass clazz, jlong ptr)
-{
-  delete JniLongToTypePtr<JniFilterEngine>(ptr);
-}
-
 static jboolean JNICALL JniIsFirstRun(JNIEnv* env, jclass clazz, jlong ptr)

[anton, 2017/09/06 06:21:27]

i'd suggest to rename all `ptr` to `platformPtr` in all methods

[sergei, 2017/09/08 09:45:01]

I'm not sure that there is a real necessity in it because here `ptr` plays a
role of an opaque pointer and since we are not working with it directly only
through GetFilterEngineRef I would propose to leave `ptr` argument in these
methods, but rename it only where its nature is actually used, namely in
GetFilterEngineRef and in JniUpdateFiltersAsync. What do you think about it?
BTW, just below we even use generic `engine` however we keep in mind the actual
kind of the engine.

[anton, 2017/09/08 10:19:18]

i'd prefer it to be renamed as previously it was c++ filter engine ref actually
and now it's platform actually (JniPlatform instance to be more detailed). This
could introduce misunderstanding.

[sergei, 2017/09/08 12:25:30]

I would like to wait for the third opinion here. I think that the only meaning
`ptr` is  carrying here is a native pointer held by Java class and it should not
show in each method what is actually behind it. Why should it generate a lot of
changes if we change the nature of this pointer if it's used as an opaque
integer value?

[diegocarloslima, 2017/09/08 20:42:41]

As I could see, the only place where the ptr had a specific name was in the
JniCtor. Also, for every other function, the ptr type is implicit. I would
suggest leaving 'ptr' in this code review, and if you really think that this may
cause readability problems, we can change it everywhere (in order to be
consistent) in a follow up issue (or even add some comments/documentation). 

 {
   try
@@ -535,10 +484,10 @@ static jstring JNICALL JniGetAcceptableAdsSubscriptionURL(JNIEnv* env, jclass cl
 static void JNICALL JniUpdateFiltersAsync(JNIEnv* env, jclass clazz, jlong ptr, jstring jSubscriptionUrl)
 {
   std::string subscriptionUrl = JniJavaToStdString(env, jSubscriptionUrl);
-  auto jniFilterEngine = JniLongToTypePtr<JniFilterEngine>(ptr);
-  jniFilterEngine->timer->SetTimer(std::chrono::milliseconds(0), [jniFilterEngine, subscriptionUrl]
+  auto jniPlatform = JniLongToTypePtr<JniPlatform>(ptr);
+  jniPlatform->scheduler([jniPlatform, subscriptionUrl]
   {
-    auto& filterEngine = *jniFilterEngine->filterEngine;
+    auto& filterEngine = jniPlatform->platform->GetFilterEngine();
     for (auto& subscription : filterEngine.GetListedSubscriptions())
     {
       if (stringBeginsWith(subscriptionUrl, subscription.GetProperty("url").AsString()))
@@ -552,7 +501,6 @@ static void JNICALL JniUpdateFiltersAsync(JNIEnv* env, jclass clazz, jlong ptr,
 
 static JNINativeMethod methods[] =
 {
-  { (char*)"ctor", (char*)"(J" TYP("IsAllowedConnectionCallback") ")J", (void*)JniCtor },
   { (char*)"isFirstRun", (char*)"(J)Z", (void*)JniIsFirstRun },
   { (char*)"getFilter", (char*)"(JLjava/lang/String;)" TYP("Filter"), (void*)JniGetFilter },
   { (char*)"getListedFilters", (char*)"(J)Ljava/util/List;", (void*)JniGetListedFilters },
@@ -580,7 +528,6 @@ static JNINativeMethod methods[] =
   { (char*)"setAcceptableAdsEnabled", (char*)"(JZ)V", (void*)JniSetAcceptableAdsEnabled },
   { (char*)"isAcceptableAdsEnabled", (char*)"(J)Z", (void*)JniIsAcceptableAdsEnabled },
   { (char*)"getAcceptableAdsSubscriptionURL", (char*)"(J)Ljava/lang/String;", (void*)JniGetAcceptableAdsSubscriptionURL },
-  { (char*)"dtor", (char*)"(J)V", (void*)JniDtor },
   { (char*)"updateFiltersAsync", (char*)"(JLjava/lang/String;)V", (void*)JniUpdateFiltersAsync }
 };