On unfocus, fade the dse icon out completely before text overlaps

Introducing a conversion function to fade the icon in/out in a way that
it never intersects with the url text.
- Adding separate functions for setting the alpha/visibility of the
status icon.
- Supporting non-google engines for alpha fading.

Bug: 1007577,1018870
Change-Id: Ibcc76087f2ca964614f8bb3b4a18c04e4edee753
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1874577
Commit-Queue: Brandon Wylie <wylieb@chromium.org>
Reviewed-by: Theresa  <twellington@chromium.org>
Reviewed-by: Patrick Noland <pnoland@chromium.org>
Reviewed-by: Ender <ender@google.com>
Cr-Commit-Position: refs/heads/master@{#712819}

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

@@ -14,6 +14,7 @@ import android.view.WindowManager;
 
 import org.chromium.chrome.R;
 import org.chromium.chrome.browser.WindowDelegate;
+import org.chromium.chrome.browser.ntp.NewTabPage;
 import org.chromium.chrome.browser.omnibox.status.StatusView;
 
 /**
@@ -187,8 +188,7 @@ public class LocationBarPhone extends LocationBarLayout {
                         - mStatusView.getEndPaddingPixelSizeForFocusState(false));
 
         if (!hasFocus && mIconView.getVisibility() == VISIBLE
-                && mToolbarDataProvider.getNewTabPageForCurrentTab() != null
-                && !mToolbarDataProvider.getTab().isLoading()) {
+                && SearchEngineLogoUtils.currentlyOnNTP(mToolbarDataProvider)) {
             // When:
             // 1. unfocusing the LocationBar on the NTP.
             // 2. scrolling the fakebox to the LocationBar on the NTP.
@@ -292,6 +292,7 @@ public class LocationBarPhone extends LocationBarLayout {
             setSoftInputMode(WindowManager.LayoutParams.SOFT_INPUT_ADJUST_PAN, false);
             getWindowAndroid().getKeyboardDelegate().showKeyboard(mUrlBar);
         }
+        mStatusViewCoordinator.onUrlAnimationFinished(hasFocus);
         setUrlFocusChangeInProgress(false);
         updateShouldAnimateIconChanges();
     }
@@ -352,9 +353,30 @@ public class LocationBarPhone extends LocationBarLayout {
         boolean isIncognito = getToolbarDataProvider().isIncognito();
         boolean shouldShowSearchEngineLogo =
                 SearchEngineLogoUtils.shouldShowSearchEngineLogo(isIncognito);
-        updateUrlBarPaddingForSearchEngineIcon();
-        if (mIconView != null) mIconView.setVisibility(shouldShowSearchEngineLogo ? VISIBLE : GONE);
         setShowIconsWhenUrlFocused(shouldShowSearchEngineLogo);
         mFirstVisibleFocusedView = shouldShowSearchEngineLogo ? mStatusView : mUrlBar;
+
+        updateStatusVisibility();
+        updateUrlBarPaddingForSearchEngineIcon();
+    }
+
+    @Override
+    public void onTabLoadingNTP(NewTabPage ntp) {
+        super.onTabLoadingNTP(ntp);
+        updateStatusVisibility();
+    }
+
+    /** Update the status visibility according to the current state held in LocationBar. */
+    private void updateStatusVisibility() {
+        boolean incognito = getToolbarDataProvider().isIncognito();
+        if (!SearchEngineLogoUtils.shouldShowSearchEngineLogo(incognito)) {
+            return;
+        }
+
+        if (SearchEngineLogoUtils.currentlyOnNTP(mToolbarDataProvider)) {
+            mStatusViewCoordinator.setStatusIconShown(hasFocus());
+        } else {
+            mStatusViewCoordinator.setStatusIconShown(true);
+        }
     }
 }

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

@@ -21,8 +21,10 @@ import org.chromium.chrome.R;
 import org.chromium.chrome.browser.ChromeFeatureList;
 import org.chromium.chrome.browser.favicon.FaviconHelper;
 import org.chromium.chrome.browser.locale.LocaleManager;
