Render the tab close button as a vector-based icon.

chrome/browser/ui/views/tabs/tab.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/views/tabs/tab.h"
 
 #include <limits>
 
 #include "base/command_line.h"
 #include "base/debug/alias.h"
@@ skipping 19 matching lines @@
 #include "ui/base/resource/resource_bundle.h"
 #include "ui/base/theme_provider.h"
 #include "ui/gfx/animation/animation_container.h"
 #include "ui/gfx/animation/multi_animation.h"
 #include "ui/gfx/animation/throb_animation.h"
 #include "ui/gfx/canvas.h"
 #include "ui/gfx/color_analysis.h"
 #include "ui/gfx/favicon_size.h"
 #include "ui/gfx/geometry/rect_conversions.h"
 #include "ui/gfx/image/image_skia_operations.h"
+#include "ui/gfx/paint_vector_icon.h"
 #include "ui/gfx/path.h"
 #include "ui/gfx/skia_util.h"
+#include "ui/gfx/vector_icons_public.h"
+#include "ui/native_theme/common_theme.h"
+#include "ui/native_theme/native_theme.h"
 #include "ui/resources/grit/ui_resources.h"
 #include "ui/views/border.h"
 #include "ui/views/controls/button/image_button.h"
 #include "ui/views/controls/label.h"
 #include "ui/views/rect_based_targeting_utils.h"
 #include "ui/views/view_targeter.h"
 #include "ui/views/widget/tooltip_manager.h"
 #include "ui/views/widget/widget.h"
 #include "ui/views/window/non_client_view.h"
 
@@ skipping 16 matching lines @@
 
 // How long the pulse throb takes.
 const int kPulseDurationMs = 200;
 
 // Width of touch tabs.
 const int kTouchWidth = 120;
 
 const int kToolbarOverlap = 1;
 const int kExtraLeftPaddingToBalanceCloseButtonPadding = 2;
 const int kFaviconTitleSpacing = 4;
-const int kAfterTitleSpacing = 3;
-const int kCloseButtonRightPaddingOverlap = 3;
+const int kAfterTitleSpacing = 4;
+const int kCloseButtonRightPaddingOverlap = 2;
 const int kStandardTitleWidth = 175;
 
 // Width of the content inside a pinned tab.
 int kPinnedTabContentWidth = 25;
 
 // When a non-pinned tab becomes a pinned tab the width of the tab animates. If
 // the width of a pinned tab is at least kPinnedTabExtraWidthToRenderAsNormal
 // larger than the desired pinned tab width then the tab is rendered as a normal
 // tab. This is done to avoid having the title immediately disappear when
 // transitioning a tab from normal to pinned tab.
@@ skipping 331 matching lines @@
       immersive_loading_step_(0),
       should_display_crashed_favicon_(false),
       close_button_(NULL),
       media_indicator_button_(NULL),
       title_(new views::Label()),
       tab_activated_with_last_tap_down_(false),
       hover_controller_(this),
       showing_icon_(false),
       showing_media_indicator_(false),
       showing_close_button_(false),
