[Mac] Fix rough-looking profile picker text, especially with a dark theme.

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

Index: chrome/browser/ui/cocoa/tabs/tab_strip_background_view.mm
diff --git a/chrome/browser/ui/cocoa/tabs/tab_strip_background_view.mm b/chrome/browser/ui/cocoa/tabs/tab_strip_background_view.mm
index 4b32010d44e65a67c702b90540b9b1f7ec77acf8..a6da861984b4a4f8cd66e95f6fceac3795619fda 100644
--- a/chrome/browser/ui/cocoa/tabs/tab_strip_background_view.mm
+++ b/chrome/browser/ui/cocoa/tabs/tab_strip_background_view.mm
@@ -2,23 +2,35 @@
 // 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_background_view.h"
+#import "chrome/browser/ui/cocoa/tabs/tab_strip_background_view_private.h"
 
+#include "chrome/browser/themes/theme_properties.h"
 #import "chrome/browser/ui/cocoa/framed_browser_window.h"
 #import "ui/base/cocoa/nsview_additions.h"
+#include "ui/base/theme_provider.h"
 
-@implementation TabStripBackgroundView
+@interface TabStripThemeBackgroundView : NSView
+@property(nonatomic) BOOL inATabDraggingOverlayWindow;
+@end
+
+@implementation TabStripThemeBackgroundView
+
+@synthesize inATabDraggingOverlayWindow = _inATabDraggingOverlayWindow;

[Robert Sesek, 2017/01/11 19:23:39]

Chrome uses trailingUnderscores_ for ObjC ivars.

[Sidney San Martín, 2017/01/13 00:10:07]

