bug 882196 - Android crash in nsXPCWrappedJS::AddRef, remove nsAppShell::CallObservers r=snorp
authorBrad Lassey <blassey@mozilla.com>
Sat, 15 Jun 2013 17:40:27 -0400
changeset 135366 f7628d639dd1b86866c66a58ba511f962ce2c6b7
parent 135365 d496fdada911f9f7647b5ef62e4815800aec4344
child 135367 35b943a379dded84def27a77d9d8dc907a4abfe5
push idunknown
push userunknown
push dateunknown
reviewerssnorp
bugs882196
milestone24.0a1
bug 882196 - Android crash in nsXPCWrappedJS::AddRef, remove nsAppShell::CallObservers r=snorp
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
--- a/mobile/android/base/GeckoAppShell.java
+++ b/mobile/android/base/GeckoAppShell.java
@@ -167,17 +167,19 @@ public class GeckoAppShell
     // Initialization methods
     public static native void nativeInit();
 
     // helper methods
     //    public static native void setSurfaceView(GeckoSurfaceView sv);
     public static native void setLayerClient(GeckoLayerClient client);
     public static native void onResume();
     public static native void onLowMemory();
-    public static native void callObserver(String observerKey, String topic, String data);
+    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 native void onChangeNetworkLinkStatus(String status);
     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
--- a/mobile/android/base/GeckoEvent.java
+++ b/mobile/android/base/GeckoEvent.java
@@ -57,17 +57,18 @@ public class GeckoEvent {
         VISITED(21),
         NETWORK_CHANGED(22),
         THUMBNAIL(25),
         SCREENORIENTATION_CHANGED(27),
         COMPOSITOR_CREATE(28),
         COMPOSITOR_PAUSE(29),
         COMPOSITOR_RESUME(30),
         NATIVE_GESTURE_EVENT(31),
-        IME_KEY_EVENT(32);
+        IME_KEY_EVENT(32),
+        CALL_OBSERVER(33);
 
         public final int value;
 
         private NativeGeckoEvent(int value) {
             this.value = value;
          }
     }
 
@@ -150,16 +151,17 @@ public class GeckoEvent {
     private int mUnicodeChar;
     private int mBaseUnicodeChar; // mUnicodeChar without meta states applied
     private int mRepeatCount;
     private int mCount;
     private int mStart;
     private int mEnd;
     private String mCharacters;
     private String mCharactersExtra;
+    private String mData;
     private int mRangeType;
     private int mRangeStyles;
     private int mRangeLineStyle;
     private boolean mRangeBoldLine;
     private int mRangeForeColor;
     private int mRangeBackColor;
     private int mRangeLineColor;
     private Location mLocation;
@@ -652,12 +654,20 @@ public class GeckoEvent {
     }
 
     public static GeckoEvent createScreenOrientationEvent(short aScreenOrientation) {
         GeckoEvent event = new GeckoEvent(NativeGeckoEvent.SCREENORIENTATION_CHANGED);
         event.mScreenOrientation = aScreenOrientation;
         return event;
     }
 
+    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 void setAckNeeded(boolean ackNeeded) {
         mAckNeeded = ackNeeded;
     }
 }
--- a/mozglue/android/jni-stubs.inc
+++ b/mozglue/android/jni-stubs.inc
@@ -72,35 +72,16 @@ Java_org_mozilla_gecko_GeckoAppShell_onL
 #endif
 
 #ifdef JNI_BINDINGS
   xul_dlsym("Java_org_mozilla_gecko_GeckoAppShell_onLowMemory", &f_Java_org_mozilla_gecko_GeckoAppShell_onLowMemory);
 #endif
 
 #ifdef JNI_STUBS
 
