bug 884792 - crash in nsXPCWrappedJS::Release, removeObserver being called off main thread r=kats
authorBrad Lassey <blassey@mozilla.com>
Wed, 19 Jun 2013 13:55:35 -0400
changeset 135825 23bcbbbfd5a04674782c940b99563c530d45e531
parent 135824 3d389787101e13ae30f381be56cb8f687b27e4f6
child 135826 84855cdd91da2c3c1495e08e24ad8b0ebbdddc57
push idunknown
push userunknown
push dateunknown
reviewerskats
bugs884792
milestone24.0a1
bug 884792 - crash in nsXPCWrappedJS::Release, removeObserver being called off main thread r=kats
mobile/android/base/GeckoAppShell.java
mobile/android/base/GeckoEvent.java
mozglue/android/jni-stubs.inc
widget/android/AndroidJNI.cpp
widget/android/AndroidJavaWrappers.cpp
widget/android/AndroidJavaWrappers.h
widget/android/nsAppShell.cpp
widget/android/nsAppShell.h
--- a/mobile/android/base/GeckoAppShell.java
+++ b/mobile/android/base/GeckoAppShell.java
@@ -169,17 +169,19 @@ public class GeckoAppShell
 
     // helper methods
     //    public static native void setSurfaceView(GeckoSurfaceView sv);
     public static native void setLayerClient(GeckoLayerClient client);
     public static native void onResume();
     public static void callObserver(String observerKey, String topic, String data) {
         sendEventToGecko(GeckoEvent.createCallObserverEvent(observerKey, topic, data));
     }
