[ToolbarMVC] Move more focus logic into LBMediator/Coordinator

This moves onSuggestionsHidden, onSuggestionsChanged,
setOmniboxEditingText, and setUrlBarFocusable to LBCoordinator/
LBMediator. Notably, this removes some redundant logic that was
previously siloed in LocationBarTablet. This logic became redundant when
we added mUrlBarTextIsSearch.

Bug: 1147581
Change-Id: Ia02c651530e6a23540ebcc67abd58c1e846ac738
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2586186Reviewed-by: Tomasz Wiszkowski <ender@google.com>
Reviewed-by: Ted Choc <tedchoc@chromium.org>
Reviewed-by: Filip Gorski <fgorski@chromium.org>
Commit-Queue: Patrick Noland <pnoland@chromium.org>
Cr-Commit-Position: refs/heads/master@{#836829}

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

@@ -302,11 +302,6 @@ public final class LocationBarCoordinator implements LocationBar, NativeInitObse
         mLocationBarMediator.onSuggestionsChanged(autocompleteText, defaultMatchIsSearch);
     }
 
-    @Override
-    public void onSuggestionsHidden() {
-        mLocationBarMediator.onSuggestionsHidden();
-    }
-
     @Override
     public void setKeyboardVisibility(boolean shouldShow, boolean delayHide) {
         mUrlCoordinator.setKeyboardVisibility(shouldShow, delayHide);
@@ -414,11 +409,18 @@ public final class LocationBarCoordinator implements LocationBar, NativeInitObse
         mLocationBarMediator.setUnfocusedWidth(unfocusedWidth);
     }
 
-    /** Returns the {@link StatusCoordiantor} for the LocationBar. */
+    /** Returns the {@link StatusCoordinator} for the LocationBar. */
     public StatusCoordinator getStatusCoordinator() {
         return mStatusCoordinator;
     }
 
+    /**
+     * @param focusable Whether the url bar should be focusable.
+     */
+    public void setUrlBarFocusable(boolean focusable) {
+        mUrlCoordinator.setAllowFocus(focusable);
+    }
+
     public void setVoiceRecognitionHandlerForTesting(
             VoiceRecognitionHandler voiceRecognitionHandler) {
         mLocationBarMediator.setVoiceRecognitionHandlerForTesting(voiceRecognitionHandler);

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

@@ -59,11 +59,6 @@ public class LocationBarCoordinatorPhone implements LocationBarCoordinator.SubCo
         mLocationBarPhone.finishUrlFocusChange(hasFocus, shouldShowKeyboard);
     }
 
-    /** Sets whether the url bar should be focusable. */
-    public void setUrlBarFocusable(boolean focusable) {
-        mLocationBarPhone.setUrlBarFocusable(focusable);
-    }
-
     /**
      * Returns {@link FrameLayout.LayoutParams} of the LocationBar view.
      *

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

@@ -224,16 +224,6 @@ public class LocationBarLayout extends FrameLayout implements OnClickListener {
         mStatusCoordinator.setLocationBarDataProviderForTesting(locationBarDataProvider);
     }
 
-    /* package */ void onSuggestionsHidden() {}
-
-    /* package */ void onSuggestionsChanged() {
-        // Handle the case where suggestions (in particular zero suggest) are received without the
-        // URL focusing happening.
-        if (mUrlFocusedWithoutAnimations && mUrlHasFocus) {
-            handleUrlFocusAnimation(mUrlHasFocus);
-        }
-    }
-
     @Override
     protected void dispatchRestoreInstanceState(SparseArray<Parcelable> container) {
         // Don't restore the state of the location bar, it can lead to all kind of bad states with
@@ -439,20 +429,6 @@ public class LocationBarLayout extends FrameLayout implements OnClickListener {
         if (!mLocationBarDataProvider.hasTab()) return;
     }
 
-    /* package */ void setOmniboxEditingText(String text) {
-        mUrlCoordinator.setUrlBarData(UrlBarData.forNonUrlText(text), UrlBar.ScrollType.NO_SCROLL,
-                UrlBarCoordinator.SelectionState.SELECT_END);
-        updateButtonVisibility();
-    }
-
-    /**
-     * @param focusable Whether the url bar should be focusable.
-     */
-    public void setUrlBarFocusable(boolean focusable) {
-        if (mUrlCoordinator == null) return;
-        mUrlCoordinator.setAllowFocus(focusable);
-    }
-
     @CallSuper
     protected void setUrlFocusChangeFraction(float fraction) {
         mUrlFocusChangeFraction = fraction;

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

@@ -230,7 +230,6 @@ class LocationBarMediator implements LocationBarDataProvider.Observer, FakeboxDe
     }
 
     /* package */ void onSuggestionsChanged(String autocompleteText, boolean defaultMatchIsSearch) {
-        mLocationBarLayout.onSuggestionsChanged();
         // TODO (https://crbug.com/1152501): Refactor the LBM/LBC relationship such that LBM doesn't
         // need to communicate with other coordinators like this.
         mStatusCoordinator.onDefaultMatchClassified(defaultMatchIsSearch);
@@ -239,7 +238,13 @@ class LocationBarMediator implements LocationBarDataProvider.Observer, FakeboxDe
             mUrlCoordinator.setAutocompleteText(userText, autocompleteText);
         }
 
-        mLocationBarLayout.onSuggestionsChanged();
+        // Handle the case where suggestions (in particular zero suggest) are received without the
+        // URL focusing happening.
+        if (mLocationBarLayout.isUrlBarFocusedWithoutAnimations()
+                && mLocationBarLayout.isUrlBarFocused()) {
+            mLocationBarLayout.handleUrlFocusAnimation(/*hasFocus=*/true);
+        }
+
         if (mNativeInitialized
                 && !CommandLine.getInstance().hasSwitch(ChromeSwitches.DISABLE_INSTANT)
                 && mPrivacyPreferencesManager.shouldPrerender()
@@ -250,10 +255,6 @@ class LocationBarMediator implements LocationBarDataProvider.Observer, FakeboxDe
         }
     }
 
