Bug 1330515 - Try to recover from IME errors; r=esawin

Instead of throwing IME exceptions, try to recover from IME errors by flushing the entire text unless we already tried that before. This prevents annoying crashes, and deals with known IME bugs that are too risky to uplift to older releases.
dothq · Jan 13, 2017 · f48add8 · f48add8
1 parent 3b07a04
commit f48add8
Show file tree

Hide file tree

Showing 3 changed files with 35 additions and 9 deletions.
diff --git a/mobile/android/geckoview/src/main/java/org/mozilla/gecko/GeckoEditable.java b/mobile/android/geckoview/src/main/java/org/mozilla/gecko/GeckoEditable.java
@@ -1135,7 +1135,7 @@ public void run() {
         });
     }
 
-    @WrapForJNI(calledFrom = "gecko")
+    @WrapForJNI(calledFrom = "gecko", exceptionMode = "ignore")
     private void onSelectionChange(final int start, final int end) {
         if (DEBUG) {
             // GeckoEditableListener methods should all be called from the Gecko thread
@@ -1164,7 +1164,7 @@ private boolean geckoIsSameText(int start, int oldEnd, CharSequence newText) {
                TextUtils.regionMatches(mText.getCurrentText(), start, newText, 0, oldEnd - start);
     }
 
-    @WrapForJNI(calledFrom = "gecko")
+    @WrapForJNI(calledFrom = "gecko", exceptionMode = "ignore")
     private void onTextChange(final CharSequence text, final int start,
                               final int unboundedOldEnd, final int unboundedNewEnd) {
         if (DEBUG) {

diff --git a/widget/android/GeneratedJNIWrappers.h b/widget/android/GeneratedJNIWrappers.h
@@ -2340,7 +2340,7 @@ class GeckoEditable : public mozilla::jni::ObjectBase<GeckoEditable>
                 "(II)V";
         static const bool isStatic = false;
         static const mozilla::jni::ExceptionMode exceptionMode =
-                mozilla::jni::ExceptionMode::ABORT;
+                mozilla::jni::ExceptionMode::IGNORE;
         static const mozilla::jni::CallingThread callingThread =
                 mozilla::jni::CallingThread::GECKO;
         static const mozilla::jni::DispatchTarget dispatchTarget =
@@ -2363,7 +2363,7 @@ class GeckoEditable : public mozilla::jni::ObjectBase<GeckoEditable>
                 "(Ljava/lang/CharSequence;III)V";
         static const bool isStatic = false;
         static const mozilla::jni::ExceptionMode exceptionMode =
-                mozilla::jni::ExceptionMode::ABORT;
+                mozilla::jni::ExceptionMode::IGNORE;
         static const mozilla::jni::CallingThread callingThread =
                 mozilla::jni::CallingThread::GECKO;
         static const mozilla::jni::DispatchTarget dispatchTarget =

diff --git a/widget/android/nsWindow.cpp b/widget/android/nsWindow.cpp
@@ -415,12 +415,16 @@ class nsWindow::GeckoViewSupport final
     void AddIMETextChange(const IMETextChange& aChange);
 
     enum FlushChangesFlag {
+        // Not retrying.
         FLUSH_FLAG_NONE,
-        FLUSH_FLAG_RETRY
+        // Retrying due to IME text changes during flush.
+        FLUSH_FLAG_RETRY,
+        // Retrying due to IME sync exceptions during flush.
+        FLUSH_FLAG_RECOVER
     };
     void PostFlushIMEChanges();
     void FlushIMEChanges(FlushChangesFlag aFlags = FLUSH_FLAG_NONE);
-    void FlushIMEText();
+    void FlushIMEText(FlushChangesFlag aFlags = FLUSH_FLAG_NONE);
     void AsyncNotifyIME(int32_t aNotification);
     void UpdateCompositionRects();
 
@@ -2750,7 +2754,7 @@ nsWindow::GeckoViewSupport::FlushIMEChanges(FlushChangesFlag aFlags)
         // A query event could have triggered more text changes to come in, as
         // indicated by our flag. If that happens, try flushing IME changes
         // again.
-        if (aFlags != FLUSH_FLAG_RETRY) {
+        if (aFlags == FLUSH_FLAG_NONE) {
             FlushIMEChanges(FLUSH_FLAG_RETRY);
         } else {
             // Don't retry if already retrying, to avoid infinite loops.
@@ -2805,22 +2809,44 @@ nsWindow::GeckoViewSupport::FlushIMEChanges(FlushChangesFlag aFlags)
         selEnd = int32_t(event.GetSelectionEnd());
     }
 
+    JNIEnv* const env = jni::GetGeckoThreadEnv();
+    auto flushOnException = [=] () -> bool {
+        if (!env->ExceptionCheck()) {
+            return false;
+        }
+        if (aFlags != FLUSH_FLAG_RECOVER) {
+            // First time seeing an exception; try flushing text.
+            env->ExceptionClear();
+            __android_log_print(ANDROID_LOG_WARN, "GeckoViewSupport",
+                    "Recovering from IME exception");
+            FlushIMEText(FLUSH_FLAG_RECOVER);
+        } else {
+            // Give up because we've already tried.
+            MOZ_CATCH_JNI_EXCEPTION(env);
+        }
+        return true;
+    };
+
     // Commit the text change and selection change transaction.
     mIMETextChanges.Clear();
 
     for (const TextRecord& record : textTransaction) {
         mEditable->OnTextChange(record.text, record.start,
                                 record.oldEnd, record.newEnd);
+        if (flushOnException()) {
+            return;
+        }
     }
 
     if (mIMESelectionChanged) {
         mIMESelectionChanged = false;
         mEditable->OnSelectionChange(selStart, selEnd);
+        flushOnException();
     }
 }
 
 void
-nsWindow::GeckoViewSupport::FlushIMEText()
+nsWindow::GeckoViewSupport::FlushIMEText(FlushChangesFlag aFlags)
 {
     // Notify Java of the newly focused content
     mIMETextChanges.Clear();
@@ -2835,7 +2861,7 @@ nsWindow::GeckoViewSupport::FlushIMEText()
     notification.mTextChangeData.mAddedEndOffset = INT32_MAX / 2;
     NotifyIME(notification);
 
-    FlushIMEChanges();
+    FlushIMEChanges(aFlags);
 }
 
 static jni::ObjectArray::LocalRef