-typedef void (*Java_org_mozilla_gecko_GeckoAppShell_callObserver_t)(JNIEnv *, jclass, jstring, jstring, jstring);
-static Java_org_mozilla_gecko_GeckoAppShell_callObserver_t f_Java_org_mozilla_gecko_GeckoAppShell_callObserver;
-extern "C" NS_EXPORT void JNICALL
-Java_org_mozilla_gecko_GeckoAppShell_callObserver(JNIEnv * arg0, jclass arg1, jstring arg2, jstring arg3, jstring arg4) {
-    if (!f_Java_org_mozilla_gecko_GeckoAppShell_callObserver) {
-        arg0->ThrowNew(arg0->FindClass("java/lang/UnsupportedOperationException"),
-                       "JNI Function called before it was loaded");
-        return ;
-    }
-     f_Java_org_mozilla_gecko_GeckoAppShell_callObserver(arg0, arg1, arg2, arg3, arg4);
-}
-#endif
-
-#ifdef JNI_BINDINGS
-  xul_dlsym("Java_org_mozilla_gecko_GeckoAppShell_callObserver", &f_Java_org_mozilla_gecko_GeckoAppShell_callObserver);
-#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 ;
--- a/widget/android/AndroidJNI.cpp
+++ b/widget/android/AndroidJNI.cpp
@@ -97,29 +97,16 @@ Java_org_mozilla_gecko_GeckoAppShell_onL
 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_callObserver(JNIEnv *jenv, jclass, jstring jObserverKey, jstring jTopic, jstring jData)
-{
-    if (!nsAppShell::gAppShell)
-        return;
-
-    nsJNIString sObserverKey(jObserverKey, jenv);
-    nsJNIString sTopic(jTopic, jenv);
-    nsJNIString sData(jData, jenv);
-
-    nsAppShell::gAppShell->CallObserver(sObserverKey, sTopic, sData);
-}
-
-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));
--- a/widget/android/AndroidJavaWrappers.cpp
+++ b/widget/android/AndroidJavaWrappers.cpp
@@ -27,16 +27,17 @@ jfieldID AndroidGeckoEvent::jXField = 0;
 jfieldID AndroidGeckoEvent::jYField = 0;
 jfieldID AndroidGeckoEvent::jZField = 0;
 jfieldID AndroidGeckoEvent::jDistanceField = 0;
 jfieldID AndroidGeckoEvent::jRectField = 0;
 jfieldID AndroidGeckoEvent::jNativeWindowField = 0;
 
 jfieldID AndroidGeckoEvent::jCharactersField = 0;
 jfieldID AndroidGeckoEvent::jCharactersExtraField = 0;
+jfieldID AndroidGeckoEvent::jDataField = 0;
 jfieldID AndroidGeckoEvent::jKeyCodeField = 0;
 jfieldID AndroidGeckoEvent::jMetaStateField = 0;
 jfieldID AndroidGeckoEvent::jDomKeyLocationField = 0;
 jfieldID AndroidGeckoEvent::jFlagsField = 0;
 jfieldID AndroidGeckoEvent::jUnicodeCharField = 0;
 jfieldID AndroidGeckoEvent::jBaseUnicodeCharField = 0;
 jfieldID AndroidGeckoEvent::jRepeatCountField = 0;
 jfieldID AndroidGeckoEvent::jCountField = 0;
@@ -230,16 +231,17 @@ AndroidGeckoEvent::InitGeckoEventClass(J
     jPointRadii = getField("mPointRadii", "[Landroid/graphics/Point;");
     jXField = getField("mX", "D");
     jYField = getField("mY", "D");
     jZField = getField("mZ", "D");
     jRectField = getField("mRect", "Landroid/graphics/Rect;");
 
     jCharactersField = getField("mCharacters", "Ljava/lang/String;");
     jCharactersExtraField = getField("mCharactersExtra", "Ljava/lang/String;");
+    jDataField = getField("mData", "Ljava/lang/String;");
     jKeyCodeField = getField("mKeyCode", "I");
     jMetaStateField = getField("mMetaState", "I");
     jDomKeyLocationField = getField("mDomKeyLocation", "Lorg/mozilla/gecko/GeckoEvent$DomKeyLocation;");
     jFlagsField = getField("mFlags", "I");
     jUnicodeCharField = getField("mUnicodeChar", "I");
     jBaseUnicodeCharField = getField("mBaseUnicodeChar", "I");
     jRepeatCountField = getField("mRepeatCount", "I");
     jCountField = getField("mCount", "I");
@@ -507,16 +509,30 @@ AndroidGeckoEvent::ReadCharactersExtraFi
     }
 
     int len = jenv->GetStringLength(s);
     mCharactersExtra.SetLength(len);
     jenv->GetStringRegion(s, 0, len, mCharactersExtra.BeginWriting());
 }
 
 void
