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 <iostream>
 #include <map>
 
 #include "base/i18n/rtl.h"
 #include "base/stl_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "build/build_config.h"
 #include "chrome/app/chrome_command_ids.h"
 #include "chrome/browser/command_updater.h"
 #include "chrome/browser/defaults.h"
 #include "chrome/browser/extensions/api/omnibox/omnibox_api.h"
@@ skipping 366 matching lines @@
   DCHECK(page_action);
   WebContents* web_contents = GetWebContents();
 
   RefreshPageActionViews();
   PageActionWithBadgeView* page_action_view =
       static_cast<PageActionWithBadgeView*>(GetPageActionView(page_action));
   DCHECK(page_action_view);
   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();
   SchedulePaint();
 }
 
 PageActionWithBadgeView* LocationBarView::GetPageActionView(
     ExtensionAction* page_action) {
   DCHECK(page_action);
   for (PageActionViews::const_iterator i(page_action_views_.begin());
@@ skipping 121 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()) {
+  } 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();
   }
 
   // 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_) +
                     IncrementalMinimumWidth(manage_passwords_icon_view_) +
                     IncrementalMinimumWidth(zoom_view_);
   for (PageActionViews::const_iterator i(page_action_views_.begin());
        i != page_action_views_.end(); ++i)
     trailing_width += IncrementalMinimumWidth((*i));
   for (ContentSettingViews::const_iterator i(content_setting_views_.begin());
        i != content_setting_views_.end(); ++i)
     trailing_width += IncrementalMinimumWidth((*i));
 
   min_size.set_width(leading_width + omnibox_view_->GetMinimumSize().width() +
                      2 * padding - omnibox_view_->GetInsets().width() +
                      trailing_width);
   return min_size;
 }
 
 void LocationBarView::Layout() {
+  std::cout << "LocationBar::Layout " << (void*)this << "\n";
   if (!IsInitialized())
     return;
 
   selected_keyword_view_->SetVisible(false);
   location_icon_view_->SetVisible(false);
   keyword_hint_view_->SetVisible(false);
 
   const int item_padding = GetLayoutConstant(LOCATION_BAR_HORIZONTAL_PADDING);
   const int edge_thickness = GetHorizontalEdgeThickness();
 
@@ skipping 29 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 (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(
         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 148 matching lines @@
 void LocationBarView::OnNativeThemeChanged(const ui::NativeTheme* theme) {
   if (ui::MaterialDesignController::IsModeMaterial()) {
     RefreshLocationIcon();
     if (!is_popup_mode_) {
       set_background(new BackgroundWith1PxBorder(GetColor(BACKGROUND),
                                                  kBorderColor));
     }
   }
 }
 
+// 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();
   RefreshPageActionViews();
   RefreshTranslateIcon();
   RefreshSaveCreditCardIconView();
   RefreshManagePasswordsIconView();
   content::WebContents* web_contents_for_sub_views =
       GetToolbarModel()->input_in_progress() ? nullptr : GetWebContents();
   open_pdf_in_reader_view_->Update(web_contents_for_sub_views);
@@ skipping 241 matching lines @@
   return suggested_text_view_->visible() &&
       !suggested_text_view_->size().IsEmpty();
 }
 
 bool LocationBarView::ShouldShowKeywordBubble() const {
   return !omnibox_view_->model()->keyword().empty() &&
       !omnibox_view_->model()->is_keyword_hint();
 }
 
 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:
 
 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();
 }