-    public static native void removeObserver(String observerKey);
+    public static void removeObserver(String observerKey) {
+        sendEventToGecko(GeckoEvent.createRemoveObserverEvent(observerKey));
+    }
     public static native Message getNextMessageFromQueue(MessageQueue queue);
     public static native void onSurfaceTextureFrameAvailable(Object surfaceTexture, int id);
 
     public static void registerGlobalExceptionHandler() {
         Thread.setDefaultUncaughtExceptionHandler(new Thread.UncaughtExceptionHandler() {
             @Override
             public void uncaughtException(Thread thread, Throwable e) {
                 // If the uncaught exception was rethrown, walk the exception `cause` chain to find
--- a/mobile/android/base/GeckoEvent.java
+++ b/mobile/android/base/GeckoEvent.java
@@ -59,18 +59,19 @@ public class GeckoEvent {
         THUMBNAIL(25),
         SCREENORIENTATION_CHANGED(27),
         COMPOSITOR_CREATE(28),
         COMPOSITOR_PAUSE(29),
         COMPOSITOR_RESUME(30),
         NATIVE_GESTURE_EVENT(31),
         IME_KEY_EVENT(32),
         CALL_OBSERVER(33),
-        LOW_MEMORY(34),
-        NETWORK_LINK_CHANGE(35);
+        REMOVE_OBSERVER(34),
+        LOW_MEMORY(35),
+        NETWORK_LINK_CHANGE(36);
 
         public final int value;
 
         private NativeGeckoEvent(int value) {
             this.value = value;
          }
     }
 
@@ -664,16 +665,22 @@ public class GeckoEvent {
     public static GeckoEvent createCallObserverEvent(String observerKey, String topic, String data) {
         GeckoEvent event = new GeckoEvent(NativeGeckoEvent.CALL_OBSERVER);
         event.mCharacters = observerKey;
         event.mCharactersExtra = topic;
         event.mData = data;
         return event;
     }
 
+    public static GeckoEvent createRemoveObserverEvent(String observerKey) {
+        GeckoEvent event = new GeckoEvent(NativeGeckoEvent.REMOVE_OBSERVER);
+        event.mCharacters = observerKey;
+        return event;
+    }
+
     public static GeckoEvent createLowMemoryEvent(int level) {
         GeckoEvent event = new GeckoEvent(NativeGeckoEvent.LOW_MEMORY);
         event.mMetaState = level;
         return event;
     }
 
     public static GeckoEvent createNetworkLinkChangeEvent(String status) {
         GeckoEvent event = new GeckoEvent(NativeGeckoEvent.NETWORK_LINK_CHANGE);
--- a/mozglue/android/jni-stubs.inc
+++ b/mozglue/android/jni-stubs.inc
@@ -53,35 +53,16 @@ Java_org_mozilla_gecko_GeckoAppShell_onR
 #endif
 
 #ifdef JNI_BINDINGS
   xul_dlsym("Java_org_mozilla_gecko_GeckoAppShell_onResume", &f_Java_org_mozilla_gecko_GeckoAppShell_onResume);
 #endif
 
 #ifdef JNI_STUBS
 
-typedef void (*Java_org_mozilla_gecko_GeckoAppShell_removeObserver_t)(JNIEnv *, jclass, jstring);
-static Java_org_mozilla_gecko_GeckoAppShell_removeObserver_t f_Java_org_mozilla_gecko_GeckoAppShell_removeObserver;
-extern "C" NS_EXPORT void JNICALL
-Java_org_mozilla_gecko_GeckoAppShell_removeObserver(JNIEnv * arg0, jclass arg1, jstring arg2) {
-    if (!f_Java_org_mozilla_gecko_GeckoAppShell_removeObserver) {
-        arg0->ThrowNew(arg0->FindClass("java/lang/UnsupportedOperationException"),
-                       "JNI Function called before it was loaded");
-        return ;
-    }
-     f_Java_org_mozilla_gecko_GeckoAppShell_removeObserver(arg0, arg1, arg2);
-}
-#endif
-
-#ifdef JNI_BINDINGS
-  xul_dlsym("Java_org_mozilla_gecko_GeckoAppShell_removeObserver", &f_Java_org_mozilla_gecko_GeckoAppShell_removeObserver);
-#endif
-
-#ifdef JNI_STUBS
-
 typedef jobject (*Java_org_mozilla_gecko_GeckoAppShell_getNextMessageFromQueue_t)(JNIEnv *, jclass, jobject);
 static Java_org_mozilla_gecko_GeckoAppShell_getNextMessageFromQueue_t f_Java_org_mozilla_gecko_GeckoAppShell_getNextMessageFromQueue;
 extern "C" NS_EXPORT jobject JNICALL
 Java_org_mozilla_gecko_GeckoAppShell_getNextMessageFromQueue(JNIEnv * arg0, jclass arg1, jobject arg2) {
     if (!f_Java_org_mozilla_gecko_GeckoAppShell_getNextMessageFromQueue) {
         arg0->ThrowNew(arg0->FindClass("java/lang/UnsupportedOperationException"),
                        "JNI Function called before it was loaded");
         return NULL;
--- a/widget/android/AndroidJNI.cpp
+++ b/widget/android/AndroidJNI.cpp
@@ -86,30 +86,16 @@ Java_org_mozilla_gecko_GeckoAppShell_set
 NS_EXPORT void JNICALL
 Java_org_mozilla_gecko_GeckoAppShell_onResume(JNIEnv *jenv, jclass jc)
 {
     if (nsAppShell::gAppShell)
         nsAppShell::gAppShell->OnResume();
 }
 
 NS_EXPORT void JNICALL
-Java_org_mozilla_gecko_GeckoAppShell_removeObserver(JNIEnv *jenv, jclass, jstring jObserverKey)
-{
-    if (!nsAppShell::gAppShell)
-        return;
-
-    const jchar *observerKey = jenv->GetStringChars(jObserverKey, NULL);
-    nsString sObserverKey(observerKey);
-    sObserverKey.SetLength(jenv->GetStringLength(jObserverKey));
-    jenv->ReleaseStringChars(jObserverKey, observerKey);
-
-    nsAppShell::gAppShell->RemoveObserver(sObserverKey);
-}
-
-NS_EXPORT void JNICALL
 Java_org_mozilla_gecko_GeckoAppShell_reportJavaCrash(JNIEnv *jenv, jclass, jstring jStackTrace)
 {
 #ifdef MOZ_CRASHREPORTER
     const nsJNIString stackTrace16(jStackTrace, jenv);
     const NS_ConvertUTF16toUTF8 stackTrace8(stackTrace16);
     CrashReporter::AnnotateCrashReport(NS_LITERAL_CSTRING("JavaStackTrace"), stackTrace8);
 #endif // MOZ_CRASHREPORTER
 
--- a/widget/android/AndroidJavaWrappers.cpp
+++ b/widget/android/AndroidJavaWrappers.cpp
@@ -685,16 +685,21 @@ AndroidGeckoEvent::Init(JNIEnv *jenv, jo
 
         case CALL_OBSERVER: {
             ReadCharactersField(jenv);
             ReadCharactersExtraField(jenv);
             ReadDataField(jenv);
             break;
         }
 
+        case REMOVE_OBSERVER: {
+            ReadCharactersField(jenv);
+            break;
+        }
+
         case LOW_MEMORY: {
             mMetaState = jenv->GetIntField(jobj, jMetaStateField);
             break;
         }
 
         case NETWORK_LINK_CHANGE: {
             ReadCharactersField(jenv);
             break;
--- a/widget/android/AndroidJavaWrappers.h
+++ b/widget/android/AndroidJavaWrappers.h
@@ -702,18 +702,19 @@ public:
         THUMBNAIL = 25,
         SCREENORIENTATION_CHANGED = 27,
         COMPOSITOR_CREATE = 28,
         COMPOSITOR_PAUSE = 29,
         COMPOSITOR_RESUME = 30,
         NATIVE_GESTURE_EVENT = 31,
         IME_KEY_EVENT = 32,
         CALL_OBSERVER = 33,
-        LOW_MEMORY = 34,
-        NETWORK_LINK_CHANGE = 35,
+        REMOVE_OBSERVER = 34,
+        LOW_MEMORY = 35,
+        NETWORK_LINK_CHANGE = 36,
         dummy_java_enum_list_end
     };
 
     enum {
         // Memory pressue levels, keep in sync with those in MemoryMonitor.java
         MEMORY_PRESSURE_NONE = 0,
         MEMORY_PRESSURE_CLEANUP = 1,
         MEMORY_PRESSURE_LOW = 2,
--- a/widget/android/nsAppShell.cpp
+++ b/widget/android/nsAppShell.cpp
@@ -515,17 +515,32 @@ nsAppShell::ProcessNextNativeEvent(bool 
             static_cast<dom::ScreenOrientation>(curEvent->ScreenOrientation());
 
         hal::NotifyScreenConfigurationChange(
             hal::ScreenConfiguration(rect, orientation, colorDepth, pixelDepth));
         break;
     }
 
     case AndroidGeckoEvent::CALL_OBSERVER:
-        CallObserver(curEvent->Characters(), curEvent->CharactersExtra(), curEvent->Data());
+    {
+        nsCOMPtr<nsIObserver> observer;
+        mObserversHash.Get(curEvent->Characters(), getter_AddRefs(observer));
+
+        if (observer) {
+            observer->Observe(nullptr, NS_ConvertUTF16toUTF8(curEvent->CharactersExtra()).get(),
+                              nsString(curEvent->Data()).get());
+        } else {
+            ALOG("Call_Observer event: Observer was not found!");
+        }
+
+        break;
+    }
+
+    case AndroidGeckoEvent::REMOVE_OBSERVER:
+        mObserversHash.Remove(curEvent->Characters());
         break;
 
     case AndroidGeckoEvent::LOW_MEMORY:
         // TODO hook in memory-reduction stuff for different levels here
         if (curEvent->MetaState() >= AndroidGeckoEvent::MEMORY_PRESSURE_MEDIUM) {
             nsCOMPtr<nsIObserverService> os = mozilla::services::GetObserverService();
             if (os) {
                 os->NotifyObservers(nullptr,
@@ -731,40 +746,16 @@ nsAppShell::OnResume()
 nsresult
 nsAppShell::AddObserver(const nsAString &aObserverKey, nsIObserver *aObserver)
 {
     NS_ASSERTION(aObserver != nullptr, "nsAppShell::AddObserver: aObserver is null!");
     mObserversHash.Put(aObserverKey, aObserver);
     return NS_OK;
 }
 
-void
-nsAppShell::CallObserver(const nsAString &aObserverKey, const nsAString &aTopic, const nsAString &aData)
-{
-    nsCOMPtr<nsIObserver> observer;
-    mObserversHash.Get(aObserverKey, getter_AddRefs(observer));
-
-    if (!observer) {
-        ALOG("nsAppShell::CallObserver: Observer was not found!");
-        return;
-    }
-
-    const NS_ConvertUTF16toUTF8 sTopic(aTopic);
-    const nsPromiseFlatString& sData = PromiseFlatString(aData);
-
-    MOZ_ASSERT(NS_IsMainThread());
-    observer->Observe(nullptr, sTopic.get(), sData.get());
-}
-
-void
-nsAppShell::RemoveObserver(const nsAString &aObserverKey)
-{
-    mObserversHash.Remove(aObserverKey);
-}
-
 // Used by IPC code
 namespace mozilla {
 
 bool ProcessNextEvent()
 {
     return nsAppShell::gAppShell->ProcessNextNativeEvent(true) ? true : false;
 }
 
--- a/widget/android/nsAppShell.h
+++ b/widget/android/nsAppShell.h
@@ -42,18 +42,16 @@ public:
     void NotifyNativeEvent();
 
     virtual bool ProcessNextNativeEvent(bool mayWait);
 
     void PostEvent(mozilla::AndroidGeckoEvent *event);
     void OnResume();
 
     nsresult AddObserver(const nsAString &aObserverKey, nsIObserver *aObserver);
-    void CallObserver(const nsAString &aObserverKey, const nsAString &aTopic, const nsAString &aData);
-    void RemoveObserver(const nsAString &aObserverKey);
     void ResendLastResizeEvent(nsWindow* aDest);
 
     void SetBrowserApp(nsIAndroidBrowserApp* aBrowserApp) {
         mBrowserApp = aBrowserApp;
     }
 
     void GetBrowserApp(nsIAndroidBrowserApp* *aBrowserApp) {
         *aBrowserApp = mBrowserApp;