+AndroidGeckoEvent::ReadDataField(JNIEnv *jenv)
+{
+    jstring s = (jstring) jenv->GetObjectField(wrapped_obj, jDataField);
+    if (!s) {
+        mData.SetIsVoid(true);
+        return;
+    }
+
+    int len = jenv->GetStringLength(s);
+    mData.SetLength(len);
+    jenv->GetStringRegion(s, 0, len, mData.BeginWriting());
+}
+
+void
 AndroidGeckoEvent::UnionRect(nsIntRect const& aRect)
 {
     mRect = aRect.Union(mRect);
 }
 
 uint32_t
 AndroidGeckoEvent::ReadDomKeyLocation(JNIEnv* jenv, jobject jGeckoEventObj)
 {
@@ -662,16 +678,23 @@ AndroidGeckoEvent::Init(JNIEnv *jenv, jo
         }
 
         case COMPOSITOR_CREATE: {
             mWidth = jenv->GetIntField(jobj, jWidthField);
             mHeight = jenv->GetIntField(jobj, jHeightField);
             break;
         }
 
+        case CALL_OBSERVER: {
+            ReadCharactersField(jenv);
+            ReadCharactersExtraField(jenv);
+            ReadDataField(jenv);
+            break;
+        }
+
         default:
             break;
     }
 
 #ifdef DEBUG_ANDROID_EVENTS
     ALOG("AndroidGeckoEvent: %p : %d", (void*)jobj, mType);
 #endif
 }
--- a/widget/android/AndroidJavaWrappers.h
+++ b/widget/android/AndroidJavaWrappers.h
@@ -540,16 +540,17 @@ public:
     const nsTArray<float>& Orientations() { return mOrientations; }
     const nsTArray<nsIntPoint>& PointRadii() { return mPointRadii; }
     double X() { return mX; }
     double Y() { return mY; }
     double Z() { return mZ; }
     const nsIntRect& Rect() { return mRect; }
     nsAString& Characters() { return mCharacters; }
     nsAString& CharactersExtra() { return mCharactersExtra; }
+    nsAString& Data() { return mData; }
     int KeyCode() { return mKeyCode; }
     int MetaState() { return mMetaState; }
     uint32_t DomKeyLocation() { return mDomKeyLocation; }
     bool IsAltPressed() const { return (mMetaState & AMETA_ALT_MASK) != 0; }
     bool IsShiftPressed() const { return (mMetaState & AMETA_SHIFT_MASK) != 0; }
     bool IsCtrlPressed() const { return (mMetaState & AMETA_CTRL_MASK) != 0; }
     bool IsMetaPressed() const { return (mMetaState & AMETA_META_MASK) != 0; }
     int Flags() { return mFlags; }
@@ -595,17 +596,17 @@ protected:
     int mRepeatCount;
     int mCount;
     int mStart, mEnd;
     int mRangeType, mRangeStyles, mRangeLineStyle;
     bool mRangeBoldLine;
     int mRangeForeColor, mRangeBackColor, mRangeLineColor;
     double mX, mY, mZ;
     int mPointerIndex;
