Gtk3: More fixes and refactorings

This CL
* Uses entry colors instead of window colors for the NTP
* Fixes a leak involving ScopedGObject
* Simplifies how background and border colors are calculated
* Adds the ".label" class only on versions of Gtk before 3.20
* Fixes text selection colors on one theme
* Updates the doc to reflect the new way colors are calculated

BUG=132847

Review-Url: https://codereview.chromium.org/2683953005
Cr-Commit-Position: refs/heads/master@{#449709}

chrome/browser/ui/libgtkui/gtk_ui.cc

@@ -822,14 +822,15 @@ void GtkUi::LoadGtkValues() {
   colors_[ThemeProperties::COLOR_BACKGROUND_TAB_TEXT] =
       color_utils::BlendTowardOppositeLuma(toolbar_text_color, 50);
 
-  // Color drawn around the location bar.
-  colors_[ThemeProperties::COLOR_LOCATION_BAR_BORDER] =
+  SkColor location_bar_border =
       GetBorderColor("GtkToolbar#toolbar GtkEntry#entry");
+  if (SkColorGetA(location_bar_border)) {
+    colors_[ThemeProperties::COLOR_LOCATION_BAR_BORDER] = location_bar_border;
+  }
 
-  inactive_selection_bg_color_ =
-      GetBgColor("GtkEntry#entry:backdrop #selection:selected");
+  inactive_selection_bg_color_ = GetSelectedBgColor("GtkEntry#entry:backdrop");
   inactive_selection_fg_color_ =
-      GetFgColor("GtkEntry#entry:backdrop #selection:selected");
+      GetSelectedTextColor("GtkEntry#entry:backdrop");
 
   SkColor toolbar_separator_horizontal =
       GetSeparatorColor("GtkToolbar#toolbar GtkSeparator#separator.horizontal");
@@ -865,10 +866,8 @@ void GtkUi::LoadGtkValues() {
         header_button_inactive_border;
   }
 
-  SkColor ntp_bg = GetBgColor("");
-  colors_[ThemeProperties::COLOR_NTP_BACKGROUND] = ntp_bg;
-  colors_[ThemeProperties::COLOR_NTP_TEXT] =
-      color_utils::GetReadableColor(GetFgColor("GtkLabel#label"), ntp_bg);
+  colors_[ThemeProperties::COLOR_NTP_BACKGROUND] = GetBgColor("GtkEntry#entry");
+  colors_[ThemeProperties::COLOR_NTP_TEXT] = GetFgColor("GtkEntry#entry");
   colors_[ThemeProperties::COLOR_NTP_HEADER] =
       GetBorderColor("GtkButton#button");
 #endif
@@ -876,13 +875,13 @@ void GtkUi::LoadGtkValues() {
   colors_[ThemeProperties::COLOR_TOOLBAR] = toolbar_color;
   colors_[ThemeProperties::COLOR_CONTROL_BACKGROUND] = toolbar_color;
 
-  colors_[ThemeProperties::COLOR_NTP_LINK] =
-      native_theme_->GetSystemColor(ui::NativeTheme::kColorId_LinkEnabled);
+  colors_[ThemeProperties::COLOR_NTP_LINK] = native_theme_->GetSystemColor(
+      ui::NativeTheme::kColorId_TextfieldSelectionBackgroundFocused);
 
   // Generate the colors that we pass to WebKit.
   SetScrollbarColors();
-  focus_ring_color_ = native_theme_->GetSystemColor(
-      ui::NativeTheme::kColorId_TextfieldSelectionBackgroundFocused);
+  focus_ring_color_ =
+      native_theme_->GetSystemColor(ui::NativeTheme::kColorId_LinkEnabled);
 
   // Some GTK themes only define the text selection colors on the GtkEntry
   // class, so we need to use that for getting selection colors.

chrome/browser/ui/libgtkui/gtk_util.cc

@@ -220,49 +220,42 @@ CairoSurface::CairoSurface(const gfx::Size& size)
     : surface_(cairo_image_surface_create(CAIRO_FORMAT_ARGB32,
                                           size.width(),
                                           size.height())),
-      cairo_(cairo_create(surface_)) {}
+      cairo_(cairo_create(surface_)) {
+  DCHECK(cairo_surface_status(surface_) == CAIRO_STATUS_SUCCESS);
+  // Clear the surface.
+  cairo_save(cairo_);
+  cairo_set_source_rgba(cairo_, 0, 0, 0, 0);
+  cairo_set_operator(cairo_, CAIRO_OPERATOR_SOURCE);
+  cairo_paint(cairo_);
+  cairo_restore(cairo_);
+}
 
 CairoSurface::~CairoSurface() {
   cairo_destroy(cairo_);
   cairo_surface_destroy(surface_);
 }
 
-SkColor CairoSurface::GetAveragePixelValue(bool only_frame_pixels) {
+SkColor CairoSurface::GetAveragePixelValue(bool frame) {
   cairo_surface_flush(surface_);
-  int num_samples = 0;
-  long a = 0, r = 0, g = 0, b = 0;
   SkColor* data =
       reinterpret_cast<SkColor*>(cairo_image_surface_get_data(surface_));
   int width = cairo_image_surface_get_width(surface_);
   int height = cairo_image_surface_get_height(surface_);
   DCHECK(4 * width == cairo_image_surface_get_stride(surface_));
-  auto accumulate = [&](int x, int y) mutable {
-    SkColor color = data[y * width + x];
-    int alpha = SkColorGetA(color);
-    a += alpha;
-    r += alpha * SkColorGetR(color);
-    g += alpha * SkColorGetG(color);
-    b += alpha * SkColorGetB(color);
-    num_samples++;
-  };
-  if (width == 1 || height == 1 || !only_frame_pixels) {
-    // Count every pixel in the surface.
-    for (int x = 0; x < width; x++)
-      for (int y = 0; y < height; y++)
-        accumulate(x, y);
-  } else {
-    // Count the pixels in the top and bottom rows.
-    for (int x = 0; x < width; x++)
-      for (int y : {0, height - 1})
-        accumulate(x, y);
-    // Count the pixels in the left and right columns.
-    for (int x : {0, width - 1})
-      for (int y = 1; y < height - 1; y++)
-        accumulate(x, y);
+  long a = 0, r = 0, g = 0, b = 0;
+  unsigned int max_alpha = 0;
+  for (int i = 0; i < width * height; i++) {
+    SkColor color = data[i];
+    max_alpha = std::max(SkColorGetA(color), max_alpha);
+    a += SkColorGetA(color);
+    r += SkColorGetR(color);
+    g += SkColorGetG(color);
+    b += SkColorGetB(color);
   }
   if (a == 0)
     return SK_ColorTRANSPARENT;
-  return SkColorSetARGB(a / num_samples, r / a, g / a, b / a);
+  return SkColorSetARGB(frame ? max_alpha : a / (width * height), r * 255 / a,
+                        g * 255 / a, b * 255 / a);
 }
 
 bool GtkVersionCheck(int major, int minor, int micro) {
@@ -336,14 +329,12 @@ ScopedStyleContext AppendCssNodeToStyleContext(GtkStyleContext* context,
           NOTREACHED();
       }
     } else {
+      static auto* _gtk_widget_path_iter_set_object_name =
+          reinterpret_cast<void (*)(GtkWidgetPath*, gint, const char*)>(dlsym(
+              GetGtk3SharedLibrary(), "gtk_widget_path_iter_set_object_name"));
       switch (part_type) {
         case CSS_NAME: {
           if (GtkVersionCheck(3, 20)) {
-            static auto* _gtk_widget_path_iter_set_object_name =
-                reinterpret_cast<void (*)(GtkWidgetPath*, gint, const char*)>(
-                    dlsym(GetGtk3SharedLibrary(),
-                          "gtk_widget_path_iter_set_object_name"));
-            DCHECK(_gtk_widget_path_iter_set_object_name);
             _gtk_widget_path_iter_set_object_name(path, -1, t.token().c_str());
           } else {
             gtk_widget_path_iter_add_class(path, -1, t.token().c_str());
@@ -354,6 +345,10 @@ ScopedStyleContext AppendCssNodeToStyleContext(GtkStyleContext* context,
           GType type = g_type_from_name(t.token().c_str());
           DCHECK(type);
           gtk_widget_path_append_type(path, type);
+          if (GtkVersionCheck(3, 20)) {
+            if (t.token() == "GtkLabel")
+              _gtk_widget_path_iter_set_object_name(path, -1, "label");
+          }
           break;
         }
         case CSS_CLASS: {
@@ -379,7 +374,7 @@ ScopedStyleContext AppendCssNodeToStyleContext(GtkStyleContext* context,
   // widgets specially if they want to.
   gtk_widget_path_iter_add_class(path, -1, "chromium");
 
-  auto child_context = ScopedStyleContext(gtk_style_context_new());
+  ScopedStyleContext child_context(gtk_style_context_new());
   gtk_style_context_set_path(child_context, path);
   gtk_style_context_set_state(child_context, state);
   gtk_style_context_set_parent(child_context, context);
@@ -388,9 +383,10 @@ ScopedStyleContext AppendCssNodeToStyleContext(GtkStyleContext* context,
 }
 
 ScopedStyleContext GetStyleContextFromCss(const char* css_selector) {
-  // Prepend "GtkWindow.background" to the selector since all widgets must live
+  // Prepend a window node to the selector since all widgets must live
   // in a window, but we don't want to specify that every time.
-  auto context = AppendCssNodeToStyleContext(nullptr, "GtkWindow.background");
+  auto context =
+      AppendCssNodeToStyleContext(nullptr, "GtkWindow#window.background");
 
   for (const auto& widget_type :
        base::SplitString(css_selector, base::kWhitespaceASCII,
@@ -405,26 +401,46 @@ SkColor GdkRgbaToSkColor(const GdkRGBA& color) {
                         color.blue * 255);
 }
 
-SkColor SkColorFromStyleContext(GtkStyleContext* context) {
+SkColor GetFgColorFromStyleContext(GtkStyleContext* context) {
   GdkRGBA color;
   gtk_style_context_get_color(context, gtk_style_context_get_state(context),
                               &color);
   return GdkRgbaToSkColor(color);
 }
 
-SkColor GetFgColor(const char* css_selector) {
-  return SkColorFromStyleContext(GetStyleContextFromCss(css_selector));
+SkColor GetBgColorFromStyleContext(GtkStyleContext* context) {
+  // Backgrounds are more general than solid colors (eg. gradients),
+  // but chromium requires us to boil this down to one color.  We
+  // cannot use the background-color here because some themes leave it
+  // set to a garbage color because a background-image will cover it
+  // anyway.  So we instead render the background into a 24x24 bitmap,
+  // removing any borders, and hope that we get a good color.
+  ApplyCssToContext(context,
+                    "* {"
+                    "border-radius: 0px;"
+                    "border-style: none;"
+                    "box-shadow: none;"
+                    "}");
+  gfx::Size size(24, 24);
+  CairoSurface surface(size);
+  RenderBackground(size, surface.cairo(), context);
+  return surface.GetAveragePixelValue(false);
 }
 
-GtkCssProvider* GetCssProvider(const char* css) {
+SkColor GetFgColor(const char* css_selector) {
+  return GetFgColorFromStyleContext(GetStyleContextFromCss(css_selector));
+}
+
+ScopedCssProvider GetCssProvider(const char* css) {
   GtkCssProvider* provider = gtk_css_provider_new();
   GError* error = nullptr;
   gtk_css_provider_load_from_data(provider, css, -1, &error);
   DCHECK(!error);
-  return provider;
+  return ScopedCssProvider(provider);
 }
 
-void ApplyCssToContext(GtkStyleContext* context, GtkCssProvider* provider) {
+void ApplyCssProviderToContext(GtkStyleContext* context,
+                               GtkCssProvider* provider) {
   while (context) {
     gtk_style_context_add_provider(context, GTK_STYLE_PROVIDER(provider),
                                    G_MAXUINT);
@@ -432,35 +448,9 @@ void ApplyCssToContext(GtkStyleContext* context, GtkCssProvider* provider) {
   }
 }
 
-void RemoveBorders(GtkStyleContext* context) {
-  static GtkCssProvider* provider = GetCssProvider(
-      "* {"
-      "border-style: none;"
-      "border-radius: 0px;"
-      "border-width: 0px;"
-      "border-image-width: 0px;"
-      "box-shadow: none;"
-      "padding: 0px;"
-      "margin: 0px;"
-      "outline: none;"
-      "outline-width: 0px;"
-      "}");
-  ApplyCssToContext(context, provider);
-}
-
-void AddBorders(GtkStyleContext* context) {
-  static GtkCssProvider* provider = GetCssProvider(
-      "* {"
-      "border-style: solid;"
-      "border-radius: 0px;"
-      "border-width: 1px;"
-      "box-shadow: none;"
-      "padding: 0px;"
-      "margin: 0px;"
-      "outline: none;"
-      "outline-width: 0px;"
-      "}");
-  ApplyCssToContext(context, provider);
+void ApplyCssToContext(GtkStyleContext* context, const char* css) {
+  auto provider = GetCssProvider(css);
+  ApplyCssProviderToContext(context, provider);
 }
 
 void RenderBackground(const gfx::Size& size,
@@ -472,66 +462,78 @@ void RenderBackground(const gfx::Size& size,
   gtk_render_background(context, cr, 0, 0, size.width(), size.height());
 }
 
-gfx::Size GetMinimumWidgetSize(GtkStyleContext* context) {
-  if (!GtkVersionCheck(3, 20))
-    return gfx::Size(1, 1);
-  int w = 1, h = 1;
-  gtk_style_context_get(context, gtk_style_context_get_state(context),
-                        "min-width", &w, "min-height", &h, NULL);
-  DCHECK(w >= 0 && h >= 0);
-  return gfx::Size(w ? w : 1, h ? h : 1);
-}
-
 SkColor GetBgColor(const char* css_selector) {
-  // Backgrounds are more general than solid colors (eg. gradients),
-  // but chromium requires us to boil this down to one color.  We
-  // cannot use the background-color here because some themes leave it
-  // set to a garbage color because a background-image will cover it
-  // anyway.  So we instead render the background into a single pixel,
-  // removing any borders, and hope that we get a good color.
-  auto context = GetStyleContextFromCss(css_selector);
-  RemoveBorders(context);
-  gfx::Size size = GetMinimumWidgetSize(context);
-  CairoSurface surface(size);
-  RenderBackground(size, surface.cairo(), context);
-  return surface.GetAveragePixelValue(false);
+  return GetBgColorFromStyleContext(GetStyleContextFromCss(css_selector));
 }
 
 SkColor GetBorderColor(const char* css_selector) {
   // Borders have the same issue as backgrounds, due to the
   // border-image property.
   auto context = GetStyleContextFromCss(css_selector);
-  GtkStateFlags state = gtk_style_context_get_state(context);
-  GtkBorderStyle border_style = GTK_BORDER_STYLE_NONE;
-  gtk_style_context_get(context, state, GTK_STYLE_PROPERTY_BORDER_STYLE,
-                        &border_style, nullptr);
-  GtkBorder border;
-  gtk_style_context_get_border(context, state, &border);
-  if ((border_style == GTK_BORDER_STYLE_NONE ||
-       border_style == GTK_BORDER_STYLE_HIDDEN) ||
-      (!border.left && !border.right && !border.top && !border.bottom)) {
-    return SK_ColorTRANSPARENT;
-  }
-
-  AddBorders(context);
-  gfx::Size size = GetMinimumWidgetSize(context);
+  gfx::Size size(24, 24);
   CairoSurface surface(size);
-  RenderBackground(size, surface.cairo(), context);
   gtk_render_frame(context, surface.cairo(), 0, 0, size.width(), size.height());
   return surface.GetAveragePixelValue(true);
 }
 
+ScopedStyleContext GetSelectedStyleContext(const char* css_selector) {
+  auto context = GetStyleContextFromCss(css_selector);
+  if (GtkVersionCheck(3, 20)) {
+    context = AppendCssNodeToStyleContext(context, "#selection");
+  } else {
+    GtkStateFlags state = gtk_style_context_get_state(context);
+    state = static_cast<GtkStateFlags>(state | GTK_STATE_FLAG_SELECTED);
+    gtk_style_context_set_state(context, state);
+  }
+  return context;
+}
+
+SkColor GetSelectedTextColor(const char* css_selector) {
+  return GetFgColorFromStyleContext(GetSelectedStyleContext(css_selector));
+}
+
+SkColor GetSelectedBgColor(const char* css_selector) {
+  auto context = GetSelectedStyleContext(css_selector);
+  if (GtkVersionCheck(3, 20))
+    return GetBgColorFromStyleContext(context);
+  // This is verbatim how Gtk gets the selection color on versions before 3.20.
+  GdkRGBA selection_color;
+  G_GNUC_BEGIN_IGNORE_DEPRECATIONS;
+  gtk_style_context_get_background_color(
+      context, gtk_style_context_get_state(context), &selection_color);
+  G_GNUC_END_IGNORE_DEPRECATIONS;
+  return GdkRgbaToSkColor(selection_color);
+}
+
 SkColor GetSeparatorColor(const char* css_selector) {
   if (!GtkVersionCheck(3, 20))
     return GetFgColor(css_selector);
 
-  // Some themes use borders to render separators, others use a
-  // background color.  Only return the border color if there is one.
-  SkColor border = GetBorderColor(css_selector);
-  if (SkColorGetA(border))
-    return border;
+  auto context = GetStyleContextFromCss(css_selector);
+  int w = 1, h = 1;
+  gtk_style_context_get(context, gtk_style_context_get_state(context),
+                        "min-width", &w, "min-height", &h, nullptr);
+  GtkBorder border, padding;
+  GtkStateFlags state = gtk_style_context_get_state(context);
+  gtk_style_context_get_border(context, state, &border);
+  gtk_style_context_get_padding(context, state, &padding);
+  w += border.left + padding.left + padding.right + border.right;
+  h += border.top + padding.top + padding.bottom + border.bottom;
 
-  return GetBgColor(css_selector);
+  bool horizontal = gtk_style_context_has_class(context, "horizontal");
+  if (horizontal) {
+    w = 24;
+    h = std::max(h, 1);
+  } else {
+    DCHECK(gtk_style_context_has_class(context, "vertical"));
+    h = 24;
+    w = std::max(w, 1);
+  }
+
+  CairoSurface surface(gfx::Size(w, h));
+  gtk_render_background(context, surface.cairo(), 0, 0, w, h);
+  gtk_render_frame(context, surface.cairo(), 0, 0, w, h);
+  return surface.GetAveragePixelValue(false);
 }
 #endif
 

chrome/browser/ui/libgtkui/gtk_util.h

@@ -98,10 +98,10 @@ class CairoSurface {
   // Get the drawing context for GTK to use.
   cairo_t* cairo() { return cairo_; }
 
-  // If |only_frame_pixels| is false, returns the average of all
-  // pixels in the surface, otherwise returns the average of only the
-  // edge pixels.
-  SkColor GetAveragePixelValue(bool only_frame_pixels);
+  // Returns the average of all pixels in the surface.  If |frame| is
+  // true, the resulting alpha will be the average alpha, otherwise it
+  // will be the max alpha across all pixels.
+  SkColor GetAveragePixelValue(bool frame);
 
  private:
   cairo_surface_t* surface_;
@@ -112,18 +112,18 @@ class CairoSurface {
 // |major|.|minor|.|micro|.
 bool GtkVersionCheck(int major, int minor = 0, int micro = 0);
 
+// Similar in spirit to a std::unique_ptr.
 template <typename T>
 class ScopedGObject {
  public:
   explicit ScopedGObject(T* obj) : obj_(obj) {
-    // Increase the reference count of |obj_|, removing the floating
-    // reference if it has one.
-    g_object_ref_sink(obj_);
+    // Remove the floating reference from |obj_| if it has one.
+    if (g_object_is_floating(obj_))
+      g_object_ref_sink(obj_);
+    DCHECK(G_OBJECT(obj_)->ref_count == 1);
   }
 
-  ScopedGObject(const ScopedGObject<T>& other) : obj_(other.obj_) {
-    g_object_ref(obj_);
-  }
+  ScopedGObject(const ScopedGObject<T>& other) = delete;
 
   ScopedGObject(ScopedGObject<T>&& other) : obj_(other.obj_) {
     other.obj_ = nullptr;
@@ -131,15 +131,10 @@ class ScopedGObject {
 
   ~ScopedGObject() {
     if (obj_)
-      g_object_unref(obj_);
+      Unref();
   }
 
-  ScopedGObject<T>& operator=(const ScopedGObject<T>& other) {
-    g_object_ref(other.obj_);
-    g_object_unref(obj_);
-    obj_ = other.obj_;
-    return *this;
-  }
+  ScopedGObject<T>& operator=(const ScopedGObject<T>& other) = delete;
 
   ScopedGObject<T>& operator=(ScopedGObject<T>&& other) {
     g_object_unref(obj_);
@@ -151,10 +146,35 @@ class ScopedGObject {
   operator T*() { return obj_; }
 
  private:
+  void Unref() { g_object_unref(obj_); }
+
   T* obj_;
 };
 
+template <>
+inline void ScopedGObject<GtkStyleContext>::Unref() {
+  // Versions of GTK earlier than 3.15.4 had a bug where a g_assert
+  // would be triggered when trying to free a GtkStyleContext that had
+  // a parent whose only reference was the child context in question.
+  // This is a hack to work around that case.  See GTK commit
+  // "gtkstylecontext: Don't try to emit a signal when finalizing".
+  GtkStyleContext* context = obj_;
+  while (context) {
+    GtkStyleContext* parent = gtk_style_context_get_parent(context);
+    if (parent && G_OBJECT(context)->ref_count == 1 &&
+        !GtkVersionCheck(3, 15, 4)) {
+      g_object_ref(parent);
+      gtk_style_context_set_parent(context, nullptr);
+    } else {
+      g_object_unref(context);
+      return;
+    }
+    context = parent;
+  }
+}
+
 typedef ScopedGObject<GtkStyleContext> ScopedStyleContext;
+typedef ScopedGObject<GtkCssProvider> ScopedCssProvider;
 
 // If |context| is NULL, creates a new top-level style context
 // specified by parsing |css_node|.  Otherwise, creates the child
@@ -170,10 +190,11 @@ ScopedStyleContext AppendCssNodeToStyleContext(GtkStyleContext* context,
 // must g_object_unref() the returned context.
 ScopedStyleContext GetStyleContextFromCss(const char* css_selector);
 
-SkColor SkColorFromStyleContext(GtkStyleContext* context);
+SkColor GetFgColorFromStyleContext(GtkStyleContext* context);
 
-// Removes all border-type properties on |context| and all of its parents.
-void RemoveBorders(GtkStyleContext* context);
+// Overrides properties on |context| and all its parents with those
+// provided by |css|.
+void ApplyCssToContext(GtkStyleContext* context, const char* css);
 
 // Get the 'color' property from the style context created by
 // GetStyleContextFromCss(|css_selector|).
@@ -186,15 +207,18 @@ void RenderBackground(const gfx::Size& size,
                       GtkStyleContext* context);
 
 // Renders a background from the style context created by
-// GetStyleContextFromCss(|css_selector|) into a single pixel and
-// returns the color.
+// GetStyleContextFromCss(|css_selector|) into a 24x24 bitmap and
+// returns the average color.
 SkColor GetBgColor(const char* css_selector);
 
-// If there is a border, renders the border from the style context
-// created by GetStyleContextFromCss(|css_selector|) into a single
-// pixel and returns the color.  Otherwise returns kInvalidColor.
+// Renders the border from the style context created by
+// GetStyleContextFromCss(|css_selector|) into a 24x24 bitmap and
+// returns the average color.
 SkColor GetBorderColor(const char* css_selector);
 
+SkColor GetSelectedTextColor(const char* css_selector);
+SkColor GetSelectedBgColor(const char* css_selector);
+
 // Get the color of the GtkSeparator specified by |css_selector|.
 SkColor GetSeparatorColor(const char* css_selector);
 #endif

chrome/browser/ui/libgtkui/native_theme_gtk3.cc

@@ -106,35 +106,35 @@ SkColor SkColorFromColorId(ui::NativeTheme::ColorId color_id) {
     case ui::NativeTheme::kColorId_FocusedMenuItemBackgroundColor:
       return GetBgColor("GtkMenu#menu GtkMenuItem#menuitem:hover");
     case ui::NativeTheme::kColorId_EnabledMenuItemForegroundColor:
-      return GetFgColor("GtkMenu#menu GtkMenuItem#menuitem GtkLabel#label");
+      return GetFgColor("GtkMenu#menu GtkMenuItem#menuitem GtkLabel");
     case ui::NativeTheme::kColorId_SelectedMenuItemForegroundColor:
-      return GetFgColor(
-          "GtkMenu#menu GtkMenuItem#menuitem:hover GtkLabel#label");
+      return GetFgColor("GtkMenu#menu GtkMenuItem#menuitem:hover GtkLabel");
     case ui::NativeTheme::kColorId_DisabledMenuItemForegroundColor:
-      return GetFgColor(
-          "GtkMenu#menu GtkMenuItem#menuitem:disabled GtkLabel#label");
+      return GetFgColor("GtkMenu#menu GtkMenuItem#menuitem:disabled GtkLabel");
     case ui::NativeTheme::kColorId_MenuItemSubtitleColor:
       return GetFgColor(
-          "GtkMenu#menu GtkMenuItem#menuitem GtkLabel#label.accelerator");
+          "GtkMenu#menu GtkMenuItem#menuitem GtkLabel.accelerator");
     case ui::NativeTheme::kColorId_MenuSeparatorColor:
     // MenuButton borders are used as vertical menu separators in Chrome.
     case ui::NativeTheme::kColorId_EnabledMenuButtonBorderColor:
     case ui::NativeTheme::kColorId_FocusedMenuButtonBorderColor:
     case ui::NativeTheme::kColorId_HoverMenuButtonBorderColor:
-      if (GtkVersionCheck(3, 20))
-        return GetBgColor("GtkMenu#menu GtkSeparator#separator");
-      else
+      if (GtkVersionCheck(3, 20)) {
+        return GetSeparatorColor(
+            "GtkMenu#menu GtkSeparator#separator.horizontal");
+      } else {
         return GetFgColor("GtkMenu#menu GtkMenuItem#menuitem.separator");
+      }
 
     // Label
     case ui::NativeTheme::kColorId_LabelEnabledColor:
-      return GetFgColor("GtkLabel#label");
+      return GetFgColor("GtkLabel");
     case ui::NativeTheme::kColorId_LabelDisabledColor:
-      return GetFgColor("GtkLabel#label:disabled");
+      return GetFgColor("GtkLabel:disabled");
     case ui::NativeTheme::kColorId_LabelTextSelectionColor:
-      return GetFgColor("GtkLabel#label #selection:selected");
+      return GetSelectedTextColor("GtkLabel");
     case ui::NativeTheme::kColorId_LabelTextSelectionBackgroundFocused:
-      return GetBgColor("GtkLabel#label #selection:selected");
+      return GetSelectedBgColor("GtkLabel");
 
     // Link
     case ui::NativeTheme::kColorId_LinkDisabled:
@@ -142,13 +142,13 @@ SkColor SkColorFromColorId(ui::NativeTheme::ColorId color_id) {
           SkColorFromColorId(ui::NativeTheme::kColorId_LinkEnabled), 0xBB);
     case ui::NativeTheme::kColorId_LinkPressed:
       if (GtkVersionCheck(3, 12))
-        return GetFgColor("GtkLabel#label.link:link:hover:active");
+        return GetFgColor("GtkLabel.link:link:hover:active");
     // fallthrough
     case ui::NativeTheme::kColorId_LinkEnabled: {
       if (GtkVersionCheck(3, 12)) {
-        return GetFgColor("GtkLabel#label.link:link");
+        return GetFgColor("GtkLabel.link:link");
       }
-      auto link_context = GetStyleContextFromCss("GtkLabel#label.view");
+      auto link_context = GetStyleContextFromCss("GtkLabel.view");
       GdkColor* color;
       gtk_style_context_get_style(link_context, "link-color", &color, nullptr);
       if (color) {
@@ -173,28 +173,28 @@ SkColor SkColorFromColorId(ui::NativeTheme::ColorId color_id) {
 
     // Button
     case ui::NativeTheme::kColorId_ButtonEnabledColor:
-      return GetFgColor("GtkButton#button.text-button GtkLabel#label");
+      return GetFgColor("GtkButton#button.text-button GtkLabel");
     case ui::NativeTheme::kColorId_ButtonDisabledColor:
-      return GetFgColor("GtkButton#button.text-button:disabled GtkLabel#label");
+      return GetFgColor("GtkButton#button.text-button:disabled GtkLabel");
     case ui::NativeTheme::kColorId_ButtonHoverColor:
-      return GetFgColor("GtkButton#button.text-button:hover GtkLabel#label");
+      return GetFgColor("GtkButton#button.text-button:hover GtkLabel");
     case ui::NativeTheme::kColorId_ButtonPressedShade:
       return SK_ColorTRANSPARENT;
 
     case ui::NativeTheme::kColorId_BlueButtonEnabledColor:
       return GetFgColor(
-          "GtkButton#button.text-button.suggested-action GtkLabel#label");
+          "GtkButton#button.text-button.suggested-action GtkLabel");
     case ui::NativeTheme::kColorId_BlueButtonDisabledColor:
       return GetFgColor(
           "GtkButton#button.text-button.suggested-action:disabled "
-          "GtkLabel#label");
+          "GtkLabel");
     case ui::NativeTheme::kColorId_BlueButtonHoverColor:
       return GetFgColor(
-          "GtkButton#button.text-button.suggested-action:hover GtkLabel#label");
+          "GtkButton#button.text-button.suggested-action:hover GtkLabel");
     case ui::NativeTheme::kColorId_BlueButtonPressedColor:
       return GetFgColor(
           "GtkButton#button.text-button.suggested-action:hover:active "
-          "GtkLabel#label");
+          "GtkLabel");
     case ui::NativeTheme::kColorId_BlueButtonShadowColor:
       return SK_ColorTRANSPARENT;
 
@@ -202,7 +202,7 @@ SkColor SkColorFromColorId(ui::NativeTheme::ColorId color_id) {
       return GetBgColor("GtkButton#button.text-button.destructive-action");
     case ui::NativeTheme::kColorId_TextOnProminentButtonColor:
       return GetFgColor(
-          "GtkButton#button.text-button.destructive-action GtkLabel#label");
+          "GtkButton#button.text-button.destructive-action GtkLabel");
 
     // Textfield
     case ui::NativeTheme::kColorId_TextfieldDefaultColor:
@@ -214,9 +214,9 @@ SkColor SkColorFromColorId(ui::NativeTheme::ColorId color_id) {
     case ui::NativeTheme::kColorId_TextfieldReadOnlyBackground:
       return GetBgColor("GtkEntry#entry:disabled");
     case ui::NativeTheme::kColorId_TextfieldSelectionColor:
-      return GetFgColor("GtkEntry#entry #selection:selected");
+      return GetSelectedTextColor("GtkEntry#entry");
     case ui::NativeTheme::kColorId_TextfieldSelectionBackgroundFocused:
-      return GetBgColor("GtkEntry#entry #selection:selected");
+      return GetSelectedBgColor("GtkEntry#entry");
 
     // Tooltips
     case ui::NativeTheme::kColorId_TooltipBackground:
@@ -233,13 +233,13 @@ SkColor SkColorFromColorId(ui::NativeTheme::ColorId color_id) {
     case ui::NativeTheme::kColorId_TreeText:
     case ui::NativeTheme::kColorId_TreeArrow:
     case ui::NativeTheme::kColorId_TableGroupingIndicatorColor:
-      return GetFgColor("GtkTreeView#treeview.view .view.cell GtkLabel#label");
+      return GetFgColor("GtkTreeView#treeview.view .view.cell GtkLabel");
     case ui::NativeTheme::kColorId_TableSelectedText:
     case ui::NativeTheme::kColorId_TableSelectedTextUnfocused:
     case ui::NativeTheme::kColorId_TreeSelectedText:
     case ui::NativeTheme::kColorId_TreeSelectedTextUnfocused:
       return GetFgColor(
-          "GtkTreeView#treeview.view .view.cell:selected:focus GtkLabel#label");
+          "GtkTreeView#treeview.view .view.cell:selected:focus GtkLabel");
     case ui::NativeTheme::kColorId_TableSelectionBackgroundFocused:
     case ui::NativeTheme::kColorId_TableSelectionBackgroundUnfocused:
     case ui::NativeTheme::kColorId_TreeSelectionBackgroundFocused:
@@ -248,43 +248,39 @@ SkColor SkColorFromColorId(ui::NativeTheme::ColorId color_id) {
 
     // Table Header
     case ui::NativeTheme::kColorId_TableHeaderText:
-      return GetFgColor(
-          "GtkTreeView#treeview.view GtkButton#button GtkLabel#label");
+      return GetFgColor("GtkTreeView#treeview.view GtkButton#button GtkLabel");
     case ui::NativeTheme::kColorId_TableHeaderBackground:
       return GetBgColor("GtkTreeView#treeview.view GtkButton#button");
     case ui::NativeTheme::kColorId_TableHeaderSeparator:
       return GetBorderColor("GtkTreeView#treeview.view GtkButton#button");
 
     // Results Table
-    // TODO(thomasanderson): The GtkEntry selectors was how the gtk2 theme got
-    // these colors.  Update this code to use a different widget.
     case ui::NativeTheme::kColorId_ResultsTableNormalBackground:
       return GetBgColor("GtkEntry#entry");
     case ui::NativeTheme::kColorId_ResultsTableHoveredBackground:
-      return color_utils::AlphaBlend(
-          GetBgColor("GtkEntry#entry"),
-          GetBgColor("GtkEntry#entry #selection:selected"), 0x80);
+      return color_utils::AlphaBlend(GetBgColor("GtkEntry#entry"),
+                                     GetSelectedBgColor("GtkEntry#entry"),
+                                     0x80);
     case ui::NativeTheme::kColorId_ResultsTableSelectedBackground:
-      return GetBgColor("GtkEntry#entry #selection:selected");
+      return GetSelectedBgColor("GtkEntry#entry");
     case ui::NativeTheme::kColorId_ResultsTableNormalText:
     case ui::NativeTheme::kColorId_ResultsTableHoveredText:
       return GetFgColor("GtkEntry#entry");
     case ui::NativeTheme::kColorId_ResultsTableSelectedText:
-      return GetFgColor("GtkEntry#entry #selection:selected");
+      return GetSelectedTextColor("GtkEntry#entry");
     case ui::NativeTheme::kColorId_ResultsTableNormalDimmedText:
     case ui::NativeTheme::kColorId_ResultsTableHoveredDimmedText:
       return color_utils::AlphaBlend(GetFgColor("GtkEntry#entry"),
                                      GetBgColor("GtkEntry#entry"), 0x80);
     case ui::NativeTheme::kColorId_ResultsTableSelectedDimmedText:
-      return color_utils::AlphaBlend(
-          GetFgColor("GtkEntry#entry #selection:selected"),
-          GetBgColor("GtkEntry#entry"), 0x80);
+      return color_utils::AlphaBlend(GetSelectedTextColor("GtkEntry#entry"),
+                                     GetBgColor("GtkEntry#entry"), 0x80);
     case ui::NativeTheme::kColorId_ResultsTableNormalUrl:
     case ui::NativeTheme::kColorId_ResultsTableHoveredUrl:
       return NormalURLColor(GetFgColor("GtkEntry#entry"));
     case ui::NativeTheme::kColorId_ResultsTableSelectedUrl:
-      return SelectedURLColor(GetFgColor("GtkEntry#entry #selection:selected"),
-                              GetBgColor("GtkEntry#entry #selection:selected"));
+      return SelectedURLColor(GetSelectedTextColor("GtkEntry#entry"),
+                              GetSelectedBgColor("GtkEntry#entry"));
 
     case ui::NativeTheme::kColorId_ResultsTablePositiveText:
       return color_utils::GetReadableColor(kPositiveTextColor,
@@ -413,7 +409,7 @@ void NativeThemeGtk3::PaintArrowButton(SkCanvas* canvas,
   }
 
   PaintWidget(canvas, rect, context, BG_RENDER_NORMAL, true);
-  PaintArrow(canvas, rect, direction, SkColorFromStyleContext(context));
+  PaintArrow(canvas, rect, direction, GetFgColorFromStyleContext(context));
 }
 
 void NativeThemeGtk3::PaintScrollbarTrack(
@@ -483,28 +479,29 @@ void NativeThemeGtk3::PaintMenuSeparator(
       case ui::UPPER_SEPARATOR:
         return 0;
       default:
-        return rect.height() / 2;
+        return (rect.height() - separator_thickness) / 2;
     }
   };
   if (GtkVersionCheck(3, 20)) {
-    auto context =
-        GetStyleContextFromCss("GtkMenu#menu GtkSeparator#separator");
+    auto context = GetStyleContextFromCss(
+        "GtkMenu#menu GtkSeparator#separator.horizontal");
     GtkBorder margin, border, padding;
     GtkStateFlags state = gtk_style_context_get_state(context);
     gtk_style_context_get_margin(context, state, &margin);
     gtk_style_context_get_border(context, state, &border);
     gtk_style_context_get_padding(context, state, &padding);
-    int min_height = 0;
+    int min_height = 1;
     gtk_style_context_get(context, state, "min-height", &min_height, NULL);
     int w = rect.width() - margin.left - margin.right;
-    int h =
-        min_height + padding.top + padding.bottom + border.top + border.bottom;
+    int h = std::max(
+        min_height + padding.top + padding.bottom + border.top + border.bottom,
+        1);
     int x = margin.left;
     int y = separator_offset(h);
     PaintWidget(canvas, gfx::Rect(x, y, w, h), context, BG_RENDER_NORMAL, true);
   } else {
-    auto context =
-        GetStyleContextFromCss("GtkMenu#menu GtkMenuItem#menuitem.separator");
+    auto context = GetStyleContextFromCss(
+        "GtkMenu#menu GtkMenuItem#menuitem.separator.horizontal");
     gboolean wide_separators = false;
     gint separator_height = 0;
     gtk_style_context_get_style(context, "wide-separators", &wide_separators,
@@ -522,7 +519,7 @@ void NativeThemeGtk3::PaintMenuSeparator(
       PaintWidget(canvas, gfx::Rect(x, y, w, h), context, BG_RENDER_NONE, true);
     } else {
       SkPaint paint;
-      paint.setColor(SkColorFromStyleContext(context));
+      paint.setColor(GetFgColorFromStyleContext(context));
       canvas->drawLine(x, y, x + w, y, paint);
     }
   }
@@ -536,7 +533,7 @@ void NativeThemeGtk3::PaintFrameTopArea(
   auto context = GetStyleContextFromCss(frame_top_area.use_custom_frame
                                             ? "#headerbar.header-bar.titlebar"
                                             : "GtkMenuBar#menubar");
-  RemoveBorders(context);
+  ApplyCssToContext(context, "* { border-radius: 0px; border-style: none; }");
   gtk_style_context_set_state(context, frame_top_area.is_active
                                            ? GTK_STATE_FLAG_NORMAL
                                            : GTK_STATE_FLAG_BACKDROP);

docs/linux_gtk_theme_integration.md

@@ -16,24 +16,18 @@ mostly a list of about 80 colors (see //src/ui/native_theme/native_theme.h)
 overridden by the theme. Chromium usually doesn't use GTK to render entire
 widgets, but instead tries to determine colors from them.
 
-There are three types of colors Chromium needs from widgets: 
-
-* Foreground color: determined by the CSS "color" property
-* Background color: determined by the CSS "background-color" and "background-image" properties
-* Border color: determined by the "border-color", "border-image",
-  "border-style", and "border-width" properties
-
-Backgrounds and borders are complicated because in general they might have
-multiple gradients or images. To get the color, Chromium uses GTK to render the
-background or border into a single pixel and uses the resulting color for
-theming. This mostly gives reasonable results, but in case theme authors do not
-like the resulting color, they have the option to theme Chromium widgets
-specially.
+Chromium needs foreground, background and border colors from widgets.  The
+foreground color is simply taken from the CSS "color" property.  Backgrounds and
+borders are complicated because in general they might have multiple gradients or
+images. To get the color, Chromium uses GTK to render the background or border
+into a 24x24 bitmap and uses the average color for theming. This mostly gives
+reasonable results, but in case theme authors do not like the resulting color,
+they have the option to theme Chromium widgets specially.
 
 ## Note to GTK theme authors: How to theme Chromium widgets
 
 Every widget Chromium uses will have a "chromium" style class added to it. For
-example, a texfield selector might look like:
+example, a textfield selector might look like:
 
 ```
 .window.background.chromium .entry.chromium
@@ -59,11 +53,6 @@ they might use:
 For GTK3.20 or later, themes will as usual have to replace ".entry" with
 "entry".
 
-Additional requirements for border colors to be picked up:
-
-* Must have a border-style that is not none.
-* Must have a border-width that is nonzero.
-
 The list of CSS selectors that Chromium uses to determine its colors is in
 //src/chrome/browser/ui/libgtkui/native_theme_gtk3.cc.