Add special case for to compensate for incorrect autocomplete text

When navigating between two pages, getTextWithAutocomplete() points
to the autocompleted text for the previous URL. This screws with the
behavior of StatusMediator, so I've added a workaround that checks
if the current autocomplete text contains the current url bar text.
If it does then we use the autocomplete text, otherwise we just use
the UrlBar text.

Bug: 1015147
Change-Id: Ie64cb4e1944005ab7273e8b66084033c2401b7ac
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1864027
Commit-Queue: Brandon Wylie <wylieb@chromium.org>
Reviewed-by: Ted Choc <tedchoc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#707025}

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

@@ -523,16 +523,24 @@ class StatusMediator {
     }
 
     /** @see android.text.TextWatcher#onTextChanged */
-    void onTextChanged(CharSequence charSequence) {
-        // TODO (crbug.com/1012870): This is a workaround for the linked bug. Once the bug is fixed,
-        //                           it should be removed.
-        String urlTextWithAutocomplete = TextUtils.isEmpty(charSequence)
-                ? ""
-                : mUrlBarEditingTextStateProvider.getTextWithAutocomplete();
-        if (TextUtils.equals(mUrlBarTextWithAutocomplete, urlTextWithAutocomplete)) {
-            return;
+    void onTextChanged(CharSequence urlBarText) {
+        String currentAutocompleteText = mUrlBarEditingTextStateProvider.getTextWithAutocomplete();
+        String urlTextWithAutocomplete;
+        if (TextUtils.isEmpty(urlBarText)) {
+            // TODO (crbug.com/1012870): This is to workaround the UrlBar text being empty but the
+            // autocomplete text still pointing at the previous url's autocomplete text.
+            urlTextWithAutocomplete = "";
+        } else if (TextUtils.indexOf(currentAutocompleteText, urlBarText) > -1) {
+            // TODO(crbug.com/1015147): This is to workaround the UrlBar text pointing to the
+            // "current" url and the the autocomplete text pointing to the "previous" url.
+            urlTextWithAutocomplete = currentAutocompleteText;
+        } else {
+            // If the above cases don't apply, then we should use the UrlBar text itself.
+            urlTextWithAutocomplete = urlBarText.toString();
         }
 
+        if (TextUtils.equals(mUrlBarTextWithAutocomplete, urlTextWithAutocomplete)) return;
+
         mUrlBarTextWithAutocomplete = urlTextWithAutocomplete;
         boolean isValid = mDelegate.isUrlValid(mUrlBarTextWithAutocomplete);
         if (isValid != mUrlBarTextIsValidUrl) {

chrome/android/junit/src/org/chromium/chrome/browser/omnibox/status/StatusMediatorUnitTest.java

@@ -6,6 +6,9 @@ package org.chromium.chrome.browser.omnibox.status;
 
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.anyBoolean;
+import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
 
 import android.content.res.Resources;
 import android.graphics.Bitmap;
@@ -50,6 +53,8 @@ public final class StatusMediatorUnitTest {
     Bitmap mBitmap;
     @Captor
     ArgumentCaptor<Callback<Bitmap>> mCallbackCaptor;
+    @Captor
+    ArgumentCaptor<String> mUrlCaptor;
 
     PropertyModel mModel;
     StatusMediator mMediator;
@@ -62,12 +67,15 @@ public final class StatusMediatorUnitTest {
         mMediator = new StatusMediator(mModel, mResources, mUrlBarEditingTextStateProvider);
         mMediator.setToolbarCommonPropertiesModel(mToolbarCommonPropertiesModel);
         mMediator.setDelegateForTesting(mDelegate);
+
+        when(mDelegate.isUrlValid(mUrlCaptor.capture()))
+                .thenAnswer(invocation -> mUrlCaptor.getValue().equals(TEST_SEARCH_URL));
     }
 
     @Test
     @Features.EnableFeatures(ChromeFeatureList.OMNIBOX_SEARCH_ENGINE_LOGO)
     public void searchEngineLogo_showGoogleLogo() {
-        setupSearchEngineLogoForTesting(true, false, false, false);
+        setupSearchEngineLogoForTesting(true, false, false);
 
         mMediator.setUrlHasFocus(true);
         mMediator.setShowIconsWhenUrlFocused(true);
@@ -79,7 +87,7 @@ public final class StatusMediatorUnitTest {
     @Test
     @Features.EnableFeatures(ChromeFeatureList.OMNIBOX_SEARCH_ENGINE_LOGO)
     public void searchEngineLogo_showGoogleLogo_whenScrolled() {
-        setupSearchEngineLogoForTesting(true, false, false, false);
+        setupSearchEngineLogoForTesting(true, false, false);
 
         mMediator.setUrlHasFocus(false);
         mMediator.setShowIconsWhenUrlFocused(true);
@@ -92,7 +100,7 @@ public final class StatusMediatorUnitTest {
     @Test
     @Features.EnableFeatures(ChromeFeatureList.OMNIBOX_SEARCH_ENGINE_LOGO)
     public void searchEngineLogo_showGoogleLogo_searchLoupeEverywhere() {
-        setupSearchEngineLogoForTesting(true, true, true, false);
+        setupSearchEngineLogoForTesting(true, true, true);
 
         mMediator.setUrlHasFocus(true);
         mMediator.setShowIconsWhenUrlFocused(true);
@@ -103,7 +111,7 @@ public final class StatusMediatorUnitTest {
     @Test
     @Features.EnableFeatures(ChromeFeatureList.OMNIBOX_SEARCH_ENGINE_LOGO)
     public void searchEngineLogo_showNonGoogleLogo() {
-        setupSearchEngineLogoForTesting(true, false, false, false);
+        setupSearchEngineLogoForTesting(true, false, false);
 
         mMediator.setUrlHasFocus(true);
         mMediator.setShowIconsWhenUrlFocused(true);
@@ -120,7 +128,7 @@ public final class StatusMediatorUnitTest {
     @Test
     @Features.EnableFeatures(ChromeFeatureList.OMNIBOX_SEARCH_ENGINE_LOGO)
     public void searchEngineLogo_showNonGoogleLogo_searchLoupeEverywhere() {
-        setupSearchEngineLogoForTesting(true, false, true, false);
+        setupSearchEngineLogoForTesting(true, false, true);
 
         mMediator.setUrlHasFocus(true);
         mMediator.setShowIconsWhenUrlFocused(true);
@@ -135,19 +143,66 @@ public final class StatusMediatorUnitTest {
     @Test
     @Features.EnableFeatures(ChromeFeatureList.OMNIBOX_SEARCH_ENGINE_LOGO)
     public void searchEngineLogo_onTextChanged_globeReplacesIconWhenTextIsSite() {
-        setupSearchEngineLogoForTesting(true, true, false, true);
+        setupSearchEngineLogoForTesting(true, true, false);
 
         mMediator.setUrlHasFocus(true);
         mMediator.setShowIconsWhenUrlFocused(true);
+        doReturn(TEST_SEARCH_URL).when(mUrlBarEditingTextStateProvider).getTextWithAutocomplete();
+
         mMediator.onTextChanged(TEST_SEARCH_URL);
         Assert.assertEquals(R.drawable.ic_globe_24dp, mModel.get(StatusProperties.STATUS_ICON_RES));
     }
 
+    @Test
+    @Features.EnableFeatures(ChromeFeatureList.OMNIBOX_SEARCH_ENGINE_LOGO)
+    public void searchEngineLogo_onTextChanged_globeReplacesIconWhenAutocompleteSiteContainsText() {
+        setupSearchEngineLogoForTesting(true, true, false);
+
+        mMediator.setUrlHasFocus(true);
+        mMediator.setShowIconsWhenUrlFocused(true);
+        doReturn(TEST_SEARCH_URL).when(mUrlBarEditingTextStateProvider).getTextWithAutocomplete();
+
+        mMediator.onTextChanged(TEST_SEARCH_URL.substring(0, TEST_SEARCH_URL.length() - 1));
+        Assert.assertEquals(R.drawable.ic_globe_24dp, mModel.get(StatusProperties.STATUS_ICON_RES));
+    }
+
+    @Test
+    @Features.EnableFeatures(ChromeFeatureList.OMNIBOX_SEARCH_ENGINE_LOGO)
+    public void searchEngineLogo_onTextChanged_noGlobeReplacementWhenUrlBarTextDoesNotMatch() {
+        setupSearchEngineLogoForTesting(true, true, false);
+
+        mMediator.setUrlHasFocus(true);
+        mMediator.setShowIconsWhenUrlFocused(true);
+        doReturn(TEST_SEARCH_URL).when(mUrlBarEditingTextStateProvider).getTextWithAutocomplete();
+
+        mMediator.onTextChanged("food near me");
+        verify(mDelegate).isUrlValid("food near me");
+        Assert.assertNotEquals(
+                R.drawable.ic_globe_24dp, mModel.get(StatusProperties.STATUS_ICON_RES));
+    }
+
+    @Test
+    @Features.EnableFeatures(ChromeFeatureList.OMNIBOX_SEARCH_ENGINE_LOGO)
+    public void searchEngineLogo_onTextChanged_noGlobeReplacementWhenUrlBarTextIsEmpty() {
+        setupSearchEngineLogoForTesting(true, true, false);
+
+        mMediator.setUrlHasFocus(true);
+        mMediator.setShowIconsWhenUrlFocused(true);
+        // Setup a valid url to prevent the default "" from matching the url.
+        doReturn(TEST_SEARCH_URL).when(mUrlBarEditingTextStateProvider).getTextWithAutocomplete();
+        mMediator.onTextChanged(TEST_SEARCH_URL);
+
+        mMediator.onTextChanged("");
+        verify(mDelegate).isUrlValid("");
+        Assert.assertNotEquals(
+                R.drawable.ic_globe_24dp, mModel.get(StatusProperties.STATUS_ICON_RES));
+    }
+
     @Test
     @Features.EnableFeatures(ChromeFeatureList.OMNIBOX_SEARCH_ENGINE_LOGO)
     public void searchEngineLogo_incognitoNoIcon() {
-        setupSearchEngineLogoForTesting(true, true, false, false);
-        Mockito.doReturn(true).when(mToolbarCommonPropertiesModel).isIncognito();
+        setupSearchEngineLogoForTesting(true, true, false);
+        doReturn(true).when(mToolbarCommonPropertiesModel).isIncognito();
 
         mMediator.setUrlHasFocus(false);
         mMediator.setShowIconsWhenUrlFocused(true);
@@ -160,7 +215,7 @@ public final class StatusMediatorUnitTest {
     @Test
     @Features.EnableFeatures(ChromeFeatureList.OMNIBOX_SEARCH_ENGINE_LOGO)
     public void searchEngineLogo_maybeUpdateStatusIconForSearchEngineIconChanges() {
-        setupSearchEngineLogoForTesting(true, true, false, false);
+        setupSearchEngineLogoForTesting(true, true, false);
 
         mMediator.setUrlHasFocus(true);
         mMediator.setShowIconsWhenUrlFocused(true);
@@ -175,7 +230,7 @@ public final class StatusMediatorUnitTest {
     @Test
     @Features.EnableFeatures(ChromeFeatureList.OMNIBOX_SEARCH_ENGINE_LOGO)
     public void searchEngineLogo_maybeUpdateStatusIconForSearchEngineIconNoChanges() {
-        setupSearchEngineLogoForTesting(true, true, false, false);
+        setupSearchEngineLogoForTesting(true, true, false);
 
         mMediator.setUrlHasFocus(true);
         mMediator.setShowIconsWhenUrlFocused(false);
@@ -219,15 +274,12 @@ public final class StatusMediatorUnitTest {
     }
 
     private void setupSearchEngineLogoForTesting(
-            boolean shouldShowLogo, boolean showGoogle, boolean loupeEverywhere, boolean validUrl) {
-        Mockito.doReturn(shouldShowLogo).when(mDelegate).shouldShowSearchEngineLogo(false);
-        Mockito.doReturn(false).when(mDelegate).shouldShowSearchEngineLogo(true);
-        Mockito.doReturn(loupeEverywhere)
-                .when(mDelegate)
-                .shouldShowSearchLoupeEverywhere(anyBoolean());
+            boolean shouldShowLogo, boolean showGoogle, boolean loupeEverywhere) {
+        doReturn(shouldShowLogo).when(mDelegate).shouldShowSearchEngineLogo(false);
+        doReturn(false).when(mDelegate).shouldShowSearchEngineLogo(true);
+        doReturn(loupeEverywhere).when(mDelegate).shouldShowSearchLoupeEverywhere(anyBoolean());
         Mockito.doNothing().when(mDelegate).getSearchEngineLogoFavicon(
                 any(), mCallbackCaptor.capture());
-        Mockito.doReturn(validUrl).when(mDelegate).isUrlValid(any());
 
         mMediator.updateSearchEngineStatusIcon(shouldShowLogo, showGoogle, TEST_SEARCH_URL);
     }