Issue 4948 - add possibility to not send data depending on connection properties

libadblockplus-android/jni/JniFilterEngine.cpp

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-2016 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 26 matching lines @@
   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);
 }
 
-static jlong JNICALL JniCtor(JNIEnv* env, jclass clazz, jlong enginePtr)
+static jlong JNICALL JniCtor(JNIEnv* env, jclass clazz, jlong jsEnginePtr, jlong isAllowedConnectionCallbackPtr)
 {
   try
   {
-    AdblockPlus::JsEnginePtr& jsEngine = *JniLongToTypePtr<AdblockPlus::JsEnginePtr>(enginePtr);
-    return JniPtrToLong(new AdblockPlus::FilterEngine(jsEngine));
+    AdblockPlus::JsEnginePtr& jsEngine = *JniLongToTypePtr<AdblockPlus::JsEnginePtr>(jsEnginePtr);
+    AdblockPlus::FilterEnginePtr filterEnginePtr = NULL;

[sergei, 2017/03/15 10:01:38]

smart pointer is already initialized to nullptr, so no need for " = NULL" here.

[sergei, 2017/03/15 10:01:38]

I would just call the variable as filterEngine, without Ptr suffix, especially
there is no need for it, like collision with another variable.

[anton, 2017/03/15 10:13:52]

Acknowledged.

+
+    if (isAllowedConnectionCallbackPtr != 0)
+    {
+      AdblockPlus::FilterEngine::CreateParameters createParameters;
+      JniIsAllowedConnectionTypeCallback* callback =
+        JniLongToTypePtr<JniIsAllowedConnectionTypeCallback>(isAllowedConnectionCallbackPtr);
+
+      AdblockPlus::FilterEngine::IsConnectionAllowedCallback cppCallback =
+        std::bind(&JniIsAllowedConnectionTypeCallback::Callback, callback, std::placeholders::_1);
+      createParameters.isConnectionAllowed = cppCallback;
+
+      filterEnginePtr = AdblockPlus::FilterEngine::Create(jsEngine, createParameters); // here (1) - create engine sync
+    }
+    else
+    {
+      filterEnginePtr = AdblockPlus::FilterEngine::Create(jsEngine);
+    }
+
+    return JniPtrToLong(filterEnginePtr.get());

[anton, 2017/03/10 06:50:40]

for some (unknown) reason new static methods (instead of ctor previously)
returns incorrect pointer. iw as unable to test because of this

[sergei, 2017/03/15 10:01:38]

TL;DR: Actually here is a bug, but my vote is to accept it as is for present.

Long story:
The bug is that instance of FilterEngine is destroyed after finishing of the
method but we continue to use the dangling pointer. It is destroyed because we
exit from the scope, no one keeps the reference to FilterEngine and
FilterEnginePtr destroys it.

There are several ways to figure it out but I think it should be firstly
addressed in libadblockplus and we should not disturb the code of
libadblockplus-android because of bugs in libadblockplus. On the interface level
I plan to not force the user of libadblockplus to use shared_ptr<FilterEngine>
and change it to std::unique_ptr<FilterEngine> (or even to FilterEngine, though
it's unlikely), it will require to change only this particular part.

I'm ready to accept it because currently we have a circular reference and
FilterEngine is not destroyed, so technically it does work now, though it's
incorrect. Secondly, I think that fixing it now in libadblockplus-android will
introduce only numerous changes most of which will be reverted afterwards, and
this actions are unnecessary and error-prone. So for the sake of simplicity I
think we can accept it for present, then fix in libadblockplus and then change
probably only one line here.

[anton, 2017/03/15 10:13:52]

Acknowledged.

   }
   CATCH_THROW_AND_RETURN(env, 0)
 }
 
 static void JNICALL JniDtor(JNIEnv* env, jclass clazz, jlong ptr)
 {
   delete JniLongToTypePtr<AdblockPlus::FilterEngine>(ptr);
 }
 
 static jboolean JNICALL JniIsFirstRun(JNIEnv* env, jclass clazz, jlong ptr)
@@ skipping 364 matching lines @@
   std::string url = JniJavaToStdString(env, jurl);
   try
   {
     std::string host = engine->GetHostFromURL(url);
 
     return JniStdStringToJava(env, host);
   }
   CATCH_THROW_AND_RETURN(env, 0)
 }
 
+static void JNICALL JniSetAllowedConnectionType(JNIEnv* env, jclass clazz, jlong ptr, jstring jvalue)
+{
+  AdblockPlus::FilterEngine* engine = JniLongToTypePtr<AdblockPlus::FilterEngine>(ptr);
+
+  std::string* value = NULL;
+  if (jvalue != NULL)
+  {
+    std::string stdValue = JniJavaToStdString(env, jvalue);
+    value = &stdValue;
+  }
+
+  try
+  {
+    engine->SetAllowedConnectionType(value);
+  }
+  CATCH_AND_THROW(env)
+}
+
+
+static jstring JNICALL JniGetAllowedConnectionType(JNIEnv* env, jclass clazz, jlong ptr)
+{
+  try
+  {
+    AdblockPlus::FilterEngine* engine = JniLongToTypePtr<AdblockPlus::FilterEngine>(ptr);
+    std::unique_ptr<std::string> value = engine->GetAllowedConnectionType();
+
+    if (value == NULL)
+    {
+      return NULL;
+    }
+
+    return JniStdStringToJava(env, *value.get());
+  }
+  CATCH_THROW_AND_RETURN(env, 0)
+}
+
 static JNINativeMethod methods[] =
 {
-  { (char*)"ctor", (char*)"(J)J", (void*)JniCtor },
+  { (char*)"ctor", (char*)"(JJ)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*)"dtor", (char*)"(J)V", (void*)JniDtor }
 };
 
 extern "C" JNIEXPORT void JNICALL Java_org_adblockplus_libadblockplus_FilterEngine_registerNatives(JNIEnv *env, jclass clazz)
 {
   env->RegisterNatives(clazz, methods, sizeof(methods) / sizeof(methods[0]));
 }