+import org.chromium.chrome.browser.ntp.NewTabPage;
 import org.chromium.chrome.browser.profiles.Profile;
 import org.chromium.chrome.browser.search_engines.TemplateUrlServiceFactory;
+import org.chromium.chrome.browser.toolbar.ToolbarCommonPropertiesModel;
 import org.chromium.chrome.browser.ui.widget.RoundedIconGenerator;
 import org.chromium.chrome.browser.util.UrlUtilities;
 import org.chromium.content_public.browser.BrowserStartupController;
@@ -154,6 +156,13 @@ public class SearchEngineLogoUtils {
         return UrlUtilities.sameDomainOrHost(url, getSearchLogoUrl(), false);
     }
 
+    /** @return Whether the status icon should be hidden when the LocationBar is unfocused. */
+    public static boolean currentlyOnNTP(
+            ToolbarCommonPropertiesModel toolbarCommonPropertiesModel) {
+        return toolbarCommonPropertiesModel.getNewTabPageForCurrentTab() != null
+                && NewTabPage.isNTPUrl(toolbarCommonPropertiesModel.getCurrentUrl());
+    }
+
     /**
      * @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

@@ -23,6 +23,7 @@ import org.chromium.chrome.browser.omnibox.suggestions.AutocompleteCoordinatorFa
 import org.chromium.chrome.browser.profiles.Profile;
 import org.chromium.chrome.browser.toolbar.ToolbarColors;
 import org.chromium.chrome.browser.toolbar.ToolbarCommonPropertiesModel;
+import org.chromium.chrome.browser.util.MathUtils;
 import org.chromium.components.security_state.ConnectionSecurityLevel;
 import org.chromium.content_public.browser.BrowserStartupController;
 import org.chromium.ui.modelutil.PropertyModel;
@@ -93,6 +94,13 @@ class StatusMediator {
     private String mUrlBarTextWithAutocomplete = "";
     private boolean mUrlBarTextIsValidUrl;
     private float mUrlFocusPercent;
+    // Factors used to offset the animation of the status icon's alpha adjustment. The full formula
+    // used: alpha = (focusAnimationProgress - mTextOffsetThreshold) / (1 - mTextOffsetThreshold)
+    // mTextOffsetThreshold will be the % space that the icon takes up during the focus animation.
+    // When un/focused padding(s) change, this formula shouldn't need to change.
+    private final float mTextOffsetThreshold;
+    // The denominator for the above formula, which will adjust the scale for the alpha.
+    private final float mTextOffsetAdjustedScale;
 
     StatusMediator(PropertyModel model, Resources resources,
             UrlBarEditingTextStateProvider urlBarEditingTextStateProvider) {
@@ -102,6 +110,15 @@ class StatusMediator {
 
         mResources = resources;
         mUrlBarEditingTextStateProvider = urlBarEditingTextStateProvider;
+
+        int iconWidth = resources.getDimensionPixelSize(R.dimen.location_bar_status_icon_width);
+        mTextOffsetThreshold = (float) iconWidth
+                / (iconWidth
+                        + resources.getDimensionPixelSize(
+                                R.dimen.sei_location_bar_icon_end_padding_focused)
+                        - resources.getDimensionPixelSize(
+                                R.dimen.sei_location_bar_icon_end_padding));
+        mTextOffsetAdjustedScale = mTextOffsetThreshold == 1 ? 1 : (1 - mTextOffsetThreshold);
     }
 
     /**
@@ -230,6 +247,30 @@ class StatusMediator {
         mUrlHasFocus = urlHasFocus;
         updateStatusVisibility();
         updateLocationBarIcon();
+
+        // Show the icon when the url focus animation starts.
+        if (urlHasFocus
+                && SearchEngineLogoUtils.shouldShowSearchEngineLogo(
+                        mToolbarCommonPropertiesModel.isIncognito())
+                && SearchEngineLogoUtils.currentlyOnNTP(mToolbarCommonPropertiesModel)) {
+            setStatusIconShown(true);
+        }
+    }
+
+    void setUrlAnimationFinished(boolean urlHasFocus) {
+        if (!SearchEngineLogoUtils.shouldShowSearchEngineLogo(
+                    mToolbarCommonPropertiesModel.isIncognito())) {
+            return;
+        }
+
+        // Hide the icon when the url unfocus animation finishes.
+        if (!urlHasFocus && SearchEngineLogoUtils.currentlyOnNTP(mToolbarCommonPropertiesModel)) {
+            setStatusIconShown(false);
+        }
+    }
+
+    void setStatusIconShown(boolean show) {
+        mModel.set(StatusProperties.SHOW_STATUS_ICON, show);
     }
 
     /**
@@ -243,10 +284,16 @@ class StatusMediator {
             return;
         }
 
-        if (mToolbarCommonPropertiesModel.getNewTabPageForCurrentTab() != null
-                && mToolbarCommonPropertiesModel.getNewTabPageForCurrentTab()
-                           .isLocationBarShownInNTP()) {
-            mModel.set(StatusProperties.STATUS_ALPHA, percent);
+        // Only fade the animation on the new tab page.
+        if (SearchEngineLogoUtils.currentlyOnNTP(mToolbarCommonPropertiesModel)) {
+            float focusAnimationProgress = percent;
+            if (!mUrlHasFocus) {
+                focusAnimationProgress = MathUtils.clamp(
+                        (percent - mTextOffsetThreshold) / mTextOffsetAdjustedScale, 0f, 1f);
+            }
+            mModel.set(StatusProperties.STATUS_ALPHA, focusAnimationProgress);
+        } else {
+            mModel.set(StatusProperties.STATUS_ALPHA, 1f);
         }
 
         updateLocationBarIcon();

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

@@ -33,6 +33,9 @@ class StatusProperties {
     /** Specifies the icon alpha. */
     static final WritableFloatPropertyKey STATUS_ALPHA = new WritableFloatPropertyKey();
 
+    /** Specifies if the icon should be shown or not. */
+    static final WritableBooleanPropertyKey SHOW_STATUS_ICON = new WritableBooleanPropertyKey();
+
     /** Specifies accessibility string presented to user upon long click on security icon. */
     public static final WritableIntPropertyKey STATUS_ICON_ACCESSIBILITY_TOAST_RES =
             new WritableIntPropertyKey();
@@ -67,8 +70,8 @@ class StatusProperties {
 
     public static final PropertyKey[] ALL_KEYS = new PropertyKey[] {ANIMATIONS_ENABLED,
             STATUS_ICON_ACCESSIBILITY_TOAST_RES, STATUS_ICON_RES, STATUS_ICON_TINT_RES, STATUS_ICON,
-            STATUS_ALPHA, STATUS_ICON_DESCRIPTION_RES, SEPARATOR_COLOR_RES, STATUS_CLICK_LISTENER,
-            VERBOSE_STATUS_TEXT_COLOR_RES, VERBOSE_STATUS_TEXT_STRING_RES,
+            STATUS_ALPHA, SHOW_STATUS_ICON, STATUS_ICON_DESCRIPTION_RES, SEPARATOR_COLOR_RES,
+            STATUS_CLICK_LISTENER, VERBOSE_STATUS_TEXT_COLOR_RES, VERBOSE_STATUS_TEXT_STRING_RES,
             VERBOSE_STATUS_TEXT_VISIBLE, VERBOSE_STATUS_TEXT_WIDTH, INCOGNITO_BADGE_VISIBLE};
 
     private StatusProperties() {}

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

@@ -136,48 +136,16 @@ public class StatusView extends LinearLayout {
                                                     oldBottom) -> updateTouchDelegate());
     }
 
