New location security strings and animation.

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 308 matching lines @@
       return native_theme->GetSystemColor(
           ui::NativeTheme::kColorId_TextfieldDefaultColor);
 
     case SELECTED_TEXT:
       return native_theme->GetSystemColor(
           ui::NativeTheme::kColorId_TextfieldSelectionColor);
 
     case DEEMPHASIZED_TEXT:
       return color_utils::AlphaBlend(GetColor(TEXT), GetColor(BACKGROUND), 128);
 
-    case EV_BUBBLE_TEXT_AND_BORDER:
+    case SECURITY_CHIP_TEXT:
       return ui::MaterialDesignController::IsModeMaterial()
                  ? GetSecureTextColor(
-                       security_state::SecurityStateModel::EV_SECURE)
+                       GetToolbarModel()->GetSecurityLevel(false))
                  : SkColorSetRGB(7, 149, 0);
   }
   NOTREACHED();
   return gfx::kPlaceholderColor;
 }
 
 SkColor LocationBarView::GetSecureTextColor(
     security_state::SecurityStateModel::SecurityLevel security_level) const {
   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)) {
     if (ui::MaterialDesignController::IsModeMaterial()) {
       if (color_utils::IsDark(GetColor(BACKGROUND)))
         return text_color;
       if (security_level == security_state::SecurityStateModel::SECURITY_ERROR)
         text_color = gfx::kGoogleRed700;
       else
         text_color = gfx::kGoogleGreen700;
     } else if (security_level ==
                security_state::SecurityStateModel::SECURITY_ERROR) {
       text_color = SkColorSetRGB(162, 0, 0);
     } else {
-      text_color = GetColor(EV_BUBBLE_TEXT_AND_BORDER);
+      text_color = GetColor(SECURITY_CHIP_TEXT);
     }
   }
   return color_utils::GetReadableColor(text_color, GetColor(BACKGROUND));
 }
 
 void LocationBarView::ZoomChangedForActiveTab(bool can_show_bubble) {
   DCHECK(zoom_view_);
   if (RefreshZoomView()) {
     Layout();
     SchedulePaint();
@@ skipping 153 matching lines @@
 
   min_size.set_height(min_size.height() * size_animation_.GetCurrentValue());
 
   const int padding = GetLayoutConstant(LOCATION_BAR_HORIZONTAL_PADDING);
 
   // Compute width of omnibox-leading content.
   const int edge_thickness = GetHorizontalEdgeThickness();
   int leading_width = edge_thickness;
   if (ShouldShowKeywordBubble()) {
     // The selected keyword view can collapse completely.
-  } else if (ShouldShowEVBubble()) {
-    leading_width += GetLayoutConstant(LOCATION_BAR_BUBBLE_HORIZONTAL_PADDING) +
-                     location_icon_view_->GetMinimumSizeForLabelText(
-                                            GetToolbarModel()->GetEVCertName())
-                         .width();
+  } else if (ShouldShowSecurityChip()) {
+    base::string16 security_text = GetSecurityText();
+    leading_width +=
+        GetLayoutConstant(LOCATION_BAR_BUBBLE_HORIZONTAL_PADDING) +
+        location_icon_view_->GetMinimumSizeForLabelText(security_text).width();
   } else {
     leading_width += padding + location_icon_view_->GetMinimumSize().width();
   }
 
   // Compute width of omnibox-trailing content.
   int trailing_width = edge_thickness;
   trailing_width += IncrementalMinimumWidth(star_view_) +
                     IncrementalMinimumWidth(translate_icon_view_) +
                     IncrementalMinimumWidth(open_pdf_in_reader_view_) +
                     IncrementalMinimumWidth(save_credit_card_icon_view_) +
@@ skipping 55 matching lines @@
           (template_url->GetType() == TemplateURL::OMNIBOX_API_EXTENSION)) {
         gfx::Image image = extensions::OmniboxAPI::Get(profile())->
             GetOmniboxIcon(template_url->GetExtensionId());
         selected_keyword_view_->SetImage(image.AsImageSkia());
         selected_keyword_view_->set_is_extension_icon(true);
       } else {
         selected_keyword_view_->ResetImage();
         selected_keyword_view_->set_is_extension_icon(false);
       }
     }
-  } else if (ShouldShowEVBubble()) {
-    location_icon_view_->SetLabel(GetToolbarModel()->GetEVCertName());
+  } else if (ShouldShowSecurityChip()) {
+    location_icon_view_->SetLabel(GetSecurityText());
     location_icon_view_->SetBackground(true);
     // The largest fraction of the omnibox that can be taken by the EV bubble.
     const double kMaxBubbleFraction = 0.5;
     leading_decorations.AddDecoration(
         vertical_padding, location_height, false, kMaxBubbleFraction,
         bubble_horizontal_padding, item_padding, location_icon_view_);
   } else {
     leading_decorations.AddDecoration(vertical_padding, location_height,
                                       location_icon_view_);
   }
