Bug 1540628 - Don't use restartInput when no composition. r=esawin
authorMakoto Kato <m_kato@ga2.so-net.ne.jp>
Mon, 15 Apr 2019 15:24:55 +0900
changeset 470373 2e9187dc17a928962173b04bf39dcdee57805dd8
parent 470329 f8541439e10186980ecea712820310b08144ec96
child 470374 ab1da7fa2ad0aa1c9d6d0a9e2f63e6492cd1b712
push id35904
push useropoprus@mozilla.com
push dateMon, 22 Apr 2019 21:49:19 +0000
treeherdermozilla-central@4c7eaf384b06 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersesawin
bugs1540628
milestone68.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1540628 - Don't use restartInput when no composition. r=esawin Since `restartInput` resets all keyboard state, it isn't good to call this everytime. So we should call this when discarding composition only. Differential Revision: https://phabricator.services.mozilla.com/D27489
mobile/android/geckoview/src/main/java/org/mozilla/geckoview/GeckoEditable.java
mobile/android/geckoview/src/main/java/org/mozilla/geckoview/GeckoInputConnection.java
mobile/android/geckoview/src/main/java/org/mozilla/geckoview/SessionTextInput.java
--- a/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/GeckoEditable.java
+++ b/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/GeckoEditable.java
@@ -306,16 +306,48 @@ import android.view.inputmethod.EditorIn
         // be read-only. Editing should be done through one of the shadow*** methods.
         public Spanned getShadowText() {
             if (DEBUG) {
                 assertOnIcThread();
             }
             return mShadowText;
         }
 
+        /**
+         * Check whether we are currently discarding the composition. It means that shadow text has composition,
+         * but current text has no composition. So syncShadowText will discard composition.
+         *
+         * @return true if discarding composition
+         */
+        private boolean isDiscardingComposition() {
+            boolean wasComposing = false;
+            Object[] spans = mShadowText.getSpans(0, mShadowText.length(), Object.class);
+            for (final Object span : spans) {
+                if ((mShadowText.getSpanFlags(span) & Spanned.SPAN_COMPOSING) != 0) {
+                    wasComposing = true;
+                    break;
+                }
+            }
+
+            if (!wasComposing) {
+                return false;
+            }
+
+            boolean isComposing = false;
+            spans = mCurrentText.getSpans(0, mCurrentText.length(), Object.class);
+            for (final Object span : spans) {
+                if ((mCurrentText.getSpanFlags(span) & Spanned.SPAN_COMPOSING) != 0) {
+                    isComposing = true;
+                    break;
+                }
+            }
+
+            return !isComposing;
+        }
+
         public synchronized void syncShadowText(
                 final SessionTextInput.EditableListener listener) {
             if (DEBUG) {
                 assertOnIcThread();
             }
 
             if (mCurrentStart > mCurrentOldEnd && mShadowStart > mShadowOldEnd) {
                 // Still check selection changes.
@@ -328,16 +360,22 @@ import android.view.inputmethod.EditorIn
                 mCurrentSelectionChanged = false;
 
                 if (listener != null) {
                     listener.onSelectionChange();
                 }
                 return;
             }
 
+            if (isDiscardingComposition()) {
+                if (listener != null) {
+                    listener.onDiscardComposition();
+                }
+            }
+
             // Copy the portion of the current text that has changed over to the shadow
             // text, with consideration for any concurrent changes in the shadow text.
             final int start = Math.min(mShadowStart, mCurrentStart);
             final int shadowEnd = mShadowNewEnd + Math.max(0, mCurrentOldEnd - mShadowOldEnd);
             final int currentEnd = mCurrentNewEnd + Math.max(0, mShadowOldEnd - mCurrentOldEnd);
 
             // Remove existing spans that may no longer be in the new text.
             Object[] spans = mShadowText.getSpans(start, shadowEnd, Object.class);
--- a/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/GeckoInputConnection.java
+++ b/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/GeckoInputConnection.java
@@ -337,16 +337,38 @@ import java.lang.reflect.Proxy;
             @Override
             public void run() {
                 getInputDelegate().updateSelection(mSession, start, end,
                                                    compositionStart, compositionEnd);
             }
         });
     }
 
+    @Override // SessionTextInput.EditableListener
+    public void onDiscardComposition() {
+        final View view = getView();
+        if (view == null) {
+            return;
+        }
+
+        // InputMethodManager.updateSelection will remove composition
+        // on most IMEs. But ATOK series do nothing. So we have to
+        // restart input method to remove composition as workaround.
+        if (!InputMethods.needsRestartInput(InputMethods.getCurrentInputMethod(view.getContext()))) {
+            return;
+        }
+
+        view.post(new Runnable() {
+            @Override
+            public void run() {
+                getInputDelegate().restartInput(mSession, GeckoSession.TextInputDelegate.RESTART_REASON_CONTENT_CHANGE);
+            }
+        });
+    }
+
     @TargetApi(21)
     @Override // SessionTextInput.EditableListener
     public void updateCompositionRects(final RectF[] rects) {
         if (!(Build.VERSION.SDK_INT >= 21)) {
             return;
         }
 
         final View view = getView();
--- a/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/SessionTextInput.java
+++ b/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/SessionTextInput.java
@@ -109,16 +109,17 @@ public final class SessionTextInput {
         @WrapForJNI final int IME_FLAG_PRIVATE_BROWSING = 1;
         @WrapForJNI final int IME_FLAG_USER_ACTION = 2;
 
         void notifyIME(int type);
         void notifyIMEContext(int state, String typeHint, String modeHint,
                               String actionHint, int flag);
         void onSelectionChange();
         void onTextChange();
+        void onDiscardComposition();
         void onDefaultKeyEvent(KeyEvent event);
         void updateCompositionRects(final RectF[] aRects);
     }
 
     private static final class DefaultDelegate implements GeckoSession.TextInputDelegate {
         public static final DefaultDelegate INSTANCE = new DefaultDelegate();
 
         private InputMethodManager getInputMethodManager(@Nullable final View view) {
@@ -203,27 +204,18 @@ public final class SessionTextInput {
                                     final int selStart, final int selEnd,
                                     final int compositionStart, final int compositionEnd) {
             ThreadUtils.assertOnUiThread();
             final View view = session.getTextInput().getView();
             final InputMethodManager imm = getInputMethodManager(view);
             if (imm != null) {
                 // When composition start and end is -1,
                 // InputMethodManager.updateSelection will remove composition
-                // on most IMEs. But ATOK series do nothing. So we have to
-                // restart input method to remove composition as workaround.
-                if (compositionStart < 0 && compositionEnd < 0 &&
-                    InputMethods.needsRestartInput(
-                        InputMethods.getCurrentInputMethod(view.getContext()))) {
-                    try {
-                        imm.restartInput(view);
-                    } catch (RuntimeException e) {
-                        Log.e(LOGTAG, "Error restarting input", e);
-                    }
-                }
+                // on most IMEs. If not working, we have to add a workaround
+                // to EditableListener.onDiscardComposition.
                 imm.updateSelection(view, selStart, selEnd, compositionStart, compositionEnd);
             }
         }
 
         @Override
         public void updateExtractedText(@NonNull final GeckoSession session,
                                         @NonNull final ExtractedTextRequest request,
                                         @NonNull final ExtractedText text) {