-    /**
-     * Start animating transition of status icon.
-     */
-    private void animateStatusIcon() {
-        Drawable targetIcon = null;
-        boolean wantIconHidden = false;
-
-        if (mIconRes != 0 && mIconTintRes != 0) {
-            targetIcon = UiUtils.getTintedDrawable(getContext(), mIconRes, mIconTintRes);
-        } else if (mIconRes != 0) {
-            targetIcon = AppCompatResources.getDrawable(getContext(), mIconRes);
-        } else if (mIconBitmap != null) {
-            targetIcon = new BitmapDrawable(getResources(), mIconBitmap);
-        } else {
-            // Do not specify any icon here and do not replace existing icon, either.
-            // TransitionDrawable uses different timing mechanism than Animations, and that may,
-            // depending on animation scale factor, produce a visible glitch.
-            targetIcon = null;
-            wantIconHidden = true;
+    private void setupAndRunStatusIconAnimations(boolean wantIconHidden, boolean isIconHidden) {
+        // This is to prevent the visibility of the view being changed both implicitly here and
+        // explicitly in setStatusIconShown. The visibility should only be set here through code not
+        // related to the dse icon.
+        if (mToolbarCommonPropertiesModel != null
+                && mDelegate.shouldShowSearchEngineLogo(
+                        mToolbarCommonPropertiesModel.isIncognito())) {
+            return;
         }
 
-        // Ensure proper handling of animations.
-        // Possible variants:
-        // 1. Is: shown,           want: hidden  => animate hiding,
-        // 2. Is: shown,           want: shown   => crossfade w/TransitionDrawable,
-        // 3. Is: animating(show), want: hidden  => cancel animation; animate hiding,
-        // 4. Is: animating(show), want: shown   => crossfade (carry on showing),
-        // 5. Is: animating(hide), want: hidden  => no op,
-        // 6. Is: animating(hide), want: shown   => cancel animation; animate showing; crossfade,
-        // 7. Is: hidden,          want: hidden  => no op,
-        // 8. Is: hidden,          want: shown   => animate showing.
-        //
-        // This gives 3 actions:
-        // - Animate showing, if hidden or previously hiding (6 + 8); cancel previous animation (6)
-        // - Animate hiding, if shown or previously showing (1 + 3); cancel previous animation (3)
-        // - crossfade w/TransitionDrawable, if visible (2, 4, 6), otherwise use image directly.
-        // All other options (5, 7) are no-op.
-        //
-        // Note: this will be compacted once we start using LayoutTransition with StatusView.
-
-        boolean isIconHidden = mIconView.getVisibility() == View.GONE || mIconView.getAlpha() == 0f;
-
         if (!wantIconHidden && (isIconHidden || mAnimatingStatusIconHide)) {
             // Action 1: animate showing, if icon was either hidden or hiding.
             if (mAnimatingStatusIconHide) mIconView.animate().cancel();
@@ -221,6 +189,53 @@ public class StatusView extends LinearLayout {
         } else {
             updateTouchDelegate();
         }
+    }
+
+    /**
+     * Start animating transition of status icon.
+     */
+    private void animateStatusIcon() {
+        Drawable targetIcon;
+        boolean wantIconHidden = false;
+
+        if (mIconRes != 0 && mIconTintRes != 0) {
+            targetIcon = UiUtils.getTintedDrawable(getContext(), mIconRes, mIconTintRes);
+        } else if (mIconRes != 0) {
+            targetIcon = AppCompatResources.getDrawable(getContext(), mIconRes);
+        } else if (mIconBitmap != null) {
+            targetIcon = new BitmapDrawable(getResources(), mIconBitmap);
+        } else {
+            // Do not specify any icon here and do not replace existing icon, either.
+            // TransitionDrawable uses different timing mechanism than Animations, and that may,
+            // depending on animation scale factor, produce a visible glitch.
+            targetIcon = null;
+            wantIconHidden = true;
+        }
+
+        // Ensure proper handling of animations.
+        // Possible variants:
+        // 1. Is: shown,           want: hidden  => animate hiding,
+        // 2. Is: shown,           want: shown   => crossfade w/TransitionDrawable,
+        // 3. Is: animating(show), want: hidden  => cancel animation; animate hiding,
+        // 4. Is: animating(show), want: shown   => crossfade (carry on showing),
+        // 5. Is: animating(hide), want: hidden  => no op,
+        // 6. Is: animating(hide), want: shown   => cancel animation; animate showing; crossfade,
+        // 7. Is: hidden,          want: hidden  => no op,
+        // 8. Is: hidden,          want: shown   => animate showing.
+        //
+        // This gives 3 actions:
+        // - Animate showing, if hidden or previously hiding (6 + 8); cancel previous animation (6)
+        // - Animate hiding, if shown or previously showing (1 + 3); cancel previous animation (3)
+        // - crossfade w/TransitionDrawable, if visible (2, 4, 6), otherwise use image directly.
+        // All other options (5, 7) are no-op.
+        //
+        // Note: this will be compacted once we start using LayoutTransition with StatusView.
+
+        boolean isIconHidden = mIconView.getVisibility() == View.GONE;
+
+        // Actions 1 and 2 occur in #setupAndRunStatusIconAnimations.
+        // TODO(crbug.com/1019488): Consolidate animation behavior once the dse icon feature ships.
+        setupAndRunStatusIconAnimations(wantIconHidden, isIconHidden);
 
         // Action 3: Specify icon content. Use TransitionDrawable whenever object is visible.
         if (targetIcon != null) {
@@ -321,7 +336,25 @@ public class StatusView extends LinearLayout {
     void setStatusIconAlpha(float alpha) {
         if (mIconView == null) return;
         mIconView.setAlpha(alpha);
-        mIconView.setVisibility(alpha > 0f ? VISIBLE : GONE);
+    }
+
+    /** Specify the status icon visibility. */
+    void setStatusIconShown(boolean showIcon) {
+        if (mIconView == null) return;
+
+        // This is to prevent the visibility of the view being changed both explicitly here and
+        // implicitly in animateStatusIcon. The visibility should only be set here through code
+        // related to the dse icon.
+        if (mToolbarCommonPropertiesModel != null
+                && !mDelegate.shouldShowSearchEngineLogo(
+                        mToolbarCommonPropertiesModel.isIncognito())) {
+            // Let developers know that they shouldn't use this code-path.
+            assert false : "Only DSE icon code should set the status icon visibility manually.";
+            return;
+        }
+
+        mIconView.setVisibility(showIcon ? VISIBLE : GONE);
+        updateTouchDelegate();
     }
 
     /**

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

@@ -24,6 +24,8 @@ class StatusViewBinder implements ViewBinder<PropertyModel, StatusView, Property
             view.setStatusIcon(model.get(StatusProperties.STATUS_ICON));
         } else if (StatusProperties.STATUS_ALPHA.equals(propertyKey)) {
             view.setStatusIconAlpha(model.get(StatusProperties.STATUS_ALPHA));
+        } else if (StatusProperties.SHOW_STATUS_ICON.equals(propertyKey)) {
+            view.setStatusIconShown(model.get(StatusProperties.SHOW_STATUS_ICON));
         } else if (StatusProperties.STATUS_ICON_ACCESSIBILITY_TOAST_RES.equals(propertyKey)) {
             view.setStatusIconAccessibilityToast(
                     model.get(StatusProperties.STATUS_ICON_ACCESSIBILITY_TOAST_RES));

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

@@ -28,7 +28,6 @@ public class StatusViewCoordinator implements View.OnClickListener, TextWatcher
     private final StatusMediator mMediator;
     private final PropertyModel mModel;
     private final boolean mIsTablet;
-
     private ToolbarDataProvider mToolbarDataProvider;
     private boolean mUrlHasFocus;
 
@@ -94,6 +93,16 @@ public class StatusViewCoordinator implements View.OnClickListener, TextWatcher
         updateVerboseStatusVisibility();
     }
 
+    /** @param urlHasFocus Whether the url currently has focus. */
+    public void onUrlAnimationFinished(boolean urlHasFocus) {
+        mMediator.setUrlAnimationFinished(urlHasFocus);
+    }
+
+    /** @param show Whether the status icon should be VISIBLE, otherwise GONE. */
+    public void setStatusIconShown(boolean show) {
+        mMediator.setStatusIconShown(show);
+    }
+
     /**
      * Set the url focus change percent.
      * @param percent The current focus percent.

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

@@ -29,6 +29,7 @@ import org.chromium.chrome.R;
 import org.chromium.chrome.browser.ChromeActivity;
 import org.chromium.chrome.browser.ChromeFeatureList;
 import org.chromium.chrome.browser.ChromeSwitches;
+import org.chromium.chrome.browser.tab.Tab;
 import org.chromium.chrome.browser.toolbar.LocationBarModel;
 import org.chromium.chrome.browser.util.UrlConstants;
 import org.chromium.chrome.test.ChromeActivityTestRule;
@@ -120,13 +121,13 @@ public class LocationBarLayoutTest {
     @Before
     public void setUp() throws InterruptedException {
         mActivityTestRule.startMainActivityOnBlankPage();
-        setupTabForTests(false);
+        setupModelsForCurrentTab();
     }
 
-    private void setupTabForTests(boolean isIncognito) {
-        if (isIncognito) mActivityTestRule.loadUrlInNewTab(UrlConstants.NTP_URL, isIncognito);
+    private void setupModelsForCurrentTab() {
         mTestLocationBarModel = new TestLocationBarModel();
-        mTestLocationBarModel.setTab(mActivityTestRule.getActivity().getActivityTab(), isIncognito);
+        Tab tab = mActivityTestRule.getActivity().getActivityTab();
+        mTestLocationBarModel.setTab(tab, tab.isIncognito());
 
         TestThreadUtils.runOnUiThreadBlocking(
                 () -> getLocationBar().setToolbarDataProvider(mTestLocationBarModel));
@@ -244,12 +245,38 @@ public class LocationBarLayoutTest {
     @Restriction(UiRestriction.RESTRICTION_TYPE_PHONE)
     @EnableFeatures(ChromeFeatureList.OMNIBOX_SEARCH_ENGINE_LOGO)
     @Feature({"OmniboxSearchEngineLogo"})
-    public void testOmniboxSearchEngineLogo_goneWhenIncognito() {
+    public void testOmniboxSearchEngineLogo_goneWhenIncognito() throws ExecutionException {
+        mActivityTestRule.loadUrlInNewTab(UrlConstants.NTP_URL, true);
+        setupModelsForCurrentTab();
+
         final LocationBarLayout locationBar = getLocationBar();
         final View iconView = locationBar.getSecurityIconView();
+
+        TestThreadUtils.runOnUiThreadBlocking(() -> {
+            mTestLocationBarModel.setDisplaySearchTerms(null);
+            locationBar.updateSearchEngineStatusIcon(false, false, "");
+        });
+
         onView(withId(R.id.location_bar_status))
-                .check((view, e) -> Assert.assertEquals(iconView.getVisibility(), VISIBLE));
-        setupTabForTests(true);
+                .check((view, e) -> Assert.assertEquals(iconView.getVisibility(), GONE));
+    }
+
+    @Test
+    @SmallTest
+    @Restriction(UiRestriction.RESTRICTION_TYPE_PHONE)
+    @EnableFeatures(ChromeFeatureList.OMNIBOX_SEARCH_ENGINE_LOGO)
+    @Feature({"OmniboxSearchEngineLogo"})
+    public void testOmniboxSearchEngineLogo_goneOnNTP() {
+        mActivityTestRule.loadUrlInNewTab(UrlConstants.NTP_URL, false);
+        setupModelsForCurrentTab();
+
+        final LocationBarLayout locationBar = getLocationBar();
+        final View iconView = locationBar.getSecurityIconView();
+        TestThreadUtils.runOnUiThreadBlocking(() -> {
+            mTestLocationBarModel.setDisplaySearchTerms(null);
+            locationBar.updateVisualsForState();
+            locationBar.updateSearchEngineStatusIcon(false, false, "");
+        });
         onView(withId(R.id.location_bar_status))
                 .check((view, e) -> Assert.assertEquals(iconView.getVisibility(), GONE));
     }