[Views][Mac] Security Chip Default Value

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

Index: chrome/browser/ui/views/location_bar/location_bar_view.cc
diff --git a/chrome/browser/ui/views/location_bar/location_bar_view.cc b/chrome/browser/ui/views/location_bar/location_bar_view.cc
index dc70d7e0941044f68f99b9128b13e542d240f70c..899c6e0467090c546b9bfed3d521430f3dce1561 100644
--- a/chrome/browser/ui/views/location_bar/location_bar_view.cc
+++ b/chrome/browser/ui/views/location_bar/location_bar_view.cc
@@ -140,7 +140,8 @@ LocationBarView::LocationBarView(Browser* browser,
       show_focus_rect_(false),
       template_url_service_(NULL),
       web_contents_null_at_last_refresh_(true),
-      should_show_secure_state_(true),
+      should_show_secure_state_(false),
+      should_show_nonsecure_state_(false),
       should_animate_secure_state_(false),
       should_animate_nonsecure_state_(false) {
   edit_bookmarks_enabled_.Init(
@@ -157,6 +158,9 @@ LocationBarView::LocationBarView(Browser* browser,
         command_line->GetSwitchValueASCII(switches::kSecurityChip);
     should_show_secure_state_ =
         security_chip_flag == switches::kSecurityChipShowAll;
+    should_show_nonsecure_state_ =
+        security_chip_flag == switches::kSecurityChipShowAll ||
+        security_chip_flag == switches::kSecurityChipShowNonSecureOnly;
   }
 
   if (command_line->HasSwitch(switches::kSecurityChipAnimation)) {
@@ -1009,17 +1013,19 @@ bool LocationBarView::ShouldShowKeywordBubble() const {
 
 bool LocationBarView::ShouldShowEVBubble() const {
   return (GetToolbarModel()->GetSecurityLevel(false) ==
-          security_state::SecurityStateModel::EV_SECURE) &&
-         should_show_secure_state_;
+          security_state::SecurityStateModel::EV_SECURE);

[Peter Kasting, 2016/10/19 18:02:36]

Nit: This function is only called in one place, and given its overlap with
ShouldShowSecurityChip() (it's conceptually confusing to me that both would
return true for EV_SECURE, which implies we're showing two different things), I
suggest just inlining it into its lone caller.

[spqchan, 2016/10/19 19:03:50]

Done.

 }
 
 bool LocationBarView::ShouldShowSecurityChip() const {
   using SecurityLevel = security_state::SecurityStateModel::SecurityLevel;
   const SecurityLevel level = GetToolbarModel()->GetSecurityLevel(false);
-  if (level == SecurityLevel::SECURE || level == SecurityLevel::EV_SECURE)
+  if (level == SecurityLevel::SECURE)
     return should_show_secure_state_;
-  return level == SecurityLevel::DANGEROUS ||
-         level == SecurityLevel::HTTP_SHOW_WARNING;
+  else if (level == SecurityLevel::EV_SECURE)

[Peter Kasting, 2016/10/19 18:02:36]

Nit: No else after return

Trivial, but I'd swap this conditional with the one above so that logically
these move from more to less secure.  (Of course, this makes me want to flip the
comparison order in the lines below, as well as in the two comparisons in the
next function.  Argh, OCD.)

[spqchan, 2016/10/19 19:03:50]

Done.

+    return true;
+  return should_show_nonsecure_state_ &&
+         (level == SecurityLevel::DANGEROUS ||
+          level == SecurityLevel::HTTP_SHOW_WARNING);
 }
 
 bool LocationBarView::ShouldAnimateSecurityChip() const {