New location security strings and animation.

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 a219ea3318717a43d5ef3b7e0b40422ac91df3d1..6bf447323377c1be03154535c09db5ce602d35ef 100644
--- a/chrome/browser/ui/views/location_bar/location_bar_view.cc
+++ b/chrome/browser/ui/views/location_bar/location_bar_view.cc
@@ -5,6 +5,7 @@
 #include "chrome/browser/ui/views/location_bar/location_bar_view.h"
 
 #include <algorithm>
+#include <iostream>
 #include <map>
 
 #include "base/i18n/rtl.h"
@@ -391,6 +392,8 @@ void LocationBarView::SetPreviewEnabledPageAction(ExtensionAction* page_action,
   if (!page_action_view)
     return;
 
+  // Tell the location icon to animate here?

[Peter Kasting, 2016/07/19 21:17:20]

Why?  This function is all about mucking with a specific page action icon.  Not
sure why that would affect the location icon state.

[Kevin Bailey, 2016/08/12 18:49:14]

ack

+
   page_action_view->image_view()->set_preview_enabled(preview_enabled);
   page_action_view->UpdateVisibility(web_contents);
   Layout();
@@ -532,10 +535,12 @@ gfx::Size LocationBarView::GetPreferredSize() const {
   int leading_width = edge_thickness;
   if (ShouldShowKeywordBubble()) {
     // The selected keyword view can collapse completely.
-  } else if (ShouldShowEVBubble()) {
+  } else if (ShouldShowSecureVerbose()) {
+    base::string16 label = ShouldShowEVBubble()
+                           ? GetToolbarModel()->GetEVCertName()
+                           : GetToolbarModel()->GetSecureVerboseText();

[Peter Kasting, 2016/07/19 21:17:20]

Nit: Could factor GetToolbarModel() out.  And since you do this same computation
later, maybe the whole thing should get a helper.

[Kevin Bailey, 2016/08/12 18:49:14]

Done.

     leading_width += GetLayoutConstant(LOCATION_BAR_BUBBLE_HORIZONTAL_PADDING) +
-                     location_icon_view_->GetMinimumSizeForLabelText(
-                                            GetToolbarModel()->GetEVCertName())
+                     location_icon_view_->GetMinimumSizeForLabelText(label)
                          .width();
   } else {
     leading_width += padding + location_icon_view_->GetMinimumSize().width();
@@ -563,6 +568,7 @@ gfx::Size LocationBarView::GetPreferredSize() const {
 }
 
 void LocationBarView::Layout() {
+  std::cout << "LocationBar::Layout " << (void*)this << "\n";
   if (!IsInitialized())
     return;
 
@@ -612,9 +618,13 @@ void LocationBarView::Layout() {
         selected_keyword_view_->set_is_extension_icon(false);
       }
     }
-  } else if (ShouldShowEVBubble()) {
-    location_icon_view_->SetLabel(GetToolbarModel()->GetEVCertName());
+  } else if (ShouldShowSecureVerbose()) {
+    location_icon_view_->SetLabel(ShouldShowEVBubble()
+                                  ? GetToolbarModel()->GetEVCertName()
+                                  : GetToolbarModel()->GetSecureVerboseText());
     location_icon_view_->SetBackground(true);
+    // Not necessarily the best place, and shouldn't be done unconditionally.
+    location_icon_view_->StartAnimation();

[Peter Kasting, 2016/07/19 21:17:20]

Yeah, definitely don't do this from layout, that can be called anywhere.

[Kevin Bailey, 2016/08/12 18:49:14]

It turns out to be quite useful to do this here. Is there a performance concern?

     // The largest fraction of the omnibox that can be taken by the EV bubble.
     const double kMaxBubbleFraction = 0.5;
     leading_decorations.AddDecoration(
@@ -783,6 +793,8 @@ void LocationBarView::OnNativeThemeChanged(const ui::NativeTheme* theme) {
   }
 }
 
+// Called twice per page visit. 'contents' never set.

[Peter Kasting, 2016/07/19 21:17:20]

That's because |contents| is set when switching tabs, rather than when the
current tab updates, IIRC.

[Kevin Bailey, 2016/08/12 18:49:14]

I only see it set on the very first page load. Strange.

+// Still looking for what's called once.

[Peter Kasting, 2016/07/19 21:17:20]

I'm not sure there is anything.  Update() is probably the only function
currently.  You may need to plumb additional data or callbacks.  Or maybe you
just need to look for whether we're changing the current URL during this
Update(), or something.

[Kevin Bailey, 2016/08/12 18:49:14]

ack

 void LocationBarView::Update(const WebContents* contents) {
   RefreshContentSettingViews();
   RefreshZoomView();
@@ -1044,10 +1056,20 @@ bool LocationBarView::ShouldShowKeywordBubble() const {
 }
 
 bool LocationBarView::ShouldShowEVBubble() const {
+  std::cout << "EV level is " << GetToolbarModel()->GetSecurityLevel(false)
+      << "\n";
   return (GetToolbarModel()->GetSecurityLevel(false) ==
           security_state::SecurityStateModel::EV_SECURE);
 }
 
+bool LocationBarView::ShouldShowSecureVerbose() const {
+  return ui::MaterialDesignController::IsModeMaterial() &&

[Peter Kasting, 2016/07/19 21:17:20]

Why condition this on material mode?

[Kevin Bailey, 2016/08/12 18:49:14]

Done.

+      !omnibox_view_->IsEditingOrEmpty() &&
+      !ShouldShowKeywordBubble() &&
+      GetToolbarModel()->GetSecurityLevel(false) !=
+          security_state::SecurityStateModel::NONE;

[Peter Kasting, 2016/07/19 21:17:20]

I thought we wanted to be verbose about HTTP too?  Or is that a later stage?

[Kevin Bailey, 2016/08/12 18:49:14]

Done.

+}
+
 ////////////////////////////////////////////////////////////////////////////////
 // LocationBarView, private LocationBar implementation: