Send correct key-codes when doing composition events instead of always 0.

content/public/android/java/src/org/chromium/content/browser/input/AdapterInputConnection.java

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 package org.chromium.content.browser.input;
 
 import android.os.SystemClock;
 import android.text.Editable;
 import android.text.InputType;
 import android.text.Selection;
-import android.text.TextUtils;
 import android.util.Log;
 import android.view.KeyEvent;
 import android.view.View;
 import android.view.inputmethod.BaseInputConnection;
 import android.view.inputmethod.EditorInfo;
 import android.view.inputmethod.ExtractedText;
 import android.view.inputmethod.ExtractedTextRequest;
 
 import com.google.common.annotations.VisibleForTesting;
 
@@ skipping 17 matching lines @@
 
     private boolean mSingleLine;
     private int mNumNestedBatchEdits = 0;
 
     private int mLastUpdateSelectionStart = INVALID_SELECTION;
     private int mLastUpdateSelectionEnd = INVALID_SELECTION;
     private int mLastUpdateCompositionStart = INVALID_COMPOSITION;
     private int mLastUpdateCompositionEnd = INVALID_COMPOSITION;
 
     @VisibleForTesting
+    public int mUpdateStateCalls = 0;
+
+    @VisibleForTesting
     AdapterInputConnection(View view, ImeAdapter imeAdapter, Editable editable,
             EditorInfo outAttrs) {
         super(view, true);
         mInternalView = view;
         mImeAdapter = imeAdapter;
         mImeAdapter.setInputConnection(this);
         mEditable = editable;
         // The editable passed in might have been in use by a prior keyboard and could have had
         // prior composition spans set.  To avoid keyboard conflicts, remove all composing spans
         // when taking ownership of an existing Editable.
@@ skipping 73 matching lines @@
      *                       selection.
      * @param isNonImeChange True when the update was caused by non-IME (e.g. Javascript).
      */
     @VisibleForTesting
     public void updateState(String text, int selectionStart, int selectionEnd, int compositionStart,
             int compositionEnd, boolean isNonImeChange) {
         if (DEBUG) {
             Log.w(TAG, "updateState [" + text + "] [" + selectionStart + " " + selectionEnd + "] ["
                     + compositionStart + " " + compositionEnd + "] [" + isNonImeChange + "]");
         }
+        mUpdateStateCalls++;

[aurimas (slooooooooow), 2014/07/21 20:35:54]

mUpdateStateCalls does not seem to be used. It if is going to only be used for
tests, can we move it to TestAdapterInputConnection in ImeTest.java?

[bcwhite, 2014/07/21 20:46:07]

Done.

+
         // If this update is from the IME, no further state modification is necessary because the
         // state should have been updated already by the IM framework directly.
         if (!isNonImeChange) return;
 
         // Non-breaking spaces can cause the IME to get confused. Replace with normal spaces.
         text = text.replace('\u00A0', ' ');
 
         selectionStart = Math.min(selectionStart, text.length());
         selectionEnd = Math.min(selectionEnd, text.length());
         compositionStart = Math.min(compositionStart, text.length());
@@ skipping 54 matching lines @@
         mLastUpdateCompositionStart = compositionStart;
         mLastUpdateCompositionEnd = compositionEnd;
     }
 
     /**
      * @see BaseInputConnection#setComposingText(java.lang.CharSequence, int)
      */
     @Override
     public boolean setComposingText(CharSequence text, int newCursorPosition) {
         if (DEBUG) Log.w(TAG, "setComposingText [" + text + "] [" + newCursorPosition + "]");
-        if (maybePerformEmptyCompositionWorkaround(text)) return true;
         super.setComposingText(text, newCursorPosition);
         updateSelectionIfRequired();
         return mImeAdapter.checkCompositionQueueAndCallNative(text, newCursorPosition, false);
     }
 
     /**
      * @see BaseInputConnection#commitText(java.lang.CharSequence, int)
      */
     @Override
     public boolean commitText(CharSequence text, int newCursorPosition) {
         if (DEBUG) Log.w(TAG, "commitText [" + text + "] [" + newCursorPosition + "]");
-        if (maybePerformEmptyCompositionWorkaround(text)) return true;
         super.commitText(text, newCursorPosition);
         updateSelectionIfRequired();
         return mImeAdapter.checkCompositionQueueAndCallNative(text, newCursorPosition,
                 text.length() > 0);
     }
 
     /**
      * @see BaseInputConnection#performEditorAction(int)
      */
     @Override
@@ skipping 214 matching lines @@
         if (b < 0) b = 0;
         if (a > textLength) a = textLength;
         if (b > textLength) b = textLength;
 
         if (a == b) {
             removeComposingSpans(mEditable);
         } else {
             super.setComposingRegion(a, b);
         }
         updateSelectionIfRequired();
-        return mImeAdapter.setComposingRegion(a, b);
+
+        CharSequence regionText = null;
+        if (b > a) {
+            regionText = mEditable.subSequence(start, end);
+        }
+        return mImeAdapter.setComposingRegion(regionText, a, b);
     }
 
     boolean isActive() {
         return getInputMethodManagerWrapper().isActive(mInternalView);
     }
 
     private InputMethodManagerWrapper getInputMethodManagerWrapper() {
         return mImeAdapter.getInputMethodManagerWrapper();
     }
 
