Issue 4146 - Fix failing UpdateCheckTest.testApplicationUpdateAvailable test in libadblockplus-andr…

libadblockplus-android/jni/JniLibrary.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
  * 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 "JniJsValue.h"
 

[anton, 2016/09/16 11:06:04]

added #include "Utils.h" for ABP_JNI_VERSION to be available in final commit

 jint JNI_OnLoad(JavaVM* vm, void* reserved)
 {
   JNIEnv* env;
-  if (vm->GetEnv(reinterpret_cast<void**>(&env), JNI_VERSION_1_6) != JNI_OK)
+  if (vm->GetEnv(reinterpret_cast<void**>(&env), ABP_JNI_VERSION) != JNI_OK)
   {
     return JNI_ERR;

[Felix Dahlke, 2016/09/15 13:01:19]

Can we actually return JNI_ERR here? How I interpret the docs
(https://docs.oracle.com/javase/8/docs/technotes/guides/jni/spec/invocation.ht...),
we should return JNI_VERSION_1_6 in either case, because JNI_OnLoad is supposed
to return the required JNI version.

[anton, 2016/09/15 13:18:11]

As it's written here
(https://developer.android.com/training/articles/perf-jni.html)
---
jint JNI_OnLoad(JavaVM* vm, void* reserved)
{
    JNIEnv* env;
    if (vm->GetEnv(reinterpret_cast<void**>(&env), JNI_VERSION_1_6) != JNI_OK) {
        return -1;
    }

    // Get jclass with env->FindClass.
    // Register methods with env->RegisterNatives.

    return JNI_VERSION_1_6;
}
---

in case of error it returns "-1" which is actually "JNI_ERR":

jni.h:

#define JNI_ERR         (-1)        /* generic error */

in case of success it returns required java version

   }
 
   JniJsValue_OnLoad(vm, env, reserved);
 
-  return JNI_VERSION_1_6;
+  return ABP_JNI_VERSION;
 }
 
-void JNI_OnUnload(JavaVM *vm, void *reserved)

[Felix Dahlke, 2016/09/15 13:01:19]

Nit: Same issue with the asterisks again, should be `JavaVM* vm` etc.

[anton, 2016/09/15 13:18:11]

Acknowledged.

[anton, 2016/09/15 13:18:12]

Acknowledged.

+void JNI_OnUnload(JavaVM* vm, void* reserved)
 {
   JNIEnv* env;
-  if (vm->GetEnv(reinterpret_cast<void**>(&env), JNI_VERSION_1_6) != JNI_OK)

[Felix Dahlke, 2016/09/15 13:01:19]

Can we use a constant for the required JNI version, rather than duplicating it?

[anton, 2016/09/15 13:18:12]

Acknowledged.

+  if (vm->GetEnv(reinterpret_cast<void**>(&env), ABP_JNI_VERSION) != JNI_OK)
   {
     return;
   }
 
   JniJsValue_OnUnload(vm, env, reserved);
 }