Unify visibility logic for omnibox results container and suggestion list

This patch reduces the number of functions independently responsible
for changing the visibility of the omnibox suggestions. Since the only
visible part of the results container is the list, the visibility of
the container should depend on whether the suggestions list
visibility.

To support this change, a new method has been added to the
UrlFocusChangeListener interface: onUrlAnimationFinished. This method
triggers after the animation for focusing or unfocusing the omnibox
has completed.

Change-Id: I110379fd59dd2d54182d03cc91ed75dd3cba765b
Reviewed-on: https://chromium-review.googlesource.com/c/1294423
Commit-Queue: Matthew Jones <mdjones@chromium.org>
Reviewed-by: Ted Choc <tedchoc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#604674}

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

@@ -398,6 +398,12 @@ public class LocationBarLayout extends FrameLayout
         if (!inProgress) {
             updateButtonVisibility();
         }
+
+        if (!inProgress) {
+            for (UrlFocusChangeListener listener : mUrlFocusChangeListeners) {
+                listener.onUrlAnimationFinished(mUrlHasFocus);
+            }
+        }
     }
 
     /**

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

@@ -13,4 +13,10 @@ public interface UrlFocusChangeListener {
      * @param hasFocus Whether the URL field has gained focus.
      */
     void onUrlFocusChange(boolean hasFocus);
+
+    /**
+     * A notification that animations for focusing or unfocusing the input field has finished.
+     * @param hasFocus Whether the URL field has gained focus.
+     */
+    default void onUrlAnimationFinished(boolean hasFocus) {};
 }

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

@@ -34,7 +34,6 @@ import org.chromium.chrome.browser.omnibox.suggestions.OmniboxResultsAdapter.Omn
 import org.chromium.chrome.browser.omnibox.suggestions.OmniboxSuggestionsList.OmniboxSuggestionListEmbedder;
 import org.chromium.chrome.browser.profiles.Profile;
 import org.chromium.chrome.browser.toolbar.ToolbarDataProvider;
-import org.chromium.chrome.browser.toolbar.ToolbarPhone;
 import org.chromium.chrome.browser.util.KeyNavigationUtil;
 import org.chromium.content_public.browser.WebContents;
 import org.chromium.ui.UiUtils;
@@ -74,6 +73,7 @@ public class AutocompleteCoordinator
     private ViewGroup mOmniboxResultsContainer;
     private float mMaxRequiredWidth;
     private float mMaxMatchContentsWidth;
+    private boolean mCanShowSuggestions;
 
     // The timestamp (using SystemClock.elapsedRealtime()) at the point when the user started
     // modifying the omnibox with new input.
@@ -91,7 +91,6 @@ public class AutocompleteCoordinator
 
     private boolean mIgnoreOmniboxItemSelection = true;
 
-    private Runnable mShowSuggestions;
     private Runnable mRequestSuggestions;
     private DeferredOnSelectionRunnable mDeferredOnSelection;
 
@@ -182,6 +181,8 @@ public class AutocompleteCoordinator
     @Override
     public void onUrlFocusChange(boolean hasFocus) {
         if (hasFocus) {
+            // TODO(mdjones): Move init into visibility update method as it's only caller.
+            initOmniboxResultsContainer();
             if (mNativeInitialized) {
                 startZeroSuggest();
             } else {
@@ -191,14 +192,12 @@ public class AutocompleteCoordinator
                     }
                 });
             }
-            maybeShowOmniboxResultsContainer();
         } else {
             // Prevent any upcoming omnibox suggestions from showing once a URL is loaded (and as
             // a consequence the omnibox is unfocused).
             stopAutocomplete(true);
 
-            updateOmniboxResultsContainerVisibility(false);
-
+            mCanShowSuggestions = false;
             mHasStartedNewOmniboxEditSession = false;
             mNewOmniboxEditSessionTimestamp = -1;
             hideSuggestions();
@@ -206,6 +205,12 @@ public class AutocompleteCoordinator
         }
     }
 
