Merge pull request #391 from brave/ui/toolbar-styles

Toolbar Styles

browser/themes/brave_theme_service.cc

@@ -13,8 +13,10 @@
 #include "base/strings/string_util.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/themes/theme_service_factory.h"
+#include "chrome/common/channel_info.h"
 #include "components/pref_registry/pref_registry_syncable.h"
 #include "components/prefs/pref_service.h"
+#include "components/version_info/channel.h"
 
 // static
 void BraveThemeService::RegisterProfilePrefs(
@@ -23,23 +25,46 @@ void BraveThemeService::RegisterProfilePrefs(
 }
 
 // static
-BraveThemeService::BraveThemeType BraveThemeService::GetBraveThemeType(Profile* profile) {
+BraveThemeType BraveThemeService::GetUserPreferredBraveThemeType(
+                                                      Profile* profile) {
   // allow override via cli flag
   const base::CommandLine& command_line =
       *base::CommandLine::ForCurrentProcess();
   if (command_line.HasSwitch(switches::kUiMode)) {
     std::string requested_theme_value = command_line.GetSwitchValueASCII(switches::kUiMode);
     std::string requested_theme_value_lower = base::ToLowerASCII(requested_theme_value);
     if (requested_theme_value_lower == "light")
-      return BraveThemeService::BraveThemeType::BRAVE_THEME_TYPE_LIGHT;
+      return BraveThemeType::BRAVE_THEME_TYPE_LIGHT;
     if (requested_theme_value_lower == "light")
-      return BraveThemeService::BraveThemeType::BRAVE_THEME_TYPE_DARK;
+      return BraveThemeType::BRAVE_THEME_TYPE_DARK;
   }
   // get value from preferences
-  return static_cast<BraveThemeService::BraveThemeType>(
+  return static_cast<BraveThemeType>(
       profile->GetPrefs()->GetInteger(kBraveThemeType));
 }
 
+// static
+BraveThemeType BraveThemeService::GetActiveBraveThemeType(
+                                                    Profile* profile) {
+  const BraveThemeType preferred_theme =
+                                        GetUserPreferredBraveThemeType(profile);
+  switch (preferred_theme) {
+    case BraveThemeType::BRAVE_THEME_TYPE_DEFAULT:
+      switch (chrome::GetChannel()) {
+        case version_info::Channel::STABLE:
+        case version_info::Channel::BETA:
+          return BraveThemeType::BRAVE_THEME_TYPE_LIGHT;
+        case version_info::Channel::DEV:
+        case version_info::Channel::CANARY:
+        case version_info::Channel::UNKNOWN:
+        default:
+          return BraveThemeType::BRAVE_THEME_TYPE_DARK;
+      }
+    default:
+      return preferred_theme;
+  }
+}
+
 BraveThemeService::BraveThemeService() {}
 
 BraveThemeService::~BraveThemeService() {}
@@ -56,12 +81,16 @@ void BraveThemeService::Init(Profile* profile) {
   ThemeService::Init(profile);
 }
 
+
+
 SkColor BraveThemeService::GetDefaultColor(int id, bool incognito) const {
-  const base::Optional<SkColor> braveColor =
-      MaybeGetDefaultColorForBraveUi(id, incognito, profile());
+  const BraveThemeType theme = GetActiveBraveThemeType(profile());
+  const base::Optional<SkColor> braveColor = MaybeGetDefaultColorForBraveUi(id, incognito, theme);
   if (braveColor)
       return braveColor.value();
-
+  // make sure we fallback to chrome's dark theme (incognito) for our dark theme
+  if (theme == BraveThemeType::BRAVE_THEME_TYPE_DARK)
+    incognito = true;
   return ThemeService::GetDefaultColor(id, incognito);
 }
 

browser/themes/brave_theme_service.h

@@ -12,23 +12,24 @@ namespace user_prefs {
 class PrefRegistrySyncable;
 }
 
+enum BraveThemeType {
+  BRAVE_THEME_TYPE_DEFAULT,  // Choose theme by channel
+  BRAVE_THEME_TYPE_DARK,     // Use dark theme regardless of channel
+  BRAVE_THEME_TYPE_LIGHT,    // Use light theme regardless of channel
+};
+
 class BraveThemeService : public ThemeService {
  public:
-  enum BraveThemeType {
-    BRAVE_THEME_TYPE_DEFAULT,  // Choose theme by channel
-    BRAVE_THEME_TYPE_DARK,     // Use dark theme regardless of channel
-    BRAVE_THEME_TYPE_LIGHT,    // Use light theme regardless of channel
-  };
-
   static void RegisterProfilePrefs(user_prefs::PrefRegistrySyncable* registry);
-  static BraveThemeType GetBraveThemeType(Profile* profile);
+  static BraveThemeType GetUserPreferredBraveThemeType(Profile* profile);
+  static BraveThemeType GetActiveBraveThemeType(Profile* profile);
 
   BraveThemeService();
   ~BraveThemeService() override;
 
   // ThemeService overrides:
   void Init(Profile* profile) override;
-  
+
  protected:
   // ThemeService overrides:
   SkColor GetDefaultColor(int id, bool incognito) const override;

browser/themes/brave_theme_service_browsertest.cc

@@ -15,25 +15,25 @@ using BraveThemeServiceTest = InProcessBrowserTest;
 using BTS = BraveThemeService;
 
 namespace {
-void SetBraveThemeType(Profile* profile, BTS::BraveThemeType type) {
+void SetBraveThemeType(Profile* profile, BraveThemeType type) {
   profile->GetPrefs()->SetInteger(kBraveThemeType, type);
 }
 }  // namespace
 
 IN_PROC_BROWSER_TEST_F(BraveThemeServiceTest, BraveThemeChangeTest) {
   Profile* profile = browser()->profile();
   const SkColor light_frame_color = SkColorSetRGB(0xD8, 0xDE, 0xE1);
-  const SkColor dark_frame_color = SkColorSetRGB(0x58, 0x5B, 0x5E);
+  const SkColor dark_frame_color = SkColorSetRGB(0x22, 0x22, 0x22);
 
   // Check default type is set initially.
-  EXPECT_EQ(BTS::BRAVE_THEME_TYPE_DEFAULT, BTS::GetBraveThemeType(profile));
+  EXPECT_EQ(BraveThemeType::BRAVE_THEME_TYPE_DEFAULT, BTS::GetUserPreferredBraveThemeType(profile));
 
   const ui::ThemeProvider& tp = ThemeService::GetThemeProviderForProfile(profile);
-  SetBraveThemeType(browser()->profile(), BTS::BRAVE_THEME_TYPE_LIGHT);
-  EXPECT_EQ(BTS::BRAVE_THEME_TYPE_LIGHT, BTS::GetBraveThemeType(profile));
+  SetBraveThemeType(browser()->profile(), BraveThemeType::BRAVE_THEME_TYPE_LIGHT);
+  EXPECT_EQ(BraveThemeType::BRAVE_THEME_TYPE_LIGHT, BTS::GetUserPreferredBraveThemeType(profile));
   EXPECT_EQ(light_frame_color, tp.GetColor(ThemeProperties::COLOR_FRAME));
 
-  SetBraveThemeType(browser()->profile(), BTS::BRAVE_THEME_TYPE_DARK);
-  EXPECT_EQ(BTS::BRAVE_THEME_TYPE_DARK, BTS::GetBraveThemeType(profile));
+  SetBraveThemeType(browser()->profile(), BraveThemeType::BRAVE_THEME_TYPE_DARK);
+  EXPECT_EQ(BraveThemeType::BRAVE_THEME_TYPE_DARK, BTS::GetUserPreferredBraveThemeType(profile));
   EXPECT_EQ(dark_frame_color, tp.GetColor(ThemeProperties::COLOR_FRAME));
 }

browser/themes/theme_properties.cc

@@ -5,92 +5,148 @@
 #include "brave/browser/themes/theme_properties.h"
 
 #include "brave/browser/themes/brave_theme_service.h"
-#include "brave/common/pref_names.h"
-#include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/themes/theme_properties.h"
-#include "chrome/common/channel_info.h"
-#include "components/prefs/pref_service.h"
-#include "components/version_info/channel.h"
 
 namespace {
 
-base::Optional<SkColor> MaybeGetDefaultColorForBraveLightUi(int id, bool incognito) {
+base::Optional<SkColor> MaybeGetDefaultColorForBraveLightUi(int id) {
   switch (id) {
     // Applies when the window is active, tabs and also tab bar everywhere except active tab
     case ThemeProperties::COLOR_FRAME:
     case ThemeProperties::COLOR_BACKGROUND_TAB:
-      return incognito ? SkColorSetRGB(0x81, 0x85, 0x89) : SkColorSetRGB(0xD8, 0xDE, 0xE1);
+      return SkColorSetRGB(0xD8, 0xDE, 0xE1);
     // Window when the window is innactive, tabs and also tab bar everywhere except active tab
     case ThemeProperties::COLOR_FRAME_INACTIVE:
     case ThemeProperties::COLOR_BACKGROUND_TAB_INACTIVE:
-      return incognito ? SkColorSetRGB(0x71, 0x75, 0x79) : SkColorSetRGB(0xC8, 0xCE, 0xC8);
+      return SkColorSetRGB(0xC8, 0xCE, 0xC8);
     // Active tab and also the URL toolbar
     // Parts of this color show up as you hover over innactive tabs too
     case ThemeProperties::COLOR_TOOLBAR:
     case ThemeProperties::COLOR_DETACHED_BOOKMARK_BAR_BACKGROUND:
     case ThemeProperties::COLOR_CONTROL_BACKGROUND:
     case ThemeProperties::COLOR_TOOLBAR_CONTENT_AREA_SEPARATOR:
-      return incognito ? SkColorSetRGB(0x91, 0x95, 0x99) : SkColorSetRGB(0xF6, 0xF7, 0xF9);
+      return SkColorSetRGB(0xF6, 0xF7, 0xF9);
     case ThemeProperties::COLOR_TAB_TEXT:
       return SkColorSetRGB(0x22, 0x23, 0x26);
     case ThemeProperties::COLOR_BOOKMARK_TEXT:
     case ThemeProperties::COLOR_BACKGROUND_TAB_TEXT:
       return SkColorSetRGB(0x22, 0x23, 0x26);
+    case ThemeProperties::COLOR_LOCATION_BAR_BORDER:
+      return SkColorSetRGB(0xd5, 0xd9, 0xdc);
     default:
       return base::nullopt;
   }
 }
 
-base::Optional<SkColor> MaybeGetDefaultColorForBraveDarkUi(int id, bool incognito) {
+const SkColor kDarkFrame = SkColorSetRGB(0x22, 0x22, 0x22);
+const SkColor kDarkToolbar = SkColorSetRGB(0x39, 0x39, 0x39);
+const SkColor kDarkToolbarIcon = SkColorSetRGB(0xed, 0xed, 0xed);
+
+base::Optional<SkColor> MaybeGetDefaultColorForBraveDarkUi(int id) {
   switch (id) {
     // Applies when the window is active, tabs and also tab bar everywhere except active tab
     case ThemeProperties::COLOR_FRAME:
     case ThemeProperties::COLOR_BACKGROUND_TAB:
-      return incognito ? SkColorSetRGB(0x68, 0x6B, 0x6E) : SkColorSetRGB(0x58, 0x5B, 0x5E);
+      return kDarkFrame;
     // Window when the window is innactive, tabs and also tab bar everywhere except active tab
     case ThemeProperties::COLOR_FRAME_INACTIVE:
     case ThemeProperties::COLOR_BACKGROUND_TAB_INACTIVE:
-      return incognito ? SkColorSetRGB(0x58, 0x5B, 0x5E) : SkColorSetRGB(0x48, 0x4B, 0x4E);
+      return color_utils::HSLShift(kDarkFrame, { -1, -1, 0.6 });
     // Active tab and also the URL toolbar
     // Parts of this color show up as you hover over innactive tabs too
     case ThemeProperties::COLOR_TOOLBAR:
+    case ThemeProperties::COLOR_TOOLBAR_TOP_SEPARATOR:
+    case ThemeProperties::COLOR_TOOLBAR_TOP_SEPARATOR_INACTIVE:
     case ThemeProperties::COLOR_DETACHED_BOOKMARK_BAR_BACKGROUND:
     case ThemeProperties::COLOR_CONTROL_BACKGROUND:
     case ThemeProperties::COLOR_TOOLBAR_CONTENT_AREA_SEPARATOR:
-      return incognito ? SkColorSetRGB(0x32, 0x33, 0x36) : SkColorSetRGB(0x22, 0x23, 0x26);
+      return kDarkToolbar;
     case ThemeProperties::COLOR_TAB_TEXT:
-      return SkColorSetRGB(0xF7, 0xF8, 0xF9);
+      return SkColorSetRGB(0xF3, 0xF3, 0xF3);
     case ThemeProperties::COLOR_BOOKMARK_TEXT:
     case ThemeProperties::COLOR_BACKGROUND_TAB_TEXT:
-      return SkColorSetRGB(0x81, 0x85, 0x89);
+      return SkColorSetRGB(0xFF, 0xFF, 0xFF);
+    case ThemeProperties::COLOR_LOCATION_BAR_BORDER:
+      // TODO: Should be location bar background, but location bar has hover
+      // color which we don't have access to here.
+      // Consider increasing height instead.
+      return kDarkToolbar;
+    case ThemeProperties::COLOR_TOOLBAR_BUTTON_ICON:
+      return kDarkToolbarIcon;
+    case ThemeProperties::COLOR_TOOLBAR_BUTTON_ICON_INACTIVE:
+      return color_utils::AlphaBlend(kDarkToolbarIcon, kDarkToolbar, 0x4d);
     default:
       return base::nullopt;
   }
 }
 
+const SkColor kPrivateFrame = SkColorSetRGB(0x1b, 0x0e, 0x2c);
+const SkColor kPrivateToolbar = SkColorSetRGB(0x3d, 0x28, 0x41);
+
+base::Optional<SkColor> MaybeGetDefaultColorForPrivateUi(int id) {
+  switch (id) {
+    // Applies when the window is active, tabs and also tab bar everywhere except active tab
+    case ThemeProperties::COLOR_FRAME:
+    case ThemeProperties::COLOR_FRAME_INCOGNITO:
+    case ThemeProperties::COLOR_BACKGROUND_TAB:
+    case ThemeProperties::COLOR_BACKGROUND_TAB_INCOGNITO:
+      return kPrivateFrame;
+    // Window when the window is innactive, tabs and also tab bar everywhere except active tab
+    case ThemeProperties::COLOR_FRAME_INACTIVE:
+    case ThemeProperties::COLOR_FRAME_INCOGNITO_INACTIVE:
+    case ThemeProperties::COLOR_BACKGROUND_TAB_INCOGNITO_INACTIVE:
+      return color_utils::HSLShift(kPrivateFrame, { -1, -1, 0.55 });
+    // Active tab and also the URL toolbar
+    // Parts of this color show up as you hover over innactive tabs too
+    case ThemeProperties::COLOR_TOOLBAR:
+    case ThemeProperties::COLOR_DETACHED_BOOKMARK_BAR_BACKGROUND:
+    case ThemeProperties::COLOR_CONTROL_BACKGROUND:
+    case ThemeProperties::COLOR_TOOLBAR_CONTENT_AREA_SEPARATOR:
+      return kPrivateToolbar;
+    case ThemeProperties::COLOR_TAB_TEXT:
+      return SkColorSetRGB(0xF3, 0xF3, 0xF3);
+    case ThemeProperties::COLOR_BOOKMARK_TEXT:
+    case ThemeProperties::COLOR_BACKGROUND_TAB_TEXT:
+      return SkColorSetRGB(0xFF, 0xFF, 0xFF);
+    case ThemeProperties::COLOR_LOCATION_BAR_BORDER:
+      // TODO: Should be location bar background, but location bar has hover
+      // color which we don't have access to here.
+      // Consider increasing height instead.
+      return kPrivateToolbar;
+    case ThemeProperties::COLOR_TOOLBAR_BUTTON_ICON:
+      return kDarkToolbarIcon;
+    case ThemeProperties::COLOR_TOOLBAR_BUTTON_ICON_INACTIVE:
+      return color_utils::AlphaBlend(kDarkToolbarIcon, kPrivateToolbar, 0x4d);
+    // The rest is covered by a dark-appropriate value
+    default:
+      return MaybeGetDefaultColorForBraveDarkUi(id);
+  }
+}
+
 }  // namespace
 
 // Returns a |nullopt| if the UI color is not handled by Brave.
-base::Optional<SkColor> MaybeGetDefaultColorForBraveUi(int id, bool incognito, Profile* profile) {
-  switch (BraveThemeService::GetBraveThemeType(profile)) {
-    case BraveThemeService::BRAVE_THEME_TYPE_DEFAULT:
-      switch (chrome::GetChannel()) {
-        case version_info::Channel::STABLE:
-        case version_info::Channel::BETA:
-          return MaybeGetDefaultColorForBraveLightUi(id, incognito);
-        case version_info::Channel::DEV:
-        case version_info::Channel::CANARY:
-        case version_info::Channel::UNKNOWN:
-        default:
-          return MaybeGetDefaultColorForBraveDarkUi(id, incognito);
-      }
-    case BraveThemeService::BRAVE_THEME_TYPE_LIGHT:
-      return MaybeGetDefaultColorForBraveLightUi(id, incognito);
-    case BraveThemeService::BRAVE_THEME_TYPE_DARK:
-      return MaybeGetDefaultColorForBraveDarkUi(id, incognito);
+base::Optional<SkColor> MaybeGetDefaultColorForBraveUi(int id, bool incognito, BraveThemeType theme) {
+  // Consistent (and stable) values across all themes
+  switch (id) {
+    case ThemeProperties::COLOR_TAB_THROBBER_SPINNING:
+      return SkColorSetRGB(0xd7, 0x55, 0x26);
     default:
-      NOTREACHED();
+      break;
+  }
 
+  // Allow Private Window theme to override dark vs light
+  if (incognito) {
+    return MaybeGetDefaultColorForPrivateUi(id);
+  }
+  // Get Dark or Light value
+  switch (theme) {
+    case BraveThemeType::BRAVE_THEME_TYPE_LIGHT:
+      return MaybeGetDefaultColorForBraveLightUi(id);
+    case BraveThemeType::BRAVE_THEME_TYPE_DARK:
+      return MaybeGetDefaultColorForBraveDarkUi(id);
+    default:
+      NOTREACHED();
   }
   return base::nullopt;
 }

browser/themes/theme_properties.h

@@ -7,11 +7,10 @@
 
 #include "base/optional.h"
 #include "third_party/skia/include/core/SkColor.h"
-
-class Profile;
+#include "brave/browser/themes/brave_theme_service.h"
 
 #define BRAVE_COLOR_FOR_TEST 0x7FFFFFFF
 
-base::Optional<SkColor> MaybeGetDefaultColorForBraveUi(int id, bool incognito, Profile* profile);
+base::Optional<SkColor> MaybeGetDefaultColorForBraveUi(int id, bool incognito, BraveThemeType theme);
 
 #endif  // BRAVE_BROWSER_THEMES_THEME_PROPERTIES_H_

browser/ui/BUILD.gn

@@ -10,6 +10,8 @@ source_set("ui") {
     "brave_browser_content_setting_bubble_model_delegate.h",
     "brave_pages.cc",
     "brave_pages.h",
+    "brave_layout_constants.cc",
+    "brave_layout_constants.h",
     "brave_actions/shields_action_view_controller.cc",
     "brave_actions/shields_action_view_controller.h",
     "brave_actions/brave_action_icon_with_badge_image_source.cc",
@@ -30,6 +32,8 @@ source_set("ui") {
     "toolbar/brave_app_menu_model.h",
     "toolbar/brave_toolbar_actions_model.cc",
     "toolbar/brave_toolbar_actions_model.h",
+    "views/brave_layout_provider.cc",
+    "views/brave_layout_provider.h",
     "views/brave_actions/brave_actions_container.cc",
     "views/brave_actions/brave_actions_container.h",
     "views/frame/brave_browser_view.cc",
@@ -38,6 +42,8 @@ source_set("ui") {
     "views/importer/brave_import_lock_dialog_view.h",
     "views/location_bar/brave_location_bar_view.cc",
     "views/location_bar/brave_location_bar_view.h",
+    "views/tabs/brave_new_tab_button.cc",
+    "views/tabs/brave_new_tab_button.h",
     "views/toolbar/bookmark_button.cc",
     "views/toolbar/bookmark_button.h",
     "views/toolbar/brave_toolbar_view.cc",