New origin security indicator icons.

chrome/browser/ui/views/location_bar/location_bar_view.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/ui/views/location_bar/location_bar_view.h"
 
 #include <algorithm>
 #include <map>
 
 #include "base/i18n/rtl.h"
@@ skipping 321 matching lines @@
       return ui::MaterialDesignController::IsModeMaterial()
                  ? gfx::kGoogleGreen700
                  : SkColorSetRGB(7, 149, 0);
   }
   NOTREACHED();
   return gfx::kPlaceholderColor;
 }
 
 SkColor LocationBarView::GetSecureTextColor(
     security_state::SecurityStateModel::SecurityLevel security_level) const {
+  bool md = ui::MaterialDesignController::IsModeMaterial();
   bool inverted = color_utils::IsDark(GetColor(BACKGROUND));
-  SkColor color;
-  switch (security_level) {
-    case security_state::SecurityStateModel::EV_SECURE:
-    case security_state::SecurityStateModel::SECURE:
-      if (ui::MaterialDesignController::IsModeMaterial() && inverted)
-        return GetColor(TEXT);
-      color = GetColor(EV_BUBBLE_TEXT_AND_BORDER);
-      break;
 
-    case security_state::SecurityStateModel::SECURITY_POLICY_WARNING:
-      return GetColor(DEEMPHASIZED_TEXT);
-      break;
+  if (security_level == security_state::SecurityStateModel::EV_SECURE ||
+      security_level == security_state::SecurityStateModel::SECURE) {
+    if (md && inverted)
+      return GetColor(TEXT);
+    return color_utils::GetReadableColor(GetColor(EV_BUBBLE_TEXT_AND_BORDER),
+                                         GetColor(BACKGROUND));
+  }
 
-    case security_state::SecurityStateModel::SECURITY_ERROR: {
-      bool md = ui::MaterialDesignController::IsModeMaterial();
-      if (md && inverted)
-        return GetColor(TEXT);
-      color = md ? gfx::kGoogleRed700 : SkColorSetRGB(162, 0, 0);
-      break;
-    }
+  if (security_level ==
+      security_state::SecurityStateModel::SECURITY_POLICY_WARNING) {
+    return GetColor(DEEMPHASIZED_TEXT);
+  }
 
-    case security_state::SecurityStateModel::SECURITY_WARNING:
+  if (security_level == security_state::SecurityStateModel::SECURITY_ERROR) {
+    if (md && inverted)
       return GetColor(TEXT);
-      break;
+    return color_utils::GetReadableColor(
+        md ? gfx::kGoogleRed700 : SkColorSetRGB(162, 0, 0),
+        GetColor(BACKGROUND));
+  }
 
-    default:
-      NOTREACHED();
-      return gfx::kPlaceholderColor;
-  }
-  return color_utils::GetReadableColor(color, GetColor(BACKGROUND));
+  DCHECK_EQ(security_state::SecurityStateModel::SECURITY_WARNING,
+            security_level);
+  return color_utils::GetReadableColor(GetColor(TEXT), GetColor(BACKGROUND));
 }

[Peter Kasting, 2016/05/19 20:36:25]

Here's an even shorter way to write this that eliminates the repeated
GetReadableColor() calls that moving away from using a switch introduced:

  if (security_level ==
      security_state::SecurityStateModel::SECURITY_POLICY_WARNING)
    return GetColor(DEEMPHASIZED_TEXT);

  const bool md = ui::MaterialDesignController::IsModeMaterial();
  SkColor text_color = GetColor(TEXT);
  if (md && color_utils::IsDark(GetColor(BACKGROUND)) &&
      ((security_level == security_state::SecurityStateModel::EV_SECURE) ||
       (security_level == security_state::SecurityStateModel::SECURE) ||
       (security_level == security_state::SecurityStateModel::SECURITY_ERROR)))
    return text_color;

  if ((security_level == security_state::SecurityStateModel::EV_SECURE) ||
      (security_level == security_state::SecurityStateModel::SECURE))
    text_color = GetColor(EV_BUBBLE_TEXT_AND_BORDER);
  else if (security_level == security_state::SecurityStateModel::SECURITY_ERROR)
    text_color = md ? gfx::kGoogleRed700 : SkColorSetRGB(162, 0, 0);

  return color_utils::GetReadableColor(text_color, GetColor(BACKGROUND));

