Bug 1192082 - De-clutter AndroidBridge init/deinit; r=snorp
authorJim Chen <nchen@mozilla.com>
Thu, 13 Aug 2015 00:53:39 -0400
changeset 257579 250e5f0da2477da1abb19ebb38a7fbdd11e95427
parent 257578 489fd24eea7bd7832cd52fc11cb3d95665a1e60f
child 257580 2302cc77b4005936917e0295c07921a257ff038d
push id14640
push userryanvm@gmail.com
push dateThu, 13 Aug 2015 15:20:02 +0000
treeherderfx-team@6636074ac286 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssnorp
bugs1192082
milestone43.0a1
Bug 1192082 - De-clutter AndroidBridge init/deinit; r=snorp Merge all the init code into AndroidBridge constructor and AndroidBridge::ConstructBridge; merge all the deinit code into AndroidBridge destructor and AndroidBridge::DeconstructBridge. In particular, the SetMainThread call is obsolete and removed.
toolkit/xre/nsAndroidStartup.cpp
widget/android/AndroidBridge.cpp
widget/android/AndroidBridge.h
--- a/toolkit/xre/nsAndroidStartup.cpp
+++ b/toolkit/xre/nsAndroidStartup.cpp
@@ -10,58 +10,33 @@
 #include <stdlib.h>
 #include <string.h>
 #include <pthread.h>
 
 #include "nsTArray.h"
 #include "nsString.h"
 #include "nsIFile.h"
 #include "nsAppRunner.h"
-#include "AndroidBridge.h"
 #include "APKOpen.h"
 #include "nsExceptionHandler.h"
 
 #define LOG(args...) __android_log_print(ANDROID_LOG_INFO, MOZ_APP_NAME, args)
 
-// We need to put Gecko on a even more separate thread, because
-// otherwise this JNI method never returns; this leads to problems
-// with local references overrunning the local refs table, among
-// other things, since GC can't ever run on them.
-
-// Note that we don't have xpcom initialized yet, so we can't use the
-// thread manager for this.  Instead, we use pthreads directly.
-
-struct AutoAttachJavaThread {
-    AutoAttachJavaThread() {
-        attached = mozilla_AndroidBridge_SetMainThread(pthread_self());
-    }
-    ~AutoAttachJavaThread() {
-        mozilla_AndroidBridge_SetMainThread(pthread_t());
-        attached = false;
-    }
-
-    bool attached;
-};
-
 extern "C" NS_EXPORT void
 GeckoStart(void *data, const nsXREAppData *appData)
 {
 #ifdef MOZ_CRASHREPORTER
     const struct mapping_info *info = getLibraryMapping();
     while (info->name) {
       CrashReporter::AddLibraryMapping(info->name, info->base,
                                        info->len, info->offset);
       info++;
     }
 #endif
 
-    AutoAttachJavaThread attacher;
-    if (!attacher.attached)
-        return;
-
     if (!data) {
         LOG("Failed to get arguments for GeckoStart\n");
         return;
     }
 
     nsTArray<char *> targs;
     char *arg = strtok(static_cast<char *>(data), " ");
     while (arg) {
--- a/widget/android/AndroidBridge.cpp
+++ b/widget/android/AndroidBridge.cpp
@@ -146,53 +146,71 @@ jfieldID AndroidBridge::GetStaticFieldID
              fieldName, fieldType);
         env->ExceptionDescribe();
         MOZ_CRASH();
     }
     return fieldID;
 }
 
 void
-AndroidBridge::ConstructBridge(JNIEnv *jEnv, Object::Param clsLoader, Object::Param msgQueue)
+AndroidBridge::ConstructBridge()
 {
     /* NSS hack -- bionic doesn't handle recursive unloads correctly,
      * because library finalizer functions are called with the dynamic
      * linker lock still held.  This results in a deadlock when trying
      * to call dlclose() while we're already inside dlclose().
      * Conveniently, NSS has an env var that can prevent it from unloading.
      */
     putenv("NSS_DISABLE_UNLOAD=1");
 
     MOZ_ASSERT(!sBridge);
-    sBridge = new AndroidBridge;
-    sBridge->Init(jEnv, clsLoader); // Success or crash
+    sBridge = new AndroidBridge();
 
-    auto msgQueueClass = ClassObject::LocalRef::Adopt(
-            jEnv, jEnv->GetObjectClass(msgQueue.Get()));
-    sBridge->mMessageQueue = msgQueue;
-    // mMessageQueueNext must not be null
-    sBridge->mMessageQueueNext = GetMethodID(
-            jEnv, msgQueueClass.Get(), "next", "()Landroid/os/Message;");
-    // mMessageQueueMessages may be null (e.g. due to proguard optimization)
-    sBridge->mMessageQueueMessages = jEnv->GetFieldID(
-            msgQueueClass.Get(), "mMessages", "Landroid/os/Message;");
 }
 
 void