-      close_button_color_(0) {
+      close_button_color_(SK_ColorTRANSPARENT) {
   DCHECK(controller);
   InitTabResources();
 
   // So we get don't get enter/exit on children and don't prematurely stop the
   // hover.
   set_notify_enter_exit_on_child(true);
 
   set_id(VIEW_ID_TAB);
 
   title_->SetHorizontalAlignment(gfx::ALIGN_TO_HEAD);
   title_->SetElideBehavior(gfx::FADE_TAIL);
   title_->SetHandlesTooltips(false);
   title_->SetAutoColorReadabilityEnabled(false);
   title_->SetText(CoreTabHelper::GetDefaultTitle());
   AddChildView(title_);
 
   SetEventTargeter(
       scoped_ptr<views::ViewTargeter>(new views::ViewTargeter(this)));
 
-  // Add the Close Button.
   close_button_ = new TabCloseButton(this);
-  ui::ResourceBundle& rb = ui::ResourceBundle::GetSharedInstance();
-  close_button_->SetImage(views::CustomButton::STATE_NORMAL,
-                          rb.GetImageSkiaNamed(IDR_CLOSE_1));
-  close_button_->SetImage(views::CustomButton::STATE_HOVERED,
-                          rb.GetImageSkiaNamed(IDR_CLOSE_1_H));
-  close_button_->SetImage(views::CustomButton::STATE_PRESSED,
-                          rb.GetImageSkiaNamed(IDR_CLOSE_1_P));
   close_button_->SetAccessibleName(
       l10n_util::GetStringUTF16(IDS_ACCNAME_CLOSE));
+
+  // We need to set the close button's normal image now, even though it will
+  // have the wrong color, so that the close button gets sized and laid out
+  // correctly.  (We can't get the correct color without a ThemeProvider, which
+  // we won't have until we're part of a Widget hierarchy.)  We'll reset the
+  // image to the right color the first time through OnPaint(), when we're
+  // guaranteed to have the ThemeProvider available.
+  SetCloseButtonNormalImage();
+
+  // The hovered andpressed images won't be changing later, so we can set them

[Evan Stade, 2015/09/18 17:30:33]

nit: "and pressed"

[Peter Kasting, 2015/09/18 19:48:30]

Done.

+  // correctly here.
+  const gfx::ImageSkia& hovered = gfx::CreateVectorIcon(
+      gfx::VectorIconId::TAB_CLOSE_HOVERED_PRESSED, 16,
+      SkColorSetARGB(0xFF, 0xDB, 0x44, 0x37));
+  const gfx::ImageSkia& pressed = gfx::CreateVectorIcon(
+      gfx::VectorIconId::TAB_CLOSE_HOVERED_PRESSED, 16,
+      SkColorSetARGB(0xFF, 0xA8, 0x35, 0x2A));
+  close_button_->SetImage(views::CustomButton::STATE_HOVERED, &hovered);
+  close_button_->SetImage(views::CustomButton::STATE_PRESSED, &pressed);
+
   // Disable animation so that the red danger sign shows up immediately
   // to help avoid mis-clicks.
   close_button_->SetAnimationDuration(0);
+
   AddChildView(close_button_);
 
   set_context_menu_controller(this);
 }
 
 Tab::~Tab() {
 }
 
 void Tab::set_animation_container(gfx::AnimationContainer* container) {
   animation_container_ = container;
@@ skipping 262 matching lines @@
 
 void Tab::OnPaint(gfx::Canvas* canvas) {
   // Don't paint if we're narrower than we can render correctly. (This should
   // only happen during animations).
   if (width() < GetMinimumUnselectedSize().width() && !data().pinned)
     return;
 
   gfx::Rect clip;
   if (!controller_->ShouldPaintTab(this, &clip))
     return;
+
+  // If we haven't set the correct close button normal image, we can do so now.
+  if (close_button_color_ == SK_ColorTRANSPARENT)
+    OnThemeChanged();

[Evan Stade, 2015/09/18 17:30:33]

why not use OnViewHierarchyChanged? I imagine that gets called before
Layout/GetPreferredSize but even if not, it seems more self-documenting than
having this special close button color value.

[Peter Kasting, 2015/09/18 19:48:30]

Done.  I also needed to handle active state changes correctly, so that's now
done too.

+
   if (!clip.IsEmpty()) {
     canvas->Save();
     canvas->ClipRect(clip);
   }
 
   if (controller_->IsImmersiveStyle())
     PaintImmersiveTab(canvas);
   else
     PaintTab(canvas);
 
@@ skipping 83 matching lines @@
       rect.set_y(lb.y() - (title_height - rect.height()) / 2);
       rect.set_height(title_height);
     }
     title_->SetBoundsRect(rect);
   }
   title_->SetVisible(show_title);
 }
 
 void Tab::OnThemeChanged() {
   LoadTabImages();
+
+  const SkColor text_color = GetThemeProvider()->GetColor(
+      IsActive() ? ThemeProperties::COLOR_TAB_TEXT
+                 : ThemeProperties::COLOR_BACKGROUND_TAB_TEXT);
+  close_button_color_ = SkColorSetA(text_color, 0x70);
+  SetCloseButtonNormalImage();
 }
 
 const char* Tab::GetClassName() const {
   return kViewClassName;
 }
 
 bool Tab::GetTooltipText(const gfx::Point& p, base::string16* tooltip) const {
   // Note: Anything that affects the tooltip text should be accounted for when
   // calling TooltipTextChanged() from Tab::DataChanged().
   *tooltip = chrome::AssembleTabTooltipText(data_.title, data_.media_state);
@@ skipping 181 matching lines @@
     StopPulse();
 }
 
 void Tab::PaintTab(gfx::Canvas* canvas) {
   // See if the model changes whether the icons should be painted.
   const bool show_icon = ShouldShowIcon();
   const bool show_media_indicator = ShouldShowMediaIndicator();
   const bool show_close_button = ShouldShowCloseBox();
   if (show_icon != showing_icon_ ||
       show_media_indicator != showing_media_indicator_ ||
-      show_close_button != showing_close_button_) {
+      show_close_button != showing_close_button_)
     Layout();
-  }
 
   PaintTabBackground(canvas);
 
   const SkColor title_color = GetThemeProvider()->GetColor(IsSelected() ?
       ThemeProperties::COLOR_TAB_TEXT :
       ThemeProperties::COLOR_BACKGROUND_TAB_TEXT);
