New origin security indicator icons.

chrome/browser/ui/cocoa/omnibox/omnibox_view_mac.mm

Index: chrome/browser/ui/cocoa/omnibox/omnibox_view_mac.mm
diff --git a/chrome/browser/ui/cocoa/omnibox/omnibox_view_mac.mm b/chrome/browser/ui/cocoa/omnibox/omnibox_view_mac.mm
index 9783982c981bc6c1ea248ec0a057f2a0c7473af6..153b021113d1b3b6f0957740cd1c51b9922f60ee 100644
--- a/chrome/browser/ui/cocoa/omnibox/omnibox_view_mac.mm
+++ b/chrome/browser/ui/cocoa/omnibox/omnibox_view_mac.mm
@@ -175,6 +175,35 @@ NSColor* OmniboxViewMac::BaseTextColor(bool in_dark_mode) {
   return skia::SkColorToCalibratedNSColor(BaseTextColorSkia(in_dark_mode));
 }
 
+// static
+SkColor OmniboxViewMac::GetSecureTextColorSkia(
+    security_state::SecurityStateModel::SecurityLevel security_level,
+    bool in_dark_mode) {
+  return skia::NSDeviceColorToSkColor(
+      GetSecureTextColor(security_level, in_dark_mode));
+}
+
+// static
+NSColor* OmniboxViewMac::GetSecureTextColor(
+    security_state::SecurityStateModel::SecurityLevel security_level,
+    bool in_dark_mode) {
+  NSColor* color;
+  if (security_level == security_state::SecurityStateModel::EV_SECURE ||
+      security_level == security_state::SecurityStateModel::SECURE) {
+    color = SecureSchemeColor(in_dark_mode);
+  } else if (security_level ==
+             security_state::SecurityStateModel::SECURITY_ERROR) {
+    color = SecurityErrorSchemeColor(in_dark_mode);
+  } else if (security_level ==
+             security_state::SecurityStateModel::SECURITY_WARNING) {
+    color = SecurityWarningSchemeColor(in_dark_mode);
+  } else {
+    NOTREACHED();
+    color = BaseTextColor(in_dark_mode);

[Evan Stade, 2016/05/19 17:58:49]

I'm often told not to handle NOTREACHED

[Peter Kasting, 2016/05/19 18:36:19]

Yeah.  And in general looking at this makes it clear that we could just
early-return to do this all more simply:

{
  if (security_level == security_state::SecurityStateModel::EV_SECURE ||
      security_level == security_state::SecurityStateModel::SECURE)
    return SecureSchemeColor(in_dark_mode);

  if (security_level == security_state::SecurityStateModel::SECURITY_ERROR)
    return SecurityErrorSchemeColor(in_dark_mode);

  DCHECK_EQ(security_state::SecurityStateModel::SECURITY_WARNING,
            security_level);
  return SecurityWarningSchemeColor(in_dark_mode);
}

[palmer, 2016/05/19 18:40:40]

This is borrowed from the pre-existing code (compare with the deleted lines in
this file); and mirrors the Views code. Again, I could go either way, but I'd
prefer not to mess with pre-existing code unless there is a compelling reason.
I've never been advised not to handle NOTREACHED.

[Peter Kasting, 2016/05/19 18:43:21]

Please do fix, here and/or there, as possible.
Not handling NOTREACHED has been part of Chrome style for as long as I can
remember.  See
http://dev.chromium.org/developers/coding-style#TOC-CHECK-DCHECK-and-NOTREACHED-
for more.

The existing code was wrong.

[palmer, 2016/05/19 19:55:44]

Fixed here in Cocoa.

The Views code by estade/estark/jcivelli (GetSecureTextColor in
chrome/browser/ui/views/location_bar/location_bar_view.cc) was structured
differently(switch/case, with return at the bottom, sometimes) than the
equivalent Cocoa function; I rewrote it to follow your early-return + DCHECK_EQ
style. See if you like what I did there in the latest patchset.

+  }
+  return color;
+}
+
 OmniboxViewMac::OmniboxViewMac(OmniboxEditController* controller,
                                Profile* profile,
                                CommandUpdater* command_updater,
@@ -598,23 +627,12 @@ void OmniboxViewMac::ApplyTextAttributes(
   if (!model()->user_input_in_progress() && model()->CurrentTextIsURL() &&
       scheme.is_nonempty() &&
       (security_level != security_state::SecurityStateModel::NONE)) {
-    NSColor* color;
-    if (security_level == security_state::SecurityStateModel::EV_SECURE ||
-        security_level == security_state::SecurityStateModel::SECURE) {
-      color = SecureSchemeColor(in_dark_mode);
-    } else if (security_level ==
-               security_state::SecurityStateModel::SECURITY_ERROR) {
-      color = SecurityErrorSchemeColor(in_dark_mode);
+    NSColor* color = GetSecureTextColor(security_level, in_dark_mode);

[Evan Stade, 2016/05/19 17:58:49]

no need for this local var

[palmer, 2016/05/19 18:40:40]

Done.

+    if (security_level == security_state::SecurityStateModel::SECURITY_ERROR) {
       // Add a strikethrough through the scheme.
       [attributedString addAttribute:NSStrikethroughStyleAttributeName
                  value:[NSNumber numberWithInt:NSUnderlineStyleSingle]
                  range:ComponentToNSRange(scheme)];
-    } else if (security_level ==
-               security_state::SecurityStateModel::SECURITY_WARNING) {
-      color = SecurityWarningSchemeColor(in_dark_mode);
-    } else {
-      NOTREACHED();
-      color = BaseTextColor(in_dark_mode);
     }
     [attributedString addAttribute:NSForegroundColorAttributeName
                              value:color