-    /**
-     * This method works around the issue crbug.com/373934 where Blink does not cancel
-     * the composition when we send a commit with the empty text.
-     *
-     * TODO(aurimas) Remove this once crbug.com/373934 is fixed.
-     *
-     * @param text Text that software keyboard requested to commit.
-     * @return Whether the workaround was performed.
-     */
-    private boolean maybePerformEmptyCompositionWorkaround(CharSequence text) {

[aurimas (slooooooooow), 2014/07/21 20:21:57]

Can we remove this workaround already? Does it work correctly on the blink side?
If so can you update the bug.

[bcwhite, 2014/07/21 20:27:18]

Possibly, but I don't want to do that as part of this CL.  I'd rather it be
independent.

[aurimas (slooooooooow), 2014/07/21 20:35:54]

Can you undo the removal of this call then?

-        int selectionStart = Selection.getSelectionStart(mEditable);
-        int selectionEnd = Selection.getSelectionEnd(mEditable);
-        int compositionStart = getComposingSpanStart(mEditable);
-        int compositionEnd = getComposingSpanEnd(mEditable);
-        if (TextUtils.isEmpty(text) && (selectionStart == selectionEnd)
-                && compositionStart != INVALID_COMPOSITION
-                && compositionEnd != INVALID_COMPOSITION) {
-            beginBatchEdit();
-            finishComposingText();
-            int selection = Selection.getSelectionStart(mEditable);
-            deleteSurroundingText(selection - compositionStart, selection - compositionEnd);
-            endBatchEdit();
-            return true;
-        }
-        return false;
-    }
-
     @VisibleForTesting
     static class ImeState {
         public final String text;
         public final int selectionStart;
         public final int selectionEnd;
         public final int compositionStart;
         public final int compositionEnd;
 
         public ImeState(String text, int selectionStart, int selectionEnd,
                 int compositionStart, int compositionEnd) {
             this.text = text;
             this.selectionStart = selectionStart;
             this.selectionEnd = selectionEnd;
             this.compositionStart = compositionStart;
             this.compositionEnd = compositionEnd;
         }
     }
 
     @VisibleForTesting
     ImeState getImeStateForTesting() {
         String text = mEditable.toString();
         int selectionStart = Selection.getSelectionStart(mEditable);
         int selectionEnd = Selection.getSelectionEnd(mEditable);
         int compositionStart = getComposingSpanStart(mEditable);
         int compositionEnd = getComposingSpanEnd(mEditable);
         return new ImeState(text, selectionStart, selectionEnd, compositionStart, compositionEnd);
     }
 }