-  title_->SetVisible(ShouldRenderAsNormalTab());

[sky, 2015/09/18 16:01:52]

How come you removed this line?

[Peter Kasting, 2015/09/18 19:48:30]

See comment in patch set 1 -- we do this already in Layout(), so this is
redundant.

[sky, 2015/09/18 20:33:38]

I only saw this patchset, so I missed that comment.

   title_->SetEnabledColor(title_color);
 
   if (show_icon)
     PaintIcon(canvas);
-
-  // If the close button color has changed, generate a new one.
-  if (!close_button_color_ || title_color != close_button_color_) {
-    close_button_color_ = title_color;
-    ui::ResourceBundle& rb = ui::ResourceBundle::GetSharedInstance();
-    close_button_->SetBackground(close_button_color_,
-        rb.GetImageSkiaNamed(IDR_CLOSE_1),
-        rb.GetImageSkiaNamed(IDR_CLOSE_1_MASK));
-  }
 }
 
 void Tab::PaintImmersiveTab(gfx::Canvas* canvas) {
   // Use transparency for the draw-attention animation.
   int alpha = 255;
   if (pulse_animation_ && pulse_animation_->is_animating() && !data().pinned) {
     alpha = pulse_animation_->CurrentValueBetween(
         255, static_cast<int>(255 * kImmersiveTabMinThrobOpacity));
   }
 
@@ skipping 422 matching lines @@
 
 void Tab::StartCrashAnimation() {
   crash_icon_animation_.reset(new FaviconCrashAnimation(this));
   crash_icon_animation_->Start();
 }
 
 bool Tab::IsPerformingCrashAnimation() const {
   return crash_icon_animation_.get() && data_.IsCrashed();
 }
 
+void Tab::SetCloseButtonNormalImage() {
+  const gfx::ImageSkia& normal = gfx::CreateVectorIcon(
+      gfx::VectorIconId::TAB_CLOSE_NORMAL, 16, close_button_color_);

[Evan Stade, 2015/09/18 17:30:33]

nit: lots of 16s hardcoded here, why not put this at the top with the other
consts (for consistency if nothing else)

[Peter Kasting, 2015/09/18 19:48:30]

Done.

+  close_button_->SetImage(views::CustomButton::STATE_NORMAL, &normal);
+}
+
 void Tab::ScheduleIconPaint() {
   gfx::Rect bounds = favicon_bounds_;
   if (bounds.IsEmpty())
     return;
 
   // Extends the area to the bottom when sad_favicon is animating.
   if (IsPerformingCrashAnimation())
     bounds.set_height(height() - bounds.y());
   bounds.set_x(GetMirroredXForRect(bounds));
   SchedulePaintInRect(bounds);
@@ skipping 143 matching lines @@
                          const gfx::ImageSkia& image) {
   DCHECK_NE(scale_factor, ui::SCALE_FACTOR_NONE);
   ImageCacheEntry entry;
   entry.resource_id = resource_id;
   entry.scale_factor = scale_factor;
   entry.image = image;
   image_cache_->push_front(entry);
   if (image_cache_->size() > kMaxImageCacheSize)
     image_cache_->pop_back();
 }