Skip to content

Commit

Permalink
Consolidate code related to colors in color_utils_android.cc 2/2
Browse files Browse the repository at this point in the history
This CL:
- Deletes ui/android/color_helpers.h
- Replaces ui/android/color_helpers.h with
  ui/android/color_utils_android.h
- Replaces WebappConstants.MANIFEST_COLOR_INVALID_OR_MISSING usage with
  ColorUtils.INVALID_COLOR

BUG=1256491

Change-Id: I84f825ce95d8b19b5656cc609c8a75d80044dcc6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3215110
Reviewed-by: Ted Choc <[email protected]>
Reviewed-by: Bo <[email protected]>
Reviewed-by: Ella Ge <[email protected]>
Commit-Queue: Peter Kotwicz <[email protected]>
Cr-Commit-Position: refs/heads/main@{#930787}
  • Loading branch information
pkotwicz authored and Chromium LUCI CQ committed Oct 12, 2021
1 parent 23c4c16 commit ecdbbc7
Show file tree
Hide file tree
Showing 23 changed files with 46 additions and 61 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
import org.chromium.components.webapps.ShortcutSource;
import org.chromium.components.webapps.WebApkDistributor;
import org.chromium.device.mojom.ScreenOrientationLockType;
import org.chromium.ui.util.ColorUtils;
import org.chromium.webapk.lib.common.WebApkCommonUtils;
import org.chromium.webapk.lib.common.WebApkMetaDataUtils;
import org.chromium.webapk.lib.common.splash.SplashLayout;
Expand Down Expand Up @@ -262,11 +263,10 @@ public static BrowserServicesIntentDataProvider create(Intent intent, String web
IntentUtils.safeGetString(bundle, WebApkMetaDataKeys.DISPLAY_MODE));
int orientation = orientationFromString(
IntentUtils.safeGetString(bundle, WebApkMetaDataKeys.ORIENTATION));
long themeColor = WebApkMetaDataUtils.getLongFromMetaData(bundle,
WebApkMetaDataKeys.THEME_COLOR, WebappConstants.MANIFEST_COLOR_INVALID_OR_MISSING);
long backgroundColor =
WebApkMetaDataUtils.getLongFromMetaData(bundle, WebApkMetaDataKeys.BACKGROUND_COLOR,
WebappConstants.MANIFEST_COLOR_INVALID_OR_MISSING);
long themeColor = WebApkMetaDataUtils.getLongFromMetaData(
bundle, WebApkMetaDataKeys.THEME_COLOR, ColorUtils.INVALID_COLOR);
long backgroundColor = WebApkMetaDataUtils.getLongFromMetaData(
bundle, WebApkMetaDataKeys.BACKGROUND_COLOR, ColorUtils.INVALID_COLOR);

// Fetch the default background color from the WebAPK's resources. Fetching the default
// background color from the WebAPK is important for consistency when:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import org.chromium.components.webapk.lib.common.WebApkConstants;
import org.chromium.components.webapps.ShortcutSource;
import org.chromium.device.mojom.ScreenOrientationLockType;
import org.chromium.ui.util.ColorUtils;

import java.io.File;

Expand Down Expand Up @@ -226,10 +227,8 @@ public Intent createWebappLaunchIntent() {
mPreferences.getString(KEY_ICON, null), version,
mPreferences.getInt(KEY_DISPLAY_MODE, DisplayMode.STANDALONE),
mPreferences.getInt(KEY_ORIENTATION, ScreenOrientationLockType.DEFAULT),
mPreferences.getLong(
KEY_THEME_COLOR, WebappConstants.MANIFEST_COLOR_INVALID_OR_MISSING),
mPreferences.getLong(
KEY_BACKGROUND_COLOR, WebappConstants.MANIFEST_COLOR_INVALID_OR_MISSING),
mPreferences.getLong(KEY_THEME_COLOR, ColorUtils.INVALID_COLOR),
mPreferences.getLong(KEY_BACKGROUND_COLOR, ColorUtils.INVALID_COLOR),
mPreferences.getBoolean(KEY_IS_ICON_GENERATED, false),
mPreferences.getBoolean(KEY_IS_ICON_ADAPTIVE, false));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import org.chromium.chrome.browser.browserservices.intents.WebappIntentUtils;
import org.chromium.components.webapps.ShortcutSource;
import org.chromium.device.mojom.ScreenOrientationLockType;
import org.chromium.ui.util.ColorUtils;
import org.chromium.webapk.lib.common.splash.SplashLayout;

/**
Expand Down Expand Up @@ -66,8 +67,8 @@ public static BrowserServicesIntentDataProvider create(Intent intent) {
return null;
}

long themeColor = IntentUtils.safeGetLongExtra(intent, WebappConstants.EXTRA_THEME_COLOR,
WebappConstants.MANIFEST_COLOR_INVALID_OR_MISSING);
long themeColor = IntentUtils.safeGetLongExtra(
intent, WebappConstants.EXTRA_THEME_COLOR, ColorUtils.INVALID_COLOR);
boolean hasValidToolbarColor = WebappIntentUtils.isLongColorValid(themeColor);
int toolbarColor = hasValidToolbarColor ? (int) themeColor
: WebappIntentDataProvider.getDefaultToolbarColor();
Expand All @@ -85,9 +86,8 @@ public static BrowserServicesIntentDataProvider create(Intent intent) {
int orientation = IntentUtils.safeGetIntExtra(
intent, WebappConstants.EXTRA_ORIENTATION, ScreenOrientationLockType.DEFAULT);
int source = sourceFromIntent(intent);
Integer backgroundColor = WebappIntentUtils.colorFromLongColor(
IntentUtils.safeGetLongExtra(intent, WebappConstants.EXTRA_BACKGROUND_COLOR,
WebappConstants.MANIFEST_COLOR_INVALID_OR_MISSING));
Integer backgroundColor = WebappIntentUtils.colorFromLongColor(IntentUtils.safeGetLongExtra(
intent, WebappConstants.EXTRA_BACKGROUND_COLOR, ColorUtils.INVALID_COLOR));
boolean isIconGenerated = IntentUtils.safeGetBooleanExtra(
intent, WebappConstants.EXTRA_IS_ICON_GENERATED, false);
boolean isIconAdaptive = IntentUtils.safeGetBooleanExtra(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@
import org.chromium.components.webapk.lib.common.WebApkMetaDataKeys;
import org.chromium.components.webapps.WebApkDistributor;
import org.chromium.device.mojom.ScreenOrientationLockType;
import org.chromium.ui.util.ColorUtils;
import org.chromium.webapk.lib.common.WebApkConstants;
import org.chromium.webapk.lib.common.splash.SplashLayout;
import org.chromium.webapk.test.WebApkTestHelper;
Expand Down Expand Up @@ -1136,11 +1137,11 @@ public void testDefaultBackgroundColorHasChangedShouldNotUpgrade() {
assertNotEquals(oldDefaultBackgroundColor, splashLayoutDefaultBackgroundColor);

ManifestData androidManifestData = defaultManifestData();
androidManifestData.backgroundColor = WebappConstants.MANIFEST_COLOR_INVALID_OR_MISSING;
androidManifestData.backgroundColor = ColorUtils.INVALID_COLOR;
androidManifestData.defaultBackgroundColor = oldDefaultBackgroundColor;

ManifestData fetchedManifestData = defaultManifestData();
fetchedManifestData.backgroundColor = WebappConstants.MANIFEST_COLOR_INVALID_OR_MISSING;
fetchedManifestData.backgroundColor = ColorUtils.INVALID_COLOR;
fetchedManifestData.defaultBackgroundColor = splashLayoutDefaultBackgroundColor;

assertFalse(checkUpdateNeededForFetchedManifest(androidManifestData, fetchedManifestData));
Expand Down
1 change: 1 addition & 0 deletions chrome/browser/android/browserservices/intents/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -59,5 +59,6 @@ android_library("junit") {
"//third_party/blink/public/mojom:mojom_platform_java",
"//third_party/hamcrest:hamcrest_library_java",
"//third_party/junit",
"//ui/android:ui_no_recycler_view_java",
]
}
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
import org.chromium.components.webapps.ShortcutSource;
import org.chromium.components.webapps.WebApkDistributor;
import org.chromium.device.mojom.ScreenOrientationLockType;
import org.chromium.ui.util.ColorUtils;
import org.chromium.webapk.lib.common.WebApkConstants;
import org.chromium.webapk.lib.common.splash.SplashLayout;
import org.chromium.webapk.test.WebApkTestHelper;
Expand Down Expand Up @@ -696,8 +697,7 @@ public void testShareDataUriList() {
public void testBackgroundColorFallbackToDefaultNoCustomDefault() {
Bundle bundle = new Bundle();
bundle.putString(WebApkMetaDataKeys.START_URL, START_URL);
bundle.putString(WebApkMetaDataKeys.BACKGROUND_COLOR,
WebappConstants.MANIFEST_COLOR_INVALID_OR_MISSING + "L");
bundle.putString(WebApkMetaDataKeys.BACKGROUND_COLOR, ColorUtils.INVALID_COLOR + "L");
WebApkTestHelper.registerWebApkWithMetaData(WEBAPK_PACKAGE_NAME, bundle, null);

Intent intent = new Intent();
Expand All @@ -720,8 +720,7 @@ public void testBackgroundColorFallbackToDefaultWebApkHasCustomDefault() {

Bundle bundle = new Bundle();
bundle.putString(WebApkMetaDataKeys.START_URL, START_URL);
bundle.putString(WebApkMetaDataKeys.BACKGROUND_COLOR,
WebappConstants.MANIFEST_COLOR_INVALID_OR_MISSING + "L");
bundle.putString(WebApkMetaDataKeys.BACKGROUND_COLOR, ColorUtils.INVALID_COLOR + "L");
bundle.putInt(
WebApkMetaDataKeys.DEFAULT_BACKGROUND_COLOR_ID, defaultBackgroundColorResourceId);
WebApkTestHelper.registerWebApkWithMetaData(WEBAPK_PACKAGE_NAME, bundle, null);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
package org.chromium.chrome.browser.browserservices.intents;

import org.chromium.content_public.common.ScreenOrientationConstants;
import org.chromium.ui.util.ColorUtils;

/**
* This class contains constants related to adding shortcuts to the Android Home
Expand Down Expand Up @@ -46,8 +45,5 @@ public class WebappConstants {
// be correctly populated into the WebappRegistry/WebappDataStorage.
public static final int WEBAPP_SHORTCUT_VERSION = 3;

// This value is equal to kInvalidOrMissingColor in the C++ blink::Manifest struct.
public static final long MANIFEST_COLOR_INVALID_OR_MISSING = ColorUtils.INVALID_COLOR;

private WebappConstants() {}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import org.chromium.components.webapps.ShortcutSource;
import org.chromium.components.webapps.WebApkDistributor;
import org.chromium.device.mojom.ScreenOrientationLockType;
import org.chromium.ui.util.ColorUtils;

import java.util.List;
import java.util.Map;
Expand Down Expand Up @@ -95,11 +96,11 @@ public int source() {

/**
* Returns the toolbar color if it is valid, and
* WebappConstants.MANIFEST_COLOR_INVALID_OR_MISSING otherwise.
* ColorUtils.INVALID_COLOR otherwise.
*/
public long toolbarColor() {
return hasValidToolbarColor() ? mProvider.getColorProvider().getToolbarColor()
: WebappConstants.MANIFEST_COLOR_INVALID_OR_MISSING;
: ColorUtils.INVALID_COLOR;
}

/**
Expand All @@ -112,7 +113,7 @@ public boolean hasValidToolbarColor() {
/**
* Background color is actually a 32 bit unsigned integer which encodes a color
* in ARGB format. Return value is a long because we also need to encode the
* error state of WebappConstants.MANIFEST_COLOR_INVALID_OR_MISSING.
* error state of ColorUtils.INVALID_COLOR.
*/
public long backgroundColor() {
return WebappIntentUtils.colorFromIntegerColor(getWebappExtras().backgroundColor);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import org.chromium.chrome.test.util.browser.webapps.WebappTestHelper;
import org.chromium.components.webapps.ShortcutSource;
import org.chromium.device.mojom.ScreenOrientationLockType;
import org.chromium.ui.util.ColorUtils;

/**
* Tests the WebappInfo class's ability to parse various URLs.
Expand Down Expand Up @@ -166,9 +167,8 @@ public void testInvalidOrMissingColors() {
intent.putExtra(WebappConstants.EXTRA_NAME, name);
intent.putExtra(WebappConstants.EXTRA_SHORT_NAME, shortName);
WebappInfo info = createWebappInfo(intent);
Assert.assertEquals(WebappConstants.MANIFEST_COLOR_INVALID_OR_MISSING, info.toolbarColor());
Assert.assertEquals(
WebappConstants.MANIFEST_COLOR_INVALID_OR_MISSING, info.backgroundColor());
Assert.assertEquals(ColorUtils.INVALID_COLOR, info.toolbarColor());
Assert.assertEquals(ColorUtils.INVALID_COLOR, info.backgroundColor());
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import android.provider.Browser;

import org.chromium.base.IntentUtils;
import org.chromium.ui.util.ColorUtils;
import org.chromium.webapk.lib.common.WebApkConstants;

import java.util.HashSet;
Expand Down Expand Up @@ -47,22 +48,22 @@ public class WebappIntentUtils {
/**
* Converts color from signed Integer where an unspecified color is represented as null to
* to unsigned long where an unspecified color is represented as
* {@link WebappConstants.MANIFEST_COLOR_INVALID_OR_MISSING}.
* {@link ColorUtils.INVALID_COLOR}.
*/
public static long colorFromIntegerColor(Integer color) {
if (color != null) {
return color.intValue();
}
return WebappConstants.MANIFEST_COLOR_INVALID_OR_MISSING;
return ColorUtils.INVALID_COLOR;
}

public static boolean isLongColorValid(long longColor) {
return (longColor != WebappConstants.MANIFEST_COLOR_INVALID_OR_MISSING);
return (longColor != ColorUtils.INVALID_COLOR);
}

/**
* Converts color from unsigned long where an unspecified color is represented as
* {@link WebappConstants.MANIFEST_COLOR_INVALID_OR_MISSING} to a signed Integer where an
* {@link ColorUtils.INVALID_COLOR} to a signed Integer where an
* unspecified color is represented as null.
*/
public static Integer colorFromLongColor(long longColor) {
Expand Down
2 changes: 1 addition & 1 deletion chrome/browser/android/shortcut_helper.cc
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
#include "components/webapps/browser/android/shortcut_info.h"
#include "content/public/browser/manifest_icon_downloader.h"
#include "content/public/browser/web_contents.h"
#include "ui/android/color_helpers.h"
#include "ui/android/color_utils_android.h"
#include "ui/gfx/android/java_bitmap.h"
#include "url/gurl.h"

Expand Down
2 changes: 1 addition & 1 deletion chrome/browser/android/webapk/webapk_handler_delegate.cc
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
#include "chrome/android/chrome_jni_headers/WebApkHandlerDelegate_jni.h"
#include "third_party/blink/public/mojom/manifest/display_mode.mojom.h"
#include "third_party/skia/include/core/SkColor.h"
#include "ui/android/color_helpers.h"
#include "ui/android/color_utils_android.h"

using base::android::JavaParamRef;

Expand Down
2 changes: 1 addition & 1 deletion chrome/browser/android/webapk/webapk_installer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@
#include "services/network/public/cpp/simple_url_loader.h"
#include "services/network/public/mojom/url_response_head.mojom.h"
#include "third_party/abseil-cpp/absl/types/optional.h"
#include "ui/android/color_helpers.h"
#include "ui/android/color_utils_android.h"
#include "ui/gfx/android/java_bitmap.h"
#include "ui/gfx/codec/png_codec.h"
#include "url/origin.h"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
#include "third_party/blink/public/common/manifest/manifest_util.h"
#include "third_party/blink/public/mojom/manifest/manifest.mojom.h"
#include "third_party/smhasher/src/MurmurHash2.h"
#include "ui/android/color_helpers.h"
#include "ui/android/color_utils_android.h"
#include "ui/gfx/android/java_bitmap.h"
#include "ui/gfx/codec/png_codec.h"
#include "url/gurl.h"
Expand Down
2 changes: 1 addition & 1 deletion chrome/browser/android/webapk/webapk_update_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
#include "content/public/browser/browser_thread.h"
#include "third_party/blink/public/mojom/manifest/manifest.mojom.h"
#include "third_party/skia/include/core/SkBitmap.h"
#include "ui/android/color_helpers.h"
#include "ui/android/color_utils_android.h"
#include "ui/gfx/android/java_bitmap.h"
#include "url/gurl.h"

Expand Down
2 changes: 1 addition & 1 deletion chrome/browser/ui/webui/webapks/webapks_handler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
#include "chrome/browser/android/shortcut_helper.h"
#include "content/public/browser/web_ui.h"
#include "third_party/blink/public/common/manifest/manifest_util.h"
#include "ui/android/color_helpers.h"
#include "ui/android/color_utils_android.h"
#include "ui/gfx/color_utils.h"

WebApksHandler::WebApksHandler()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,11 @@

import org.chromium.blink.mojom.DisplayMode;
import org.chromium.chrome.browser.browserservices.intents.BrowserServicesIntentDataProvider;
import org.chromium.chrome.browser.browserservices.intents.WebappConstants;
import org.chromium.chrome.browser.webapps.WebApkIntentDataProviderFactory;
import org.chromium.components.webapps.ShortcutSource;
import org.chromium.components.webapps.WebApkDistributor;
import org.chromium.device.mojom.ScreenOrientationLockType;
import org.chromium.ui.util.ColorUtils;

import java.util.ArrayList;
import java.util.HashMap;
Expand Down Expand Up @@ -54,8 +54,7 @@ public void setWebApkVersionCode(int versionCode) {
public BrowserServicesIntentDataProvider build() {
return WebApkIntentDataProviderFactory.create(new Intent(), mUrl, mScope, null, null, null,
null, mDisplayMode, ScreenOrientationLockType.DEFAULT, ShortcutSource.UNKNOWN,
WebappConstants.MANIFEST_COLOR_INVALID_OR_MISSING,
WebappConstants.MANIFEST_COLOR_INVALID_OR_MISSING, Color.WHITE,
ColorUtils.INVALID_COLOR, ColorUtils.INVALID_COLOR, Color.WHITE,
false /* isPrimaryIconMaskable */, false /* isSplashIconMaskable */,
mWebApkPackageName, /* shellApkVersion */ 1, mManifestUrl, mUrl,
WebApkDistributor.BROWSER,
Expand Down
2 changes: 1 addition & 1 deletion components/thin_webview/internal/compositor_view_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
#include "components/thin_webview/internal/jni_headers/CompositorViewImpl_jni.h"
#include "content/public/browser/android/compositor.h"
#include "third_party/skia/include/core/SkColor.h"
#include "ui/android/color_helpers.h"
#include "ui/android/color_utils_android.h"
#include "ui/android/window_android.h"

using base::android::JavaParamRef;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
#include "third_party/blink/public/common/manifest/manifest_util.h"
#include "third_party/blink/public/mojom/manifest/manifest.mojom.h"
#include "third_party/skia/include/core/SkBitmap.h"
#include "ui/android/color_helpers.h"
#include "ui/android/color_utils_android.h"
#include "ui/gfx/codec/png_codec.h"

namespace webapps {
Expand Down
3 changes: 1 addition & 2 deletions ui/android/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ component("android") {
output_name = "ui_android"
sources = [
"animation_utils.h",
"color_helpers.h",
"color_utils_android.cc",
"color_utils_android.h",
"delegated_frame_host_android.cc",
Expand Down Expand Up @@ -519,7 +518,7 @@ test("ui_android_unittests") {
# Clipboard unittests are run here for Android as gtests on Android are not
# sharded. On other OSs these are run as part of interactive_ui_tests.
"//ui/base/clipboard/clipboard_unittest.cc",
"color_helpers_unittest.cc",
"color_utils_android_unittest.cc",
"overscroll_refresh_unittest.cc",
"resources/resource_manager_impl_unittest.cc",
"run_all_unittests.cc",
Expand Down
11 changes: 0 additions & 11 deletions ui/android/color_helpers.h

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
// Copyright 2018 The Chromium Authors. All rights reserved.
// Copyright 2021 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.

#include "ui/android/color_helpers.h"
#include "ui/android/color_utils_android.h"

#include <stdint.h>

Expand Down
2 changes: 1 addition & 1 deletion weblayer/browser/webapps/weblayer_webapps_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
#include "base/guid.h"
#include "components/webapps/browser/android/add_to_homescreen_params.h"
#include "components/webapps/browser/android/shortcut_info.h"
#include "ui/android/color_helpers.h"
#include "ui/android/color_utils_android.h"
#include "ui/gfx/android/java_bitmap.h"
#include "url/gurl.h"
#endif
Expand Down

0 comments on commit ecdbbc7

Please sign in to comment.