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
@@ skipping 30 matching lines @@
   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
 {
-  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<JniPlatform>(ptr)->platform->GetFilterEngine();
+  AdblockPlus::FilterEngine& GetFilterEngineRef(jlong jniPlatformPtr)
+  {
+    return JniLongToTypePtr<JniPlatform>(jniPlatformPtr)->platform->GetFilterEngine();
   }
 }
 
 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 406 matching lines @@
 {
   try
   {
     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)
+static void JNICALL JniUpdateFiltersAsync(JNIEnv* env, jclass clazz, jlong jniPlatformPtr, jstring jSubscriptionUrl)
 {
   std::string subscriptionUrl = JniJavaToStdString(env, jSubscriptionUrl);
-  auto jniPlatform = JniLongToTypePtr<JniPlatform>(ptr);
+  auto jniPlatform = JniLongToTypePtr<JniPlatform>(jniPlatformPtr);
   jniPlatform->scheduler([jniPlatform, subscriptionUrl]
   {
     auto& filterEngine = jniPlatform->platform->GetFilterEngine();
     for (auto& subscription : filterEngine.GetListedSubscriptions())
     {
       if (stringBeginsWith(subscriptionUrl, subscription.GetProperty("url").AsString()))
       {
         subscription.UpdateFilters();
         return;
       }
@@ skipping 30 matching lines @@
   { (char*)"setAcceptableAdsEnabled", (char*)"(JZ)V", (void*)JniSetAcceptableAdsEnabled },
   { (char*)"isAcceptableAdsEnabled", (char*)"(J)Z", (void*)JniIsAcceptableAdsEnabled },
   { (char*)"getAcceptableAdsSubscriptionURL", (char*)"(J)Ljava/lang/String;", (void*)JniGetAcceptableAdsSubscriptionURL },
   { (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]));
 }