-AndroidBridge::Init(JNIEnv *jEnv, Object::Param clsLoader)
+AndroidBridge::DeconstructBridge()
+{
+    if (sBridge) {
+        delete sBridge;
+        // AndroidBridge destruction requires sBridge to still be valid,
+        // so we set sBridge to nullptr after deleting it.
+        sBridge = nullptr;
+    }
+}
+
+AndroidBridge::~AndroidBridge()
+{
+}
+
+AndroidBridge::AndroidBridge()
+  : mLayerClient(nullptr),
+    mPresentationWindow(nullptr),
+    mPresentationSurface(nullptr)
 {
     ALOG_BRIDGE("AndroidBridge::Init");
 
+    JNIEnv* const jEnv = jni::GetGeckoThreadEnv();
     AutoLocalJNIFrame jniFrame(jEnv);
 
-    mClassLoader = Object::GlobalRef(jEnv, clsLoader);
+    mClassLoader = Object::GlobalRef(jEnv, widget::GeckoThread::ClsLoader());
     mClassLoaderLoadClass = GetMethodID(
-            jEnv, jEnv->GetObjectClass(clsLoader.Get()),
+            jEnv, jEnv->GetObjectClass(mClassLoader.Get()),
             "loadClass", "(Ljava/lang/String;)Ljava/lang/Class;");
 
+    mMessageQueue = widget::GeckoThread::MsgQueue();
+    auto msgQueueClass = ClassObject::LocalRef::Adopt(
+            jEnv, jEnv->GetObjectClass(mMessageQueue.Get()));
+    // mMessageQueueNext must not be null
+    mMessageQueueNext = GetMethodID(
+            jEnv, msgQueueClass.Get(), "next", "()Landroid/os/Message;");
+    // mMessageQueueMessages may be null (e.g. due to proguard optimization)
+    mMessageQueueMessages = jEnv->GetFieldID(
+            msgQueueClass.Get(), "mMessages", "Landroid/os/Message;");
+
     mGLControllerObj = nullptr;
     mOpenedGraphicsLibraries = false;
     mHasNativeBitmapAccess = false;
     mHasNativeWindowAccess = false;
     mHasNativeWindowFallback = false;
 
 #ifdef MOZ_WEBSMS_BACKEND
     AutoJNIClass smsMessage(jEnv, "android/telephony/SmsMessage");
@@ -238,40 +256,16 @@ AndroidBridge::Init(JNIEnv *jEnv, Object
 
     AutoJNIClass inputStream(jEnv, "java/io/InputStream");
     jInputStream = inputStream.getGlobalRef();
     jClose = inputStream.getMethod("close", "()V");
     jAvailable = inputStream.getMethod("available", "()I");
 
     InitAndroidJavaWrappers(jEnv);
     ANRReporter::Init();
-
-    // jEnv should NOT be cached here by anything -- the jEnv here
-    // is not valid for the real gecko main thread, which is set
-    // at SetMainThread time.
-}
-
-bool
-AndroidBridge::SetMainThread(pthread_t thr)
-{
-    ALOG_BRIDGE("AndroidBridge::SetMainThread");
-
-    if (thr) {
-        return true;
-    }
-
-    // SetMainThread(0) is called on Gecko shutdown,
-    // so we should clean up the bridge here.
-    if (sBridge) {
-        delete sBridge;
-        // AndroidBridge destruction requires sBridge to still be valid,
-        // so we set sBridge to nullptr after deleting it.
-        sBridge = nullptr;
-    }
-    return true;
 }
 
 // Raw JNIEnv variants.
 jstring AndroidBridge::NewJavaString(JNIEnv* env, const char16_t* string, uint32_t len) {
    jstring ret = env->NewString(reinterpret_cast<const jchar*>(string), len);
    if (env->ExceptionCheck()) {
        ALOG_BRIDGE("Exceptional exit of: %s", __PRETTY_FUNCTION__);
        env->ExceptionDescribe();
@@ -779,23 +773,16 @@ AndroidBridge::GetStaticStringField(cons
     jstring jstr = (jstring) jEnv->GetStaticObjectField(cls.getRawRef(), field);
     if (!jstr)
         return false;
 
     result.Assign(nsJNIString(jstr, jEnv));
     return true;
 }
 
-// Available for places elsewhere in the code to link to.
-bool
-mozilla_AndroidBridge_SetMainThread(pthread_t thr)
-{
-    return AndroidBridge::Bridge()->SetMainThread(thr);
-}
-
 void*
 AndroidBridge::GetNativeSurface(JNIEnv* env, jobject surface) {
     if (!env || !mHasNativeWindowFallback || !jSurfacePointerField)
         return nullptr;
 
     return (void*)env->GetIntField(surface, jSurfacePointerField);
 }
 
@@ -1521,27 +1508,16 @@ void AndroidBridge::SyncFrameMetrics(con
     aFixedLayerMargins.right = viewTransform->FixedLayerMarginRight();
     aFixedLayerMargins.bottom = viewTransform->FixedLayerMarginBottom();
     aFixedLayerMargins.left = viewTransform->FixedLayerMarginLeft();
 
     aOffset.x = viewTransform->OffsetX();
     aOffset.y = viewTransform->OffsetY();
 }
 
-AndroidBridge::AndroidBridge()
-  : mLayerClient(nullptr),
-    mPresentationWindow(nullptr),
-    mPresentationSurface(nullptr)
-{
-}
-
-AndroidBridge::~AndroidBridge()
-{
-}
-
 /* Implementation file */
 NS_IMPL_ISUPPORTS(nsAndroidBridge, nsIAndroidBridge)
 
 nsAndroidBridge::nsAndroidBridge()
 {
 }
 
 nsAndroidBridge::~nsAndroidBridge()
--- a/widget/android/AndroidBridge.h
+++ b/widget/android/AndroidBridge.h
@@ -34,18 +34,16 @@
 
 // Some debug #defines
 // #define DEBUG_ANDROID_EVENTS
 // #define DEBUG_ANDROID_WIDGET
 
 class nsIObserver;
 class Task;
 
-extern bool mozilla_AndroidBridge_SetMainThread(pthread_t);
-
 namespace base {
 class Thread;
 } // end namespace base
 
 typedef void* EGLSurface;
 
 namespace mozilla {
 
@@ -128,31 +126,23 @@ public:
     static void RegisterJavaUiThread() {
         sJavaUiThread = pthread_self();
     }
 
     static bool IsJavaUiThread() {
         return pthread_equal(pthread_self(), sJavaUiThread);
     }
 
-    static void ConstructBridge(JNIEnv *jEnv,
-                                jni::Object::Param clsLoader,
-                                jni::Object::Param msgQueue);
+    static void ConstructBridge();
+    static void DeconstructBridge();
 
     static AndroidBridge *Bridge() {
         return sBridge;
     }
 
-    // The bridge needs to be constructed via ConstructBridge first,
-    // and then once the Gecko main thread is spun up (Gecko side),
-    // SetMainThread should be called which will create the JNIEnv for
-    // us to use.  toolkit/xre/nsAndroidStartup.cpp calls
-    // SetMainThread.
-    bool SetMainThread(pthread_t thr);
-
     /* These are all implemented in Java */
     bool GetThreadNameJavaProfiling(uint32_t aThreadId, nsCString & aResult);
     bool GetFrameNameJavaProfiling(uint32_t aThreadId, uint32_t aSampleId, uint32_t aFrameId, nsCString & aResult);
 
     nsresult CaptureZoomedView(nsIDOMWindow *window, nsIntRect zoomedViewRect, jni::Object::Param buffer, float zoomFactor);
     nsresult CaptureThumbnail(nsIDOMWindow *window, int32_t bufW, int32_t bufH, int32_t tabId, jni::Object::Param buffer, bool &shouldStore);
     void GetDisplayPort(bool aPageSizeUpdate, bool aIsBrowserContentDisplayed, int32_t tabId, nsIAndroidViewport* metrics, nsIAndroidDisplayport** displayPort);
     void ContentDocumentChanged();
@@ -321,19 +311,16 @@ protected:
     widget::GeckoLayerClient::GlobalRef mLayerClient;
 
     // the android.telephony.SmsMessage class
     jclass mAndroidSmsMessageClass;
 
     AndroidBridge();
     ~AndroidBridge();
 
-    void InitStubs(JNIEnv *jEnv);
-    void Init(JNIEnv *jEnv, jni::Object::Param clsLoader);
-
     bool mOpenedGraphicsLibraries;
     void OpenGraphicsLibraries();
     void* GetNativeSurface(JNIEnv* env, jobject surface);
 
     bool mHasNativeBitmapAccess;
     bool mHasNativeWindowAccess;
     bool mHasNativeWindowFallback;