Gtk: Refactor LoadGtkValues

chrome/browser/ui/libgtkui/gtk_ui.cc

 // Copyright (c) 2012 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 "chrome/browser/ui/libgtkui/gtk_ui.h"
 
 #include <math.h>
 #include <pango/pango.h>
 #include <X11/Xcursor/Xcursor.h>
 
@@ skipping 415 matching lines @@
 
 void GtkUi::Initialize() {
   GtkSettings* settings = gtk_settings_get_default();
   g_signal_connect_after(settings, "notify::gtk-theme-name",
                          G_CALLBACK(OnThemeChanged), this);
   g_signal_connect_after(settings, "notify::gtk-icon-theme-name",
                          G_CALLBACK(OnThemeChanged), this);
 
   LoadGtkValues();
 
-  LoadCursorTheme();
-
 #if BUILDFLAG(ENABLE_BASIC_PRINTING)
   printing::PrintingContextLinux::SetCreatePrintDialogFunction(
       &PrintDialogGtk2::CreatePrintDialog);
   printing::PrintingContextLinux::SetPdfPaperSizeFunction(
       &GetPdfPaperSizeDeviceUnitsGtk);
 #endif
 
 #if defined(USE_GCONF)
   // We must build this after GTK gets initialized.
   gconf_listener_.reset(new GConfListener(this));
@@ skipping 331 matching lines @@
   GetChromeStyleColor("scrollbar-slider-normal-color", &thumb_inactive_color_);
   GetChromeStyleColor("scrollbar-trough-color", &track_color_);
 }
 
 void GtkUi::LoadGtkValues() {
   // TODO(erg): GtkThemeService had a comment here about having to muck with
   // the raw Prefs object to remove prefs::kCurrentThemeImages or else we'd
   // regress startup time. Figure out how to do that when we can't access the
   // prefs system from here.
 
-  SkColor label_color = native_theme_->GetSystemColor(
-      ui::NativeTheme::kColorId_LabelEnabledColor);
+  UpdateDeviceScaleFactor();
+  UpdateCursorTheme();
+
+  BuildFrameColors();
 
 #if GTK_MAJOR_VERSION == 2
   SkColor toolbar_color =
       native_theme_->GetSystemColor(ui::NativeTheme::kColorId_DialogBackground);
+  SkColor label_color = native_theme_->GetSystemColor(
+      ui::NativeTheme::kColorId_LabelEnabledColor);
 
   colors_[ThemeProperties::COLOR_TOOLBAR_BUTTON_ICON] =
       color_utils::DeriveDefaultIconColor(label_color);
 
   colors_[ThemeProperties::COLOR_TAB_TEXT] = label_color;
   colors_[ThemeProperties::COLOR_BOOKMARK_TEXT] = label_color;
   colors_[ThemeProperties::COLOR_BACKGROUND_TAB_TEXT] =
       color_utils::BlendTowardOppositeLuma(label_color, 50);
 
   inactive_selection_bg_color_ = native_theme_->GetSystemColor(
       ui::NativeTheme::kColorId_TextfieldReadOnlyBackground);
   inactive_selection_fg_color_ = native_theme_->GetSystemColor(
       ui::NativeTheme::kColorId_TextfieldReadOnlyColor);
+
+  // We pick the text and background colors for the NTP out of the
+  // colors for a GtkEntry. We do this because GtkEntries background
+  // color is never the same as |toolbar_color|, is usually a white,
+  // and when it isn't a white, provides sufficient contrast to
+  // |toolbar_color|. Try this out with Darklooks, HighContrastInverse
+  // or ThinIce.
+  colors_[ThemeProperties::COLOR_NTP_BACKGROUND] =
+      native_theme_->GetSystemColor(
+          ui::NativeTheme::kColorId_TextfieldDefaultBackground);
+  colors_[ThemeProperties::COLOR_NTP_TEXT] = native_theme_->GetSystemColor(
+      ui::NativeTheme::kColorId_TextfieldDefaultColor);
+  // The NTP header is the color that surrounds the current active
+  // thumbnail on the NTP, and acts as the border of the "Recent
+  // Links" box. It would be awesome if they were separated so we
+  // could use GetBorderColor() for the border around the "Recent
+  // Links" section, but matching the frame color is more important.
+  colors_[ThemeProperties::COLOR_NTP_HEADER] =
+      colors_[ThemeProperties::COLOR_FRAME];
 #else
   SkColor toolbar_color = GetBgColor("GtkToolbar#toolbar");
 
   colors_[ThemeProperties::COLOR_TOOLBAR_BUTTON_ICON] =
       GetFgColor("GtkToolbar#toolbar GtkLabel#label");
 
   // Tabs use the same background color as the toolbar, so use the
   // toolbar text color as the tab text color.
   SkColor tab_text_color =
       GetFgColor("GtkToolbar#toolbar.horizontal GtkLabel#label");
@@ skipping 32 matching lines @@
       GetBorderColor("#headerbar.header-bar.titlebar GtkEntry#entry");
   SkColor entry_inactive_border =
       GetBorderColor("#headerbar.header-bar.titlebar:backdrop GtkEntry#entry");
   // Unlike with toolbars, we always want a border around tabs, so let
   // ThemeService choose the border color if the theme doesn't provide one.
   if (SkColorGetA(entry_border) && SkColorGetA(entry_inactive_border)) {
     colors_[ThemeProperties::COLOR_TOOLBAR_TOP_SEPARATOR] = entry_border;
     colors_[ThemeProperties::COLOR_TOOLBAR_TOP_SEPARATOR_INACTIVE] =
         entry_inactive_border;
   }
+
+  colors_[ThemeProperties::COLOR_NTP_BACKGROUND] = GetBgColor("");
+  colors_[ThemeProperties::COLOR_NTP_TEXT] = GetFgColor("");
+  colors_[ThemeProperties::COLOR_NTP_HEADER] = GetBorderColor("GtkEntry#entry");
 #endif
 
+  colors_[ThemeProperties::COLOR_TOOLBAR] = toolbar_color;
   colors_[ThemeProperties::COLOR_CONTROL_BACKGROUND] = toolbar_color;
-  colors_[ThemeProperties::COLOR_TOOLBAR] = toolbar_color;
 
-  UpdateDeviceScaleFactor();
-
-  // We pick the text and background colors for the NTP out of the colors for a
-  // GtkEntry. We do this because GtkEntries background color is never the same
-  // as |toolbar_color|, is usually a white, and when it isn't a white,
-  // provides sufficient contrast to |toolbar_color|. Try this out with
-  // Darklooks, HighContrastInverse or ThinIce.
-
-  SkColor ntp_background = native_theme_->GetSystemColor(
-      ui::NativeTheme::kColorId_TextfieldDefaultBackground);
-  SkColor ntp_foreground = native_theme_->GetSystemColor(
-      ui::NativeTheme::kColorId_TextfieldDefaultColor);
-
-  colors_[ThemeProperties::COLOR_NTP_BACKGROUND] = ntp_background;
-  colors_[ThemeProperties::COLOR_NTP_TEXT] = ntp_foreground;
-
-  // The NTP header is the color that surrounds the current active thumbnail on
-  // the NTP, and acts as the border of the "Recent Links" box. It would be
-  // awesome if they were separated so we could use GetBorderColor() for the
-  // border around the "Recent Links" section, but matching the frame color is
-  // more important.
-
-  BuildFrameColors();
-  SkColor frame_color = colors_[ThemeProperties::COLOR_FRAME];
-  colors_[ThemeProperties::COLOR_NTP_HEADER] = frame_color;
-  colors_[ThemeProperties::COLOR_NTP_SECTION] = toolbar_color;

[Elliot Glaysher, 2017/01/31 18:30:51]

If these values are actually unusued, you probably want to remove them
everywhere.

[Tom (Use chromium acct), 2017/01/31 18:57:40]

sgtm, but maybe that can be done in a different CL?  There's a lot of places
that need to change:

$ git grep COLOR_NTP_SECTION
chrome/browser/themes/browser_theme_pack.cc:  { "ntp_section",
ThemeProperties::COLOR_NTP_SECTION },
chrome/browser/themes/browser_theme_pack.cc:  { "ntp_section_text",
ThemeProperties::COLOR_NTP_SECTION_TEXT },
chrome/browser/themes/browser_theme_pack.cc:  { "ntp_section_link",
ThemeProperties::COLOR_NTP_SECTION_LINK },
chrome/browser/themes/browser_theme_pack.cc:   
ThemeProperties::COLOR_NTP_SECTION_LINK_UNDERLINE },
chrome/browser/themes/browser_theme_pack.cc:     
colors->count(ThemeProperties::COLOR_NTP_SECTION)) {
chrome/browser/themes/browser_theme_pack.cc:       
(*colors)[ThemeProperties::COLOR_NTP_SECTION];
chrome/browser/themes/browser_theme_pack.cc:  if
(!colors->count(ThemeProperties::COLOR_NTP_SECTION_LINK_UNDERLINE) &&
chrome/browser/themes/browser_theme_pack.cc:     
colors->count(ThemeProperties::COLOR_NTP_SECTION_LINK)) {
chrome/browser/themes/browser_theme_pack.cc:       
(*colors)[ThemeProperties::COLOR_NTP_SECTION_LINK];
chrome/browser/themes/browser_theme_pack.cc:   
(*colors)[ThemeProperties::COLOR_NTP_SECTION_LINK_UNDERLINE] =
chrome/browser/themes/browser_theme_pack_unittest.cc: 
colors[ThemeProperties::COLOR_NTP_SECTION_LINK] = link_color;
chrome/browser/themes/browser_theme_pack_unittest.cc: 
colors[ThemeProperties::COLOR_NTP_SECTION_LINK_UNDERLINE] =
chrome/browser/themes/browser_theme_pack_unittest.cc: 
colors[ThemeProperties::COLOR_NTP_SECTION_LINK] = link_color;
chrome/browser/themes/browser_theme_pack_unittest.cc: 
colors[ThemeProperties::COLOR_NTP_SECTION_LINK_UNDERLINE] =
chrome/browser/themes/browser_theme_pack_unittest.cc: 
colors[ThemeProperties::COLOR_NTP_SECTION] = ntp_color;
chrome/browser/themes/browser_theme_pack_unittest.cc: 
colors[ThemeProperties::COLOR_NTP_SECTION] = SkColorSetRGB(190, 190, 190);
chrome/browser/themes/theme_properties.cc:    case COLOR_NTP_SECTION:
chrome/browser/themes/theme_properties.cc:    case COLOR_NTP_SECTION_TEXT:
chrome/browser/themes/theme_properties.cc:    case COLOR_NTP_SECTION_LINK:
chrome/browser/themes/theme_properties.cc:    case
COLOR_NTP_SECTION_LINK_UNDERLINE:
chrome/browser/themes/theme_properties.h:    COLOR_NTP_SECTION,
chrome/browser/themes/theme_properties.h:    COLOR_NTP_SECTION_TEXT,
chrome/browser/themes/theme_properties.h:    COLOR_NTP_SECTION_LINK,
chrome/browser/themes/theme_properties.h:    COLOR_NTP_SECTION_LINK_UNDERLINE,
chrome/browser/themes/theme_properties.h:    COLOR_NTP_SECTION_HEADER_TEXT,
chrome/browser/themes/theme_properties.h:   
COLOR_NTP_SECTION_HEADER_TEXT_HOVER,
chrome/browser/themes/theme_properties.h:    COLOR_NTP_SECTION_HEADER_RULE,
chrome/browser/themes/theme_properties.h:   
COLOR_NTP_SECTION_HEADER_RULE_LIGHT,
chrome/browser/themes/theme_service.cc:    case
ThemeProperties::COLOR_NTP_SECTION_HEADER_TEXT:
chrome/browser/themes/theme_service.cc:    case
ThemeProperties::COLOR_NTP_SECTION_HEADER_TEXT_HOVER:
chrome/browser/themes/theme_service.cc:    case
ThemeProperties::COLOR_NTP_SECTION_HEADER_RULE:
chrome/browser/themes/theme_service.cc:    case
ThemeProperties::COLOR_NTP_SECTION_HEADER_RULE_LIGHT:
chrome/browser/ui/libgtkui/gtk_ui.cc: 
colors_[ThemeProperties::COLOR_NTP_SECTION] = toolbar_color;
chrome/browser/ui/libgtkui/gtk_ui.cc: 
colors_[ThemeProperties::COLOR_NTP_SECTION_TEXT] = label_color;
chrome/browser/ui/libgtkui/gtk_ui.cc: 
colors_[ThemeProperties::COLOR_NTP_SECTION_LINK] = link_color;
chrome/browser/ui/libgtkui/gtk_ui.cc: 
colors_[ThemeProperties::COLOR_NTP_SECTION_LINK_UNDERLINE] = link_color;


$ git grep ntp_section
chrome/browser/themes/browser_theme_pack.cc:  { "ntp_section",
ThemeProperties::COLOR_NTP_SECTION },
chrome/browser/themes/browser_theme_pack.cc:  { "ntp_section_text",
ThemeProperties::COLOR_NTP_SECTION_TEXT },
chrome/browser/themes/browser_theme_pack.cc:  { "ntp_section_link",
ThemeProperties::COLOR_NTP_SECTION_LINK },
chrome/browser/themes/browser_theme_pack.cc:  { "ntp_section_link_underline",
chrome/browser/themes/browser_theme_pack_unittest.cc:                          
"  \"ntp_section_link\": [128, 128, 128] }";
chrome/browser/themes/browser_theme_pack_unittest.cc:                          
"  \"ntp_section_link\": [128, 128, 128],"
chrome/browser/themes/browser_theme_pack_unittest.cc:                          
"  \"ntp_section_link_underline\": [255, 255, 255]"
chrome/browser/themes/browser_theme_pack_unittest.cc:  std::string color_json =
"{ \"ntp_section\": [190, 190, 190] }";
chrome/browser/themes/browser_theme_pack_unittest.cc:                          
"  \"ntp_section\": [190, 190, 190] }";
chrome/common/extensions/docs/templates/articles/themes.html:      "ntp_section"
: [207, 221, 192],
chrome/test/data/extensions/theme/manifest.json:         "ntp_section": [ 207,
221, 192 ],
chrome/test/data/extensions/theme2/manifest.json:         "ntp_section": [ 213,
233, 245 ],
Binary file chrome/test/data/extensions/theme_missing_image.crx matches
chrome/test/data/profiles/profile_with_complex_theme/Default/Extensions/mblmlcbknbnfebdfjnolmcapmdofhmme/1.1/manifest.json:
        "ntp_section": [ 131, 138, 146, 0.8 ],
chrome/test/data/profiles/profile_with_complex_theme/Default/Extensions/mblmlcbknbnfebdfjnolmcapmdofhmme/1.1/manifest.json:
        "ntp_section_link": [ 255, 255, 255 ],
chrome/test/data/profiles/profile_with_complex_theme/Default/Extensions/mblmlcbknbnfebdfjnolmcapmdofhmme/1.1/manifest.json:
        "ntp_section_text": [ 255, 255, 255 ],
chrome/test/data/profiles/profile_with_complex_theme/Default/PreferencesTemplate:
                    "ntp_section": [ 131, 138, 146, 0.8 ],
chrome/test/data/profiles/profile_with_complex_theme/Default/PreferencesTemplate:
                    "ntp_section_link": [ 255, 255, 255 ],
chrome/test/data/profiles/profile_with_complex_theme/Default/PreferencesTemplate:
                    "ntp_section_text": [ 255, 255, 255 ],

-  colors_[ThemeProperties::COLOR_NTP_SECTION_TEXT] = label_color;
-
-  SkColor link_color =
+  colors_[ThemeProperties::COLOR_NTP_LINK] =
       native_theme_->GetSystemColor(ui::NativeTheme::kColorId_LinkEnabled);
-  colors_[ThemeProperties::COLOR_NTP_LINK] = link_color;
-  colors_[ThemeProperties::COLOR_NTP_LINK_UNDERLINE] = link_color;
-  colors_[ThemeProperties::COLOR_NTP_SECTION_LINK] = link_color;
-  colors_[ThemeProperties::COLOR_NTP_SECTION_LINK_UNDERLINE] = link_color;
 
   // Generate the colors that we pass to WebKit.
-  focus_ring_color_ = frame_color;
-
   SetScrollbarColors();
+  focus_ring_color_ = native_theme_->GetSystemColor(
+      ui::NativeTheme::kColorId_TextfieldSelectionBackgroundFocused);
 
   // Some GTK themes only define the text selection colors on the GtkEntry
   // class, so we need to use that for getting selection colors.
   active_selection_bg_color_ = native_theme_->GetSystemColor(
       ui::NativeTheme::kColorId_TextfieldSelectionBackgroundFocused);
   active_selection_fg_color_ = native_theme_->GetSystemColor(
       ui::NativeTheme::kColorId_TextfieldSelectionColor);
 
   colors_[ThemeProperties::COLOR_TAB_THROBBER_SPINNING] =
       native_theme_->GetSystemColor(
           ui::NativeTheme::kColorId_ThrobberSpinningColor);
   colors_[ThemeProperties::COLOR_TAB_THROBBER_WAITING] =
       native_theme_->GetSystemColor(
           ui::NativeTheme::kColorId_ThrobberWaitingColor);
 }
 
-void GtkUi::LoadCursorTheme() {
+void GtkUi::UpdateCursorTheme() {
   GtkSettings* settings = gtk_settings_get_default();
 
   gchar* theme = nullptr;
   gint size = 0;
   g_object_get(settings, "gtk-cursor-theme-name", &theme,
                "gtk-cursor-theme-size", &size, nullptr);
 
   if (theme)
     XcursorSetTheme(gfx::GetXDisplay(), theme);
   if (size)
@@ skipping 115 matching lines @@
 
 float GtkUi::GetDeviceScaleFactor() const {
   return device_scale_factor_;
 }
 
 }  // namespace libgtkui
 
 views::LinuxUI* BuildGtkUi() {
   return new libgtkui::GtkUi;
 }