Changes for PR:

- Add tests for overrides
- Improve approach for SysColorMixer override

browser/themes/brave_theme_service.cc

@@ -8,7 +8,6 @@
 #include <memory>
 
 #include "brave/browser/extensions/brave_theme_event_router.h"
-#include "chrome/browser/themes/theme_service.h"
 
 BraveThemeService::BraveThemeService(Profile* profile,
                                      const ThemeHelper& theme_helper)

chromium_src/chrome/browser/ui/views/location_bar/location_bar_view_browsertest.cc

@@ -3,15 +3,18 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this file,
  * You can obtain one at http://mozilla.org/MPL/2.0/. */
 
+#include "chrome/browser/ui/views/location_bar/location_bar_view.h"
+
 #include <optional>
 
 #include "base/strings/string_util.h"
 #include "chrome/browser/ssl/security_state_tab_helper.h"
 #include "chrome/browser/ui/views/frame/browser_view.h"
-#include "chrome/browser/ui/views/location_bar/location_bar_view.h"
+#include "chrome/browser/ui/views/omnibox/omnibox_view_views.h"
 #include "chrome/test/base/in_process_browser_test.h"
 #include "chrome/test/base/ui_test_utils.h"
 #include "components/omnibox/browser/location_bar_model_impl.h"
+#include "components/omnibox/browser/omnibox_edit_model.h"
 #include "content/public/test/browser_test.h"
 #include "content/public/test/url_loader_interceptor.h"
 #include "mojo/public/cpp/system/data_pipe.h"
@@ -62,8 +65,9 @@ class SecurityIndicatorTest
 
   bool InterceptURLLoad(net::CertStatus cert_status,
                         content::URLLoaderInterceptor::RequestParams* params) {
-    if (params->url_request.url.host() != kMockSecureHostname)
+    if (params->url_request.url.host() != kMockSecureHostname) {
       return false;
+    }
     net::SSLInfo ssl_info;
     ssl_info.cert = cert_;
     ssl_info.cert_status = cert_status;
@@ -129,3 +133,39 @@ INSTANTIATE_TEST_SUITE_P(
                                     kEmptyString},
         SecurityIndicatorTestParams{false, 0, security_state::NONE, false,
                                     kEmptyString}));
+
+class BraveLocationBarViewColorOverridesTest : public InProcessBrowserTest {
+ public:
+  BraveLocationBarViewColorOverridesTest() = default;
+  BraveLocationBarViewColorOverridesTest(
+      const BraveLocationBarViewColorOverridesTest&) = delete;
+  BraveLocationBarViewColorOverridesTest& operator=(
+      const BraveLocationBarViewColorOverridesTest&) = delete;
+  ~BraveLocationBarViewColorOverridesTest() override = default;
+
+  OmniboxViewViews* GetOmniboxView() {
+    return BrowserView::GetBrowserViewForBrowser(browser())
+        ->GetLocationBarView()
+        ->omnibox_view();
+  }
+};
+
+// We override the behavior of the LocationBar when the user is editing text.
+// This test makes sure the override is being applied.
+IN_PROC_BROWSER_TEST_F(BraveLocationBarViewColorOverridesTest,
+                       UnfocusedEditingColorIsSameAsUnfocused) {
+  // NTP is special, so we navigate somewhere else - this also unfocused the
+  // LocationBar.
+  ui_test_utils::NavigateToURLBlockUntilNavigationsComplete(
+      browser(), GURL("https://example.com"), 1);
+  EXPECT_FALSE(GetOmniboxView()->model()->is_caret_visible());
+  EXPECT_FALSE(GetOmniboxView()->model()->user_input_in_progress());
+  auto default_color = GetOmniboxView()->GetBackgroundColor();
+
+  // Set the user text
+  GetOmniboxView()->SetUserText(u"hello world");
+
+  EXPECT_FALSE(GetOmniboxView()->model()->is_caret_visible());
+  EXPECT_TRUE(GetOmniboxView()->model()->user_input_in_progress());
+  EXPECT_EQ(default_color, GetOmniboxView()->GetBackgroundColor());
+}

chromium_src/ui/color/BUILD.gn