Done.

 
 - (void)drawRect:(NSRect)dirtyRect {
   // Only the top corners are rounded. For simplicity, round all 4 corners but
   // draw the bottom corners outside of the visible bounds.
   float cornerRadius = 4.0;
+  bool isFullScreen = (self.window.styleMask & NSFullScreenWindowMask) != 0;

[spqchan, 2017/01/11 19:33:56]

nit: Change to isFullscreen

We want to use "Fullscreen" instead of "Full Screen" to make things more
consistent

[Sidney San Martín, 2017/01/13 00:10:07]

Done.

+
   NSRect roundedRect = [self bounds];
-  roundedRect.origin.y -= cornerRadius;
-  roundedRect.size.height += cornerRadius;
-  [[NSBezierPath bezierPathWithRoundedRect:roundedRect
-                                   xRadius:cornerRadius
-                                   yRadius:cornerRadius] addClip];
+  if (!isFullScreen) {
+    roundedRect.origin.y -= cornerRadius;
+    roundedRect.size.height += cornerRadius;
+    [[NSBezierPath bezierPathWithRoundedRect:roundedRect
+                                     xRadius:cornerRadius
+                                     yRadius:cornerRadius] addClip];
+  }
   BOOL themed = [FramedBrowserWindow drawWindowThemeInDirtyRect:dirtyRect
                                                         forView:self
                                                          bounds:roundedRect
@@ -26,28 +38,148 @@
 
   // Draw a 1px border on the top edge and top corners.
   if (themed) {
-    CGFloat lineWidth = [self cr_lineWidth];
-    // Inset the vertical lines by 0.5px so that the top line gets a full pixel.
-    // Outset the horizontal lines by 0.5px so that they are not visible, but
-    // still get the rounded corners to get a border.
-    NSRect strokeRect = NSInsetRect(roundedRect, -lineWidth/2, lineWidth/2);
-    NSBezierPath* path = [NSBezierPath bezierPathWithRoundedRect:strokeRect
-                                                         xRadius:cornerRadius
-                                                         yRadius:cornerRadius];
-    [path setLineWidth:lineWidth];
-    [[NSColor colorWithCalibratedWhite:1.0 alpha:0.5] set];
-    [path stroke];
+    if (!isFullScreen) {
+      CGFloat lineWidth = [self cr_lineWidth];
+      // Inset the vertical lines by 0.5px so that the top line gets a full
+      // pixel. Outset the horizontal lines by 0.5px so that they are not
+      // visible, but still get the rounded corners to get a border.
+      NSRect strokeRect =
+          NSInsetRect(roundedRect, -lineWidth / 2, lineWidth / 2);
+      NSBezierPath* path =
+          [NSBezierPath bezierPathWithRoundedRect:strokeRect
+                                          xRadius:cornerRadius
+                                          yRadius:cornerRadius];
+      [path setLineWidth:lineWidth];
+      [[NSColor colorWithCalibratedWhite:1.0 alpha:0.5] set];
+      [path stroke];
+    }
+  } else if (!_inATabDraggingOverlayWindow) {
+    // If the window is not themed, and not being used to drag tabs between
+    // browser windows, decrease the tab strip background's translucency by
+    // overlaying it with a partially-transparent gray. The gray is somewhat
+    // opaque for Incognito mode, very opaque for non-Incognito mode, and
+    // completely opaque when the window is not active.
+    if (const ui::ThemeProvider* themeProvider =
+            [[self window] themeProvider]) {
+      NSColor* overlayColor = nil;
+      if (self.window.isMainWindow) {
+        NSAppearance* appearance = self.effectiveAppearance;
+        if ([appearance respondsToSelector:@selector(allowsVibrancy)] &&
+            appearance.allowsVibrancy) {
+          overlayColor = themeProvider->GetNSColor(
+              ThemeProperties::COLOR_FRAME_VIBRANCY_OVERLAY);
+        } else if (themeProvider->InIncognitoMode()) {
+          overlayColor = [NSColor colorWithSRGBRed:20 / 255.
+                                             green:22 / 255.
+                                              blue:24 / 255.
+                                             alpha:1];
+        } else {
+          overlayColor =
+              themeProvider->GetNSColor(ThemeProperties::COLOR_FRAME);
+        }
+      } else {
+        overlayColor =
+            themeProvider->GetNSColor(ThemeProperties::COLOR_FRAME_INACTIVE);
+      }
+
+      if (overlayColor) {
+        [overlayColor set];
+        NSRectFillUsingOperation(dirtyRect, NSCompositeSourceOver);
+      }
+    }
   }
 }
+@end
+
+@implementation TabStripBackgroundView {
+  TabStripThemeBackgroundView* _themeBackgroundView;

[Robert Sesek, 2017/01/11 19:23:39]

Here too use trailing underscores.

[Sidney San Martín, 2017/01/13 00:10:07]

Done.

+  NSVisualEffectView* _visualEffectView;
+}
+
+- (instancetype)initWithFrame:(NSRect)frame {
+  if ((self = [super initWithFrame:frame])) {
+    _themeBackgroundView = [[[TabStripThemeBackgroundView alloc]
+        initWithFrame:self.bounds] autorelease];

[Robert Sesek, 2017/01/11 19:23:39]

Who owns this? Needs a comment in the ivar.

[Sidney San Martín, 2017/01/13 00:10:07]

Done.

+    _themeBackgroundView.autoresizingMask =
+        NSViewWidthSizable | NSViewHeightSizable;
+    [self addSubview:_themeBackgroundView];
+  }
+  return self;
+}
+
+- (BOOL)inATabDraggingOverlayWindow {
+  return _themeBackgroundView.inATabDraggingOverlayWindow;
+}
+
+- (void)setInATabDraggingOverlayWindow:(BOOL)inATabDraggingOverlayWindow {
+  _themeBackgroundView.inATabDraggingOverlayWindow =
+      inATabDraggingOverlayWindow;
+}
+
+- (void)updateVisualEffectView {

[Robert Sesek, 2017/01/11 19:23:40]

Does this need to be gated by a runtime version check?

[Sidney San Martín, 2017/01/13 00:10:07]

It doesn't _need_ to be. NSVisualEffectView is a weak symbol so this method will
bail at line 130. I could optimize by, for example, doing a runtime check in
+alloc and returning a different class without any NSVisualEffectView logic, but
I'm not sure if it's worth it.

+  const ui::ThemeProvider* themeProvider = [[self window] themeProvider];
+  const bool isFullScreen =
+      (self.window.styleMask & NSFullScreenWindowMask) != 0;
+
+  // Visual effects cause higher power consumption in full screen.

[spqchan, 2017/01/11 19:33:56]

nit: "full screen" -> "fullscreen"

[Sidney San Martín, 2017/01/13 00:10:07]

Done.

+  if (!isFullScreen && themeProvider->UsingSystemTheme()) {
+    if (!_visualEffectView) {
+      _visualEffectView =
+          [[[NSVisualEffectView alloc] initWithFrame:self.bounds] autorelease];
+      if (!_visualEffectView)
+        return;
+
+      _visualEffectView.autoresizingMask =
+          NSViewWidthSizable | NSViewHeightSizable;
+      _visualEffectView.appearance =
+          [NSAppearance appearanceNamed:themeProvider->InIncognitoMode()
+                                            ? NSAppearanceNameVibrantDark
+                                            : NSAppearanceNameVibrantLight];
+      [self addSubview:_visualEffectView];
+      [_visualEffectView addSubview:_themeBackgroundView];
+    }
+  } else {
+    if (_visualEffectView) {
+      [self addSubview:_themeBackgroundView];
+      [_visualEffectView removeFromSuperview];
+      _visualEffectView = nil;
+    }
+  }
+}
+
+- (void)viewWillMoveToWindow:(NSWindow*)newWindow {
+  // AppKit calls this method when the view's position in the view hierarchy
+  // changes, even if its parent window won't change.
+  if (newWindow == self.window)
+    return;
+  if (self.window)
+    [self.window removeObserver:self forKeyPath:@"styleMask"];

[Robert Sesek, 2017/01/11 19:23:39]

Do you need to remove the observer in dealloc, or does AppKit call this on
window teardown too?

[Sidney San Martín, 2017/01/13 00:10:07]

AppKit takes care of it:

+  if (newWindow)
+    [newWindow addObserver:self forKeyPath:@"styleMask" options:0 context:nil];
+}
+
+- (void)observeValueForKeyPath:(NSString*)keyPath
+                      ofObject:(id)object
+                        change:(NSDictionary*)change
+                       context:(void*)context {
+  DCHECK(object == self.window);

[Robert Sesek, 2017/01/11 19:23:40]

DCHECK_EQ

[Sidney San Martín, 2017/01/13 00:10:07]

Done.

+  DCHECK([keyPath isEqualToString:@"styleMask"]);
+  [self updateVisualEffectView];
+}
 
 // ThemedWindowDrawing implementation.
 
 - (void)windowDidChangeTheme {
-  [self setNeedsDisplay:YES];
+  [self updateVisualEffectView];
+  [_themeBackgroundView setNeedsDisplay:YES];
 }
 
 - (void)windowDidChangeActive {
-  [self setNeedsDisplay:YES];
+  // TODO(sdy): It shouldn't be necessary to update the visual effect view
+  // here, since it only changes when the theme changes), but the window isn't
+  // associated with a themeProvider at init time.
+  [self updateVisualEffectView];
+  [_themeBackgroundView setNeedsDisplay:YES];
 }
 
 @end