Change chrome:// favicons in tabstrip based on theming.

chrome/browser/ui/views/tabs/tab.cc

Index: chrome/browser/ui/views/tabs/tab.cc
diff --git a/chrome/browser/ui/views/tabs/tab.cc b/chrome/browser/ui/views/tabs/tab.cc
index 3da5cd70530f9c9ca0cef8c232b9ef73d776a9ed..7872c5639c3a35e4530771aa348737422380f246 100644
--- a/chrome/browser/ui/views/tabs/tab.cc
+++ b/chrome/browser/ui/views/tabs/tab.cc
@@ -25,6 +25,7 @@
 #include "chrome/common/chrome_switches.h"
 #include "chrome/grit/generated_resources.h"
 #include "content/public/browser/user_metrics.h"
+#include "content/public/common/url_constants.h"
 #include "grit/components_scaled_resources.h"
 #include "grit/components_strings.h"
 #include "grit/theme_resources.h"
@@ -163,7 +164,7 @@ class Tab::FaviconCrashAnimation : public gfx::LinearAnimation,
           static_cast<int>(floor(kHidingOffset * 2.0 * state)));
     } else {
       // Animate the crashed icon up.
-      target_->set_should_display_crashed_favicon();
+      target_->SetShouldDisplayCrashedFavicon(true);
       target_->SetFaviconHidingOffset(
           static_cast<int>(
               floor(kHidingOffset - ((state - .5) * 2.0 * kHidingOffset))));
@@ -606,7 +607,7 @@ void Tab::SetData(const TabRendererData& data) {
 
   if (!data_.IsCrashed()) {
     crash_icon_animation_->Stop();
-    should_display_crashed_favicon_ = false;
+    SetShouldDisplayCrashedFavicon(false);
     favicon_hiding_offset_ = 0;
   } else if (!should_display_crashed_favicon_ &&
              !crash_icon_animation_->is_animating()) {
@@ -978,6 +979,7 @@ void Tab::Layout() {
 void Tab::OnThemeChanged() {
   LoadTabImages();
   OnButtonColorMaybeChanged();
+  favicon_ = gfx::ImageSkia();
 }
 
 const char* Tab::GetClassName() const {
@@ -1160,6 +1162,11 @@ void Tab::MaybeAdjustLeftForPinnedTab(gfx::Rect* bounds) const {
 }
 
 void Tab::DataChanged(const TabRendererData& old) {
+  // Clear the cached themed favicon except in the case of about:crash, which
+  // should go on using the old icon, whatever it was.
+  if (data().url != GURL(content::kChromeUICrashURL))
+    favicon_ = gfx::ImageSkia();

[Peter Kasting, 2016/06/30 00:07:04]

Do we have to clear on every change?  Are there changes that wouldn't change the
favicon, which we can avoid clearing for?

[Evan Stade, 2016/06/30 15:39:45]

I think the answer is yes, but I'm not sure if it's easy to know with certainty
whether we can avoid clearing the cache. In the normal case (non-chrome://
pages) clearing the cache is harmless as there's no computation done during
paint.

In any case, I've changed this check such that it should catch some more cases
where we don't need to reset the cache, which also has the nice side effect of
eliminating this reference to about:crash.

+
   if (data().alert_state != old.alert_state || data().title != old.title)
     TooltipTextChanged();
 
@@ -1419,18 +1426,17 @@ void Tab::PaintTabFill(gfx::Canvas* canvas,
 
 void Tab::PaintPinnedTabTitleChangedIndicatorAndIcon(
     gfx::Canvas* canvas,
-    const gfx::ImageSkia& favicon,
     const gfx::Rect& favicon_draw_bounds) {
   // The pinned tab title changed indicator consists of two parts:
   // . a clear (totally transparent) part over the bottom right (or left in rtl)
   //   of the favicon. This is done by drawing the favicon to a canvas, then
   //   drawing the clear part on top of the favicon.
   // . a circle in the bottom right (or left in rtl) of the favicon.
-  if (!favicon.isNull()) {
+  if (!favicon_.isNull()) {
     const float kIndicatorCropRadius = 4.5;
     gfx::Canvas icon_canvas(gfx::Size(gfx::kFaviconSize, gfx::kFaviconSize),
                             canvas->image_scale(), false);
-    icon_canvas.DrawImageInt(favicon, 0, 0);
+    icon_canvas.DrawImageInt(favicon_, 0, 0);
     SkPaint clear_paint;
     clear_paint.setAntiAlias(true);
     clear_paint.setXfermodeMode(SkXfermode::kClear_Mode);
@@ -1466,21 +1472,34 @@ void Tab::PaintIcon(gfx::Canvas* canvas) {
   if (bounds.IsEmpty())
     return;
 
-  if (data().network_state != TabRendererData::NETWORK_STATE_NONE) {
-    // Throbber will do its own painting.
+  // Throbber will do its own painting.
+  if (data().network_state != TabRendererData::NETWORK_STATE_NONE)
     return;
+
+  // Ensure that |favicon_| is created.
+  if (favicon_.isNull()) {
+    ui::ResourceBundle* rb = &ui::ResourceBundle::GetSharedInstance();
+    favicon_ = should_display_crashed_favicon_
+                   ? *rb->GetImageSkiaNamed(IDR_CRASH_SAD_FAVICON)
+                   : data().favicon;
+    // Themify the icon if it's a chrome:// page or if it's the sadtab favicon.

[Peter Kasting, 2016/06/30 00:07:04]

Should this share code with the bookmark bar?  Should the two note the existence
of the other?

[Evan Stade, 2016/06/30 15:39:45]

I didn't share code because I didn't see a clear and useful way to do so. The
only shared bits are url.SchemeIs(content::kChromeUIScheme) and the
CreateHSLShiftedImage invocation.

I can add cross-references in comments.

+    if (!favicon_.isNull() &&
+        (should_display_crashed_favicon_ ||
+         (data().url.SchemeIs(content::kChromeUIScheme)) ||
+         favicon_.BackedBySameObjectAs(
+             *rb->GetImageSkiaNamed(IDR_DEFAULT_FAVICON)))) {
+      color_utils::HSL icon_shift =
+          GetThemeProvider()->GetTint(ThemeProperties::TINT_BUTTONS);
+      favicon_ =
+          gfx::ImageSkiaOperations::CreateHSLShiftedImage(favicon_, icon_shift);
+    }
   }
-  const gfx::ImageSkia& favicon =
-      should_display_crashed_favicon_
-          ? *ui::ResourceBundle::GetSharedInstance().GetImageSkiaNamed(
-                IDR_CRASH_SAD_FAVICON)
-          : data().favicon;
 
   if (showing_pinned_tab_title_changed_indicator_ &&
       !should_display_crashed_favicon_) {
-    PaintPinnedTabTitleChangedIndicatorAndIcon(canvas, favicon, bounds);
-  } else if (!favicon.isNull()) {
-    canvas->DrawImageInt(favicon, 0, 0, bounds.width(), bounds.height(),
+    PaintPinnedTabTitleChangedIndicatorAndIcon(canvas, bounds);
+  } else if (!favicon_.isNull()) {
+    canvas->DrawImageInt(favicon_, 0, 0, bounds.width(), bounds.height(),
                          bounds.x(), bounds.y(), bounds.width(),
                          bounds.height(), false);
   }
@@ -1594,6 +1613,14 @@ double Tab::GetThrobValue() {
   return val;
 }
 
+void Tab::SetShouldDisplayCrashedFavicon(bool value) {
+  if (value == should_display_crashed_favicon_)
+    return;
+
+  should_display_crashed_favicon_ = value;
+  favicon_ = gfx::ImageSkia();
+}
+
 void Tab::SetFaviconHidingOffset(int offset) {
   favicon_hiding_offset_ = offset;
   ScheduleIconPaint();