[SiteChip] Add UMA, finish wiring site chip experiment up to the omnibox.

chrome/browser/ui/views/toolbar/site_chip_view.cc

 // Copyright 2013 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/toolbar/site_chip_view.h"
 
 #include "base/files/file_path.h"
+#include "base/metrics/histogram.h"
 #include "base/prefs/pref_service.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/extensions/extension_icon_image.h"
 #include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/extensions/extension_system.h"
 #include "chrome/browser/favicon/favicon_tab_helper.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/safe_browsing/client_side_detection_host.h"
 #include "chrome/browser/safe_browsing/safe_browsing_service.h"
 #include "chrome/browser/safe_browsing/safe_browsing_tab_observer.h"
 #include "chrome/browser/safe_browsing/ui_manager.h"
 #include "chrome/browser/search/search.h"
 #include "chrome/browser/themes/theme_properties.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/omnibox/omnibox_view.h"
 #include "chrome/browser/ui/toolbar/toolbar_model.h"
 #include "chrome/browser/ui/views/location_bar/location_bar_view.h"
 #include "chrome/browser/ui/views/toolbar/toolbar_view.h"
 #include "chrome/common/extensions/extension_constants.h"
 #include "chrome/common/extensions/manifest_handlers/icons_handler.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/common/url_constants.h"
+#include "content/public/browser/user_metrics.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/common/url_constants.h"
 #include "extensions/common/constants.h"
 #include "grit/component_strings.h"
 #include "grit/generated_resources.h"
 #include "grit/theme_resources.h"
 #include "net/base/net_util.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/base/resource/resource_bundle.h"
 #include "ui/base/theme_provider.h"