@@ -0,0 +1,14 @@
+# Copyright (c) 2024 The Brave Authors. All rights reserved.
+# This Source Code Form is subject to the terms of the Mozilla Public
+# License, v. 2.0. If a copy of the MPL was not distributed with this file,
+# You can obtain one at https://mozilla.org/MPL/2.0/.
+
+source_set("unit_tests") {
+  testonly = true
+  sources = [ "brave_sys_color_mixer_unittest.cc" ]
+
+  deps = [
+    "//testing/gtest",
+    "//ui/color",
+  ]
+}

chromium_src/ui/color/sys_color_mixer.cc

@@ -7,12 +7,20 @@
 
 #include "brave/ui/color/brave_sys_color_mixer.h"
 
-#define kGrayscale kGrayscale) {                        \
-    AddGrayscaleSysColorOverrides(mixer, key);          \
-    ui::AddBraveGrayscaleSysColorOverrides(mixer, key); \
-  }                                                     \
-  else if (false
+#define AddSysColorMixer AddSysColorMixer_Chromium
+// #define kGrayscale kGrayscale) {                        \
+//     AddGrayscaleSysColorOverrides(mixer, key);          \
+//     ui::AddBraveGrayscaleSysColorOverrides(mixer, key); \
+//   }                                                     \
+//   else if (false
 
 #include "src/ui/color/sys_color_mixer.cc"
 
-#undef kGrayscale
+#undef AddSysColorMixer
+
+namespace ui {
+void AddSysColorMixer(ColorProvider* provider, const ColorProviderKey& key) {
+  AddSysColorMixer_Chromium(provider, key);
+  ui::AddBraveSysColorMixer(provider, key);
+}
+}  // namespace ui

test/BUILD.gn

@@ -269,6 +269,7 @@ test("brave_unit_tests") {
     "//brave/mojo/brave_ast_patcher:unit_tests",
     "//brave/net:unit_tests",
     "//brave/third_party/blink/renderer:renderer",
+    "//brave/ui/color:unit_tests",
     "//brave/vendor/brave_base",
     "//chrome:dependencies",
     "//chrome/app:command_ids",

ui/color/BUILD.gn

@@ -38,3 +38,13 @@ source_set("brave_color_mixers") {
     "//ui/color/dynamic_color",
   ]
 }
+
+source_set("unit_tests") {
+  testonly = true
+  sources = [ "brave_sys_color_mixer_unittest.cc" ]
+
+  deps = [
+    "//testing/gtest",
+    "//ui/color",
+  ]
+}

ui/color/brave_sys_color_mixer.cc

@@ -6,20 +6,23 @@
 #include "brave/ui/color/brave_sys_color_mixer.h"
 
 #include "ui/color/color_id.h"
+#include "ui/color/color_provider.h"
 #include "ui/color/color_provider_key.h"
 #include "ui/color/color_recipe.h"
 
 namespace ui {
 
 // We lightly tweak the dark theme in Grayscale mode (the default theme in
 // Brave) to be a bit darker, to not upset dark mode users.
-void AddBraveGrayscaleSysColorOverrides(ColorMixer& mixer,
-                                        const ColorProviderKey& key) {
+void AddBraveSysColorMixer(ColorProvider* provider,
+                           const ColorProviderKey& key) {
   if (key.color_mode != ColorProviderKey::ColorMode::kDark ||
       key.user_color_source != ColorProviderKey::UserColorSource::kGrayscale) {
     return;
   }
 
+  auto& mixer = provider->AddMixer();
+
   mixer[kColorSysHeader] = {kColorRefNeutral5};
   mixer[kColorSysHeaderInactive] = {kColorRefNeutral5};
 

ui/color/brave_sys_color_mixer.h

@@ -6,12 +6,12 @@
 #ifndef BRAVE_UI_COLOR_BRAVE_SYS_COLOR_MIXER_H_
 #define BRAVE_UI_COLOR_BRAVE_SYS_COLOR_MIXER_H_
 
-#include "ui/color/color_mixer.h"
 #include "ui/color/color_provider_key.h"
 
 namespace ui {
-void AddBraveGrayscaleSysColorOverrides(ColorMixer& mixer,
-                                        const ColorProviderKey& key);
+class ColorProvider;
+
+void AddBraveSysColorMixer(ColorProvider*, const ColorProviderKey& key);
 }  // namespace ui
 
 #endif  // BRAVE_UI_COLOR_BRAVE_SYS_COLOR_MIXER_H_

ui/color/brave_sys_color_mixer_unittest.cc

@@ -0,0 +1,79 @@
+// Copyright (c) 2024 The Brave Authors. All rights reserved.
+// This Source Code Form is subject to the terms of the Mozilla Public
+// License, v. 2.0. If a copy of the MPL was not distributed with this file,
+// You can obtain one at https://mozilla.org/MPL/2.0/.
+
+#include "brave/ui/color/brave_sys_color_mixer.h"
+
+#include "testing/gtest/include/gtest/gtest.h"
+#include "ui/color/color_id.h"
+#include "ui/color/color_provider.h"
+#include "ui/color/color_provider_key.h"
+#include "ui/color/ref_color_mixer.h"
+#include "ui/color/sys_color_mixer.h"
+
+namespace ui {
+
+TEST(BraveSysColorMixer, DarkModeGrayscaleOverridesAreApplied) {
+  ui::ColorProviderKey key;
+  key.color_mode = ColorProviderKey::ColorMode::kDark;
+  key.user_color_source = ColorProviderKey::UserColorSource::kGrayscale;
+
+  ColorProvider provider;
+  AddRefColorMixer(&provider, key);
+  AddSysColorMixer(&provider, key);
+
+  // Sanity check these two colors are overriden.
+  EXPECT_EQ(provider.GetColor(kColorRefNeutral5),
+            provider.GetColor(kColorSysHeader));
+  EXPECT_EQ(provider.GetColor(kColorRefNeutral15),
+            provider.GetColor(kColorSysBase));
+}
+
+TEST(BraveSysColorMixer, LightModeGrayscaleOverridesAreNotApplied) {
+  ui::ColorProviderKey key;
+  key.color_mode = ColorProviderKey::ColorMode::kLight;
+  key.user_color_source = ColorProviderKey::UserColorSource::kGrayscale;
+
+  ColorProvider provider;
+  AddRefColorMixer(&provider, key);
+  AddSysColorMixer(&provider, key);
+
+  // Sanity check these two colors are not set to our custom dark grayscale
+  // theme.
+  EXPECT_NE(provider.GetColor(kColorRefNeutral5),
+            provider.GetColor(kColorSysHeader));
+  EXPECT_NE(provider.GetColor(kColorRefNeutral15),
+            provider.GetColor(kColorSysBase));
+}
+
+TEST(BraveSysColorMixer, OverridesAreNotAppliedInNonGrayscale) {
+  ui::ColorProviderKey key;
+  key.color_mode = ColorProviderKey::ColorMode::kDark;
+  key.user_color_source = ColorProviderKey::UserColorSource::kBaseline;
+
+  ColorProvider provider_dark;
+  AddRefColorMixer(&provider_dark, key);
+  AddSysColorMixer(&provider_dark, key);
+
+  // Sanity check these two colors are not set to our custom dark grayscale
+  // theme.
+  EXPECT_NE(provider_dark.GetColor(kColorRefNeutral5),
+            provider_dark.GetColor(kColorSysHeader));
+  EXPECT_NE(provider_dark.GetColor(kColorRefNeutral15),
+            provider_dark.GetColor(kColorSysBase));
+
+  key.color_mode = ColorProviderKey::ColorMode::kLight;
+  ColorProvider provider_light;
+  AddRefColorMixer(&provider_light, key);
+  AddSysColorMixer(&provider_light, key);
+
+  // Sanity check these two colors are not set to our custom dark grayscale
+  // theme.
+  EXPECT_NE(provider_light.GetColor(kColorRefNeutral5),
+            provider_light.GetColor(kColorSysHeader));
+  EXPECT_NE(provider_light.GetColor(kColorRefNeutral15),
+            provider_light.GetColor(kColorSysBase));
+}
+
+}  // namespace ui