Skip to content

Commit

Permalink
Add a build flag for using color pipeline
Browse files Browse the repository at this point in the history
Add 'use_color_pipeline' build flag to enable building with color
pipeline feature. Excluding color_unittests so it can continue running
on buildbots without config changes.

BUG=1003612

Change-Id: Ie90fe5da6671a0f0d3d0f93bdbb5dc1df8497ce0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1928308
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/master@{#718901}
  • Loading branch information
weili0 authored and Commit Bot committed Nov 26, 2019
1 parent bce86b5 commit 69f7229
Show file tree
Hide file tree
Showing 9 changed files with 55 additions and 12 deletions.
5 changes: 4 additions & 1 deletion BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,6 @@ group("gn_all") {
if (!is_ios && !is_android && !is_chromecast && !is_fuchsia) {
deps += [
"//chrome",
"//chrome/browser/ui/color:dump_colors",
"//chrome/test:browser_tests",
"//chrome/test:interactive_ui_tests",
"//chrome/test:sync_integration_tests",
Expand Down Expand Up @@ -812,6 +811,10 @@ group("gn_all") {
]
}

if (use_color_pipeline) {
deps += [ "//chrome/browser/ui/color:dump_colors" ]
}

# PFFFT.
deps += [
"//third_party/pffft:fuzzers",
Expand Down
1 change: 1 addition & 0 deletions chrome/browser/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -2217,6 +2217,7 @@ jumbo_static_library("browser") {
"//ui/base:ui_data_pack",
"//ui/base/idle",
"//ui/base/ime",
"//ui/color:color_buildflags",
"//ui/events:events_base",
"//ui/gfx",
"//ui/gfx/geometry",
Expand Down
13 changes: 9 additions & 4 deletions chrome/browser/themes/browser_theme_pack.cc
Original file line number Diff line number Diff line change
Expand Up @@ -24,18 +24,14 @@
#include "base/values.h"
#include "build/build_config.h"
#include "chrome/browser/themes/theme_properties.h"
#include "chrome/browser/ui/color/chrome_color_id.h"
#include "chrome/browser/ui/frame/window_frame_util.h"
#include "chrome/common/extensions/manifest_handlers/theme_handler.h"
#include "chrome/common/themes/autogenerated_theme_util.h"
#include "chrome/grit/theme_resources.h"
#include "components/crx_file/id_util.h"
#include "content/public/browser/browser_thread.h"
#include "third_party/skia/include/core/SkCanvas.h"
#include "third_party/skia/include/core/SkColor.h"
#include "ui/base/resource/data_pack.h"
#include "ui/color/color_mixer.h"
#include "ui/color/color_provider.h"
#include "ui/gfx/canvas.h"
#include "ui/gfx/codec/png_codec.h"
#include "ui/gfx/color_analysis.h"
Expand All @@ -49,6 +45,13 @@
#include "ui/gfx/image/image_skia_operations.h"
#include "ui/gfx/skia_util.h"

#if BUILDFLAG(USE_COLOR_PIPELINE)
#include "chrome/browser/ui/color/chrome_color_id.h"
#include "third_party/skia/include/core/SkColor.h"
#include "ui/color/color_mixer.h"
#include "ui/color/color_provider.h"
#endif

using content::BrowserThread;
using extensions::Extension;
using TP = ThemeProperties;
Expand Down Expand Up @@ -1015,6 +1018,7 @@ bool BrowserThemePack::HasCustomImage(int idr_id) const {
return false;
}

#if BUILDFLAG(USE_COLOR_PIPELINE)
void BrowserThemePack::AddCustomThemeColorMixers(
ui::ColorProvider* provider) const {
// A map from theme property IDs to color IDs for use in color mixers.
Expand All @@ -1037,6 +1041,7 @@ void BrowserThemePack::AddCustomThemeColorMixers(
return;
provider->AddMixer().AddSet({kColorSetCustomTheme, std::move(theme_colors)});
}
#endif

// private:

Expand Down
8 changes: 7 additions & 1 deletion chrome/browser/themes/browser_theme_pack.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#include "extensions/common/extension.h"
#include "third_party/skia/include/core/SkColor.h"
#include "ui/base/layout.h"
#include "ui/color/color_buildflags.h"
#include "ui/gfx/color_utils.h"

namespace base {
Expand All @@ -31,8 +32,11 @@ class Image;
}

namespace ui {
class ColorProvider;
class DataPack;

#if BUILDFLAG(USE_COLOR_PIPELINE)
class ColorProvider;
#endif
}

// An optimized representation of a theme, backed by a mmapped DataPack.
Expand Down Expand Up @@ -95,9 +99,11 @@ class BrowserThemePack : public CustomThemeSupplier {
const override;
bool HasCustomImage(int id) const override;

#if BUILDFLAG(USE_COLOR_PIPELINE)
// Builds the color mixers that represent the state of the current browser
// theme instance.
void AddCustomThemeColorMixers(ui::ColorProvider* provider) const;
#endif

private:
friend class BrowserThemePackTest;
Expand Down
14 changes: 10 additions & 4 deletions chrome/browser/themes/browser_theme_pack_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -13,21 +13,25 @@
#include "base/values.h"
#include "build/build_config.h"
#include "chrome/browser/themes/theme_properties.h"
#include "chrome/browser/ui/color/chrome_color_id.h"
#include "chrome/browser/ui/frame/window_frame_util.h"
#include "chrome/common/chrome_paths.h"
#include "chrome/common/themes/autogenerated_theme_util.h"
#include "chrome/grit/theme_resources.h"
#include "content/public/test/browser_task_environment.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "ui/color/color_mixer.h"
#include "ui/color/color_provider.h"
#include "ui/color/color_test_ids.h"
#include "ui/color/color_buildflags.h"
#include "ui/gfx/color_utils.h"
#include "ui/gfx/image/image.h"
#include "ui/gfx/image/image_skia.h"
#include "ui/gfx/image/image_skia_rep.h"

#if BUILDFLAG(USE_COLOR_PIPELINE)
#include "chrome/browser/ui/color/chrome_color_id.h"
#include "ui/color/color_mixer.h"
#include "ui/color/color_provider.h"
#include "ui/color/color_test_ids.h"
#endif

using extensions::Extension;
using TP = ThemeProperties;

Expand Down Expand Up @@ -737,6 +741,7 @@ TEST_F(BrowserThemePackTest, TestNonExistantImages) {
EXPECT_FALSE(LoadRawBitmapsTo(out_file_paths));
}

#if BUILDFLAG(USE_COLOR_PIPELINE)
TEST_F(BrowserThemePackTest, TestCreateColorMixersOmniboxNoValues) {
// Tests to make sure that existing colors within the color provider are not
// overwritten or lost in the absence of any user provided theme values.
Expand Down Expand Up @@ -786,6 +791,7 @@ TEST_F(BrowserThemePackTest, TestCreateColorMixersOmniboxAllValues) {
EXPECT_EQ(SkColorSetRGB(120, 140, 160),
provider.GetColor(kColorOmniboxBackground));
}
#endif

// TODO(erg): This test should actually test more of the built resources from
// the extension data, but for now, exists so valgrind can test some of the
Expand Down
9 changes: 7 additions & 2 deletions chrome/browser/ui/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -1386,7 +1386,6 @@ jumbo_static_library("ui") {
"//chrome/browser/resource_coordinator:tab_metrics_event_proto",
"//chrome/browser/resource_coordinator/tab_ranker",
"//chrome/browser/safe_browsing:advanced_protection",
"//chrome/browser/ui/color:color",
"//chrome/browser/ui/webui/app_management:mojo_bindings",
"//chrome/common:buildflags",
"//chrome/common:search_mojom",
Expand Down Expand Up @@ -1435,6 +1434,13 @@ jumbo_static_library("ui") {
"//google_apis/drive",
]
}

if (use_color_pipeline) {
deps += [
"//chrome/browser/ui/color:color",
"//chrome/browser/ui/color:mixers",
]
}
}

if (enable_supervised_users) {
Expand Down Expand Up @@ -3291,7 +3297,6 @@ jumbo_static_library("ui") {
"webauthn/transport_utils.h",
]
deps += [
"//chrome/browser/ui/color:mixers",
"//chrome/browser/ui/views",
"//components/constrained_window",
"//components/media_message_center",
Expand Down
4 changes: 4 additions & 0 deletions chrome/browser/ui/color/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@
# Use of this source code is governed by a BSD-style license that can be
# found in the LICENSE file.

import("//ui/base/ui_features.gni")

assert(use_color_pipeline)

source_set("color") {
sources = [
"chrome_color_id.h",
Expand Down
4 changes: 4 additions & 0 deletions ui/base/ui_features.gni
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,10 @@ declare_args() {

# Whether the message center should be included for displaying notifications.
enable_message_center = is_win || is_mac || is_linux

# Whether to compute UI colors with the color pipeline framework,
# instead of NativeTheme/ThemeService.
use_color_pipeline = false
}

enable_hidpi = is_mac || is_win || is_linux || is_ios
9 changes: 9 additions & 0 deletions ui/color/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,16 @@
# Use of this source code is governed by a BSD-style license that can be
# found in the LICENSE file.

import("//build/buildflag_header.gni")
import("//build/config/jumbo.gni")
import("//testing/test.gni")
import("//ui/base/ui_features.gni")

buildflag_header("color_buildflags") {
header = "color_buildflags.h"

flags = [ "USE_COLOR_PIPELINE=$use_color_pipeline" ]
}

jumbo_component("color") {
sources = [
Expand All @@ -23,6 +31,7 @@ jumbo_component("color") {
defines = [ "IS_COLOR_IMPL" ]

public_deps = [
":color_buildflags",
"//base",
"//skia",
"//ui/gfx:color_utils",
Expand Down

0 comments on commit 69f7229

Please sign in to comment.