+    @Override
+    public void onUrlAnimationFinished(boolean hasFocus) {
+        mCanShowSuggestions = hasFocus;
+        updateOmniboxSuggestionsVisibility();
+    }
+
     /**
      * Provides data and state for the toolbar component.
      * @param toolbarDataProvider The data provider.
@@ -324,9 +329,6 @@ public class AutocompleteCoordinator
             mSuggestionList = new OmniboxSuggestionsList(mContext, mSuggestionListEmbedder);
         }
 
-        // Ensure the results container is initialized and add the suggestion list to it.
-        initOmniboxResultsContainer();
-
         // Start with visibility GONE to ensure that show() is called. http://crbug.com/517438
         mSuggestionList.setVisibility(View.GONE);
         mSuggestionList.setAdapter(mSuggestionListAdapter);
@@ -455,33 +457,6 @@ public class AutocompleteCoordinator
         mMaxMatchContentsWidth = 0;
     }
 
-    /**
-     * Handles showing/hiding the suggestions list.
-     * @param visible Whether the suggestions list should be visible.
-     */
-    private void setSuggestionsListVisibility(final boolean visible) {
-        if (mSuggestionsShown == visible) return;
-        mSuggestionsShown = visible;
-        if (mSuggestionList != null) {
-            final boolean isShowing = mSuggestionList.getVisibility() == View.VISIBLE;
-            if (visible && !isShowing) {
-                mIgnoreOmniboxItemSelection = true; // Reset to default value.
-
-                if (mSuggestionList.getParent() == null) {
-                    mOmniboxResultsContainer.addView(mSuggestionList);
-                }
-
-                mSuggestionList.show();
-                updateSuggestionListLayoutDirection();
-            } else if (!visible && isShowing) {
-                mSuggestionList.setVisibility(View.GONE);
-
-                UiUtils.removeViewFromParent(mSuggestionList);
-            }
-        }
-        maybeShowOmniboxResultsContainer();
-    }
-
     private void initOmniboxResultsContainer() {
         if (mOmniboxResultsContainer != null) return;
         ViewStub overlayStub =
@@ -490,25 +465,34 @@ public class AutocompleteCoordinator
     }
 
     /**
-     * Conditionally show the omnibox suggestions container.
+     * Update whether the omnibox suggestions are visible.
      */
