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

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

Index: chrome/browser/ui/cocoa/tabs/tab_strip_controller.mm
diff --git a/chrome/browser/ui/cocoa/tabs/tab_strip_controller.mm b/chrome/browser/ui/cocoa/tabs/tab_strip_controller.mm
index 44d1d9808a484e1df8a12a9caa4b2dfdab503528..6531d5c15c49988cfe77a7f9250584a339ff0be4 100644
--- a/chrome/browser/ui/cocoa/tabs/tab_strip_controller.mm
+++ b/chrome/browser/ui/cocoa/tabs/tab_strip_controller.mm
@@ -142,7 +142,8 @@ 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
@@ -1495,7 +1496,8 @@ private:
 
 // 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();
@@ -1508,12 +1510,8 @@ private:
     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
@@ -1584,6 +1582,8 @@ private:
   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),
@@ -1592,7 +1592,10 @@ private:
       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 {