Steamline omnibox dse logo showing logic

This change gives StatusMediator access to ToolbarDataProvider which
allows omnibox dse logo showing logic to be consolidated to the
StatusMediator class. This eliminates many special cases throughout
the codebase.

Bug: 989775,991017,989773
Change-Id: Iab3d21a03e680790a01b0a7a2164618aeb1ce753
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1741443
Commit-Queue: Brandon Wylie <wylieb@chromium.org>
Reviewed-by: Ted Choc <tedchoc@chromium.org>
Reviewed-by: Theresa  <twellington@chromium.org>
Cr-Commit-Position: refs/heads/master@{#686235}

chrome/android/chrome_java_sources.gni

@@ -1584,6 +1584,7 @@ chrome_java_sources = [
   "java/src/org/chromium/chrome/browser/toolbar/IncognitoToggleTabLayout.java",
   "java/src/org/chromium/chrome/browser/toolbar/KeyboardNavigationListener.java",
   "java/src/org/chromium/chrome/browser/toolbar/LocationBarModel.java",
+  "java/src/org/chromium/chrome/browser/toolbar/ToolbarCommonPropertiesModel.java",
   "java/src/org/chromium/chrome/browser/toolbar/MenuButton.java",
   "java/src/org/chromium/chrome/browser/toolbar/NewTabButton.java",
   "java/src/org/chromium/chrome/browser/toolbar/TabCountProvider.java",

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

@@ -1006,8 +1006,6 @@ public class LocationBarLayout extends FrameLayout
 
         mStatusViewCoordinator.updateSearchEngineStatusIcon(
                 mShouldShowSearchEngineLogo, mIsSearchEngineGoogle, mSearchEngineUrl);
-        mToolbarDataProvider.updateSearchEngineStatusIcon(
-                mShouldShowSearchEngineLogo, mIsSearchEngineGoogle, mSearchEngineUrl);
     }
 
     @Override

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

@@ -7,6 +7,7 @@ package org.chromium.chrome.browser.omnibox;
 import android.content.res.Resources;
 import android.graphics.Bitmap;
 import android.support.annotation.Nullable;
+import android.text.TextUtils;
 
 import org.chromium.base.Callback;
 import org.chromium.chrome.R;
@@ -44,6 +45,14 @@ public class SearchEngineLogoUtils {
         return TemplateUrlServiceFactory.get().isDefaultSearchEngineGoogle();
     }
 
+    /**
+     * @return True if the given url is the same domain as the DSE.
+     */
+    public static boolean doesUrlMatchDefaultSearchEngine(String url) {
+        if (TextUtils.isEmpty(url)) return false;
+        return UrlUtilities.sameDomainOrHost(url, getSearchLogoUrl(), false);
+    }
+
     /**
      * @return The search URL of the current DSE or null if one cannot be found.
      */

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

@@ -14,6 +14,7 @@ import org.chromium.base.VisibleForTesting;
 import org.chromium.chrome.R;
 import org.chromium.chrome.browser.omnibox.SearchEngineLogoUtils;
 import org.chromium.chrome.browser.profiles.Profile;
+import org.chromium.chrome.browser.toolbar.ToolbarCommonPropertiesModel;
 import org.chromium.chrome.browser.util.ColorUtils;
 import org.chromium.components.security_state.ConnectionSecurityLevel;
 import org.chromium.ui.modelutil.PropertyModel;
@@ -38,6 +39,7 @@ class StatusMediator {
     private boolean mIsSecurityButtonShown;
     private boolean mShouldShowSearchEngineLogo;
     private boolean mIsSearchEngineGoogle;
+    private boolean mShouldCancelCustomFavicon;
 
     private int mUrlMinWidth;
     private int mSeparatorMinWidth;
@@ -51,6 +53,7 @@ class StatusMediator {
     private @DrawableRes int mNavigationIconTintRes;
 
     private Resources mResources;
+    private ToolbarCommonPropertiesModel mToolbarCommonPropertiesModel;
 
     StatusMediator(PropertyModel model, Resources resources) {
         mModel = model;
@@ -59,6 +62,13 @@ class StatusMediator {
         mResources = resources;
     }
 
+    /**
+     * Set the ToolbarDataProvider for this class.
+     */
+    void setToolbarDataProvider(ToolbarCommonPropertiesModel toolbarCommonPropertiesModel) {
+        mToolbarCommonPropertiesModel = toolbarCommonPropertiesModel;
+    }
+
     /**
      * Toggle animations of icon changes.
      */
@@ -311,40 +321,55 @@ class StatusMediator {
      *     - not shown if URL is focused.
      */
     private void updateLocationBarIcon() {
-        int icon = 0;
-        int tint = 0;
-        int description = 0;
-        int toast = 0;
-
-        mIsSecurityButtonShown = false;
-
         // When the search engine logo should be shown, but the engine isn't Google. In this case,
         // we download the icon on the fly.
-        if (!mIsSearchEngineGoogle && mShowStatusIconWhenUrlFocused
-                && mShouldShowSearchEngineLogo) {
-            mModel.set(StatusProperties.STATUS_ICON_RES, R.drawable.ic_search);
+        boolean showFocused = mUrlHasFocus && mShowStatusIconWhenUrlFocused;
+        // Show the logo unfocused if "Query in the omnibox" is active or we're on the NTP. Current
+        // "Query in the omnibox" behavior makes it active for non-dse searches if you've just
+        // changed your default search engine.The included workaround below
+        // (doesUrlMatchDefaultSearchEngine) can be removed once this is fixed.
+        // TODO(crbug.com/991017): Remove doesUrlMatchDefaultSearchEngine when "Query in the
+        //                         omnibox" properly reacts to dse changes.
+        boolean showUnfocusedNewTabPage = !mUrlHasFocus && mToolbarCommonPropertiesModel != null
+                && mToolbarCommonPropertiesModel.getNewTabPageForCurrentTab() != null;
+        boolean showUnfocusedSearchResultsPage = !mUrlHasFocus
+                && mToolbarCommonPropertiesModel != null
+                && mToolbarCommonPropertiesModel.getDisplaySearchTerms() != null
+                && SearchEngineLogoUtils.doesUrlMatchDefaultSearchEngine(
+                        mToolbarCommonPropertiesModel.getCurrentUrl());
+        if (mShouldShowSearchEngineLogo
+                && (showFocused || showUnfocusedNewTabPage || showUnfocusedSearchResultsPage)) {
+            mShouldCancelCustomFavicon = false;
             mModel.set(StatusProperties.STATUS_ICON_TINT_RES, /* no tint */ 0);
-
-            // TODO(crbug.com/985565): Cache this favicon in Java.
-            SearchEngineLogoUtils.getSearchEngineLogoFavicon(
-                    Profile.getLastUsedProfile().getOriginalProfile(), mResources, (favicon) -> {
-                        if (favicon == null) return;
-                        mModel.set(StatusProperties.STATUS_ICON, favicon);
-                    });
-
+            if (mIsSearchEngineGoogle) {
+                mModel.set(StatusProperties.STATUS_ICON_RES, R.drawable.ic_logo_googleg_24dp);
+            } else {
+                mModel.set(StatusProperties.STATUS_ICON_RES, R.drawable.ic_search);
+                // TODO(crbug.com/985565): Cache this favicon in Java.
+                SearchEngineLogoUtils.getSearchEngineLogoFavicon(
+                        Profile.getLastUsedProfile().getOriginalProfile(), mResources,
+                        (favicon) -> {
+                            if (favicon == null || mShouldCancelCustomFavicon) return;
+                            mModel.set(StatusProperties.STATUS_ICON, favicon);
+                        });
+            }
             return;
+        } else {
+            mShouldCancelCustomFavicon = true;
         }
 
+        int icon = 0;
+        int tint = 0;
+        int description = 0;
+        int toast = 0;
+
+        mIsSecurityButtonShown = false;
         if (mUrlHasFocus) {
             if (mShowStatusIconWhenUrlFocused) {
-                if (mShouldShowSearchEngineLogo && mIsSearchEngineGoogle) {
-                    icon = R.drawable.ic_logo_googleg_24dp;
-                } else {
-                    icon = mFirstSuggestionIsSearchQuery ? R.drawable.omnibox_search
-                                                         : R.drawable.ic_omnibox_page;
-                    tint = mNavigationIconTintRes;
-                    description = R.string.accessibility_toolbar_btn_site_info;
-                }
+                icon = mFirstSuggestionIsSearchQuery ? R.drawable.omnibox_search
+                                                     : R.drawable.ic_omnibox_page;
+                tint = mNavigationIconTintRes;
+                description = R.string.accessibility_toolbar_btn_site_info;
             }
         } else if (mSecurityIconRes != 0) {
             mIsSecurityButtonShown = true;

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

@@ -63,6 +63,7 @@ public class StatusViewCoordinator implements View.OnClickListener {
      */
     public void setToolbarDataProvider(ToolbarDataProvider toolbarDataProvider) {
         mToolbarDataProvider = toolbarDataProvider;
+        mMediator.setToolbarDataProvider(mToolbarDataProvider);
         // Update status immediately after receiving the data provider to avoid initial presence
         // glitch on tablet devices. This glitch would be typically seen upon launch of app, right
         // before the landing page is presented to the user.

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

@@ -123,8 +123,4 @@ class SearchBoxDataProvider implements ToolbarDataProvider {
     public @ColorRes int getSecurityIconColorStateList() {
         return 0;
     }
-
-    @Override
-    public void updateSearchEngineStatusIcon(boolean shouldShowSearchEngineLogo,
-            boolean isSearchEngineGoogle, String searchEngineUrl) {}
 }

chrome/android/java/src/org/chromium/chrome/browser/toolbar/LocationBarModel.java

@@ -42,7 +42,7 @@ import java.net.URISyntaxException;
 /**
  * Provides a way of accessing toolbar data and state.
  */
-public class LocationBarModel implements ToolbarDataProvider {
+public class LocationBarModel implements ToolbarDataProvider, ToolbarCommonPropertiesModel {
     private final Context mContext;
 
     private Tab mTab;
@@ -52,8 +52,6 @@ public class LocationBarModel implements ToolbarDataProvider {
     private boolean mIsIncognito;
     private boolean mIsUsingBrandColor;
     private boolean mShouldShowOmniboxInOverviewMode;
-    private boolean mShouldShowSearchEngineLogo;
-    private boolean mIsSearchEngineGoogle;
 
     private long mNativeLocationBarModelAndroid;
 
@@ -362,17 +360,7 @@ public class LocationBarModel implements ToolbarDataProvider {
         // If we're showing a query in the omnibox, and the security level is high enough to show
         // the search icon, return that instead of the security icon.
         if (getDisplaySearchTerms() != null) {
-            if (mShouldShowSearchEngineLogo && mIsSearchEngineGoogle) {
-                return R.drawable.ic_logo_googleg_24dp;
-            } else {
                 return R.drawable.omnibox_search;
-            }
-        } else if (getNewTabPageForCurrentTab() != null && mShouldShowSearchEngineLogo) {
-            if (mIsSearchEngineGoogle) {
-                return R.drawable.ic_logo_googleg_24dp;
-            } else {
-                return R.drawable.omnibox_search;
-            }
         }
 
         return getSecurityIconResource(getSecurityLevel(), !isTablet, isOfflinePage(), isPreview());
@@ -426,12 +414,6 @@ public class LocationBarModel implements ToolbarDataProvider {
 
     @Override
     public @ColorRes int getSecurityIconColorStateList() {
-        // Don't apply tint to the search logo, which is shown on the NTP and the SRP pages.
-        if ((getNewTabPageForCurrentTab() != null || getDisplaySearchTerms() != null)
-                && mShouldShowSearchEngineLogo) {
-            return 0;
-        }
-
         int securityLevel = getSecurityLevel();
         int color = getPrimaryColor();
         boolean needLightIcon = ColorUtils.shouldUseLightForegroundOnBackground(color);
@@ -482,13 +464,6 @@ public class LocationBarModel implements ToolbarDataProvider {
         return nativeGetDisplaySearchTerms(mNativeLocationBarModelAndroid);
     }
 
-    @Override
-    public void updateSearchEngineStatusIcon(boolean shouldShowSearchEngineLogo,
-            boolean isSearchEngineGoogle, String searchEngineUrl) {
-        mShouldShowSearchEngineLogo = shouldShowSearchEngineLogo;
-        mIsSearchEngineGoogle = isSearchEngineGoogle;
-    }
-
     /** @return The formatted URL suitable for editing. */
     public String getFormattedFullUrl() {
         if (mNativeLocationBarModelAndroid == 0) return "";

chrome/android/java/src/org/chromium/chrome/browser/toolbar/ToolbarCommonPropertiesModel.java

+// Copyright 2019 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.chrome.browser.toolbar;
+
+import android.support.annotation.NonNull;
+import android.support.annotation.Nullable;
+
+import org.chromium.chrome.browser.ntp.NewTabPage;
+
+/**
+ * Defines an interface that provides common properties to toolbar and omnibox classes.
+ */
+public interface ToolbarCommonPropertiesModel {
+    /**
+     * @return The current url for the current tab. Returns empty string when there is no tab.
+     */
+    @NonNull
+    String getCurrentUrl();
+
+    /**
+     * @return The NewTabPage shown for the current Tab or null if one is not being shown.
+     */
+    NewTabPage getNewTabPageForCurrentTab();
+
+    /**
+     * @return Whether the toolbar is currently being displayed for incognito.
+     */
+    boolean isIncognito();
+
+    /**
+     * If the current tab state is eligible for displaying the search query terms instead of the
+     * URL, this extracts the query terms from the current URL.
+     *
+     * @return The search terms. Returns null if the tab is ineligible to display the search terms
+     *         instead of the URL.
+     */
+    @Nullable
+    default public String getDisplaySearchTerms() {
+        return null;
+    }
+}

chrome/android/java/src/org/chromium/chrome/browser/toolbar/ToolbarDataProvider.java

@@ -21,7 +21,9 @@ import org.chromium.components.security_state.ConnectionSecurityLevel;
 /**
  * Defines the data that is exposed to properly render the Toolbar.
  */
-public interface ToolbarDataProvider {
+// TODO(crbug.com/865801): Refine split between common/generally toolbar properties and
+//                         sub-component properties.
+public interface ToolbarDataProvider extends ToolbarCommonPropertiesModel {
     /**
      * @return The tab that contains the information currently displayed in the toolbar.
      */
@@ -37,16 +39,19 @@ public interface ToolbarDataProvider {
      * @return The current url for the current tab. Returns empty string when there is no tab.
      */
     @NonNull
+    @Override
     String getCurrentUrl();
 
     /**
      * @return The NewTabPage shown for the current Tab or null if one is not being shown.
      */
+    @Override
     NewTabPage getNewTabPageForCurrentTab();
 
     /**
      * @return Whether the toolbar is currently being displayed for incognito.
      */
+    @Override
     boolean isIncognito();
 
     /**
@@ -150,18 +155,8 @@ public interface ToolbarDataProvider {
      *         instead of the URL.
      */
     @Nullable
+    @Override
     default public String getDisplaySearchTerms() {
         return null;
     }
-
-    /**
-     * Update the information required to display the search engine logo in the omnibox.
-     *
-     * @param shouldShowSearchEngineLogo True if we should show the search engine logo in the
-     *         omnibox.
-     * @param isSearchEngineGoogle True if the default search engine is Google.
-     * @param searchEngineUrl The url for the search engine, used to fetch the favicon.
-     */
-    void updateSearchEngineStatusIcon(boolean shouldShowSearchEngineLogo,
-            boolean isSearchEngineGoogle, String searchEngineUrl);
 }

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

@@ -314,11 +314,6 @@ public abstract class ToolbarLayout
             public @ColorRes int getSecurityIconColorStateList() {
                 return 0;
             }
-
-            @Override
-            public void updateSearchEngineStatusIcon(boolean shouldShowSearchEngineLogo,
-                    boolean isSearchEngineGoogle, String searchEngineUrl) {}
-
         };
 
         // Set menu button background in case it was previously called before inflation

chrome/android/javatests/src/org/chromium/chrome/browser/omnibox/LocationBarVoiceRecognitionHandlerTest.java

@@ -257,10 +257,6 @@ public class LocationBarVoiceRecognitionHandlerTest {
         public @ColorRes int getSecurityIconColorStateList() {
             return 0;
         }
-
-        @Override
-        public void updateSearchEngineStatusIcon(boolean shouldShowSearchEngineLogo,
-                boolean isSearchEngineGoogle, String searchEngineUrl) {}
     }
 
     /**