[Material][Mac] Fix Default Favicon's Color

chrome/browser/ui/cocoa/tabs/tab_strip_controller.mm

 // 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.
 
 #import "chrome/browser/ui/cocoa/tabs/tab_strip_controller.h"
 
 #import <QuartzCore/QuartzCore.h>
 
 #include <cmath>
 #include <limits>
@@ skipping 124 matching lines @@
   bool animate_;
   DISALLOW_COPY_AND_ASSIGN(ScopedNSAnimationContextGroup);
 };
 
 }  // namespace
 
 @interface TabStripController (Private)
 - (void)addSubviewToPermanentList:(NSView*)aView;
 - (void)regenerateSubviewList;
 - (NSInteger)indexForContentsView:(NSView*)view;
-- (NSImage*)iconImageForContents:(content::WebContents*)contents;
+- (NSImage*)iconImageForContents:(content::WebContents*)contents
+                         atIndex:(NSInteger)modelIndex;
 - (void)updateIconsForContents:(content::WebContents*)contents
                        atIndex:(NSInteger)modelIndex;
 - (void)layoutTabsWithAnimation:(BOOL)animate
              regenerateSubviews:(BOOL)doUpdate;
 - (void)animationDidStop:(CAAnimation*)animation
            forController:(TabController*)controller
                 finished:(BOOL)finished;
 - (NSInteger)indexFromModelIndex:(NSInteger)index;
 - (void)clickNewTabButton:(id)sender;
 - (NSInteger)numberOfOpenTabs;
@@ skipping 1332 matching lines @@
     // tabs. Instead, simply mark it as closing to prevent the tab from
     // generating any drags or selections.
     [[tab tabView] setClosing:YES];
   }
 
   [delegate_ onTabDetachedWithContents:contents];
 }
 
 // A helper routine for creating an NSImageView to hold the favicon or app icon
 // for |contents|.
-- (NSImage*)iconImageForContents:(content::WebContents*)contents {
+- (NSImage*)iconImageForContents:(content::WebContents*)contents
+                         atIndex:(NSInteger)modelIndex {
   extensions::TabHelper* extensions_tab_helper =
       extensions::TabHelper::FromWebContents(contents);
   BOOL isApp = extensions_tab_helper->is_app();
   NSImage* image = nil;
   // Favicons come from the renderer, and the renderer draws everything in the
   // system color space.
   CGColorSpaceRef colorSpace = base::mac::GetSystemColorSpace();
   if (isApp) {
     SkBitmap* icon = extensions_tab_helper->GetExtensionAppIcon();
     if (icon)
       image = skia::SkBitmapToNSImageWithColorSpace(*icon, colorSpace);
   } else {
-    TabController* tab = [tabArray_ firstObject];
-    NSColor* titleColor = [[tab tabView] titleColor];
-    NSColor* deviceColor =
-        [titleColor colorUsingColorSpace:[NSColorSpace deviceRGBColorSpace]];
-    image = mac::FaviconForWebContents(
-        contents, skia::NSDeviceColorToSkColor(deviceColor));
+    TabController* tab = [tabArray_ objectAtIndex:modelIndex];
+    image = mac::FaviconForWebContents(contents, [[tab tabView] iconColor]);
   }
 
   // Either we don't have a valid favicon or there was some issue converting it
   // from an SkBitmap. Either way, just show the default.
   if (!image)
     image = defaultFavicon_.get();
 
   return image;
 }
 
@@ skipping 50 matching lines @@
         [[[tabController view] window] hasDarkTheme]) {
       throbberImage = throbberLoadingIncognitoImage;
     } else {
       throbberImage = throbberLoadingImage;
     }
   }
 
   if (oldState != newState)
     [tabController setLoadingState:newState];
 
+  [tabController setShouldUseDefaultFavicon:NO];

[Robert Sesek, 2016/07/12 23:43:56]

Does the TabController really need to know about whether or not it should use
the default favicon? Can the TabStripController just setIconImage: correctly
below, and in themeDidChangeNotification:?

[spqchan, 2016/07/13 17:16:14]

The default favicon also needs to be updated at internalSetSelected:, which
isn't handled by TabStripController.

One alternative is to pass in the WebContents to the TabController and then set
a flag to "showFavicon".
The TabController will then call imageForColor from favicon_util_mac when the
image needs to be updated, regardless if it's the default favicon or not. WDYT?

[Robert Sesek, 2016/07/13 22:37:59]

Why can't the TabStripController updates the icon when it observes the selection
changing?
No, the TabController shouldn't know about WebContents. Ideally it shouldn't
know about favicons, either.

+
   // While loading, this function is called repeatedly with the same state.
   // To avoid expensive unnecessary view manipulation, only make changes when
   // the state is actually changing.  When loading is complete (kTabDone),
   // every call to this function is significant.
   if (newState == kTabDone || oldState != newState ||
       oldHasIcon != newHasIcon) {
     if (newHasIcon) {
       if (newState == kTabDone) {
-        [tabController setIconImage:[self iconImageForContents:contents]];
+        [tabController setIconImage:[self iconImageForContents:contents
+                                                       atIndex:modelIndex]];
+        [tabController
+            setShouldUseDefaultFavicon:mac::ShouldUseDefaultFavicon(contents)];
       } else if (newState == kTabCrashed) {
         [tabController setIconImage:sadFaviconImage withToastAnimation:YES];
       } else {
         [tabController setIconImage:throbberImage];
       }
     } else {
       [tabController setIconImage:nil];
     }
   }
 
@@ skipping 722 matching lines @@
 
 - (void)themeDidChangeNotification:(NSNotification*)notification {
   [newTabButton_ setImages];
 }
 
 - (void)setVisualEffectsDisabledForFullscreen:(BOOL)fullscreen {
   [tabStripView_ setVisualEffectsDisabledForFullscreen:fullscreen];
 }
 
 @end