[Peter Kasting, 2016/05/19 20:42:53]

Even shorter:

  if (security_level ==
      security_state::SecurityStateModel::SECURITY_POLICY_WARNING)
    return GetColor(DEEMPHASIZED_TEXT);

  SkColor text_color = GetColor(TEXT);
  if ((security_level == security_state::SecurityStateModel::EV_SECURE) ||
      (security_level == security_state::SecurityStateModel::SECURE) ||
      (security_level == security_state::SecurityStateModel::SECURITY_ERROR)) {
    const bool md = ui::MaterialDesignController::IsModeMaterial();
    if (md && color_utils::IsDark(GetColor(BACKGROUND)))
      return text_color;
    if (security_level == security_state::SecurityStateModel::SECURITY_ERROR)
      text_color = md ? gfx::kGoogleRed700 : SkColorSetRGB(162, 0, 0);
    else
      text_color = GetColor(EV_BUBBLE_TEXT_AND_BORDER);
  }
  return color_utils::GetReadableColor(text_color, GetColor(BACKGROUND));

[palmer, 2016/05/19 20:59:06]

That is good; but: it no longer has the DCHECK_EQ, if that matters. It's also
less clearly-parallel to the Cocoa version, if that matters. You all are the
OWNERS of this code, so I'm happy to go with whatever your preference is. Just
thought I should raise those points first.

I do like short code...

[Peter Kasting, 2016/05/19 21:02:28]

I considered that when I wrote it and decided that it actually doesn't need the
DCHECK, because even if a caller were to pass NONE, it does something very
reasonable as written.
I think that's in large part because the two have noticeable functional
differences.  I don't know if that's correct or not, but given that they don't
return the same values, it seems reasonable that they're not structured the same
either.

 
 void LocationBarView::ZoomChangedForActiveTab(bool can_show_bubble) {
   DCHECK(zoom_view_);
   if (RefreshZoomView()) {
     Layout();
     SchedulePaint();
   }
 
   WebContents* web_contents = GetWebContents();
   if (can_show_bubble && zoom_view_->visible() && web_contents)
@@ skipping 478 matching lines @@
     return;
 
   if (ui::MaterialDesignController::IsModeMaterial()) {
     gfx::VectorIconId icon_id = gfx::VectorIconId::VECTOR_ICON_NONE;
     const int kIconSize = 16;
     SkColor icon_color = gfx::kPlaceholderColor;
     if (ShouldShowEVBubble()) {
       icon_id = gfx::VectorIconId::LOCATION_BAR_HTTPS_VALID_IN_CHIP;
       icon_color = location_icon_view_->GetTextColor();
     } else {
-      icon_id = omnibox_view_->GetVectorIcon(
-          color_utils::IsDark(GetColor(BACKGROUND)));
-      icon_color = color_utils::DeriveDefaultIconColor(GetColor(TEXT));
+      icon_id = omnibox_view_->GetVectorIcon();
+      security_state::SecurityStateModel::SecurityLevel security_level =
+          GetToolbarModel()->GetSecurityLevel(false);
+      icon_color = (security_level == security_state::SecurityStateModel::NONE)
+                       ? color_utils::DeriveDefaultIconColor(GetColor(TEXT))
+                       : GetSecureTextColor(security_level);
     }
     location_icon_view_->SetImage(
         gfx::CreateVectorIcon(icon_id, kIconSize, icon_color));
   } else {
     location_icon_view_->SetImage(
         *GetThemeProvider()->GetImageSkiaNamed(omnibox_view_->GetIcon()));
   }
 }
 
 bool LocationBarView::RefreshContentSettingViews() {
@@ skipping 499 matching lines @@
 // LocationBarView, private TemplateURLServiceObserver implementation:
 
 void LocationBarView::OnTemplateURLServiceChanged() {
   template_url_service_->RemoveObserver(this);
   template_url_service_ = nullptr;
   // If the browser is no longer active, let's not show the info bubble, as this
   // would make the browser the active window again.
   if (omnibox_view_ && omnibox_view_->GetWidget()->IsActive())
     ShowFirstRunBubble();
 }