@@ skipping 52 matching lines @@
   if (icon_view_)
     icon_view_->SetImage(&icon_image_->image_skia());
 }
 
 
 // SiteChipView ---------------------------------------------------------------
 
 namespace {
 
 const int kEdgeThickness = 5;
-const int k16x16IconLeadingSpacing = 3;
-const int k16x16IconTrailingSpacing = 3;
+const int k16x16IconLeadingSpacing = 1;
+const int k16x16IconTrailingSpacing = 2;
 const int kIconTextSpacing = 3;
 const int kTrailingLabelMargin = 0;
 
+const SkColor kEVBackgroundColor = SkColorSetRGB(163, 226, 120);
+const SkColor kMalwareBackgroundColor = SkColorSetRGB(145, 0, 0);
+const SkColor kBrokenSSLBackgroundColor = SkColorSetRGB(253, 196, 36);

[Peter Kasting, 2013/12/18 00:33:35]

Do we want these colors on every possible toolbar background color?  (For
reference, the bubble colors in the omnibox are semitransparent; you could copy
the colors + alpha values from those.)

[Greg Billock, 2013/12/18 16:43:07]

That's a good question. The images have a bit of transparency, so I could be
using SetRGBA here. I wasn't sure how that would interact with the label
background and picking the high-contrast color, is why I didn't use it. The
transparency probably isn't enough that it'll make a difference in practice.

[Peter Kasting, 2013/12/18 21:30:59]

I think rather than hardcode these values (which makes for subtle maintenance
issues if we change the images, since we won't automatically notice these are
out of date), you should look in the code for e.g. the EV bubble and find how it
samples the correct color from the image grid and then blends it against the
background.  Then the code below should do that and these constants can
disappear entirely.

+
 // Detect client-side or SB malware/phishing hits.
 bool IsMalware(const GURL& url, content::WebContents* tab) {
   if (tab->GetURL() != url)
     return false;
 
   safe_browsing::SafeBrowsingTabObserver* sb_observer =
       safe_browsing::SafeBrowsingTabObserver::FromWebContents(tab);
   return sb_observer && sb_observer->detection_host() &&
       sb_observer->detection_host()->DidPageReceiveSafeBrowsingMatch();
 }
@@ skipping 201 matching lines @@
 
   if ((url == url_displayed_) &&
       (security_level == security_level_) &&
       (url_malware == url_malware_))
     return;
 
   url_displayed_ = url;
   url_malware_ = url_malware;
   security_level_ = security_level;
 
-  if (url_malware_)
+  SkColor label_background =
+      GetThemeProvider()->GetColor(ThemeProperties::COLOR_TOOLBAR);
+  if (url_malware_) {
     painter_ = malware_background_painter_.get();
-  else if (security_level_ == ToolbarModel::SECURITY_ERROR)
+    label_background = kMalwareBackgroundColor;
+  } else if (security_level_ == ToolbarModel::SECURITY_ERROR) {
     painter_ = broken_ssl_background_painter_.get();
-  else if (security_level_ == ToolbarModel::EV_SECURE)
+    label_background = kBrokenSSLBackgroundColor;
+  } else if (security_level_ == ToolbarModel::EV_SECURE) {
     painter_ = ev_background_painter_.get();
-  else
+    label_background = kEVBackgroundColor;
+  } else {
     painter_ = NULL;
+  }
 
   string16 host = SiteLabelFromURL(url_displayed_);
   if (security_level_ == ToolbarModel::EV_SECURE) {
     host = l10n_util::GetStringFUTF16(IDS_SITE_CHIP_EV_SSL_LABEL,
         toolbar_view_->GetToolbarModel()->GetEVCertName(),
         host);
   }
 
   host_label_->SetText(host);
   host_label_->SetTooltipText(host);
-  // TODO(gbillock): Instead of this, just set
-  // host_label_->SetBackgroundColor() to either the COLOR_TOOLBAR or
-  // the painter background color if we're using a background painter.
-  SkColor toolbar_background =
-      GetThemeProvider()->GetColor(ThemeProperties::COLOR_TOOLBAR);
-  host_label_->SetEnabledColor(
-      color_utils::GetReadableColor(SK_ColorBLACK, toolbar_background));
+  host_label_->SetBackgroundColor(label_background);

[Peter Kasting, 2013/12/18 00:33:35]

Hmm, maybe we should be using GetReadableColor() somewhere in all this mix to
ensure the text is always readable over the background, and perhaps also that
the background colors here always stick out against the toolbar color.

[Greg Billock, 2013/12/18 16:43:07]

Michael helped me work this out -- that's what Label does internally to ensure
high contrast.

 
   int icon = toolbar_view_->GetToolbarModel()->GetIconForSecurityLevel(
       security_level_);
   showing_16x16_icon_ = false;
 
   if (url_displayed_.is_empty() ||
       url_displayed_.SchemeIs(chrome::kChromeUIScheme) ||
       url_displayed_.SchemeIs(chrome::kAboutScheme)) {
     icon = IDR_PRODUCT_LOGO_16;
     showing_16x16_icon_ = true;
@@ skipping 68 matching lines @@
   if (painter_)
     views::Painter::PaintPainterAt(canvas, painter_, rect);
 
   ToolbarButton::OnPaint(canvas);
 }
 
 // TODO(gbillock): Make the LocationBarView or OmniboxView the listener for
 // this button.
 void SiteChipView::ButtonPressed(views::Button* sender,
                                  const ui::Event& event) {
+  UMA_HISTOGRAM_COUNTS("SiteChip.Pressed", 1);
+  content::RecordAction(content::UserMetricsAction("SiteChipPress"));
+
   toolbar_view_->location_bar()->GetOmniboxView()->SetFocus();
-  toolbar_view_->location_bar()->GetOmniboxView()->SelectAll(true);
   toolbar_view_->location_bar()->GetOmniboxView()->model()->
       SetCaretVisibility(true);
+  toolbar_view_->location_bar()->GetOmniboxView()->ShowURL();
 }
 
 void SiteChipView::WriteDragDataForView(View* sender,
                                         const gfx::Point& press_pt,
                                         OSExchangeData* data) {
   // TODO(gbillock): Consolidate this with the identical logic in
   // LocationBarView.
   content::WebContents* web_contents = toolbar_view_->GetWebContents();
   FaviconTabHelper* favicon_tab_helper =
       FaviconTabHelper::FromWebContents(web_contents);
@@ skipping 19 matching lines @@
 // Note: When OnSafeBrowsingHit would be called, OnSafeBrowsingMatch will
 // have already been called.
 void SiteChipView::OnSafeBrowsingHit(
     const SafeBrowsingUIManager::UnsafeResource& resource) {}
 
 void SiteChipView::OnSafeBrowsingMatch(
     const SafeBrowsingUIManager::UnsafeResource& resource) {
   OnChanged();
 }