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 id25939
push userkwierso@gmail.com
push dateTue, 07 Jan 2014 01:03:18 +0000
treeherdermozilla-central@302362be5075 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersblassey
bugs945327
milestone29.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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;