@@ skipping 156 matching lines @@
   }
 }
 
 void LocationBarView::Update(const WebContents* contents) {
   RefreshContentSettingViews();
   RefreshZoomView();
   RefreshPageActionViews();
   RefreshTranslateIcon();
   RefreshSaveCreditCardIconView();
   RefreshManagePasswordsIconView();
-  content::WebContents* web_contents_for_sub_views =
+  WebContents* web_contents_for_sub_views =
       GetToolbarModel()->input_in_progress() ? nullptr : GetWebContents();
   open_pdf_in_reader_view_->Update(web_contents_for_sub_views);
 
   if (star_view_)
     UpdateBookmarkStarVisibility();
 
   if (contents)
     omnibox_view_->OnTabChanged(contents);
   else
     omnibox_view_->Update();
 
+  if (!ShouldShowSecurityChip()) {
+    location_icon_view_->HideSecurityChip();
+  } else if (contents) {
+    location_icon_view_->ShowSecurityChip();
+  } else {
+    location_icon_view_->AnimateSecurityChip();
+  }

[Peter Kasting, 2016/08/26 20:59:12]

AnimateSecurityChip() is unclear, because it doesn't say whether we're showing
or hiding the chip.

I still think this API is not only more complicated but less clear than
something like:

Here:
  location_icon_view_->SetSecurityState(ShouldShowSecurityChip(), !contents);

In LocationIconView:
  // Triggers a security chip state change.  |showing| describes the desired new
  // state.  |updates_current_tab| is true if this is a change to the current
  // tab's state (as opposed to a switch to a different tab).
  LocationIconView::SetSecurityState(bool showing, bool updates_current_tab);

(You could also convert the second arg to an enum, which might be more clear, or
change the name or reverse the meaning of it.)

It's not just about having a smaller API surface, although that does matter. 
It's also about making the details of when and how the chip animates an
implementation detail inside LocationIconView, instead of having LocationBarView
control them.  LocationBarView is already extremely complicated, and the more
logic it can push into its children, the better.

[Kevin Bailey, 2016/08/29 13:38:10]

That would be trivially fixed by adopting Sarah's name, AnimateIn.
I'm sorry but you won't convince me that a parameter of '!contents' is more
clear than AnimateIn(). That you need enums to make it readable is proof of
this.
You don't move lines from one file to another simply to make them balanced. You
put lines where they belong. If something is too complicated, one can move the
logic to a subroutine, which we also could have done here.

The problem with smearing responsibility across classes is that it requires the
reader to understand the whole collection to understand any of it. It makes it
so that a tiny change here causes a problem over there. It makes the classes
unusable elsewhere. Smearing undermines decoupling.

I hope you like my version. I think it at least draws a line between these 2
classes.

+
   OnChanged();  // NOTE: Calls Layout().
 }
 
 void LocationBarView::ResetTabState(WebContents* contents) {
   omnibox_view_->ResetTabState(contents);
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 // LocationBarView, public OmniboxEditController implementation:
 
@@ skipping 216 matching lines @@
   if (browser)
     FirstRunBubble::ShowBubble(browser, location_icon_view_);
 #endif
 }
 
 bool LocationBarView::HasValidSuggestText() const {
   return suggested_text_view_->visible() &&
       !suggested_text_view_->size().IsEmpty();
 }
 
+base::string16 LocationBarView::GetSecurityText() const {
+  return ShouldShowEVBubble() ? GetToolbarModel()->GetEVCertName()
+                              : GetToolbarModel()->GetSecureVerboseText();
+}
+
 bool LocationBarView::ShouldShowKeywordBubble() const {
   return !omnibox_view_->model()->keyword().empty() &&
       !omnibox_view_->model()->is_keyword_hint();
 }
 
 bool LocationBarView::ShouldShowEVBubble() const {
   return (GetToolbarModel()->GetSecurityLevel(false) ==
           security_state::SecurityStateModel::EV_SECURE);
 }
 
+bool LocationBarView::ShouldShowSecurityChip() const {
+  using SecurityLevel = security_state::SecurityStateModel::SecurityLevel;
+  SecurityLevel level = GetToolbarModel()->GetSecurityLevel(false);
+  return level == SecurityLevel::SECURE || level == SecurityLevel::EV_SECURE ||
+         level == SecurityLevel::SECURITY_ERROR;
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 // LocationBarView, private LocationBar implementation:
 
 void LocationBarView::ShowFirstRunBubble() {
   // Wait until search engines have loaded to show the first run bubble.
   TemplateURLService* url_service =
       TemplateURLServiceFactory::GetForProfile(profile());
   if (!url_service->loaded()) {
     template_url_service_ = url_service;
     template_url_service_->AddObserver(this);
@@ skipping 324 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();
 }