Bug 1464096 - 3. Remove SessionTextInput.isInputActive; r?esawin draft
authorJim Chen <nchen@mozilla.com>
Tue, 05 Jun 2018 17:49:01 -0400
changeset 804394 674f0c9115b99c7b6bdccf2c468ce067ae004ded
parent 804393 867965bab9fa4ddc423dee7e01cc034105456c80
child 804395 6de4b7ef1106e0a8457e2e62fc025acad9d40f0b
push id112368
push userbmo:nchen@mozilla.com
push dateTue, 05 Jun 2018 21:49:45 +0000
reviewersesawin
bugs1464096
milestone62.0a1
Bug 1464096 - 3. Remove SessionTextInput.isInputActive; r?esawin The SessionTextInput.isInputActive API is prone to races. Its functionality can be replicated through the SessionTextInput.Delegate.restartInput callback, so it's okay to remove it. MozReview-Commit-ID: GFYjbH8cQv0
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
@@ -865,32 +865,31 @@ import android.view.inputmethod.EditorIn
                               " : " + Integer.toHexString(rangeStyles) +
                               " : " + Integer.toHexString(rangeForeColor) +
                               " : " + Integer.toHexString(rangeBackColor));
             }
         } while (rangeStart < composingEnd);
     }
 
     @Override // SessionTextInput.EditableClient