-    nsString mCharacters, mCharactersExtra;
+    nsString mCharacters, mCharactersExtra, mData;
     nsRefPtr<nsGeoPosition> mGeoPosition;
     double mBandwidth;
     bool mCanBeMetered;
     short mScreenOrientation;
     nsRefPtr<RefCountedJavaObject> mByteBuffer;
     int mWidth, mHeight;
 
     void ReadIntArray(nsTArray<int> &aVals,
@@ -618,16 +619,17 @@ protected:
                         int32_t count);
     void ReadPointArray(nsTArray<nsIntPoint> &mPoints,
                         JNIEnv *jenv,
                         jfieldID field,
                         int32_t count);
     void ReadRectField(JNIEnv *jenv);
     void ReadCharactersField(JNIEnv *jenv);
     void ReadCharactersExtraField(JNIEnv *jenv);
+    void ReadDataField(JNIEnv *jenv);
 
     uint32_t ReadDomKeyLocation(JNIEnv* jenv, jobject jGeckoEventObj);
 
     static jclass jGeckoEventClass;
     static jfieldID jActionField;
     static jfieldID jTypeField;
     static jfieldID jAckNeededField;
     static jfieldID jTimeField;
@@ -640,16 +642,17 @@ protected:
     static jfieldID jYField;
     static jfieldID jZField;
     static jfieldID jDistanceField;
     static jfieldID jRectField;
     static jfieldID jNativeWindowField;
 
     static jfieldID jCharactersField;
     static jfieldID jCharactersExtraField;
+    static jfieldID jDataField;
     static jfieldID jKeyCodeField;
     static jfieldID jMetaStateField;
     static jfieldID jDomKeyLocationField;
     static jfieldID jFlagsField;
     static jfieldID jCountField;
     static jfieldID jStartField;
     static jfieldID jEndField;
     static jfieldID jPointerIndexField;
@@ -698,16 +701,17 @@ public:
         NETWORK_CHANGED = 22,
         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,
         dummy_java_enum_list_end
     };
 
     enum {
         // Internal Gecko events
         IME_FLUSH_CHANGES = -2,
         IME_UPDATE_CONTEXT = -1,
         // Events from Java to Gecko
--- a/widget/android/nsAppShell.cpp
+++ b/widget/android/nsAppShell.cpp
@@ -513,16 +513,20 @@ nsAppShell::ProcessNextNativeEvent(bool 
         orientation =
             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());
+        break;
+
     case AndroidGeckoEvent::NOOP:
         break;
 
     default:
         nsWindow::OnGlobalAndroidEvent(curEvent);
         break;
     }
 
@@ -703,63 +707,32 @@ 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;
 }
 
-/**
- * The XPCOM event that will call the observer on the main thread.
- */
-class ObserverCaller : public nsRunnable {
-public:
-    ObserverCaller(nsIObserver *aObserver, const char *aTopic, const PRUnichar *aData) :
-        mObserver(aObserver), mTopic(aTopic), mData(aData) {
-        NS_ASSERTION(aObserver != nullptr, "ObserverCaller: aObserver is null!");
-    }
-
-    NS_IMETHOD Run() {
-        ALOG("ObserverCaller::Run: observer = %p, topic = '%s')",
-             (nsIObserver*)mObserver, mTopic.get());
-        mObserver->Observe(nullptr, mTopic.get(), mData.get());
-        return NS_OK;
-    }
-
-private:
-    nsCOMPtr<nsIObserver> mObserver;
-    nsCString mTopic;
-    nsString mData;
-};
-
 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);
 
-    if (NS_IsMainThread()) {
-        // This branch will unlikely be hit, have it just in case
-        observer->Observe(nullptr, sTopic.get(), sData.get());
-    } else {
-        // Java is not running on main thread, so we have to use NS_DispatchToMainThread
-        nsCOMPtr<nsIRunnable> observerCaller = new ObserverCaller(observer, sTopic.get(), sData.get());
-        nsresult rv = NS_DispatchToMainThread(observerCaller);
-        ALOG("NS_DispatchToMainThread result: %d", rv);
-        unused << rv;
-    }
+    MOZ_ASSERT(NS_IsMainThread());
+    observer->Observe(nullptr, sTopic.get(), sData.get());
 }
 
 void
 nsAppShell::RemoveObserver(const nsAString &aObserverKey)
 {
     mObserversHash.Remove(aObserverKey);
 }