-    /* package */ void onSuggestionsHidden() {
-        mLocationBarLayout.onSuggestionsHidden();
-    }
-
     /* package */ void loadUrl(String url, int transition, long inputStart) {
         loadUrlWithPostData(url, transition, inputStart, null, null);
     }

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

@@ -233,20 +233,6 @@ class LocationBarTablet extends LocationBarLayout {
         }
     }
 
-    @Override
-    void onSuggestionsHidden() {
-        super.onSuggestionsHidden();
-        mStatusCoordinator.setFirstSuggestionIsSearchType(false);
-    }
-
-    @Override
-    void onSuggestionsChanged() {
-        super.onSuggestionsChanged();
-        mStatusCoordinator.setFirstSuggestionIsSearchType(
-                mAutocompleteCoordinator.getSuggestionCount() > 0
-                && mAutocompleteCoordinator.getSuggestionAt(0).isSearchSuggestion());
-    }
-
     @Override
     protected void onMeasure(int widthMeasureSpec, int heightMeasureSpec) {
         int measuredWidth = getMeasuredWidth();

chrome/android/java/src/org/chromium/chrome/browser/omnibox/status/StatusCoordinator.java

@@ -252,11 +252,6 @@ public class StatusCoordinator implements View.OnClickListener {
         mMediator.setShowIconsWhenUrlFocused(showIconsWithUrlFocused);
     }
 
-    /** Specify whether suggestion for URL bar is a search action. */
-    public void setFirstSuggestionIsSearchType(boolean firstSuggestionIsSearchQuery) {
-        mMediator.setFirstSuggestionIsSearchType(firstSuggestionIsSearchQuery);
-    }
-
     /** Update information required to display the search engine icon. */
     public void updateSearchEngineStatusIcon(boolean shouldShowSearchEngineLogo,
             boolean isSearchEngineGoogle, String searchEngineUrl) {

chrome/android/java/src/org/chromium/chrome/browser/omnibox/status/StatusMediator.java

@@ -58,7 +58,6 @@ class StatusMediator implements IncognitoStateProvider.IncognitoStateObserver {
     private final PropertyModel mModel;
     private boolean mDarkTheme;
     private boolean mUrlHasFocus;
-    private boolean mFirstSuggestionIsSearchQuery;
     private boolean mVerboseStatusSpaceAvailable;
     private boolean mPageIsPreview;
     private boolean mPageIsPaintPreview;
@@ -340,14 +339,6 @@ class StatusMediator implements IncognitoStateProvider.IncognitoStateObserver {
         updateLocationBarIcon();
     }
 
-    /**
-     * Reports whether the first omnibox suggestion is a search query.
-     */
-    void setFirstSuggestionIsSearchType(boolean firstSuggestionIsSearchQuery) {
-        mFirstSuggestionIsSearchQuery = firstSuggestionIsSearchQuery;
-        updateLocationBarIcon();
-    }
-
     /**
      * Specify minimum width of an URL field.
      */
@@ -487,8 +478,8 @@ class StatusMediator implements IncognitoStateProvider.IncognitoStateObserver {
         mIsSecurityButtonShown = false;
         if (mUrlHasFocus) {
             if (mShowStatusIconWhenUrlFocused) {
-                icon = mFirstSuggestionIsSearchQuery ? R.drawable.ic_suggestion_magnifier
-                                                     : R.drawable.ic_globe_24dp;
+                icon = mUrlBarTextIsSearch ? R.drawable.ic_suggestion_magnifier
+                                           : R.drawable.ic_globe_24dp;
                 tint = mNavigationIconTintRes;
             }
         } else if (mSecurityIconRes != 0) {

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

@@ -24,11 +24,6 @@ public interface AutocompleteDelegate extends UrlBarDelegate {
      */
     void onSuggestionsChanged(String autocompleteText, boolean defaultMatchIsSearch);
 
-    /**
-     * Notified that the suggestions have been hidden.
-     */
-    void onSuggestionsHidden();
-
     /**
      * Requests the keyboard visibility update.
      *

chrome/android/java/src/org/chromium/chrome/browser/searchwidget/SearchActivity.java

@@ -181,6 +181,7 @@ public class SearchActivity extends AsyncInitializationActivity
                     loadUrl(url, transition, postDataType, postData);
                     return true;
                 });
+        mLocationBarCoordinator.setUrlBarFocusable(true);
 
         // Kick off everything needed for the user to type into the box.
         beginQuery();

chrome/android/java/src/org/chromium/chrome/browser/searchwidget/SearchActivityLocationBarLayout.java

@@ -66,7 +66,6 @@ public class SearchActivityLocationBarLayout extends LocationBarLayout {
         super.initialize(autocompleteCoordinator, urlCoordinator, statusCoordinator,
                 locationBarDataProvider, windowDelegate, windowAndroid, voiceRecognitionHandler,
                 assistantVoiceSearchServiceSupplier);
-        setUrlBarFocusable(true);
         mPendingSearchPromoDecision = LocaleManager.getInstance().needToCheckForSearchEnginePromo();
         getAutocompleteCoordinator().setShouldPreventOmniboxAutocomplete(
                 mPendingSearchPromoDecision);

chrome/android/java/src/org/chromium/chrome/browser/toolbar/top/ToolbarPhone.java

@@ -1789,7 +1789,7 @@ public class ToolbarPhone extends ToolbarLayout implements OnClickListener, TabC
         // crbug.com/974745.
         requestLayout();
 
-        mLocationBar.getPhoneCoordinator().setUrlBarFocusable(false);
+        mLocationBar.setUrlBarFocusable(false);
 
         finishAnimations();
 
@@ -1880,7 +1880,7 @@ public class ToolbarPhone extends ToolbarLayout implements OnClickListener, TabC
 
         // Detect what was being transitioned from and set the new state appropriately.
         if (mTabSwitcherState == EXITING_TAB_SWITCHER) {
-            mLocationBar.getPhoneCoordinator().setUrlBarFocusable(true);
+            mLocationBar.setUrlBarFocusable(true);
             mTabSwitcherState = STATIC_TAB;
             updateVisualsForLocationBarState();
         }

chrome/android/javatests/src/org/chromium/chrome/browser/omnibox/suggestions/AutocompleteMediatorUnitTest.java

@@ -93,9 +93,6 @@ public class AutocompleteMediatorUnitTest {
         @Override
         public void onSuggestionsChanged(String autocompleteText, boolean defaultMatchIsSearch) {}
 
-        @Override
-        public void onSuggestionsHidden() {}
-
         @Override
         public void setKeyboardVisibility(boolean shouldShow, boolean delayHide) {}
 

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

@@ -224,12 +224,15 @@ public class LocationBarMediatorTest {
         doReturn(456L).when(mAutocompleteCoordinator).getCurrentNativeAutocompleteResult();
         doReturn("text").when(mUrlCoordinator).getTextWithoutAutocomplete();
         doReturn(true).when(mUrlCoordinator).shouldAutocomplete();
+        doReturn(true).when(mLocationBarLayout).isUrlBarFocusedWithoutAnimations();
+        doReturn(true).when(mLocationBarLayout).isUrlBarFocused();
 
         mMediator.onSuggestionsChanged("textWithAutocomplete", true);
         verify(mPrerenderJni)
                 .prerenderMaybe(123L, omniboxPrerenderCaptor.getValue(), "text", "originalUrl",
                         456L, profile, mTab);
         verify(mUrlCoordinator).setAutocompleteText("text", "textWithAutocomplete");
+        verify(mLocationBarLayout).handleUrlFocusAnimation(true);
     }
 
     @Test