Bug 1236643 - Reorder GeckoEditable destruction sequence; r=esawin

To guarantee that GeckoInputConnection and GeckoEditable are not used by
GeckoView after GeckoEditable has been destroyed, we need to make sure a
certain sequence is followed. We should first unset the
InputConnectionListener in GeckoView on the UI thread; then unset the
GeckoEditableListener on the IC thread; and finally finish destroying
the GeckoEditable instance through disposeNative. This patch merges this
logic with the initialization logic in GeckoEditable.onViewChange, so
that onViewChange can be used for both initialization and destruction.

mobile/android/base/java/org/mozilla/gecko/GeckoEditable.java

@@ -65,7 +65,8 @@ final class GeckoEditable extends JNIObject
     private Handler mIcRunHandler;
     private Handler mIcPostHandler;
 
-    private GeckoEditableListener mListener;
+    /* package */ GeckoEditableListener mListener;
+    /* package */ GeckoView mView;
     /* package */ boolean mInBatchMode; // Used by IC thread
     /* package */ boolean mNeedCompositionUpdate; // Used by IC thread
     private boolean mFocused; // Used by IC thread
@@ -235,6 +236,11 @@ final class GeckoEditable extends JNIObject
                               getConstantName(Action.class, "TYPE_", action.mType) + ")");
             }
 
+            if (mListener == null) {
+                // We haven't initialized or we've been destroyed.
+                return;
+            }
+
             if (mActions.isEmpty()) {
                 mActionsActive.acquireUninterruptibly();
                 mActions.offer(action);
@@ -394,24 +400,6 @@ final class GeckoEditable extends JNIObject
     @WrapForJNI @Override
     protected native void disposeNative();
 
-    @WrapForJNI
-    private void onDestroy() {
-        if (DEBUG) {
-            // Called by nsWindow.
-            ThreadUtils.assertOnGeckoThread();
-            Log.d(LOGTAG, "onDestroy()");
-        }
-
-        // Make sure we clear all pending Runnables on the IC thread first,
-        // by calling disposeNative from the IC thread.
-        geckoPostToIc(new Runnable() {
-            @Override
-            public void run() {
-                GeckoEditable.this.disposeNative();
-            }
-        });
-    }
-
     @WrapForJNI
     /* package */ void onViewChange(final GeckoView v) {
         if (DEBUG) {
@@ -420,26 +408,51 @@ final class GeckoEditable extends JNIObject
             Log.d(LOGTAG, "onViewChange(" + v + ")");
         }
 
-        final GeckoEditableListener newListener = GeckoInputConnection.create(v, this);
-        geckoPostToIc(new Runnable() {
+        final GeckoEditableListener newListener =
+            v != null ? GeckoInputConnection.create(v, this) : null;
+
+        final Runnable setListenerRunnable = new Runnable() {
             @Override
             public void run() {
                 if (DEBUG) {
                     Log.d(LOGTAG, "onViewChange (set listener)");
                 }
-                // Make sure there are no other things going on
-                mActionQueue.syncWithGecko();
-                mListener = newListener;
-            }
-        });
 
+                if (newListener != null) {
+                    // Make sure there are no other things going on.
+                    mActionQueue.syncWithGecko();
+                    mListener = newListener;
+                } else {
+                    // We're being destroyed. By this point, we should have cleared all
+                    // pending Runnables on the IC thread, so it's safe to call
+                    // disposeNative here.
+                    mListener = null;
+                    GeckoEditable.this.disposeNative();
+                }
+            }
+        };
+
+        // Post to UI thread first to make sure any code that is using the old input
+        // connection has finished running, before we switch to a new input connection or
+        // before we clear the input connection on destruction.
         ThreadUtils.postToUiThread(new Runnable() {
             @Override
             public void run() {
                 if (DEBUG) {
                     Log.d(LOGTAG, "onViewChange (set IC)");
                 }
-                v.setInputConnectionListener((InputConnectionListener) newListener);
+
+                if (mView != null) {
+                    // Detach the previous view.
+                    mView.setInputConnectionListener(null);
+                }
+                if (v != null) {
+                    // And attach the new view.
+                    v.setInputConnectionListener((InputConnectionListener) newListener);
+                }
+
+                mView = v;
+                mIcPostHandler.post(setListenerRunnable);
             }
         });
     }
@@ -660,6 +673,10 @@ final class GeckoEditable extends JNIObject
             }
             return null;
         }
+        if (mListener == null) {
+            // We haven't initialized or we've been destroyed.
+            return null;
+        }
         return mProxy;
     }
 

widget/android/GeneratedJNIWrappers.cpp

@@ -763,14 +763,6 @@ auto GeckoEditable::NotifyIMEContext(int32_t a0, mozilla::jni::String::Param a1,
     return mozilla::jni::Method<NotifyIMEContext_t>::Call(this, nullptr, a0, a1, a2, a3);
 }
 
-constexpr char GeckoEditable::OnDestroy_t::name[];
-constexpr char GeckoEditable::OnDestroy_t::signature[];
-
-auto GeckoEditable::OnDestroy() const -> void
-{
-    return mozilla::jni::Method<OnDestroy_t>::Call(this, nullptr);
-}
-
 constexpr char GeckoEditable::OnImeAcknowledgeFocus_t::name[];
 constexpr char GeckoEditable::OnImeAcknowledgeFocus_t::signature[];
 

widget/android/GeneratedJNIWrappers.h

@@ -1829,23 +1829,6 @@ public:
 
     auto NotifyIMEContext(int32_t, mozilla::jni::String::Param, mozilla::jni::String::Param, mozilla::jni::String::Param) const -> void;
 
-public:
-    struct OnDestroy_t {
-        typedef GeckoEditable Owner;
-        typedef void ReturnType;
-        typedef void SetterType;
-        typedef mozilla::jni::Args<> Args;
-        static constexpr char name[] = "onDestroy";
-        static constexpr char signature[] =
-                "()V";
-        static const bool isStatic = false;
-        static const bool isMultithreaded = false;
-        static const mozilla::jni::ExceptionMode exceptionMode =
-                mozilla::jni::ExceptionMode::ABORT;
-    };
-
-    auto OnDestroy() const -> void;
-
 public:
     struct OnImeAcknowledgeFocus_t {
         typedef GeckoEditable Owner;

widget/android/nsWindow.cpp

@@ -616,7 +616,7 @@ nsWindow::GeckoViewSupport::~GeckoViewSupport()
     // OnDestroy will call disposeNative after any pending native calls have
     // been made.
     MOZ_ASSERT(mEditable);
-    mEditable->OnDestroy();
+    mEditable->OnViewChange(nullptr);
 }
 
 /* static */ void