Commit bbc3d86b authored by Brandon Wylie's avatar Brandon Wylie Committed by Commit Bot

Use dark theme colors for some dse status view icons

The following icons should be colored according to the current theme:
- search loupe
- globe

I've changed the functionality from setting properties in the model to
setting the resource values intermediate variables (icon/tint) and
settings those in the model at the end of the branch. This simplifies
the code a bit and makes it easier to read. To allow for this change,
I've altered the favicon fetching logic to post a task back to the
ui thread for the callback. This way the callback isn't invoked
immediately, which avoids crbug.com/997343 from resurfacing due to
this change.

Bug: 1004903
Change-Id: I6e8102f47e81d0037761d9b111b98d4f1999a225
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1818604
Commit-Queue: Brandon Wylie <wylieb@chromium.org>
Reviewed-by: default avatarTheresa  <twellington@chromium.org>
Reviewed-by: default avatarEnder <ender@google.com>
Cr-Commit-Position: refs/heads/master@{#699992}
parent 419d632e
...@@ -15,6 +15,8 @@ import androidx.annotation.Nullable; ...@@ -15,6 +15,8 @@ import androidx.annotation.Nullable;
import org.chromium.base.Callback; import org.chromium.base.Callback;
import org.chromium.base.VisibleForTesting; import org.chromium.base.VisibleForTesting;
import org.chromium.base.library_loader.LibraryProcessType; import org.chromium.base.library_loader.LibraryProcessType;
import org.chromium.base.task.PostTask;
import org.chromium.base.task.TaskTraits;
import org.chromium.chrome.R; import org.chromium.chrome.R;
import org.chromium.chrome.browser.ChromeFeatureList; import org.chromium.chrome.browser.ChromeFeatureList;
import org.chromium.chrome.browser.favicon.FaviconHelper; import org.chromium.chrome.browser.favicon.FaviconHelper;
...@@ -24,6 +26,7 @@ import org.chromium.chrome.browser.search_engines.TemplateUrlServiceFactory; ...@@ -24,6 +26,7 @@ import org.chromium.chrome.browser.search_engines.TemplateUrlServiceFactory;
import org.chromium.chrome.browser.util.UrlUtilities; import org.chromium.chrome.browser.util.UrlUtilities;
import org.chromium.chrome.browser.widget.RoundedIconGenerator; import org.chromium.chrome.browser.widget.RoundedIconGenerator;
import org.chromium.content_public.browser.BrowserStartupController; import org.chromium.content_public.browser.BrowserStartupController;
import org.chromium.content_public.browser.UiThreadTaskTraits;
import java.util.HashMap; import java.util.HashMap;
import java.util.Map; import java.util.Map;
...@@ -167,18 +170,25 @@ public class SearchEngineLogoUtils { ...@@ -167,18 +170,25 @@ public class SearchEngineLogoUtils {
*/ */
public static void getSearchEngineLogoFavicon( public static void getSearchEngineLogoFavicon(
Profile profile, Resources resources, Callback<Bitmap> callback) { Profile profile, Resources resources, Callback<Bitmap> callback) {
PostTask.postTask(TaskTraits.USER_VISIBLE,
() -> getSearchEngineLogoFaviconInternal(profile, resources, callback));
}
/** @see {@link #getSearchEngineLogoFavicon}. */
private static void getSearchEngineLogoFaviconInternal(
Profile profile, Resources resources, Callback<Bitmap> callback) {
if (sFaviconHelper == null) sFaviconHelper = new FaviconHelper(); if (sFaviconHelper == null) sFaviconHelper = new FaviconHelper();
String logoUrl = getSearchLogoUrl(); String logoUrl = getSearchLogoUrl();
if (logoUrl == null) { if (logoUrl == null) {
callback.onResult(null); returnLogoOnUiThread(null, callback);
return; return;
} }
// Return a cached copy if it's available. // Return a cached copy if it's available.
if (sCachedComposedBackground != null if (sCachedComposedBackground != null
&& sCachedComposedBackgroundLogoUrl.equals(getSearchLogoUrl())) { && sCachedComposedBackgroundLogoUrl.equals(getSearchLogoUrl())) {
callback.onResult(sCachedComposedBackground); returnLogoOnUiThread(sCachedComposedBackground, callback);
return; return;
} }
...@@ -186,13 +196,15 @@ public class SearchEngineLogoUtils { ...@@ -186,13 +196,15 @@ public class SearchEngineLogoUtils {
boolean willReturn = sFaviconHelper.getLocalFaviconImageForURL( boolean willReturn = sFaviconHelper.getLocalFaviconImageForURL(
profile, logoUrl, logoSizePixels, (image, iconUrl) -> { profile, logoUrl, logoSizePixels, (image, iconUrl) -> {
if (image == null) { if (image == null) {
callback.onResult(image); returnLogoOnUiThread(image, callback);
return; return;
} }
processReturnedLogo(logoUrl, image, resources, callback); processReturnedLogo(logoUrl, image, resources, callback);
}); });
if (!willReturn) callback.onResult(null); if (!willReturn) {
returnLogoOnUiThread(null, callback);
}
} }
/** /**
...@@ -236,7 +248,15 @@ public class SearchEngineLogoUtils { ...@@ -236,7 +248,15 @@ public class SearchEngineLogoUtils {
sCachedComposedBackground = composedIcon; sCachedComposedBackground = composedIcon;
sCachedComposedBackgroundLogoUrl = logoUrl; sCachedComposedBackgroundLogoUrl = logoUrl;
callback.onResult(sCachedComposedBackground); returnLogoOnUiThread(sCachedComposedBackground, callback);
}
/**
* @param logo The logo to return to callback.
* @param callback The callback for the calling client.
*/
private static void returnLogoOnUiThread(Bitmap logo, Callback<Bitmap> callback) {
PostTask.postTask(UiThreadTaskTraits.USER_VISIBLE, () -> callback.onResult(logo));
} }
/** /**
......
...@@ -368,6 +368,10 @@ class StatusMediator { ...@@ -368,6 +368,10 @@ class StatusMediator {
* - not shown if URL is focused. * - not shown if URL is focused.
*/ */
private void updateLocationBarIcon() { private void updateLocationBarIcon() {
int icon = 0;
int tint = 0;
int toast = 0;
// Update the accessibility description before continuing since we need it either way. // Update the accessibility description before continuing since we need it either way.
mModel.set(StatusProperties.STATUS_ICON_DESCRIPTION_RES, getAccessibilityDescriptionRes()); mModel.set(StatusProperties.STATUS_ICON_DESCRIPTION_RES, getAccessibilityDescriptionRes());
...@@ -394,32 +398,31 @@ class StatusMediator { ...@@ -394,32 +398,31 @@ class StatusMediator {
mShouldCancelCustomFavicon = false; mShouldCancelCustomFavicon = false;
// If the current url text is a valid url, then swap the dse icon for a globe. // If the current url text is a valid url, then swap the dse icon for a globe.
if (mUrlBarTextIsValidUrl) { if (mUrlBarTextIsValidUrl) {
mModel.set(StatusProperties.STATUS_ICON_RES, R.drawable.ic_globe_24dp); icon = R.drawable.ic_globe_24dp;
} else if (mIsSearchEngineGoogle) { } else if (mIsSearchEngineGoogle) {
mModel.set(StatusProperties.STATUS_ICON_RES, if (mDelegate.shouldShowSearchLoupeEverywhere(isIncognito)) {
mDelegate.shouldShowSearchLoupeEverywhere(isIncognito) icon = R.drawable.ic_search;
? R.drawable.ic_search } else {
: R.drawable.ic_logo_googleg_24dp); icon = R.drawable.ic_logo_googleg_24dp;
}
} else { } else {
mModel.set(StatusProperties.STATUS_ICON_RES, R.drawable.ic_search); icon = R.drawable.ic_search;
if (!mDelegate.shouldShowSearchLoupeEverywhere(isIncognito)) { if (!mDelegate.shouldShowSearchLoupeEverywhere(isIncognito)) {
mDelegate.getSearchEngineLogoFavicon(mResources, (favicon) -> { mDelegate.getSearchEngineLogoFavicon(mResources, (favicon) -> {
if (favicon == null || mShouldCancelCustomFavicon) return; if (favicon == null || mShouldCancelCustomFavicon) return;
mModel.set(StatusProperties.STATUS_ICON, favicon); mModel.set(StatusProperties.STATUS_ICON, favicon);
mModel.set(StatusProperties.STATUS_ICON_TINT_RES, 0);
}); });
} }
} }
// None of the icons associated with dse should be tinted. tint = icon == R.drawable.ic_logo_googleg_24dp ? 0 : mSecurityIconTintRes;
mModel.set(StatusProperties.STATUS_ICON_TINT_RES, 0); mModel.set(StatusProperties.STATUS_ICON_RES, icon);
mModel.set(StatusProperties.STATUS_ICON_TINT_RES, tint);
return; return;
} else { } else {
mShouldCancelCustomFavicon = true; mShouldCancelCustomFavicon = true;
} }
int icon = 0;
int tint = 0;
int toast = 0;
mIsSecurityButtonShown = false; mIsSecurityButtonShown = false;
if (mUrlHasFocus) { if (mUrlHasFocus) {
if (mShowStatusIconWhenUrlFocused) { if (mShowStatusIconWhenUrlFocused) {
......
...@@ -141,6 +141,18 @@ public final class StatusMediatorUnitTest { ...@@ -141,6 +141,18 @@ public final class StatusMediatorUnitTest {
Assert.assertEquals(0, mModel.get(StatusProperties.STATUS_ICON_RES)); Assert.assertEquals(0, mModel.get(StatusProperties.STATUS_ICON_RES));
} }
@Test
@Features.EnableFeatures(ChromeFeatureList.OMNIBOX_SEARCH_ENGINE_LOGO)
public void searchEngineLogo_tintAppliedToLoupe() {
setupSearchEngineLogoForTesting(true, true, true, false);
Mockito.doReturn(false).when(mToolbarCommonPropertiesModel).isIncognito();
mMediator.setUrlHasFocus(true);
mMediator.setShowIconsWhenUrlFocused(true);
mMediator.setSecurityIconTint(11);
Assert.assertEquals(11, mModel.get(StatusProperties.STATUS_ICON_TINT_RES));
}
private void setupSearchEngineLogoForTesting( private void setupSearchEngineLogoForTesting(
boolean shouldShowLogo, boolean showGoogle, boolean loupeEverywhere, boolean validUrl) { boolean shouldShowLogo, boolean showGoogle, boolean loupeEverywhere, boolean validUrl) {
Mockito.doReturn(shouldShowLogo).when(mDelegate).shouldShowSearchEngineLogo(false); Mockito.doReturn(shouldShowLogo).when(mDelegate).shouldShowSearchEngineLogo(false);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment