Bug 1330515 - Try to recover from IME errors. r=esawin. a=lizzard
authorJim Chen <nchen@mozilla.com>
Wed, 18 Jan 2017 15:55:37 +0800
changeset 465639 65c61eec146d3d46eafe622641d49ef665ae3786
parent 465638 dd62b8f1c3620150afc8b4742f7541f8c652ccc4
child 465640 4025860dec4be8fa9490e9351fadd9119c7c1262
push id42661
push userfelipc@gmail.com
push dateTue, 24 Jan 2017 16:14:20 +0000
reviewersesawin, lizzard
bugs1330515
milestone51.0
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
@@ -969,17 +969,17 @@ final class GeckoEditable extends JNIObj
                 if (mListener == null) {
                     return;
                 }
                 mListener.notifyIMEContext(state, typeHint, modeHint, actionHint);
             }
         });
     }
 
-    @WrapForJNI(calledFrom = "gecko") @Override
+    @WrapForJNI(calledFrom = "gecko", exceptionMode = "ignore") @Override
     public void onSelectionChange(int start, int end) {
         if (DEBUG) {
             // GeckoEditableListener methods should all be called from the Gecko thread
             ThreadUtils.assertOnGeckoThread();
             Log.d(LOGTAG, "onSelectionChange(" + start + ", " + end + ")");
         }
         if (start < 0 || start > mText.length() || end < 0 || end > mText.length()) {
             Log.e(LOGTAG, "invalid selection notification range: " +
@@ -1013,17 +1013,17 @@ final class GeckoEditable extends JNIObj
         mText.replace(start, oldEnd, newText);
     }
 
     private boolean geckoIsSameText(int start, int oldEnd, CharSequence newText) {
         return oldEnd - start == newText.length() &&
                TextUtils.regionMatches(mText, start, newText, 0, oldEnd - start);
     }
 
-    @WrapForJNI(calledFrom = "gecko") @Override
+    @WrapForJNI(calledFrom = "gecko", exceptionMode = "ignore") @Override
     public 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
@@ -2328,17 +2328,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;
 
@@ -2351,17 +2351,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,21 +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(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();
@@ -2772,17 +2777,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;
     };
@@ -2827,30 +2832,71 @@ 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(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(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);
     for (size_t i = 0; i < length; i++) {
         sdk::RectF::LocalRef rect(aJNIEnv);