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

libadblockplus-android/jni/JniFilterEngine.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 <AdblockPlus.h>
 #include "Utils.h"
 #include "JniCallbacks.h"
 #include <thread>
-#include "JniJsEngine.h"
+#include "JniPlatform.h"
 
 static jobject SubscriptionsToArrayList(JNIEnv* env, std::vector<AdblockPlus::Subscription>&& subscriptions)
 {
   jobject list = NewJniArrayList(env);
 
   for (std::vector<AdblockPlus::Subscription>::iterator it = subscriptions.begin(), end = subscriptions.end(); it != end; it++)
   {
     JniAddObjectToList(env, list, NewJniSubscription(env, std::move(*it)));
   }
 
   return list;
 }
 
 static AdblockPlus::FilterEngine::ContentType ConvertContentType(JNIEnv *env,
     jobject jContentType)
 {
   JniLocalReference<jclass> contentTypeClass(env,
       env->GetObjectClass(jContentType));
   jmethodID nameMethod = env->GetMethodID(*contentTypeClass, "name",
       "()Ljava/lang/String;");
   JniLocalReference<jstring> jValue(env,
       (jstring) env->CallObjectMethod(jContentType, nameMethod));
   const std::string value = JniJavaToStdString(env, *jValue);
   return AdblockPlus::FilterEngine::StringToContentType(value);
 }
 
 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
   {
     AdblockPlus::FilterEngine& engine = GetFilterEngineRef(ptr);
 
     return engine.IsFirstRun() ? JNI_TRUE : JNI_FALSE;
   }
   CATCH_THROW_AND_RETURN(env, JNI_FALSE);
 }
 
@@ skipping 409 matching lines @@
     AdblockPlus::FilterEngine& engine = GetFilterEngineRef(ptr);
     std::string url = engine.GetAAUrl();
     return JniStdStringToJava(env, url);
   }
   CATCH_THROW_AND_RETURN(env, 0)
 }
 
 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()))
       {
         subscription.UpdateFilters();
         return;
       }
     }
   });
 }
 
 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 },
   { (char*)"getSubscription", (char*)"(JLjava/lang/String;)" TYP("Subscription"), (void*)JniGetSubscription },
   { (char*)"showNextNotification", (char*)"(JLjava/lang/String;)V", (void*)JniShowNextNotification },
   { (char*)"setShowNotificationCallback", (char*)"(JJ)V", (void*)JniSetShowNotificationCallback },
   { (char*)"removeShowNotificationCallback", (char*)"(J)V", (void*)JniRemoveShowNotificationCallback },
   { (char*)"getListedSubscriptions", (char*)"(J)Ljava/util/List;", (void*)JniGetListedSubscriptions },
   { (char*)"fetchAvailableSubscriptions", (char*)"(J)Ljava/util/List;", (void*)JniFetchAvailableSubscriptions },
   { (char*)"setUpdateAvailableCallback", (char*)"(JJ)V", (void*)JniSetUpdateAvailableCallback },
   { (char*)"removeUpdateAvailableCallback", (char*)"(J)V", (void*)JniRemoveUpdateAvailableCallback },
   { (char*)"setFilterChangeCallback", (char*)"(JJ)V", (void*)JniSetFilterChangeCallback },
   { (char*)"removeFilterChangeCallback", (char*)"(J)V", (void*)JniRemoveFilterChangeCallback },
   { (char*)"forceUpdateCheck", (char*)"(JJ)V", (void*)JniForceUpdateCheck },
   { (char*)"getElementHidingSelectors", (char*)"(JLjava/lang/String;)Ljava/util/List;", (void*)JniGetElementHidingSelectors },
   { (char*)"matches", (char*)"(JLjava/lang/String;" TYP("FilterEngine$ContentType") "Ljava/lang/String;)" TYP("Filter"), (void*)JniMatches },
   { (char*)"matches", (char*)"(JLjava/lang/String;" TYP("FilterEngine$ContentType") "[Ljava/lang/String;)" TYP("Filter"), (void*)JniMatchesMany },
   { (char*)"isDocumentWhitelisted", (char*)"(JLjava/lang/String;[Ljava/lang/String;)Z", (void*)JniIsDocumentWhitelisted },
   { (char*)"isElemhideWhitelisted", (char*)"(JLjava/lang/String;[Ljava/lang/String;)Z", (void*)JniIsElemhideWhitelisted },
   { (char*)"getPref", (char*)"(JLjava/lang/String;)" TYP("JsValue"), (void*)JniGetPref },
   { (char*)"setPref", (char*)"(JLjava/lang/String;J)V", (void*)JniSetPref },
   { (char*)"getHostFromURL", (char*)"(JLjava/lang/String;)Ljava/lang/String;", (void*)JniGetHostFromURL },
   { (char*)"setAllowedConnectionType", (char*)"(JLjava/lang/String;)V", (void*)JniSetAllowedConnectionType },
   { (char*)"getAllowedConnectionType", (char*)"(J)Ljava/lang/String;", (void*)JniGetAllowedConnectionType },
   { (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 }
 };
 
 extern "C" JNIEXPORT void JNICALL Java_org_adblockplus_libadblockplus_FilterEngine_registerNatives(JNIEnv *env, jclass clazz)
 {
   env->RegisterNatives(clazz, methods, sizeof(methods) / sizeof(methods[0]));
 }