Stop notifying autocomplete text state change on selection change

Selection change at focus gain triggers onAutocompleteTextStateChanged()
and this causes LocationBarLayout and AutocompleteController to stop
generating omnibox suggestions.

BUG=759876

(cherry picked from commit bc99b84)

Change-Id: I96fd12e5ef116862ee44f082b9234a9f4b9c11bd
Reviewed-on: https://chromium-review.googlesource.com/651572
Reviewed-by: Ted Choc <[email protected]>
Commit-Queue: Changwan Ryu <[email protected]>
Cr-Original-Commit-Position: refs/heads/master@{#500052}
Reviewed-on: https://chromium-review.googlesource.com/656533
Reviewed-by: Changwan Ryu <[email protected]>
Cr-Commit-Position: refs/branch-heads/3202@{#73}
Cr-Branched-From: fa6a5d8-refs/heads/master@{#499098}

Author: Changwan Ryu

chrome/android/java/src/org/chromium/chrome/browser/omnibox/AutocompleteState.java

@@ -143,10 +143,6 @@ public void commitAutocompleteText() {
         mAutocompleteText = "";
     }
 
-    public boolean equalsExceptAutocompleteText(AutocompleteState a) {
-        return mUserText.equals(a.mUserText) && mSelStart == a.mSelStart && mSelEnd == a.mSelEnd;
-    }
-
     @Override
     public boolean equals(Object o) {
         if (!(o instanceof AutocompleteState)) return false;

chrome/android/java/src/org/chromium/chrome/browser/omnibox/SpannableAutocompleteEditTextModel.java

@@ -182,11 +182,12 @@ private void notifyAutocompleteTextStateChanged() {
         if (mBatchEditNestCount > 0) return;
         if (mCurrentState.equals(mPreviouslyNotifiedState)) return;
         notifyAccessibilityService();
-        // Nothing has changed except that autocomplete text has been set or modified.
-        if (mCurrentState.equalsExceptAutocompleteText(mPreviouslyNotifiedState)
-                && mCurrentState.hasAutocompleteText()) {
-            // Autocomplete text is set by the controller, we should not notify the controller with
-            // the same information.
+        if (mCurrentState.getUserText().equals(mPreviouslyNotifiedState.getUserText())
+                && (mCurrentState.hasAutocompleteText()
+                           || !mPreviouslyNotifiedState.hasAutocompleteText())) {
+            // Nothing has changed except that autocomplete text has been set or modified. Or
+            // selection change did not affect autocomplete text. Autocomplete text is set by the
+            // controller, so only text change or deletion of autocomplete text should be notified.
             mPreviouslyNotifiedState.copyFrom(mCurrentState);
             return;
         }

chrome/android/junit/src/org/chromium/chrome/browser/omnibox/AutocompleteEditTextTest.java

@@ -19,6 +19,7 @@
 import android.view.accessibility.AccessibilityEvent;
 import android.view.inputmethod.EditorInfo;
 import android.view.inputmethod.InputConnection;
+import android.widget.LinearLayout;
 
 import org.junit.Before;
 import org.junit.Rule;
@@ -60,6 +61,8 @@ public class AutocompleteEditTextTest {
 
     private InOrder mInOrder;
     private TestAutocompleteEditText mAutocomplete;
+    private LinearLayout mFocusPlaceHolder;
+
     private Context mContext;
     private InputConnection mInputConnection;
     private Verifier mVerifier;
@@ -160,13 +163,16 @@ public void setUp() throws Exception {
 
         mVerifier = spy(new Verifier());
         mAutocomplete = new TestAutocompleteEditText(mContext, null);
+        mFocusPlaceHolder = new LinearLayout(mContext);
+        mFocusPlaceHolder.setFocusable(true);
+        mFocusPlaceHolder.addView(mAutocomplete);
         assertNotNull(mAutocomplete);
 
         // Pretend that the view is shown in the activity hierarchy, which is for accessibility
         // testing.
         Activity activity = Robolectric.buildActivity(Activity.class).create().get();
-        activity.setContentView(mAutocomplete);
-        assertNotNull(mAutocomplete.getParent());
+        activity.setContentView(mFocusPlaceHolder);
+        assertNotNull(mFocusPlaceHolder.getParent());
         mIsShown = true;
         assertTrue(mAutocomplete.isShown());
 
@@ -181,7 +187,7 @@ public void setUp() throws Exception {
         mInOrder = inOrder(mVerifier);
         assertTrue(mAutocomplete.requestFocus());
         verifyOnPopulateAccessibilityEvent(
-                AccessibilityEvent.TYPE_VIEW_FOCUSED, "", "", 1, -1, -1, -1, -1);
+                AccessibilityEvent.TYPE_VIEW_FOCUSED, "", "", 2, -1, -1, -1, -1);
         assertNotNull(mAutocomplete.onCreateInputConnection(new EditorInfo()));
         mInputConnection = mAutocomplete.getInputConnection();
         assertNotNull(mInputConnection);
@@ -1104,4 +1110,61 @@ public void testOnSaveInstanceStateDoesNotCrash() {
         // should not crash.
         new SpannableString(mAutocomplete.getText());
     }
+
+    // crbug.com/759876
+    @Test
+    @Features(@Features.Register(
+            value = ChromeFeatureList.SPANNABLE_INLINE_AUTOCOMPLETE, enabled = true))
+    public void testFocusInAndSelectAllWithSpannableModel() {
+        internalTestFocusInAndSelectAll();
+    }
+
+    // crbug.com/759876
+    @Test
+    @Features(@Features.Register(
+            value = ChromeFeatureList.SPANNABLE_INLINE_AUTOCOMPLETE, enabled = false))
+    public void testFocusInAndSelectAllWithoutSpannableModel() {
+        internalTestFocusInAndSelectAll();
+    }
+
+    private void internalTestFocusInAndSelectAll() {
+        final String url = "https://google.com";
+        final int len = url.length();
+        mAutocomplete.setIgnoreTextChangesForAutocomplete(true);
+        mAutocomplete.setText(url);
+        mAutocomplete.setIgnoreTextChangesForAutocomplete(false);
+
+        mInOrder.verifyNoMoreInteractions();
+        assertVerifierCallCounts(0, 0);
+
+        assertTrue(mFocusPlaceHolder.requestFocus());
+
+        mInOrder.verifyNoMoreInteractions();
+        assertVerifierCallCounts(0, 0);
+
+        // LocationBarLayout does this.
+        mAutocomplete.setSelectAllOnFocus(true);
+
+        assertTrue(mAutocomplete.requestFocus());
+
+        if (isUsingSpannableModel()) {
+            verifyOnPopulateAccessibilityEvent(AccessibilityEvent.TYPE_VIEW_TEXT_SELECTION_CHANGED,
+                    url, "", 18, 18, 18, -1, -1);
+        }
+        mInOrder.verify(mVerifier).onUpdateSelection(len, len);
+        verifyOnPopulateAccessibilityEvent(
+                AccessibilityEvent.TYPE_VIEW_TEXT_SELECTION_CHANGED, url, "", 18, 18, 18, -1, -1);
+        mInOrder.verify(mVerifier).onUpdateSelection(0, len);
+        verifyOnPopulateAccessibilityEvent(
+                AccessibilityEvent.TYPE_VIEW_TEXT_SELECTION_CHANGED, url, "", 18, 0, 18, -1, -1);
+        verifyOnPopulateAccessibilityEvent(
+                AccessibilityEvent.TYPE_VIEW_FOCUSED, url, "", 2, -1, -1, -1, -1);
+
+        if (isUsingSpannableModel()) {
+            assertVerifierCallCounts(2, 4);
+        } else {
+            assertVerifierCallCounts(2, 3);
+        }
+        mInOrder.verifyNoMoreInteractions();
+    }
 }