[Mac][Material Design] Rework how location bar shadow is drawn.

chrome/browser/ui/cocoa/location_bar/autocomplete_text_field.mm

Index: chrome/browser/ui/cocoa/location_bar/autocomplete_text_field.mm
diff --git a/chrome/browser/ui/cocoa/location_bar/autocomplete_text_field.mm b/chrome/browser/ui/cocoa/location_bar/autocomplete_text_field.mm
index 36b9a024ad561c5de3a1eac6b1a9db6e8777a9e6..c1e265c7f149ad311ffcf2d424140bec07263be0 100644
--- a/chrome/browser/ui/cocoa/location_bar/autocomplete_text_field.mm
+++ b/chrome/browser/ui/cocoa/location_bar/autocomplete_text_field.mm
@@ -22,8 +22,32 @@
 
 namespace {
 const CGFloat kAnimationDuration = 0.2;
+const CGFloat kShadowInset = 3;
 }
 
+// A view that draws a 1px shadow line beneath the autocomplete textfield.
+@interface AutocompleteTextFieldShadowView : NSView {

[tapted, 2016/04/22 01:11:46]

nit: curlies not needed

[shrike, 2016/04/26 19:19:30]

Done.

+}
+@end
+
+@implementation AutocompleteTextFieldShadowView
+
+- (void)drawRect:(NSRect)rect {
+  // Don't draw anything on a Retina display because on Retina there's room
+  // for the shadow just beneath the autocomplete textfield path stroke. Why
+  // even add this view? If the user drags the Incognito window between Retina
+  // and non-Retina screens there would have to be logic to add and remove the
+  // view. It's easier just to always add it for Incognito mode and draw
+  // nothing into it.
+  if ([self cr_lineWidth] < 1) {

[tapted, 2016/04/22 01:11:46]

optional nit: curlies not needed (but I'd keep a blank line after the `return`

[shrike, 2016/04/26 19:19:30]

Acknowledged.

+    return;
+  }
+  [[NSColor colorWithCalibratedWhite:69 / 255. alpha:1] set];

[tapted, 2016/04/22 01:11:46]

hmmmm this should be `colorWithGenericGamma22White` (sRGB is gamma 2.2, I think
"calibrated" will use a gamma of 1.8 which is what OSX used before 10.6 -- see
https://support.apple.com/en-au/ht3712 or http://crbug.com/254361 ).

But there's more `colorWithCalibratedWhite` over in -[AutocompleteTextFieldCell
drawWithFrame:], so they should be consistent. I guess it depends how the levels
were determined - e.g. are they "fresh" from a recent spec? Or were they revised
after a designer looked at them in Canary and said "this is too light".

[shrike, 2016/04/22 02:09:00]

I'm confused, because the link you sent says 2.2 is the default gamma that's
used by the system as of 10.6 so to me that implies it's not necessary to jump
through hoops to get a color with that gamma. There are zero occurrences of
colorWithGenericGamma22White in the Chromium.

[tapted, 2016/04/22 04:41:34]

I don't think it's clear what gamma you get with `colorWithCalibratedWhite`, so
we shouldn't use it. If a designer gives us a color value, they probably assume
sRGB which is gamma of 2.2, so we should ensure we get 2.2. I'm pretty sure you
get 1.8 otherwise, which won't match up with the same color value shown in the
content area, or on any other platform.

There's also a tidbit in the 10.4 AppKit release notes -
https://developer.apple.com/library/mac/releasenotes/AppKit/RN-AppKitOlderNot...
- on NSColor it says,
"""
Calibrated NSColors (those created with colorWithCalibratedRed:..,
colorWithCalibratedHue:.., or colorWithCalibratedWhite:...) now use Quartz
generic color spaces, rather than the "display" (aka "device") color spaces.
"""

So in 10.4 it switched from "device" AKA
"whatever-the-heck-the-user's-screen-is-like" to "generic" which, at the time,
was gamma 1.8. Then in 10.6 having the OS using 2.2 means that color values
specified with colorWithCalibratedWhite will get converted from a gamma of 1.8
to 2.2, which usually makes them lighter.


But, after all that's said, it's probably more important that these color values
are consistent with other parts of the cocoa UI than with stuff in the
webcontent area. And since we never audited the color values used for the Cocoa
UI to see whether they each one should actually have been specified to have an
sRGB color space, we can probably ignore this particular case.
colorWithGenericGamma22White wasn't in the 10.6 SDK, so that's likely why. But
we should be using it - that's why I filed http://crbug.com/254361 :)

+  NSRectFill(rect);
+}
+
+@end
+
 @implementation AutocompleteTextField
 
 @synthesize observer = observer_;
@@ -217,6 +241,18 @@ const CGFloat kAnimationDuration = 0.2;
   [self updateMouseTracking];
 }
 
+// Overridden to adjust the shadow view whenever the textfield's origin changes.

[tapted, 2016/04/22 01:11:46]

I think the typical solution for this under c/b/ui/cocoa is to subscribe to
NSViewFrameDidChangeNotification inside the AutocompleteTextFieldShadowView
itself

Then adjustShadowViewFrame can be declared on AutocompleteTextFieldShadowView as
well

[shrike, 2016/04/22 02:09:00]

OK. I can't remember off the top of my head if it's easy to get to the
autocompletion textfield from an arbitrary place in the view hierarchy. If it's
not, it means the shadow view having an ivar for the textfield. You are OK with
that?

[tapted, 2016/04/22 04:41:34]

Would it work to pass in `self` into new initializer for
AutocompleteTextFieldShadowView, when it's initialized inside
-[AutocompleteTextField updateColorsToMatchTheme]?

+- (void)setFrameOrigin:(NSPoint)frameOrigin {
+  [super setFrameOrigin:frameOrigin];
+  [self adjustShadowViewFrame];
+}
+
+// Overridden to adjust the shadow view whenever the textfield's size changes.
+- (void)setFrameSize:(NSSize)frameSize {
+  [super setFrameSize:frameSize];
+  [self adjustShadowViewFrame];
+}
+
 - (void)setAttributedStringValue:(NSAttributedString*)aString {
   AutocompleteTextFieldEditor* editor =
       static_cast<AutocompleteTextFieldEditor*>([self currentEditor]);
@@ -377,11 +413,31 @@ const CGFloat kAnimationDuration = 0.2;
   [self setNeedsDisplay];
 }
 
+- (void)adjustShadowViewFrame {
+  // Make the shadow view 1pt tall and slightly inset from the edges of the
+  // autocomplete textfield.
+  NSRect shadowViewFrame = [self frame];
+  shadowViewFrame.origin.x += kShadowInset;
+  shadowViewFrame.size.width -= kShadowInset * 2;
+  shadowViewFrame.origin.y -= 1;
+  shadowViewFrame.size.height = 1;
+  [shadowView_ setFrame:shadowViewFrame];
+}
+
 - (void)updateColorsToMatchTheme {
   if (![[self window] inIncognitoMode]) {
+    [shadowView_ removeFromSuperview];
     return;
   }
 
+  // Add 1px shadow below the autocomplete textfield.
+  if (!shadowView_.get()) {
+    shadowView_.reset(
+        [[AutocompleteTextFieldShadowView alloc] initWithFrame:NSZeroRect]);
+  }
+  [self adjustShadowViewFrame];
+  [[self superview] addSubview:shadowView_];
+
   // Invert the textfield's colors when Material Design and Incognito and not
   // a custom theme.
   bool inDarkMode = [[self window] inIncognitoModeWithSystemTheme];