[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 391 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);

[msw, 2013/12/12 00:17:49]

What are you trying to record? This will do a decent job of recording the number
of times a single user has clicked the SiteChip, and presenting that locally at
about:histograms. But, you'll need to modify
tools/metrics/histograms/histograms.xml to get this shown correctly on the UMA
dashboard and aggregated for respondant Chrome users, and this format isn't good
for that type of data collection anyway (you'd just see the aggregate number of
times users have clicked the site chip, not a distribution of how many times
each user clicks the button in a given session or anything). Also, this number
alone doesn't give much context, you might want to record the number of
SiteChips made visible and/or updated, how many times any are hovered, or
something else that would give this a frame of reference.

[Greg Billock, 2013/12/12 20:36:37]

Yeah, I'll add the histograms.xml descriptor separately. Measuring hover is
probably a good plan. I don't know that we really have a plan for what we'd do
with measurements like per-user per-session clicks or something. We want to
measure usage, but we don't expect much interaction, so it's not clear what we'd
do with more information.

+  content::RecordAction(content::UserMetricsAction("SiteChipPress"));
   toolbar_view_->location_bar()->GetOmniboxView()->SetFocus();

[msw, 2013/12/12 00:17:49]

nit: make a local OmniboxView* omnibox_view =
toolbar_view_->location_bar()->GetOmniboxView(); for convenience here.

[Greg Billock, 2013/12/12 20:36:37]

After Justin's change, we won't need SetFocus, and probably not
SetCaretVisibility either. I'm planning to delete those lines at some point.

-  toolbar_view_->location_bar()->GetOmniboxView()->SelectAll(true);
   toolbar_view_->location_bar()->GetOmniboxView()->model()->
       SetCaretVisibility(true);
+  toolbar_view_->location_bar()->GetOmniboxView()->ShowURL();
+  //toolbar_view_->location_bar()->GetOmniboxView()->SelectAll(true);

[msw, 2013/12/12 00:17:49]

Remove this.

[Greg Billock, 2013/12/12 20:36:37]

Done.

 }
 
 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();
 }