Bug 1330515 - Try to recover from IME errors. r=esawin, a=lizzard
authorJim Chen <nchen@mozilla.com>
Fri, 13 Jan 2017 14:39:27 -0500
changeset 353640 11c4cd93ffc02884634f45abec0ca737df7c4d3b
parent 353639 b68fa747093c8e4100dcbbb696b24ac200d1d03e
child 353641 1e9635dd5012725bd5a7d8559e3c8dd3c2ce676f
push id6795
push userjlund@mozilla.com
push dateMon, 23 Jan 2017 14:19:46 +0000
treeherdermozilla-esr52@76101b503191 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersesawin, lizzard
bugs1330515
milestone52.0a2
Bug 1330515 - Try to recover from IME errors. r=esawin, a=lizzard 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.
mobile/android/geckoview/src/main/java/org/mozilla/gecko/GeckoEditable.java
widget/android/GeneratedJNIWrappers.h
widget/android/nsWindow.cpp
--- a/mobile/android/geckoview/src/main/java/org/mozilla/gecko/GeckoEditable.java
+++ b/mobile/android/geckoview/src/main/java/org/mozilla/gecko/GeckoEditable.java
@@ -1105,17 +1105,17 @@ final class GeckoEditable extends JNIObj
                 if (mListener == null) {
                     return;
                 }
                 mListener.notifyIMEContext(state, typeHint, modeHint, actionHint);
             }
         });
     }
 
-    @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
             ThreadUtils.assertOnGeckoThread();
             Log.d(LOGTAG, "onSelectionChange(" + start + ", " + end + ")");
         }
 
         final int currentLength = mText.getCurrentText().length();
@@ -1134,17 +1134,17 @@ final class GeckoEditable extends JNIObj
         geckoScheduleSyncShadowText();
     }
 
     private boolean geckoIsSameText(int start, int oldEnd, CharSequence newText) {
         return oldEnd - start == newText.length() &&
                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) {
             // GeckoEditableListener methods should all be called from the Gecko thread
             ThreadUtils.assertOnGeckoThread();
             StringBuilder sb = new StringBuilder("onTextChange(");
             debugAppend(sb, text);
             sb.append(", ").append(start).append(", ")
--- a/widget/android/GeneratedJNIWrappers.h
+++ b/widget/android/GeneratedJNIWrappers.h
@@ -2089,17 +2089,17 @@ public:
         typedef mozilla::jni::Args<
                 int32_t,
                 int32_t> Args;
         static constexpr char name[] = "onSelectionChange";
         static constexpr char signature[] =
                 "(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 =
                 mozilla::jni::DispatchTarget::CURRENT;
     };
 
     auto OnSelectionChange(int32_t, int32_t) const -> void;
 
@@ -2112,17 +2112,17 @@ public:
                 int32_t,
                 int32_t,
                 int32_t> Args;
         static constexpr char name[] = "onTextChange";
         static constexpr char signature[] =
                 "(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 =
                 mozilla::jni::DispatchTarget::CURRENT;
     };
 
     auto OnTextChange(mozilla::jni::String::Param, int32_t, int32_t, int32_t) const -> void;
 
--- a/widget/android/nsWindow.cpp
+++ b/widget/android/nsWindow.cpp
@@ -405,22 +405,26 @@ private:
     bool mIMESelectionChanged;
     bool mIMETextChangedDuringFlush;
     bool mIMEMonitorCursor;
 
     void SendIMEDummyKeyEvents();
     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();
 
 public:
     bool NotifyIME(const IMENotification& aIMENotification);
     void SetInputContext(const InputContext& aContext,
                          const InputContextAction& aAction);
     InputContext GetInputContext();
@@ -2746,17 +2750,17 @@ nsWindow::GeckoViewSupport::FlushIMEChan
 
     auto shouldAbort = [=] () -> bool {
         if (!mIMETextChangedDuringFlush) {
             return false;
         }
         // 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.
             __android_log_print(ANDROID_LOG_WARN, "GeckoViewSupport",
                     "Already retrying IME flush");
         }
         return true;
     };
@@ -2801,47 +2805,69 @@ nsWindow::GeckoViewSupport::FlushIMEChan
         if (shouldAbort()) {
             return;
         }
 
         selStart = int32_t(event.GetSelectionStart());
         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();
     mIMESelectionChanged = true;
 
     // Use 'INT32_MAX / 2' here because subsequent text changes might combine
     // with this text change, and overflow might occur if we just use
     // INT32_MAX.
     IMENotification notification(NOTIFY_IME_OF_TEXT_CHANGE);
     notification.mTextChangeData.mStartOffset = 0;
     notification.mTextChangeData.mRemovedEndOffset = INT32_MAX / 2;
     notification.mTextChangeData.mAddedEndOffset = INT32_MAX / 2;
     NotifyIME(notification);
 
-    FlushIMEChanges();
+    FlushIMEChanges(aFlags);
 }
 
 static jni::ObjectArray::LocalRef
 ConvertRectArrayToJavaRectFArray(JNIEnv* aJNIEnv, const nsTArray<LayoutDeviceIntRect>& aRects, const LayoutDeviceIntPoint& aOffset, const CSSToLayoutDeviceScale aScale)
 {
     size_t length = aRects.Length();
     jobjectArray rects = aJNIEnv->NewObjectArray(length, sdk::RectF::Context().ClassRef(), nullptr);
     auto rectsRef = jni::ObjectArray::LocalRef::Adopt(aJNIEnv, rects);