-    public void sendKeyEvent(final @Nullable View view, final boolean inputActive, final int action,
-                             @NonNull KeyEvent event) {
+    public void sendKeyEvent(final @Nullable View view, final int action, @NonNull KeyEvent event) {
         final Editable editable = getEditable();
         if (editable == null) {
             return;
         }
 
         final KeyListener keyListener = TextKeyListener.getInstance();
         event = translateKey(event.getKeyCode(), event);
 
         // We only let TextKeyListener do UI things on the UI thread.
         final View v = ThreadUtils.isOnUiThread() ? view : null;
         final int keyCode = event.getKeyCode();
         final boolean handled;
 
-        if (!inputActive || shouldSkipKeyListener(keyCode, event)) {
+        if (shouldSkipKeyListener(keyCode, event)) {
             handled = false;
         } else if (action == KeyEvent.ACTION_DOWN) {
             setSuppressKeyUp(true);
             handled = keyListener.onKeyDown(v, editable, keyCode, event);
         } else if (action == KeyEvent.ACTION_UP) {
             handled = keyListener.onKeyUp(v, editable, keyCode, event);
         } else {
             handled = keyListener.onKeyOther(v, editable, event);
@@ -941,16 +940,20 @@ import android.view.inputmethod.EditorIn
                        /* isSynthesizedImeKey */ false);
             icOfferAction(new Action(Action.TYPE_EVENT));
         } catch (final RemoteException e) {
             Log.e(LOGTAG, "Remote call failed", e);
         }
     }
 
     private boolean shouldSkipKeyListener(final int keyCode, final @NonNull KeyEvent event) {
+        if (mIMEState == SessionTextInput.EditableListener.IME_STATE_DISABLED) {
+            return true;
+        }
+
         // Preserve enter and tab keys for the browser
         if (keyCode == KeyEvent.KEYCODE_ENTER ||
             keyCode == KeyEvent.KEYCODE_TAB) {
             return true;
         }
         // BaseKeyListener returns false even if it handled these keys for us,
         // so we skip the key listener entirely and handle these ourselves
         if (keyCode == KeyEvent.KEYCODE_DEL ||
@@ -1956,60 +1959,59 @@ import android.view.inputmethod.EditorIn
         throw new UnsupportedOperationException("method must be called through mProxy");
     }
 
     @Override
     public String toString() {
         throw new UnsupportedOperationException("method must be called through mProxy");
     }
 
-    public boolean onKeyPreIme(final @Nullable View view, final boolean inputActive,
-                               final int keyCode, final @NonNull KeyEvent event) {
+    public boolean onKeyPreIme(final @Nullable View view, final int keyCode,
+                               final @NonNull KeyEvent event) {
         return false;
     }
 
-    public boolean onKeyDown(final @Nullable View view, final boolean inputActive,
-                             final int keyCode, final @NonNull KeyEvent event) {
-        return processKey(view, inputActive, KeyEvent.ACTION_DOWN, keyCode, event);
+    public boolean onKeyDown(final @Nullable View view, final int keyCode,
+                             final @NonNull KeyEvent event) {
+        return processKey(view, KeyEvent.ACTION_DOWN, keyCode, event);
     }
 
-    public boolean onKeyUp(final @Nullable View view, final boolean inputActive,
-                           final int keyCode, final @NonNull KeyEvent event) {
-        return processKey(view, inputActive, KeyEvent.ACTION_UP, keyCode, event);
+    public boolean onKeyUp(final @Nullable View view, final int keyCode,
+                           final @NonNull KeyEvent event) {
+        return processKey(view, KeyEvent.ACTION_UP, keyCode, event);
     }
 
-    public boolean onKeyMultiple(final @Nullable View view, final boolean inputActive,
-                                 final int keyCode, int repeatCount,
+    public boolean onKeyMultiple(final @Nullable View view, final int keyCode, int repeatCount,
                                  final @NonNull KeyEvent event) {
         if (keyCode == KeyEvent.KEYCODE_UNKNOWN) {
             // KEYCODE_UNKNOWN means the characters are in KeyEvent.getCharacters()
             final String str = event.getCharacters();
             for (int i = 0; i < str.length(); i++) {
                 final KeyEvent charEvent = getCharKeyEvent(str.charAt(i));
-                if (!processKey(view, inputActive, KeyEvent.ACTION_DOWN,
+                if (!processKey(view, KeyEvent.ACTION_DOWN,
                                 KeyEvent.KEYCODE_UNKNOWN, charEvent) ||
-                    !processKey(view, inputActive, KeyEvent.ACTION_UP,
+                    !processKey(view, KeyEvent.ACTION_UP,
                                 KeyEvent.KEYCODE_UNKNOWN, charEvent)) {
                     return false;
                 }
             }
             return true;
         }
 
         while ((repeatCount--) > 0) {
-            if (!processKey(view, inputActive, KeyEvent.ACTION_DOWN, keyCode, event) ||
-                !processKey(view, inputActive, KeyEvent.ACTION_UP, keyCode, event)) {
+            if (!processKey(view, KeyEvent.ACTION_DOWN, keyCode, event) ||
+                !processKey(view, KeyEvent.ACTION_UP, keyCode, event)) {
                 return false;
             }
         }
         return true;
     }
 
-    public boolean onKeyLongPress(final @Nullable View view, final boolean inputActive,
-                                  final int keyCode, final @NonNull KeyEvent event) {
+    public boolean onKeyLongPress(final @Nullable View view, final int keyCode,
+                                  final @NonNull KeyEvent event) {
         return false;
     }
 
     /**
      * Get a key that represents a given character.
      */
     private static KeyEvent getCharKeyEvent(final char c) {
         final long time = SystemClock.uptimeMillis();
@@ -2022,26 +2024,26 @@ import android.view.inputmethod.EditorIn
 
             @Override
             public int getUnicodeChar(int metaState) {
                 return c;
             }
         };
     }
 
-    private boolean processKey(final @Nullable View view, final boolean inputActive,
-                               final int action, final int keyCode, final @NonNull KeyEvent event) {
+    private boolean processKey(final @Nullable View view, final int action, final int keyCode,
+                               final @NonNull KeyEvent event) {
         if (keyCode > KeyEvent.getMaxKeyCode() || !shouldProcessKey(keyCode, event)) {
             return false;
         }
 
         postToInputConnection(new Runnable() {
             @Override
             public void run() {
-                sendKeyEvent(view, inputActive, action, event);
+                sendKeyEvent(view, action, event);
             }
         });
         return true;
     }
 
     private static boolean shouldProcessKey(int keyCode, KeyEvent event) {
         switch (keyCode) {
             case KeyEvent.KEYCODE_MENU:
--- a/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/GeckoInputConnection.java
+++ b/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/GeckoInputConnection.java
@@ -585,17 +585,17 @@ import java.lang.reflect.Proxy;
             return true;
         }
         return super.setSelection(start, end);
     }
 
     @Override
     public boolean sendKeyEvent(@NonNull KeyEvent event) {
         event = translateKey(event.getKeyCode(), event);
-        mEditableClient.sendKeyEvent(getView(), isInputActive(), event.getAction(), event);
+        mEditableClient.sendKeyEvent(getView(), event.getAction(), event);
         return false; // seems to always return false
     }
 
     private KeyEvent translateKey(final int keyCode, final @NonNull KeyEvent event) {
         switch (keyCode) {
             case KeyEvent.KEYCODE_ENTER:
                 if ((event.getFlags() & KeyEvent.FLAG_EDITOR_ACTION) != 0 &&
                         mIMEActionHint.equalsIgnoreCase("next")) {
@@ -632,22 +632,16 @@ import java.lang.reflect.Proxy;
                     Context viewContext = getView().getContext();
                     AudioManager am = (AudioManager)viewContext.getSystemService(Context.AUDIO_SERVICE);
                     am.dispatchMediaKeyEvent(event);
                 }
                 break;
         }
     }
 
-    @Override // SessionTextInput.InputConnectionClient
-    public synchronized boolean isInputActive() {
-        // Make sure this picks up PASSWORD state as well.
-        return mIMEState != IME_STATE_DISABLED;
-    }
-
     @Override // SessionTextInput.EditableListener
     public void notifyIME(final int type) {
         switch (type) {
             case NOTIFY_IME_OF_FOCUS:
                 // Showing/hiding vkb is done in notifyIMEContext
                 if (mBatchEditCount != 0) {
                     Log.w(LOGTAG, "resetting with mBatchEditCount = " + mBatchEditCount);
                     mBatchEditCount = 0;
--- a/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/SessionTextInput.java
+++ b/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/SessionTextInput.java
@@ -140,33 +140,31 @@ public final class SessionTextInput {
         void updateCursorAnchorInfo(@NonNull GeckoSession session, @NonNull CursorAnchorInfo info);
     }
 
     // Interface to access GeckoInputConnection from SessionTextInput.
     /* package */ interface InputConnectionClient {
         View getView();
         Handler getHandler(Handler defHandler);
         InputConnection onCreateInputConnection(EditorInfo attrs);
-        boolean isInputActive();
     }
 
     // Interface to access GeckoEditable from GeckoInputConnection.
     /* package */ interface EditableClient {
         // The following value is used by requestCursorUpdates
         // ONE_SHOT calls updateCompositionRects() after getting current composing
         // character rects.
         @WrapForJNI final int ONE_SHOT = 1;
         // START_MONITOR start the monitor for composing character rects.  If is is
         // updaed,  call updateCompositionRects()
         @WrapForJNI final int START_MONITOR = 2;
         // ENDT_MONITOR stops the monitor for composing character rects.
         @WrapForJNI final int END_MONITOR = 3;
 
-        void sendKeyEvent(@Nullable View view, boolean inputActive, int action,
-                          @NonNull KeyEvent event);
+        void sendKeyEvent(@Nullable View view, int action, @NonNull KeyEvent event);
         Editable getEditable();
         void setBatchMode(boolean isBatchMode);
         Handler setInputConnectionHandler(@NonNull Handler handler);
         void postToInputConnection(@NonNull Runnable runnable);
         void requestCursorUpdates(int requestMode);
     }
 
     // Interface to access GeckoInputConnection from GeckoEditable.
@@ -422,78 +420,67 @@ public final class SessionTextInput {
      * Process a KeyEvent as a pre-IME event.
      *
      * @param keyCode Key code.
      * @param event KeyEvent instance.
      * @return True if the event was handled.
      */
     public boolean onKeyPreIme(final int keyCode, final @NonNull KeyEvent event) {
         ThreadUtils.assertOnUiThread();
-        return mEditable.onKeyPreIme(getView(), isInputActive(), keyCode, event);
+        return mEditable.onKeyPreIme(getView(), keyCode, event);
     }
 
     /**
      * Process a KeyEvent as a key-down event.
      *
      * @param keyCode Key code.
      * @param event KeyEvent instance.
      * @return True if the event was handled.
      */
     public boolean onKeyDown(final int keyCode, final @NonNull KeyEvent event) {
         ThreadUtils.assertOnUiThread();
-        return mEditable.onKeyDown(getView(), isInputActive(), keyCode, event);
+        return mEditable.onKeyDown(getView(), keyCode, event);
     }
 
     /**
      * Process a KeyEvent as a key-up event.
      *
      * @param keyCode Key code.
      * @param event KeyEvent instance.
      * @return True if the event was handled.
      */
     public boolean onKeyUp(final int keyCode, final @NonNull KeyEvent event) {
         ThreadUtils.assertOnUiThread();
-        return mEditable.onKeyUp(getView(), isInputActive(), keyCode, event);
+        return mEditable.onKeyUp(getView(), keyCode, event);
     }
 
     /**
      * Process a KeyEvent as a long-press event.
      *
      * @param keyCode Key code.
      * @param event KeyEvent instance.
      * @return True if the event was handled.
      */
     public boolean onKeyLongPress(final int keyCode, final @NonNull KeyEvent event) {
         ThreadUtils.assertOnUiThread();
-        return mEditable.onKeyLongPress(getView(), isInputActive(), keyCode, event);
+        return mEditable.onKeyLongPress(getView(), keyCode, event);
     }
 
     /**
      * Process a KeyEvent as a multiple-press event.
      *
      * @param keyCode Key code.
      * @param repeatCount Key repeat count.
      * @param event KeyEvent instance.
      * @return True if the event was handled.
      */
     public boolean onKeyMultiple(final int keyCode, final int repeatCount,
                                  final @NonNull KeyEvent event) {
         ThreadUtils.assertOnUiThread();
-        return mEditable.onKeyMultiple(getView(), isInputActive(), keyCode, repeatCount, event);
-    }
-
-    /**
-     * Return whether there is an active input connection, usually as a result of a
-     * focused input field.
-     *
-     * @return True if input is active.
-     */
-    public boolean isInputActive() {
-        ThreadUtils.assertOnUiThread();
-        return mInputConnection != null && mInputConnection.isInputActive();
+        return mEditable.onKeyMultiple(getView(), keyCode, repeatCount, event);
     }
 
     /**
      * Set the current text input delegate.
      *
      * @param delegate Delegate instance or null to restore to default.
      */
     public void setDelegate(@Nullable final Delegate delegate) {