Clean up key event dispatching for Omnibox suggestions.

This moves the bulk of the key handling into the view
classes themselves and not the coordinator.

BUG=898522

Change-Id: Ie2b517eca6d7ae9e27bc12a020880bc668897b56
Reviewed-on: https://chromium-review.googlesource.com/c/1331689Reviewed-by: Pedro Amaral <amaralp@chromium.org>
Commit-Queue: Ted Choc <tedchoc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#607704}

chrome/android/java/src/org/chromium/chrome/browser/omnibox/suggestions/AutocompleteCoordinator.java

@@ -289,45 +289,14 @@ public class AutocompleteCoordinator implements UrlFocusChangeListener, UrlTextC
      * @return Whether the key event was handled.
      */
     public boolean handleKeyEvent(int keyCode, KeyEvent event) {
-        if (KeyNavigationUtil.isGoDown(event) && mListView != null && mListView.isShown()) {
-            int suggestionCount = mMediator.getSuggestionCount();
-            if (mListView.getSelectedItemPosition() < suggestionCount - 1) {
-                if (suggestionCount > 0) mMediator.allowPendingItemSelection();
-            } else {
-                // Do not pass down events when the last item is already selected as it will
-                // dismiss the suggestion list.
-                return true;
-            }
-
-            if (mListView.getSelectedItemPosition() == ListView.INVALID_POSITION) {
-                // When clearing the selection after a text change, state is not reset
-                // correctly so hitting down again will cause it to start from the previous
-                // selection point. We still have to send the key down event to let the list
-                // view items take focus, but then we select the first item explicitly.
-                boolean result = mListView.onKeyDown(keyCode, event);
-                mListView.setSelection(0);
-                return result;
-            } else {
-                return mListView.onKeyDown(keyCode, event);
-            }
-        } else if (KeyNavigationUtil.isGoUp(event) && mListView != null && mListView.isShown()) {
-            if (mListView.getSelectedItemPosition() != 0 && mMediator.getSuggestionCount() > 0) {
-                mMediator.allowPendingItemSelection();
-            }
-            return mListView.onKeyDown(keyCode, event);
-        } else if (KeyNavigationUtil.isGoRight(event) && mListView != null && mListView.isShown()
-                && mListView.getSelectedItemPosition() != ListView.INVALID_POSITION) {
-            mMediator.onSetUrlToSuggestion(
-                    mMediator.getSuggestionAt(mListView.getSelectedItemPosition()));
-            onTextChangedForAutocomplete();
-            mListView.setSelection(0);
-            return true;
-        } else if (KeyNavigationUtil.isEnter(event) && mParent.getVisibility() == View.VISIBLE) {
-            int selectedItemPosition = ListView.INVALID_POSITION;
-            if (mListView != null && mListView.isShown()) {
-                selectedItemPosition = mListView.getSelectedItemPosition();
-            }
-            mMediator.loadSuggestionAtIndex(selectedItemPosition, event.getEventTime());
+        boolean isShowingList = mListView != null && mListView.isShown();
+        if (isShowingList && mMediator.getSuggestionCount() > 0
+                && (KeyNavigationUtil.isGoDown(event) || KeyNavigationUtil.isGoUp(event))) {
+            mMediator.allowPendingItemSelection();
+        }
+        if (isShowingList && mListView.onKeyDown(keyCode, event)) return true;
+        if (KeyNavigationUtil.isEnter(event) && mParent.getVisibility() == View.VISIBLE) {
+            mMediator.loadTypedOmniboxText(event.getEventTime());
             return true;
         }
         return false;
@@ -339,9 +308,6 @@ public class AutocompleteCoordinator implements UrlFocusChangeListener, UrlTextC
      */
     @Override
     public void onTextChangedForAutocomplete() {
-        if (!mParent.isInTouchMode() && mListView != null) {
-            mListView.setSelection(0);
-        }
         mMediator.onTextChangedForAutocomplete();
     }
 

chrome/android/java/src/org/chromium/chrome/browser/omnibox/suggestions/AutocompleteMediator.java

@@ -18,7 +18,6 @@ import android.text.style.StyleSpan;
 import android.util.Pair;
 import android.util.TypedValue;
 import android.view.View;
-import android.widget.ListView;
 import android.widget.TextView;
 
 import org.chromium.base.Log;
@@ -802,6 +801,7 @@ class AutocompleteMediator implements OnSuggestionsReceivedListener {
 
         if (mShouldPreventOmniboxAutocomplete) return;
 
+        mIgnoreOmniboxItemSelection = true;
         cancelPendingAutocompleteStart();
 
         if (!mHasStartedNewOmniboxEditSession && mNativeInitialized) {
@@ -903,44 +903,34 @@ class AutocompleteMediator implements OnSuggestionsReceivedListener {
     }
 
     /**
-     * Load the suggestion at the given index.
-     * @param index The index that was selected.
+     * Load the url corresponding to the typed omnibox text.
      * @param eventTime The timestamp the load was triggered by the user.
      */
-    void loadSuggestionAtIndex(int index, long eventTime) {
+    void loadTypedOmniboxText(long eventTime) {
         mDelegate.hideKeyboard();
 
         final String urlText = mUrlBarEditingTextProvider.getTextWithAutocomplete();
         if (mNativeInitialized) {
-            findMatchAndLoadUrl(index, urlText, eventTime);
+            findMatchAndLoadUrl(urlText, eventTime);
         } else {
-            mDeferredNativeRunnables.add(() -> findMatchAndLoadUrl(index, urlText, eventTime));
+            mDeferredNativeRunnables.add(() -> findMatchAndLoadUrl(urlText, eventTime));
         }
     }
 
-    private void findMatchAndLoadUrl(int suggestionIndex, String urlText, long inputStart) {
-        int suggestionMatchPosition;
+    private void findMatchAndLoadUrl(String urlText, long inputStart) {
         OmniboxSuggestion suggestionMatch;
         boolean inSuggestionList = true;
 
-        if (suggestionIndex != ListView.INVALID_POSITION
-                && suggestionIndex < getSuggestionCount()) {
-            // Bluetooth keyboard case: the user highlighted a suggestion with the arrow
-            // keys, then pressed enter.
-            suggestionMatchPosition = suggestionIndex;
-            suggestionMatch = getSuggestionAt(suggestionMatchPosition);
-        } else if (getSuggestionCount() > 0
+        if (getSuggestionCount() > 0
                 && urlText.trim().equals(mUrlTextAfterSuggestionsReceived.trim())) {
             // Common case: the user typed something, received suggestions, then pressed enter.
             suggestionMatch = getSuggestionAt(0);
-            suggestionMatchPosition = 0;
         } else {
             // Less common case: there are no valid omnibox suggestions. This can happen if the
             // user tapped the URL bar to dismiss the suggestions, then pressed enter. This can
             // also happen if the user presses enter before any suggestions have been received
             // from the autocomplete controller.
             suggestionMatch = mAutocomplete.classify(urlText, mDelegate.didFocusUrlFromFakebox());
-            suggestionMatchPosition = 0;
             // Classify matches don't propagate to java, so skip the OOB check.
             inSuggestionList = false;
 
@@ -948,8 +938,7 @@ class AutocompleteMediator implements OnSuggestionsReceivedListener {
             if (suggestionMatch == null) return;
         }
 
-        loadUrlFromOmniboxMatch(
-                suggestionMatchPosition, suggestionMatch, inputStart, inSuggestionList);
+        loadUrlFromOmniboxMatch(0, suggestionMatch, inputStart, inSuggestionList);
     }
 
     /**

chrome/android/java/src/org/chromium/chrome/browser/omnibox/suggestions/OmniboxSuggestionsList.java

@@ -10,6 +10,7 @@ import android.graphics.Rect;
 import android.graphics.drawable.ColorDrawable;
 import android.support.annotation.Nullable;
 import android.support.v4.view.ViewCompat;
+import android.view.KeyEvent;
 import android.view.View;
 import android.view.ViewGroup;
 import android.view.ViewParent;
@@ -19,6 +20,7 @@ import android.widget.ListView;
 import org.chromium.base.VisibleForTesting;
 import org.chromium.chrome.R;
 import org.chromium.chrome.browser.WindowDelegate;
+import org.chromium.chrome.browser.util.KeyNavigationUtil;
 import org.chromium.chrome.browser.util.ViewUtils;
 
 import java.util.ArrayList;
@@ -178,6 +180,40 @@ public class OmniboxSuggestionsList extends ListView {
                         mEmbedder.isTablet() ? MeasureSpec.AT_MOST : MeasureSpec.EXACTLY));
     }
 
+    @Override
+    public boolean onKeyDown(int keyCode, KeyEvent event) {
+        if (!isShown()) return false;
+
+        int selectedPosition = getSelectedItemPosition();
+        int itemCount = getAdapter().getCount();
+        if (KeyNavigationUtil.isGoDown(event)) {
+            if (selectedPosition >= itemCount - 1) {
+                // Do not pass down events when the last item is already selected as it will
+                // dismiss the suggestion list.
+                return true;
+            }
+
+            if (selectedPosition == ListView.INVALID_POSITION) {
+                // When clearing the selection after a text change, state is not reset
+                // correctly so hitting down again will cause it to start from the previous
+                // selection point. We still have to send the key down event to let the list
+                // view items take focus, but then we select the first item explicitly.
+                boolean result = super.onKeyDown(keyCode, event);
+                setSelection(0);
+                return result;
+            }
+        } else if (KeyNavigationUtil.isGoRight(event)
+                && selectedPosition != ListView.INVALID_POSITION) {
+            View selectedView = getSelectedView();
+            if (selectedView != null) return selectedView.onKeyDown(keyCode, event);
+        } else if (KeyNavigationUtil.isEnter(event)
+                && selectedPosition != ListView.INVALID_POSITION) {
+            View selectedView = getSelectedView();
+            if (selectedView != null) return selectedView.performClick();
+        }
+        return super.onKeyDown(keyCode, event);
+    }
+
     @Override
     protected void layoutChildren() {
         super.layoutChildren();

chrome/android/java/src/org/chromium/chrome/browser/omnibox/suggestions/SuggestionListViewBinder.java

@@ -53,6 +53,7 @@ class SuggestionListViewBinder {
             view.listView.setEmbedder(model.get(SuggestionListProperties.EMBEDDER));
         } else if (SuggestionListProperties.SUGGESTION_MODELS.equals(propertyKey)) {
             view.adapter.updateSuggestions(model.get(SuggestionListProperties.SUGGESTION_MODELS));
+            view.listView.setSelection(0);
         } else if (SuggestionListProperties.USE_DARK_BACKGROUND.equals(propertyKey)) {
             view.listView.refreshPopupBackground(
                     model.get(SuggestionListProperties.USE_DARK_BACKGROUND));

chrome/android/java/src/org/chromium/chrome/browser/omnibox/suggestions/SuggestionView.java

@@ -14,14 +14,17 @@ import android.support.annotation.DrawableRes;
 import android.support.annotation.IntDef;
 import android.support.annotation.VisibleForTesting;
 import android.support.v7.content.res.AppCompatResources;
+import android.view.KeyEvent;
 import android.view.MotionEvent;
 import android.view.View;
+import android.view.View.OnClickListener;
 import android.view.ViewGroup;
 import android.widget.ImageView;
 import android.widget.TextView;
 
 import org.chromium.base.ApiCompatibilityUtils;
 import org.chromium.chrome.R;
+import org.chromium.chrome.browser.util.KeyNavigationUtil;
 import org.chromium.chrome.browser.widget.TintedDrawable;
 
 import java.lang.annotation.Retention;
@@ -32,7 +35,7 @@ import java.lang.annotation.RetentionPolicy;
  * any unnecessary measures and layouts.
  */
 @VisibleForTesting
-public class SuggestionView extends ViewGroup {
+public class SuggestionView extends ViewGroup implements OnClickListener {
     private static final float ANSWER_IMAGE_SCALING_FACTOR = 1.15f;
 
     @IntDef({SuggestionLayoutType.TEXT_SUGGESTION, SuggestionLayoutType.ANSWER,
@@ -104,6 +107,8 @@ public class SuggestionView extends ViewGroup {
     public SuggestionView(Context context) {
         super(context);
 
+        setOnClickListener(this);
+
         mSuggestionHeight =
                 context.getResources().getDimensionPixelOffset(R.dimen.omnibox_suggestion_height);
         mSuggestionAnswerHeight = context.getResources().getDimensionPixelOffset(
@@ -245,6 +250,20 @@ public class SuggestionView extends ViewGroup {
         return super.dispatchTouchEvent(ev);
     }
 
+    @Override
+    public void onClick(View v) {
+        mContentsView.callOnClick();
+    }
+
+    @Override
+    public boolean onKeyDown(int keyCode, KeyEvent event) {
+        if (KeyNavigationUtil.isGoRight(event)) {
+            mSuggestionDelegate.onRefineSuggestion();
+            return true;
+        }
+        return super.onKeyDown(keyCode, event);
+    }
+
     /** Sets the delegate for the actions on the suggestion view. */
     void setDelegate(SuggestionViewDelegate delegate) {
         mSuggestionDelegate = delegate;