Omnibox - Clipboard Provider - Record Number of Times Suggestion Offered

components/omnibox/browser/clipboard_url_provider.cc

Index: components/omnibox/browser/clipboard_url_provider.cc
diff --git a/components/omnibox/browser/clipboard_url_provider.cc b/components/omnibox/browser/clipboard_url_provider.cc
index 3fe1074966161124158d27f699cfd3e4a9a5e553..97c942631decdca8025c44f3679c57c0d8189d57 100644
--- a/components/omnibox/browser/clipboard_url_provider.cc
+++ b/components/omnibox/browser/clipboard_url_provider.cc
@@ -39,10 +39,15 @@ void ClipboardURLProvider::Start(const AutocompleteInput& input,
     return;
 
   GURL url;
-  // If the clipboard does not contain any URL, or the URL on the page is the
-  // same as the URL in the clipboard, early return.
-  if (!clipboard_content_->GetRecentURLFromClipboard(&url) ||
-      url == input.current_url())
+  // The clipboard does not contain a URL worth suggesting.
+  if (!clipboard_content_->GetRecentURLFromClipboard(&url)) {
+    current_url_suggested_ = GURL();
+    current_url_suggested_times_ = 0;
+    return;
+  }
+  // The URL on the page is the same as the URL in the clipboard.  Don't
+  // bother suggesting it.
+  if (url == input.current_url())
     return;
 
   DCHECK(url.is_valid());
@@ -62,6 +67,17 @@ void ClipboardURLProvider::Start(const AutocompleteInput& input,
                         !matches_.empty());
   UMA_HISTOGRAM_LONG_TIMES_100("Omnibox.ClipboardSuggestionShownAge",
                                clipboard_content_->GetClipboardContentAge());
+  // Record the number of times the currently-offered URL has been suggested.
+  // This only works over this run of Chrome; if the URL was in the clipboard
+  // on a previous run, those offerings will not be counted.
+  if (url == current_url_suggested_) {
+    current_url_suggested_times_++;
+  } else {
+    current_url_suggested_ = url;
+    current_url_suggested_times_ = 1;
+  }
+  UMA_HISTOGRAM_EXACT_LINEAR("Omnibox.ClipboardSuggestionShownNumTimes",
+                             current_url_suggested_times_, 20);

[Ilya Sherman, 2017/05/31 23:40:23]

nit: Is this better than UMA_HISTOGRAM_SPARSE_SLOWLY with explicit capping of
the recorded value?

[Mark P, 2017/06/01 04:16:33]

No, not really.  Changed.

Note that I had to use a static_cast (ugh) because I couldn't figure out a
size_t constant that works on all platforms.  (E.g., 20ul works on linux but not
android; 20u works on android but not linux.)

 
   // Add the clipboard match. The relevance is 800 to beat ZeroSuggest results.
   AutocompleteMatch match(this, 800, false, AutocompleteMatchType::CLIPBOARD);
@@ -80,3 +96,17 @@ void ClipboardURLProvider::Start(const AutocompleteInput& input,
 
   matches_.push_back(match);
 }
+
+void ClipboardURLProvider::AddProviderInfo(ProvidersInfo* provider_info) const {

[jif, 2017/05/31 09:27:47]

Can we pass |provider_info| by reference? This would make it clear that
|provider_info| can't be null.
This can of course be done in a separate CL.

[Mark P, 2017/05/31 23:41:52]

This is an inherited function from the base class.  It would be a pain to chance
the interface and everywhere that uses it for relatively little gain.

So, I'll punt this to a separate changelist, then probably postpone the
changelist indefinitely. /honesty  :-)

+  // If a URL wasn't suggested on this most recent focus event, don't bother
+  // setting |times_returned_results_in_session|, as in effect this URL has
+  // never been suggested during the current session.  (For the purpose of
+  // this provider, we define a session as intervals between when a URL
+  // clipboard suggestion changes.)
+  if (current_url_suggested_times_ == 0)
+    return;
+  provider_info->push_back(metrics::OmniboxEventProto_ProviderInfo());
+  metrics::OmniboxEventProto_ProviderInfo& new_entry = provider_info->back();
+  new_entry.set_provider(AsOmniboxEventProviderType());
+  new_entry.set_times_returned_results_in_session(current_url_suggested_times_);
+}