[Mac][Material Design] Adjust EV chip to match Material Design spec.

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

Index: chrome/browser/ui/cocoa/location_bar/ev_bubble_decoration.mm
diff --git a/chrome/browser/ui/cocoa/location_bar/ev_bubble_decoration.mm b/chrome/browser/ui/cocoa/location_bar/ev_bubble_decoration.mm
index 0547d31263d4c6743a557055be5c0b5c770ca383..ea894c30e90a253a2e159c75a8b9367708b6f860 100644
--- a/chrome/browser/ui/cocoa/location_bar/ev_bubble_decoration.mm
+++ b/chrome/browser/ui/cocoa/location_bar/ev_bubble_decoration.mm
@@ -7,8 +7,10 @@
 #import "base/logging.h"
 #include "base/strings/sys_string_conversions.h"
 #import "chrome/browser/ui/cocoa/location_bar/location_icon_decoration.h"
+#import "chrome/browser/ui/cocoa/themed_window.h"
 #include "grit/theme_resources.h"
 #include "skia/ext/skia_utils_mac.h"
+#import "ui/base/cocoa/nsview_additions.h"
 #include "ui/base/material_design/material_design_controller.h"
 #include "ui/gfx/color_palette.h"
 #include "ui/gfx/font_list.h"
@@ -16,6 +18,9 @@
 
 namespace {
 
+// This is used to increase the right margin of this decoration.
+const CGFloat kRightSideMargin = 1.0;
+
 // TODO(shess): In general, decorations that don't fit in the
 // available space are omitted.  This one never goes to omitted, it
 // sticks at 150px, which AFAICT follows the Windows code.  Since the
@@ -54,6 +59,10 @@ EVBubbleDecoration::EVBubbleDecoration(LocationIconDecoration* location_icon)
     : location_icon_(location_icon) {
   if (ui::MaterialDesignController::IsModeMaterial()) {
     SetTextColor(GetBackgroundBorderColor());
+    // The EV chip text in Material Design uses a slightly smaller font than the
+    // default, and has to be moved down slightly.
+    SetFont([NSFont systemFontOfSize:11]);

[tapted, 2016/04/22 00:41:52]

`const int kMaterialFontSize = 11;` in the anon namespace? Or I think this is
also [NSFont smallSystemFontSize].

There's also ResourceBundle::GetFontWithDelta(-2).GetNativeFont(), but that's
probably not worth it since we don't need a gfx::Font at any point

[shrike, 2016/04/26 18:03:40]

I think kMaterialFontSize is best - the spec calls for 11pt, and I don't think
there's a contract that says smallSystemFontSize does and will always return 11

[tapted, 2016/04/27 02:37:39]

hehe - specs tend not to consider that users can "globally" rescale their fonts
- at least on Windows. On Windows, Chrome obeys that by doing everything
relative to "The font size used for a system ::MessageBox()", and the UI
adjusts. So font pt-sizes in specs are usually mapped to deltas. ResourceBundle
encapsulates this craziness. Let's just hope Apple don't decide to bring this
font-scaling concept to OSX any time soon...  (the help centre article
"Yosemite: Make it easier to see what's on the screen."
https://support.apple.com/kb/PH18831 is surprisingly quaint for an OS that goes
to such lengths to look nice).

+    SetBaselineOffset(1);
   } else {
     // Color tuples stolen from location_bar_view_gtk.cc.
     SetTextColor(ColorWithRGBBytes(0x07, 0x95, 0x00));
@@ -71,6 +80,46 @@ void EVBubbleDecoration::SetFullLabel(NSString* label) {
   SetLabel(full_label_);
 }
 
+void EVBubbleDecoration::DrawWithBackgroundInFrame(NSRect background_frame,
+                                                   NSRect frame,
+                                                   NSView* control_view) {
+  if (!ui::MaterialDesignController::IsModeMaterial()) {
+    BubbleDecoration::DrawWithBackgroundInFrame(background_frame,
+                                                frame,
+                                                control_view);
+    return;
+  }
+
+  NSRect rect = NSInsetRect(background_frame, 0, 3);
+  rect.size.width -= kRightSideMargin;
+
+  CGFloat lineWidth = [control_view cr_lineWidth];

[tapted, 2016/04/22 00:41:52]

nit: since this isn't between @implementation/@end, we use hacker_style:
line_width, in_dark_mode

[shrike, 2016/04/26 18:03:40]

Done.

+  bool inDarkMode = [[control_view window] inIncognitoModeWithSystemTheme];
+  // Only adjust the path rect by 1/2 the line width if it's going to be
+  // stroked (so that the stroke lines fall along pixel lines).
+  if (!inDarkMode) {
+    rect = NSInsetRect(rect, lineWidth / 2., lineWidth / 2.);
+  }
+  NSBezierPath* path = [NSBezierPath bezierPathWithRoundedRect:rect
+                                                       xRadius:2

[tapted, 2016/04/22 00:41:52]

`const CFGloat kMaterialBorderRadius = 2;` in anon namespace

[shrike, 2016/04/26 18:03:40]

Done.

+                                                       yRadius:2];
+  [path setLineWidth:lineWidth];
+  if (inDarkMode) {
+    [[NSColor whiteColor] set];
+    [path fill];
+  } else {
+    [[NSColor colorWithCalibratedRed:46 / 255.

[tapted, 2016/04/22 00:41:52]

This will use the wrong colorspace (assuming the value #2E8732 was given in
srgb, which is what css is). The NSColor documentation now even says "Where
possible, it is preferable to specify the colorspace explicitly using the
colorWithSRGBRed:green:blue:alpha: or colorWithGenericGamma22White:alpha:
method.", and now we've ditched 10.6, we can actually use colorWithSRGBRed: \o/.
But actually the typical way we do this is:

const SkColor kMaterialBackgroundColor = SkColorSetARGB(0x0d, 0x2e, 0x87, 0x32);
..
[skia::SkColorToSRGBNSColor(kMaterialBackgroundColor) set];


(using stuff in skia/ext/skia_utils_mac.h)

[shrike, 2016/04/22 05:46:30]

The comment you quote is only in reference to colorWithRed:green:blue:alpha:,
not colorWithCalibratedRed:.... 

At this point I think it's better to leave it as is, and switch to
SkColorToSRGBNSColor() in this and all other places in a different cl. I didn't
run across this function before (all the other places I've seen use
calibratedColor).

[tapted, 2016/04/22 06:09:48]

If it's left as is the color will be wrong, see e.g. http://crbug.com/257275 and
http://crbug.com/254361. This is because the colorspace for "calibrated" is also
badly specified. (e.g. before 10.4 it was "device", now it is "the `generic`
colorspace that OSX stopped using for system UI in 10.6"). sRGB on the other
hand, is well-defined and widely used.

The problem with switching to SkColorToSRGBNSColor _afterwards_ is that the
color will actually change, and we don't have access to designer specs that say
what the color "should" be. But in this case, we have a HTML color value from a
designer, so we should assume they want sRGB for it.

(and, certainly, all toolkit-views UI is rendered into canvases assuming sRGB)

[shrike, 2016/04/22 06:24:15]

Yes, you are correct the color will be wrong. But what I am saying is that
colors are already wrong in many other places because I used the calibratedColor
method. At least using calibratedColor here makes the colors consistently wrong
instead of one right plus the rest wrong.

I'm not sure what the issue is with switching over to SkColorToSRGBNSColor at a
later date. Even if I use SkColorToSRGBNSColor here I should make that change
everywhere else anyway (which will be switching over at a later date). I do have
specs today - they don't specify a color space but they are saved in sRGB. I can
also double-check with the designer.

[tapted, 2016/04/25 21:48:10]

Ah, yep - definitely go for consistency with the existing UI :).

With regard to switching later - switching for a subset of the UI might be OK.
But my concern with updating calls mechanically has always been that we don't
know whether a designer had looked at the colors as they appear in the browser
and thought 'this should be darker' and so updated the spec to be darker. I
guess it should be subtle enough to not really worry though. It's also possible
some icon assets are colored to match a particular background. But that, too,
should be subtle and less of a concern as we move to vector assets (and since
they use Skia, they're likely to be *actually* in sRGB already when drawn).

[shrike, 2016/04/26 18:03:40]

In Cocoa I call NSImageFromImageSkia() to convert the skia vector-derived image
to an NSImage, and that returns an NSImage with NSBitmapImageReps in the generic
RGB color space. There's a variant that takes a color space so that would be a
way to ensure it's sRGB.

I think it's fine to use an sRGB color here so I will do that. I plan to make a
color correction pass at a later date to fix all the others (and the vector
image cases).

+                               green:135 / 255.
+                                blue:50 / 255.
+                               alpha:0.05] set];
+    [path fill];
+    [GetBackgroundBorderColor() set];
+    [path stroke];
+  }
+
+  DrawInFrame(frame, control_view);
+}
+
 NSPoint EVBubbleDecoration::GetBubblePointInFrame(NSRect frame) {
   NSRect image_rect = GetImageRectInFrame(frame);
   return NSMakePoint(NSMidX(image_rect),