Bug 945327 - Improve local ref management in AndroidBridge; r=blassey
authorJim Chen <nchen@mozilla.com>
Mon, 06 Jan 2014 11:54:22 -0600
changeset 162199 d7dfd3217a5436d0ebf00eba96db616f9898c8b5
parent 162198 f772c534bcafc59998250bf16eec64e8364f1f83
child 162200 48f95993e938328b9b32d8c2428e102f5bcff339
push idunknown
push userunknown
push dateunknown
reviewersblassey
bugs945327
milestone29.0a1
Bug 945327 - Improve local ref management in AndroidBridge; r=blassey
widget/android/AndroidBridge.cpp
--- a/widget/android/AndroidBridge.cpp
+++ b/widget/android/AndroidBridge.cpp
@@ -315,16 +315,18 @@ static void
 getHandlersFromStringArray(JNIEnv *aJNIEnv, jobjectArray jArr, jsize aLen,
                            nsIMutableArray *aHandlersArray,
                            nsIHandlerApp **aDefaultApp,
                            const nsAString& aAction = EmptyString(),
                            const nsACString& aMimeType = EmptyCString())
 {
     nsString empty = EmptyString();
     for (jsize i = 0; i < aLen; i+=4) {
+
+        AutoLocalJNIFrame jniFrame(aJNIEnv, 4);
         nsJNIString name(
             static_cast<jstring>(aJNIEnv->GetObjectArrayElement(jArr, i)), aJNIEnv);
         nsJNIString isDefault(
             static_cast<jstring>(aJNIEnv->GetObjectArrayElement(jArr, i + 1)), aJNIEnv);
         nsJNIString packageName(
             static_cast<jstring>(aJNIEnv->GetObjectArrayElement(jArr, i + 2)), aJNIEnv);
         nsJNIString className(
             static_cast<jstring>(aJNIEnv->GetObjectArrayElement(jArr, i + 3)), aJNIEnv);
@@ -345,116 +347,111 @@ AndroidBridge::GetHandlersForMimeType(co
                                       const nsAString& aAction)
 {
     ALOG_BRIDGE("AndroidBridge::GetHandlersForMimeType");
 
     JNIEnv *env = GetJNIEnv();
     if (!env)
         return false;
 
+    AutoLocalJNIFrame jniFrame(env, 1);
     jobjectArray arr = GeckoAppShell::GetHandlersForMimeTypeWrapper(aMimeType, aAction);
     if (!arr)
         return false;
 
     jsize len = env->GetArrayLength(arr);
 
     if (!aHandlersArray)
         return len > 0;
 
     getHandlersFromStringArray(env, arr, len, aHandlersArray,
                                aDefaultApp, aAction,
                                NS_ConvertUTF16toUTF8(aMimeType));
-
-    env->DeleteLocalRef(arr);
     return true;
 }
 
 bool
 AndroidBridge::GetHandlersForURL(const nsAString& aURL,
                                  nsIMutableArray* aHandlersArray,
                                  nsIHandlerApp **aDefaultApp,
                                  const nsAString& aAction)
 {
     ALOG_BRIDGE("AndroidBridge::GetHandlersForURL");
 
     JNIEnv *env = GetJNIEnv();
     if (!env)
         return false;
 
+    AutoLocalJNIFrame jniFrame(env, 1);
     jobjectArray arr = GeckoAppShell::GetHandlersForURLWrapper(aURL, aAction);
     if (!arr)
         return false;
 
     jsize len = env->GetArrayLength(arr);
 
     if (!aHandlersArray)
         return len > 0;
 
     getHandlersFromStringArray(env, arr, len, aHandlersArray,
                                aDefaultApp, aAction);
-
-    env->DeleteLocalRef(arr);
     return true;
 }
 
 void
 AndroidBridge::GetMimeTypeFromExtensions(const nsACString& aFileExt, nsCString& aMimeType)
 {
     ALOG_BRIDGE("AndroidBridge::GetMimeTypeFromExtensions");
 
     JNIEnv *env = GetJNIEnv();
     if (!env)
         return;
 
+    AutoLocalJNIFrame jniFrame(env, 1);
     jstring jstrType = GeckoAppShell::GetMimeTypeFromExtensionsWrapper(NS_ConvertUTF8toUTF16(aFileExt));
     if (!jstrType) {
         return;
     }
     nsJNIString jniStr(jstrType, env);
     CopyUTF16toUTF8(jniStr.get(), aMimeType);
-
-    env->DeleteLocalRef(jstrType);
 }
 
 void
 AndroidBridge::GetExtensionFromMimeType(const nsACString& aMimeType, nsACString& aFileExt)
 {
     ALOG_BRIDGE("AndroidBridge::GetExtensionFromMimeType");
 
     JNIEnv *env = GetJNIEnv();
     if (!env)
         return;
 
+    AutoLocalJNIFrame jniFrame(env, 1);
     jstring jstrExt = GeckoAppShell::GetExtensionFromMimeTypeWrapper(NS_ConvertUTF8toUTF16(aMimeType));
     if (!jstrExt) {
         return;
     }
     nsJNIString jniStr(jstrExt, env);
     CopyUTF16toUTF8(jniStr.get(), aFileExt);
-
-    env->DeleteLocalRef(jstrExt);
 }
 
 bool
 AndroidBridge::GetClipboardText(nsAString& aText)
 {
     ALOG_BRIDGE("AndroidBridge::GetClipboardText");
 
     JNIEnv *env = GetJNIEnv();
     if (!env)
         return false;
 
+    AutoLocalJNIFrame jniFrame(env, 1);
     jstring result = Clipboard::GetClipboardTextWrapper();
     if (!result)
         return false;
 
     nsJNIString jniStr(result, env);
     aText.Assign(jniStr);
-
-    env->DeleteLocalRef(result);
     return true;
 }
 
 void
 AndroidBridge::ShowAlertNotification(const nsAString& aImageUrl,
                                      const nsAString& aAlertTitle,
                                      const nsAString& aAlertText,
                                      const nsAString& aAlertCookie,
@@ -506,39 +503,39 @@ AndroidBridge::GetScreenDepth()
 
 void
 AndroidBridge::ShowFilePickerForExtensions(nsAString& aFilePath, const nsAString& aExtensions)
 {
     JNIEnv *env = GetJNIEnv();
     if (!env)
         return;
 
+    AutoLocalJNIFrame jniFrame(env, 1);
     jstring jstr = GeckoAppShell::ShowFilePickerForExtensionsWrapper(aExtensions);
     if (jstr == nullptr) {
         return;
     }
 
     aFilePath.Assign(nsJNIString(jstr, env));
-    env->DeleteLocalRef(jstr);
 }
 
 void
 AndroidBridge::ShowFilePickerForMimeType(nsAString& aFilePath, const nsAString& aMimeType)
 {
     JNIEnv *env = GetJNIEnv();
     if (!env)
         return;
 
+    AutoLocalJNIFrame jniFrame(env, 1);
     jstring jstr = GeckoAppShell::ShowFilePickerForMimeTypeWrapper(aMimeType);
     if (jstr == nullptr) {
         return;
     }
 
     aFilePath.Assign(nsJNIString(jstr, env));
-    env->DeleteLocalRef(jstr);
 }
 
 void
 AndroidBridge::ShowFilePickerAsync(const nsAString& aMimeType, nsFilePickerCallback* callback)
 {
     callback->AddRef();
     GeckoAppShell::ShowFilePickerAsyncWrapper(aMimeType, (int64_t) callback);
 }
@@ -553,17 +550,17 @@ AndroidBridge::Vibrate(const nsTArray<ui
         ALOG_BRIDGE("  invalid 0-length array");
         return;
     }
 
     JNIEnv *env = GetJNIEnv();
     if (!env)
         return;
 
-    AutoLocalJNIFrame jniFrame(env);
+    AutoLocalJNIFrame jniFrame(env, 1);
 
     // It's clear if this worth special-casing, but it creates less
     // java junk, so dodges the GC.
     if (len == 1) {
         jlong d = aPattern[0];
         if (d < 0) {
             ALOG_BRIDGE("  invalid vibration duration < 0");
             return;
@@ -604,17 +601,17 @@ AndroidBridge::GetSystemColors(AndroidSy
     NS_ASSERTION(aColors != nullptr, "AndroidBridge::GetSystemColors: aColors is null!");
     if (!aColors)
         return;
 
     JNIEnv *env = GetJNIEnv();
     if (!env)
         return;
 
-    AutoLocalJNIFrame jniFrame(env);
+    AutoLocalJNIFrame jniFrame(env, 1);
 
     jintArray arr = GeckoAppShell::GetSystemColoursWrapper();
     if (!arr)
         return;
 
     uint32_t len = static_cast<uint32_t>(env->GetArrayLength(arr));
     jint *elements = env->GetIntArrayElements(arr, 0);
 
@@ -642,17 +639,17 @@ AndroidBridge::GetIconForExtension(const
     NS_ASSERTION(aBuf != nullptr, "AndroidBridge::GetIconForExtension: aBuf is null!");
     if (!aBuf)
         return;
 
     JNIEnv *env = GetJNIEnv();
     if (!env)
         return;
 
-    AutoLocalJNIFrame jniFrame(env);
+    AutoLocalJNIFrame jniFrame(env, 1);
 
     jbyteArray arr = GeckoAppShell::GetIconForExtensionWrapper(NS_ConvertUTF8toUTF16(aFileExt), aIconSize);
 
     NS_ASSERTION(arr != nullptr, "AndroidBridge::GetIconForExtension: Returned pixels array is null!");
     if (!arr)
         return;
 
     uint32_t len = static_cast<uint32_t>(env->GetArrayLength(arr));
@@ -715,22 +712,22 @@ AndroidBridge::CreateEGLSurfaceForCompos
     }
     MOZ_ASSERT(mGLControllerObj, "AndroidBridge::CreateEGLSurfaceForCompositor called with a null GL controller ref");
 
     JNIEnv* env = GetJNIForThread(); // called on the compositor thread
     if (!env) {
         return nullptr;
     }
 
+    AutoLocalJNIFrame jniFrame(env, 1);
     jobject eglSurface = mGLControllerObj->CreateEGLSurfaceForCompositorWrapper();
     if (!eglSurface)
         return nullptr;
 
     EGLSurface ret = reinterpret_cast<EGLSurface>(env->GetIntField(eglSurface, jEGLSurfacePointerField));
-    env->DeleteLocalRef(eglSurface);
     return ret;
 }
 
 bool
 AndroidBridge::GetStaticIntField(const char *className, const char *fieldName, int32_t* aInt, JNIEnv* jEnv /* = nullptr */)
 {
     ALOG_BRIDGE("AndroidBridge::GetStaticIntField %s", fieldName);
 
@@ -761,32 +758,32 @@ AndroidBridge::GetStaticStringField(cons
     ALOG_BRIDGE("AndroidBridge::GetStaticStringField %s", fieldName);
 
     if (!jEnv) {
         jEnv = GetJNIEnv();
         if (!jEnv)
             return false;
     }
 
+    AutoLocalJNIFrame jniFrame(jEnv, 1);
     initInit();
     getClassGlobalRef(className);
     jfieldID field = getStaticField(fieldName, "Ljava/lang/String;");
 
     if (!field) {
         jEnv->DeleteGlobalRef(jClass);
         return false;
     }
 
     jstring jstr = (jstring) jEnv->GetStaticObjectField(jClass, field);
     jEnv->DeleteGlobalRef(jClass);
     if (!jstr)
         return false;
 
     result.Assign(nsJNIString(jstr, jEnv));
-    jEnv->DeleteLocalRef(jstr);
     return true;
 }
 
 // Available for places elsewhere in the code to link to.
 bool
 mozilla_AndroidBridge_SetMainThread(pthread_t thr)
 {
     return AndroidBridge::Bridge()->SetMainThread(thr);
@@ -982,17 +979,17 @@ AndroidBridge::ValidateBitmap(jobject bi
 
 bool
 AndroidBridge::InitCamera(const nsCString& contentType, uint32_t camera, uint32_t *width, uint32_t *height, uint32_t *fps)
 {
     JNIEnv *env = GetJNIEnv();
     if (!env)
         return false;
 
-    AutoLocalJNIFrame jniFrame(env);
+    AutoLocalJNIFrame jniFrame(env, 1);
     jintArray arr = GeckoAppShell::InitCameraWrapper(NS_ConvertUTF8toUTF16(contentType), (int32_t) camera, (int32_t) width, (int32_t) height);
 
     if (!arr)
         return false;
 
     jint *elements = env->GetIntArrayElements(arr, 0);
 
     *width = elements[1];
@@ -1010,17 +1007,17 @@ void
 AndroidBridge::GetCurrentBatteryInformation(hal::BatteryInformation* aBatteryInfo)
 {
     ALOG_BRIDGE("AndroidBridge::GetCurrentBatteryInformation");
 
     JNIEnv *env = GetJNIEnv();
     if (!env)
         return;
 
-    AutoLocalJNIFrame jniFrame(env);
+    AutoLocalJNIFrame jniFrame(env, 1);
 
     // To prevent calling too many methods through JNI, the Java method returns
     // an array of double even if we actually want a double and a boolean.
     jdoubleArray arr = GeckoAppShell::GetCurrentBatteryInformationWrapper();
     if (!arr || env->GetArrayLength(arr) != 3) {
         return;
     }
 
@@ -1037,17 +1034,17 @@ void
 AndroidBridge::HandleGeckoMessage(const nsAString &aMessage, nsAString &aRet)
 {
     ALOG_BRIDGE("%s", __PRETTY_FUNCTION__);
 
     JNIEnv *env = GetJNIEnv();
     if (!env)
         return;
 
-    AutoLocalJNIFrame jniFrame(env);
+    AutoLocalJNIFrame jniFrame(env, 1);
     jstring returnMessage = GeckoAppShell::HandleGeckoMessageWrapper(aMessage);
 
     if (!returnMessage)
         return;
 
     nsJNIString jniStr(returnMessage, env);
     aRet.Assign(jniStr);
     ALOG_BRIDGE("leaving %s", __PRETTY_FUNCTION__);
@@ -1067,17 +1064,17 @@ AndroidBridge::GetSegmentInfoForText(con
     data.segments() = 0;
     data.charsPerSegment() = 0;
     data.charsAvailableInLastSegment() = 0;
 
     JNIEnv *env = GetJNIEnv();
     if (!env)
         return NS_ERROR_FAILURE;
 
-    AutoLocalJNIFrame jniFrame(env);
+    AutoLocalJNIFrame jniFrame(env, 2);
     jstring jText = NewJavaString(&jniFrame, aText);
     jobject obj = env->CallStaticObjectMethod(mAndroidSmsMessageClass,
                                               jCalculateLength, jText, JNI_FALSE);
     if (jniFrame.CheckForException())
         return NS_ERROR_FAILURE;
 
     jintArray arr = static_cast<jintArray>(obj);
     if (!arr || env->GetArrayLength(arr) != 4)
@@ -1146,26 +1143,27 @@ AndroidBridge::CreateMessageList(const d
     JNIEnv *env = GetJNIEnv();
     if (!env)
         return;
 
     uint32_t requestId;
     if (!QueueSmsRequest(aRequest, &requestId))
         return;
 
-    AutoLocalJNIFrame jniFrame(env);
+    AutoLocalJNIFrame jniFrame(env, 2);
 
     jobjectArray numbers =
         (jobjectArray)env->NewObjectArray(aFilter.numbers().Length(),
                                           jStringClass,
                                           NewJavaString(&jniFrame, EmptyString()));
 
     for (uint32_t i = 0; i < aFilter.numbers().Length(); ++i) {
-        env->SetObjectArrayElement(numbers, i,
-                                   NewJavaString(&jniFrame, aFilter.numbers()[i]));
+        jstring elem = NewJavaString(&jniFrame, aFilter.numbers()[i]);
+        env->SetObjectArrayElement(numbers, i, elem);
+        env->DeleteLocalRef(elem);
     }
 
     GeckoAppShell::CreateMessageListWrapper(aFilter.startDate(), aFilter.endDate(),
                              numbers, aFilter.numbers().Length(),
                              aFilter.delivery(), aReverse, requestId);
 }
 
 void
@@ -1219,17 +1217,17 @@ void
 AndroidBridge::GetCurrentNetworkInformation(hal::NetworkInformation* aNetworkInfo)
 {
     ALOG_BRIDGE("AndroidBridge::GetCurrentNetworkInformation");
 
     JNIEnv *env = GetJNIEnv();
     if (!env)
         return;
 
-    AutoLocalJNIFrame jniFrame(env);
+    AutoLocalJNIFrame jniFrame(env, 1);
 
     // To prevent calling too many methods through JNI, the Java method returns
     // an array of double even if we actually want a double, two booleans, and an integer.
 
     jdoubleArray arr = GeckoAppShell::GetCurrentNetworkInformationWrapper();
     if (!arr || env->GetArrayLength(arr) != 4) {
         return;
     }
@@ -1246,17 +1244,17 @@ AndroidBridge::GetCurrentNetworkInformat
 
 void *
 AndroidBridge::LockBitmap(jobject bitmap)
 {
     JNIEnv *env = GetJNIEnv();
     if (!env)
         return nullptr;
 
-    AutoLocalJNIFrame jniFrame(env);
+    AutoLocalJNIFrame jniFrame(env, 0);
 
     int err;
     void *buf;
 
     if ((err = AndroidBitmap_lockPixels(env, bitmap, &buf)) != 0) {
         ALOG_BRIDGE("AndroidBitmap_lockPixels failed! (error %d)", err);
         buf = nullptr;
     }
@@ -1266,17 +1264,17 @@ AndroidBridge::LockBitmap(jobject bitmap
 
 void
 AndroidBridge::UnlockBitmap(jobject bitmap)
 {
     JNIEnv *env = GetJNIEnv();
     if (!env)
         return;
 
-    AutoLocalJNIFrame jniFrame(env);
+    AutoLocalJNIFrame jniFrame(env, 0);
 
     int err;
 
     if ((err = AndroidBitmap_unlockPixels(env, bitmap)) != 0)
         ALOG_BRIDGE("AndroidBitmap_unlockPixels failed! (error %d)", err);
 }
 
 
@@ -1647,17 +1645,17 @@ AndroidBridge::GetProxyForURI(const nsAC
                               const nsACString & aHost,
                               const int32_t      aPort,
                               nsACString & aResult)
 {
     JNIEnv* env = GetJNIEnv();
     if (!env)
         return NS_ERROR_FAILURE;
 
-    AutoLocalJNIFrame jniFrame(env);
+    AutoLocalJNIFrame jniFrame(env, 1);
     jstring jstrRet = GeckoAppShell::GetProxyForURIWrapper(NS_ConvertUTF8toUTF16(aSpec),
                                             NS_ConvertUTF8toUTF16(aScheme),
                                             NS_ConvertUTF8toUTF16(aHost),
                                             aPort);
 
     if (!jstrRet)
         return NS_ERROR_FAILURE;
 
@@ -1699,17 +1697,17 @@ Java_org_mozilla_gecko_GeckoAppShell_all
 
 bool
 AndroidBridge::GetThreadNameJavaProfiling(uint32_t aThreadId, nsCString & aResult)
 {
     JNIEnv* env = GetJNIForThread();
     if (!env)
         return false;
 
-    AutoLocalJNIFrame jniFrame(env);
+    AutoLocalJNIFrame jniFrame(env, 1);
 
     jstring jstrThreadName = GeckoJavaSampler::GetThreadNameJavaProfilingWrapper(aThreadId);
 
     if (!jstrThreadName)
         return false;
 
     nsJNIString jniStr(jstrThreadName, env);
     CopyUTF16toUTF8(jniStr.get(), aResult);
@@ -1719,26 +1717,25 @@ AndroidBridge::GetThreadNameJavaProfilin
 bool
 AndroidBridge::GetFrameNameJavaProfiling(uint32_t aThreadId, uint32_t aSampleId,
                                           uint32_t aFrameId, nsCString & aResult)
 {
     JNIEnv* env = GetJNIForThread();
     if (!env)
         return false;
 
-    AutoLocalJNIFrame jniFrame(env);
+    AutoLocalJNIFrame jniFrame(env, 1);
 
     jstring jstrSampleName = GeckoJavaSampler::GetFrameNameJavaProfilingWrapper(aThreadId, aSampleId, aFrameId);
 
     if (!jstrSampleName)
         return false;
 
     nsJNIString jniStr(jstrSampleName, env);
     CopyUTF16toUTF8(jniStr.get(), aResult);
-    env->DeleteLocalRef(jstrSampleName);
     return true;
 }
 
 nsresult AndroidBridge::CaptureThumbnail(nsIDOMWindow *window, int32_t bufW, int32_t bufH, int32_t tabId, jobject buffer)
 {
     nsresult rv;
     float scale = 1.0;