Fix various JNI array accesses.

Various places that use the JNI functions for accessing array elements
directly either forgot to release the array elements afterward, or are
doing something that can be done with one of the array helpers in base.
Fix them up to behave properly.

Change-Id: Iab3fe07949c67c884d60f7ffaaf0bd0faf0f13dc
Reviewed-on: https://chromium-review.googlesource.com/c/1279005Reviewed-by: Yaron Friedman <yfriedman@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Reviewed-by: Mohamed Amir Yosef <mamir@chromium.org>
Commit-Queue: Richard Coles <torne@chromium.org>
Cr-Commit-Position: refs/heads/master@{#599331}

base/android/animation_frame_time_histogram.cc

@@ -15,7 +15,8 @@ void JNI_AnimationFrameTimeHistogram_SaveHistogram(
     const JavaParamRef<jstring>& j_histogram_name,
     const JavaParamRef<jlongArray>& j_frame_times_ms,
     jint j_count) {
-  jlong *frame_times_ms = env->GetLongArrayElements(j_frame_times_ms, NULL);
+  jlong* frame_times_ms =
+      env->GetLongArrayElements(j_frame_times_ms.obj(), nullptr);
   std::string histogram_name = base::android::ConvertJavaStringToUTF8(
       env, j_histogram_name);
 
@@ -23,4 +24,6 @@ void JNI_AnimationFrameTimeHistogram_SaveHistogram(
     UMA_HISTOGRAM_TIMES(histogram_name.c_str(),
                         base::TimeDelta::FromMilliseconds(frame_times_ms[i]));
   }
+  env->ReleaseLongArrayElements(j_frame_times_ms.obj(), frame_times_ms,
+                                JNI_ABORT);
 }

base/android/important_file_writer_android.cc

@@ -4,6 +4,7 @@
 
 #include <string>
 
+#include "base/android/jni_array.h"
 #include "base/android/jni_string.h"
 #include "base/files/important_file_writer.h"
 #include "base/threading/thread_restrictions.h"
@@ -23,13 +24,10 @@ static jboolean JNI_ImportantFileWriterAndroid_WriteFileAtomically(
   std::string native_file_name;
   base::android::ConvertJavaStringToUTF8(env, file_name, &native_file_name);
   base::FilePath path(native_file_name);
-  int data_length = env->GetArrayLength(data);
-  jbyte* native_data = env->GetByteArrayElements(data, NULL);
-  std::string native_data_string(reinterpret_cast<char *>(native_data),
-                                 data_length);
+  std::string native_data_string;
+  JavaByteArrayToString(env, data, &native_data_string);
   bool result = base::ImportantFileWriter::WriteFileAtomically(
       path, native_data_string);
-  env->ReleaseByteArrayElements(data, native_data, JNI_ABORT);
   return result;
 }
 

base/android/jni_array.cc

@@ -247,6 +247,7 @@ void JavaBooleanArrayToBoolVector(JNIEnv* env,
   for (size_t i = 0; i < len; ++i) {
     out->at(i) = static_cast<bool>(values[i]);
   }
+  env->ReleaseBooleanArrayElements(boolean_array.obj(), values, JNI_ABORT);
 }
 
 void JavaIntArrayToIntVector(JNIEnv* env,

chrome/browser/android/chrome_backup_agent_unittest.cc

@@ -27,6 +27,7 @@ using base::android::AppendJavaStringArrayToStringVector;
 using base::android::AttachCurrentThread;
 using base::android::ScopedJavaLocalRef;
 using base::android::ToJavaArrayOfStrings;
+using base::android::ToJavaBooleanArray;
 using base::android::JavaParamRef;
 
 class ChromeBackupAgentTest : public ::testing::Test {
@@ -67,11 +68,10 @@ TEST_F(ChromeBackupAgentTest, GetBoolBackupNames) {
 TEST_F(ChromeBackupAgentTest, GetBoolBackupValues_AllDefault) {
   ScopedJavaLocalRef<jbooleanArray> result =
       GetBoolBackupValuesForTesting(env_, JavaParamRef<jobject>(nullptr));
-  jsize size = env_->GetArrayLength(result.obj());
-  ASSERT_EQ(expected_pref_names_.size(), static_cast<size_t>(size));
-
-  jboolean* values = env_->GetBooleanArrayElements(result.obj(), nullptr);
-  for (int i = 0; i < size; i++) {
+  std::vector<bool> values;
+  JavaBooleanArrayToBoolVector(env_, result, &values);
+  ASSERT_EQ(expected_pref_names_.size(), values.size());
+  for (size_t i = 0; i < values.size(); i++) {
     bool expected_value;
     ASSERT_TRUE(pref_service_->GetDefaultPrefValue(expected_pref_names_[i])
                     ->GetAsBoolean(&expected_value));
@@ -86,10 +86,10 @@ TEST_F(ChromeBackupAgentTest, GetBoolBackupValues_IrrelevantChange) {
 
   ScopedJavaLocalRef<jbooleanArray> result =
       GetBoolBackupValuesForTesting(env_, JavaParamRef<jobject>(nullptr));
-  jsize size = env_->GetArrayLength(result.obj());
-  ASSERT_EQ(expected_pref_names_.size(), static_cast<size_t>(size));
-  jboolean* values = env_->GetBooleanArrayElements(result.obj(), nullptr);
-  for (int i = 0; i < size; i++) {
+  std::vector<bool> values;
+  JavaBooleanArrayToBoolVector(env_, result, &values);
+  ASSERT_EQ(expected_pref_names_.size(), values.size());
+  for (size_t i = 0; i < values.size(); i++) {
     bool expected_value;
     ASSERT_TRUE(pref_service_->GetDefaultPrefValue(expected_pref_names_[i])
                     ->GetAsBoolean(&expected_value));
@@ -103,11 +103,10 @@ TEST_F(ChromeBackupAgentTest, GetBoolBackupValues_RelevantChange) {
   pref_service_->SetBoolean(expected_pref_names_[3], false);
   ScopedJavaLocalRef<jbooleanArray> result =
       GetBoolBackupValuesForTesting(env_, JavaParamRef<jobject>(nullptr));
-  jsize size = env_->GetArrayLength(result.obj());
-  ASSERT_EQ(expected_pref_names_.size(), static_cast<size_t>(size));
-
-  jboolean* values = env_->GetBooleanArrayElements(result.obj(), nullptr);
-  for (int i = 0; i < size; i++) {
+  std::vector<bool> values;
+  JavaBooleanArrayToBoolVector(env_, result, &values);
+  ASSERT_EQ(expected_pref_names_.size(), values.size());
+  for (size_t i = 0; i < values.size(); i++) {
     EXPECT_EQ(pref_service_->GetBoolean(expected_pref_names_[i]), values[i])
         << "i = " << i << ", " << expected_pref_names_[i];
   }
@@ -116,17 +115,15 @@ TEST_F(ChromeBackupAgentTest, GetBoolBackupValues_RelevantChange) {
 TEST_F(ChromeBackupAgentTest, SetBoolBackupValues) {
   ScopedJavaLocalRef<jobjectArray> narray =
       ToJavaArrayOfStrings(env_, expected_pref_names_);
-  jboolean* values = new jboolean[expected_pref_names_.size()];
+  bool* values = new bool[expected_pref_names_.size()];
   for (size_t i = 0; i < expected_pref_names_.size(); i++) {
     values[i] = false;
   }
   // Set a couple of the values to true.
   values[5] = true;
   values[8] = true;
-  ScopedJavaLocalRef<jbooleanArray> varray(
-      env_, env_->NewBooleanArray(expected_pref_names_.size()));
-  env_->SetBooleanArrayRegion(varray.obj(), 0, expected_pref_names_.size(),
-                              values);
+  ScopedJavaLocalRef<jbooleanArray> varray =
+      ToJavaBooleanArray(env_, values, expected_pref_names_.size());
   SetBoolBackupPrefsForTesting(env_, JavaParamRef<jobject>(nullptr),
                                JavaParamRef<jobjectArray>(env_, narray.obj()),
                                JavaParamRef<jbooleanArray>(env_, varray.obj()));

components/sync/test/fake_server/android/fake_server_helper_android.cc

@@ -198,13 +198,11 @@ void FakeServerHelperAndroid::DeserializeEntity(JNIEnv* env,
 
 void FakeServerHelperAndroid::DeserializeEntitySpecifics(
     JNIEnv* env,
-    jbyteArray serialized_entity_specifics,
+    const JavaParamRef<jbyteArray>& serialized_entity_specifics,
     sync_pb::EntitySpecifics* entity_specifics) {
-  int specifics_bytes_length = env->GetArrayLength(serialized_entity_specifics);
-  jbyte* specifics_bytes =
-      env->GetByteArrayElements(serialized_entity_specifics, nullptr);
-  std::string specifics_string(reinterpret_cast<char*>(specifics_bytes),
-                               specifics_bytes_length);
+  std::string specifics_string;
+  base::android::JavaByteArrayToString(env, serialized_entity_specifics,
+                                       &specifics_string);
 
   if (!entity_specifics->ParseFromString(specifics_string))
     NOTREACHED() << "Could not deserialize EntitySpecifics";

components/sync/test/fake_server/android/fake_server_helper_android.h

@@ -152,7 +152,8 @@ class FakeServerHelperAndroid {
 
   // Deserializes |serialized_entity_specifics| into |entity_specifics|.
   void DeserializeEntitySpecifics(JNIEnv* env,
-                                  jbyteArray serialized_entity_specifics,
+                                  const base::android::JavaParamRef<jbyteArray>&
+                                      serialized_entity_specifics,
                                   sync_pb::EntitySpecifics* entity_specifics);
 
   // Creates a bookmark entity.

content/browser/android/select_popup.cc

@@ -139,12 +139,8 @@ void SelectPopup::SelectMenuItems(JNIEnv* env,
     return;
   }
 
-  int selected_count = env->GetArrayLength(indices);
   std::vector<int> selected_indices;
-  jint* indices_ptr = env->GetIntArrayElements(indices, NULL);
-  for (int i = 0; i < selected_count; ++i)
-    selected_indices.push_back(indices_ptr[i]);
-  env->ReleaseIntArrayElements(indices, indices_ptr, JNI_ABORT);
+  base::android::JavaIntArrayToIntVector(env, indices, &selected_indices);
   rfhi->DidSelectPopupMenuItems(selected_indices);
 }
 

ui/gl/android/surface_texture.cc

@@ -59,8 +59,7 @@ void SurfaceTexture::GetTransformMatrix(float mtx[16]) {
   Java_SurfaceTexturePlatformWrapper_getTransformMatrix(env, j_surface_texture_,
                                                         jmatrix);
 
-  jboolean is_copy;
-  jfloat* elements = env->GetFloatArrayElements(jmatrix.obj(), &is_copy);
+  jfloat* elements = env->GetFloatArrayElements(jmatrix.obj(), nullptr);
   for (int i = 0; i < 16; ++i) {
     mtx[i] = static_cast<float>(elements[i]);
   }