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

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

Index: chrome/browser/ui/views/toolbar/site_chip_view.cc
diff --git a/chrome/browser/ui/views/toolbar/site_chip_view.cc b/chrome/browser/ui/views/toolbar/site_chip_view.cc
index 4a010ec634cfb4bd8a9d0c7c614ffa34fdb74a6f..feccab2a9f3aeb8a8dd938fa6955838ad9ca7194 100644
--- a/chrome/browser/ui/views/toolbar/site_chip_view.cc
+++ b/chrome/browser/ui/views/toolbar/site_chip_view.cc
@@ -5,6 +5,7 @@
 #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"
@@ -29,6 +30,7 @@
 #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"
@@ -101,11 +103,15 @@ void SiteChipExtensionIcon::OnExtensionIconImageChanged(
 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)
@@ -327,14 +333,20 @@ void SiteChipView::Update(content::WebContents* web_contents) {
   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) {
@@ -345,13 +357,7 @@ void SiteChipView::Update(content::WebContents* web_contents) {
 
   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_);
@@ -440,10 +446,13 @@ void SiteChipView::OnPaint(gfx::Canvas* canvas) {
 // 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,