-    private void maybeShowOmniboxResultsContainer() {
-        if (isSuggestionsListShown() || mDelegate.isUrlBarFocused()) {
-            initOmniboxResultsContainer();
-            updateOmniboxResultsContainerVisibility(true);
-        }
-    }
+    private void updateOmniboxSuggestionsVisibility() {
+        if (mOmniboxResultsContainer == null || mSuggestionList == null) return;
 
-    /**
-     * Update whether the omnibox suggestions container is visible.
-     */
-    private void updateOmniboxResultsContainerVisibility(boolean visible) {
-        if (mOmniboxResultsContainer == null) return;
+        boolean isContainerVisible = mOmniboxResultsContainer.getVisibility() == View.VISIBLE;
+        boolean shouldBeVisible = mCanShowSuggestions && getSuggestionCount() > 0;
+        if (isContainerVisible == shouldBeVisible) return;
+
+        mSuggestionsShown = shouldBeVisible;
+
+        final boolean isListVisible = mSuggestionList.getVisibility() == View.VISIBLE;
+        if (shouldBeVisible && !isListVisible) {
+            mIgnoreOmniboxItemSelection = true; // Reset to default value.
 
-        boolean currentlyVisible = mOmniboxResultsContainer.getVisibility() == View.VISIBLE;
-        if (currentlyVisible == visible) return;
+            if (mSuggestionList.getParent() == null) {
+                mOmniboxResultsContainer.addView(mSuggestionList);
+            }
+
+            mSuggestionList.show();
+            updateSuggestionListLayoutDirection();
+        } else if (!shouldBeVisible && isListVisible) {
+            mSuggestionList.setVisibility(View.GONE);
+
+            UiUtils.removeViewFromParent(mSuggestionList);
+        }
 
-        if (visible) {
+        if (shouldBeVisible) {
             mOmniboxResultsContainer.setVisibility(View.VISIBLE);
         } else {
             mOmniboxResultsContainer.setVisibility(View.INVISIBLE);
@@ -809,19 +793,7 @@ public class AutocompleteCoordinator
 
         if (itemsChanged) mSuggestionListAdapter.notifySuggestionsChanged();
 
-        if (mDelegate.isUrlBarFocused()) {
-            if (mShowSuggestions != null) mParent.removeCallbacks(mShowSuggestions);
-            mShowSuggestions = () -> {
-                setSuggestionsListVisibility(true);
-                mShowSuggestions = null;
-            };
-            if (!mDelegate.isUrlFocusChangeInProgress()) {
-                mShowSuggestions.run();
-            } else {
-                mParent.postDelayed(
-                        mShowSuggestions, ToolbarPhone.URL_FOCUS_CHANGE_ANIMATION_DURATION_MS);
-            }
-        }
+        updateOmniboxSuggestionsVisibility();
     }
 
     private void loadUrlFromOmniboxMatch(
@@ -898,8 +870,7 @@ public class AutocompleteCoordinator
     /**
      * Sets to show cached zero suggest results. This will start both caching zero suggest results
      * in shared preferences and also attempt to show them when appropriate without needing native
-     * initialization. See {@link #showCachedZeroSuggestResultsIfAvailable()} for
-     * showing the loaded results before native initialization.
+     * initialization.
      * @param showCachedZeroSuggestResults Whether cached zero suggest should be shown.
      */
     public void setShowCachedZeroSuggestResults(boolean showCachedZeroSuggestResults) {
@@ -907,15 +878,6 @@ public class AutocompleteCoordinator
         if (mShowCachedZeroSuggestResults) mAutocomplete.startCachedZeroSuggest();
     }
 
-    /**
-     * Signals the omnibox to shows the cached zero suggest results if they have been loaded from
-     * cache successfully.
-     */
-    public void showCachedZeroSuggestResultsIfAvailable() {
-        if (!mShowCachedZeroSuggestResults || mSuggestionList == null) return;
-        setSuggestionsListVisibility(true);
-    }
-
     /**
      * Update the visuals of the autocomplete UI.
      * @param useDarkColors Whether dark colors should be applied to the UI.
@@ -946,12 +908,10 @@ public class AutocompleteCoordinator
     private void hideSuggestions() {
         if (mAutocomplete == null || !mNativeInitialized) return;
 
-        if (mShowSuggestions != null) mParent.removeCallbacks(mShowSuggestions);
-
         stopAutocomplete(true);
 
-        setSuggestionsListVisibility(false);
         clearSuggestions(true);
+        updateOmniboxSuggestionsVisibility();
     }
 
     /**

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

@@ -142,7 +142,6 @@ public class SearchActivity extends AsyncInitializationActivity
 
         // Kick off everything needed for the user to type into the box.
         beginQuery();
-        mSearchBox.getAutocompleteCoordinator().showCachedZeroSuggestResultsIfAvailable();
 
         // Kick off loading of the native library.
         if (!getActivityDelegate().shouldDelayNativeInitialization()) {

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

@@ -144,6 +144,7 @@ public class SearchActivityLocationBarLayout extends LocationBarLayout {
     //                is finalized after native has been initialized.
     private void focusTextBox() {
         if (!mUrlBar.hasFocus()) mUrlBar.requestFocus();
+        setUrlFocusChangeInProgress(false);
         getAutocompleteCoordinator().setShowCachedZeroSuggestResults(true);
 
         new Handler().post(new Runnable() {