Omnibox: Have UMA Log Interactions Even When Dropdown is Closed

Adds two omnibox proto fields to describe the state
of this dropdown-closed interaction.

AutocompleteActionPredictor predictor is meant to
change relatively little; I think the current code is a
slight improvement.

RLZ RecordFirstSearch() triggering is exactly the same as before.

The histogram name change is because this histogram will
be recorded under different conditions than before.
Hence, I also updated histograms.xml.

Omnibox.EventCount is a non-UMA histogram, so doesn't
need to have its name changed and does not appear
in histograms.xml.

TBR=dominich

BUG=264992

Review URL: https://codereview.chromium.org/149513008

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@267411 0039d316-1c4b-4281-b951-d872f2087c98

7 files changed, 131 insertions, 70 deletions

diff --git a/chrome/browser/metrics/metrics_log.cc b/chrome/browser/metrics/metrics_log.cc
index 239657f..8ef2484 100644
--- a/chrome/browser/metrics/metrics_log.cc
+++ b/chrome/browser/metrics/metrics_log.cc
@@ -846,19 +846,33 @@ void MetricsLog::RecordOmniboxOpenedURL(const OmniboxLog& log) {
   omnibox_event->set_selected_index(log.selected_index);
   if (log.completed_length != base::string16::npos)
     omnibox_event->set_completed_length(log.completed_length);
+  const base::TimeDelta default_time_delta =
+      base::TimeDelta::FromMilliseconds(-1);
   if (log.elapsed_time_since_user_first_modified_omnibox !=
-      base::TimeDelta::FromMilliseconds(-1)) {
+      default_time_delta) {
     // Only upload the typing duration if it is set/valid.
     omnibox_event->set_typing_duration_ms(
         log.elapsed_time_since_user_first_modified_omnibox.InMilliseconds());
   }
-  omnibox_event->set_duration_since_last_default_match_update_ms(
-      log.elapsed_time_since_last_change_to_default_match.InMilliseconds());
+  if (log.elapsed_time_since_last_change_to_default_match !=
+      default_time_delta) {
+    omnibox_event->set_duration_since_last_default_match_update_ms(
+        log.elapsed_time_since_last_change_to_default_match.InMilliseconds());
+  }
   omnibox_event->set_current_page_classification(
       AsOmniboxEventPageClassification(log.current_page_classification));
   omnibox_event->set_input_type(AsOmniboxEventInputType(log.input_type));
-  omnibox_event->set_is_top_result_hidden_in_dropdown(
-      log.result.ShouldHideTopMatch());
+  // We consider a paste-and-search/paste-and-go action to have a closed popup
+  // (as explained in omnibox_event.proto) even if it was not, because such
+  // actions ignore the contents of the popup so it doesn't matter that it was
+  // open.
+  const bool consider_popup_open = log.is_popup_open && !log.is_paste_and_go;
+  omnibox_event->set_is_popup_open(consider_popup_open);
+  omnibox_event->set_is_paste_and_go(log.is_paste_and_go);
+  if (consider_popup_open) {
+    omnibox_event->set_is_top_result_hidden_in_dropdown(
+        log.result.ShouldHideTopMatch());
+  }
 
   for (AutocompleteResult::const_iterator i(log.result.begin());
        i != log.result.end(); ++i) {
diff --git a/chrome/browser/omnibox/omnibox_log.cc b/chrome/browser/omnibox/omnibox_log.cc
index 2ad76a0..48f6241 100644
--- a/chrome/browser/omnibox/omnibox_log.cc
+++ b/chrome/browser/omnibox/omnibox_log.cc
@@ -8,7 +8,9 @@ OmniboxLog::OmniboxLog(
     const base::string16& text,
     bool just_deleted_text,
     AutocompleteInput::Type input_type,
+    bool is_popup_open,
     size_t selected_index,
+    bool is_paste_and_go,
     SessionID::id_type tab_id,
     AutocompleteInput::PageClassification current_page_classification,
     base::TimeDelta elapsed_time_since_user_first_modified_omnibox,
@@ -18,7 +20,9 @@ OmniboxLog::OmniboxLog(
     : text(text),
       just_deleted_text(just_deleted_text),
       input_type(input_type),
+      is_popup_open(is_popup_open),
       selected_index(selected_index),
+      is_paste_and_go(is_paste_and_go),
       tab_id(tab_id),
       current_page_classification(current_page_classification),
       elapsed_time_since_user_first_modified_omnibox(
diff --git a/chrome/browser/omnibox/omnibox_log.h b/chrome/browser/omnibox/omnibox_log.h
index f3b4809..3b5d2eb 100644
--- a/chrome/browser/omnibox/omnibox_log.h
+++ b/chrome/browser/omnibox/omnibox_log.h
@@ -22,7 +22,9 @@ struct OmniboxLog {
       const base::string16& text,
       bool just_deleted_text,
       AutocompleteInput::Type input_type,
+      bool is_popup_open,
       size_t selected_index,
+      bool is_paste_and_go,
       SessionID::id_type tab_id,
       AutocompleteInput::PageClassification current_page_classification,
       base::TimeDelta elapsed_time_since_user_first_modified_omnibox,
@@ -41,9 +43,17 @@ struct OmniboxLog {
   // The detected type of the user's input.
   AutocompleteInput::Type input_type;
 
-  // Selected index (if selected) or -1 (OmniboxPopupModel::kNoMatch).
+  // True if the popup is open.
+  bool is_popup_open;
+
+  // The index of the item selected in the dropdown list.  Set to 0 if the
+  // dropdown is closed (and therefore there is only one implicit suggestion).
   size_t selected_index;
 
+  // True if this is a paste-and-search or paste-and-go omnibox interaction.
+  // (The codebase refers to both these types as paste-and-go.)
+  bool is_paste_and_go;
+
   // ID of the tab the selected autocomplete suggestion was opened in.
   // Set to -1 if we haven't yet determined the destination tab.
   SessionID::id_type tab_id;
@@ -69,7 +79,9 @@ struct OmniboxLog {
 
   // The amount of time since the last time the default (i.e., inline)
   // match changed.  This will certainly be less than
-  // elapsed_time_since_user_first_modified_omnibox.
+  // elapsed_time_since_user_first_modified_omnibox.  Measuring this
+  // may be inappropriate in some cases (e.g., if editing is not in
+  // progress).  In such cases, it's set to -1 milliseconds.
   base::TimeDelta elapsed_time_since_last_change_to_default_match;
 
   // Result set.
diff --git a/chrome/browser/predictors/autocomplete_action_predictor.cc b/chrome/browser/predictors/autocomplete_action_predictor.cc
index 99a2244..923b25b 100644
--- a/chrome/browser/predictors/autocomplete_action_predictor.cc
+++ b/chrome/browser/predictors/autocomplete_action_predictor.cc
@@ -31,6 +31,7 @@
 #include "chrome/browser/prerender/prerender_manager.h"
 #include "chrome/browser/prerender/prerender_manager_factory.h"
 #include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/ui/omnibox/omnibox_popup_model.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/notification_details.h"
 #include "content/public/browser/notification_service.h"
@@ -247,9 +248,6 @@ void AutocompleteActionPredictor::Observe(
       break;
     }
 
-    // This notification does not catch all instances of the user navigating
-    // from the Omnibox, but it does catch the cases where the dropdown is open
-    // and those are the events we're most interested in.
     case chrome::NOTIFICATION_OMNIBOX_OPENED_URL: {
       DCHECK(initialized_);
 
@@ -338,6 +336,16 @@ void AutocompleteActionPredictor::OnOmniboxOpenedUrl(const OmniboxLog& log) {
   if (log.text.length() < kMinimumUserTextLength)
     return;
 
+  // Do not attempt to learn from omnibox interactions where the omnibox
+  // dropdown is closed.  In these cases the user text (|log.text|) that we
+  // learn from is either empty or effectively identical to the destination
+  // string.  In either case, it can't teach us much.  Also do not attempt
+  // to learn from paste-and-go actions even if the popup is open because
+  // the paste-and-go destination has no relation to whatever text the user
+  // may have typed.
+  if (!log.is_popup_open || log.is_paste_and_go)
+    return;
+
   // Abandon the current prerender. If it is to be used, it will be used very
   // soon, so use the lower timeout.
   if (prerender_handle_) {
@@ -346,13 +354,12 @@ void AutocompleteActionPredictor::OnOmniboxOpenedUrl(const OmniboxLog& log) {
     // next StartPrerendering call.
   }
 
-  const AutocompleteMatch& match = log.result.match_at(log.selected_index);
-
   UMA_HISTOGRAM_BOOLEAN(
       base::StringPrintf("Prerender.OmniboxNavigationsCouldPrerender%s",
                          prerender::PrerenderManager::GetModeString()).c_str(),
       prerender::IsOmniboxEnabled(profile_));
 
+  const AutocompleteMatch& match = log.result.match_at(log.selected_index);
   const GURL& opened_url = match.destination_url;
   const base::string16 lower_user_text(base::i18n::ToLower(log.text));
 
diff --git a/chrome/browser/rlz/rlz.cc b/chrome/browser/rlz/rlz.cc
index a355450..ff64bac 100644
--- a/chrome/browser/rlz/rlz.cc
+++ b/chrome/browser/rlz/rlz.cc
@@ -20,6 +20,7 @@
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/google/google_util.h"
+#include "chrome/browser/omnibox/omnibox_log.h"
 #include "chrome/browser/prefs/session_startup_pref.h"
 #include "chrome/browser/search_engines/template_url.h"
 #include "chrome/browser/search_engines/template_url_service.h"
@@ -443,6 +444,13 @@ void RLZTracker::Observe(int type,
                          const content::NotificationDetails& details) {
   switch (type) {
     case chrome::NOTIFICATION_OMNIBOX_OPENED_URL:
+      // In M-36, we made NOTIFICATION_OMNIBOX_OPENED_URL fire more often than
+      // it did previously.  The RLZ folks want RLZ's "first search" detection
+      // to remain as unaffected as possible by this change.  This test is
+      // there to keep the old behavior.
+      const AutocompleteLog& log = *content::Details<OmniboxLog>(details).ptr();
+      if (!log.is_popup_open)
+        break;
       RecordFirstSearch(CHROME_OMNIBOX);
       registrar_.Remove(this, chrome::NOTIFICATION_OMNIBOX_OPENED_URL,
                         content::NotificationService::AllSources());
diff --git a/chrome/browser/ui/omnibox/omnibox_edit_model.cc b/chrome/browser/ui/omnibox/omnibox_edit_model.cc
index bc17cfd..284d472 100644
--- a/chrome/browser/ui/omnibox/omnibox_edit_model.cc
+++ b/chrome/browser/ui/omnibox/omnibox_edit_model.cc
@@ -103,7 +103,7 @@ const char kFocusToEditTimeHistogram[] = "Omnibox.FocusToEditTime";
 
 // Histogram name which counts the number of milliseconds a user takes
 // between focusing and opening an omnibox match.
-const char kFocusToOpenTimeHistogram[] = "Omnibox.FocusToOpenTime";
+const char kFocusToOpenTimeHistogram[] = "Omnibox.FocusToOpenTimeAnyPopupState";
 
 // Split the percentage match histograms into buckets based on the width of the
 // omnibox.
@@ -712,63 +712,66 @@ void OmniboxEditModel::OpenMatch(AutocompleteMatch match,
               input_text, alternate_nav_url,
               AutocompleteInput::HasHTTPScheme(input_text))));
 
-  // We only care about cases where there is a selection (i.e. the popup is
-  // open).
-  if (popup_model()->IsOpen()) {
-    base::TimeDelta elapsed_time_since_last_change_to_default_match(
-        now - autocomplete_controller()->last_time_default_match_changed());
-    // These elapsed times don't really make sense for ZeroSuggest matches
-    // (because the user does not modify the omnibox for ZeroSuggest), so for
-    // those we set the elapsed times to something that will be ignored by
-    // metrics_log.cc.
-    if (match.provider &&
-        (match.provider->type() == AutocompleteProvider::TYPE_ZERO_SUGGEST)) {
-      elapsed_time_since_user_first_modified_omnibox =
-          base::TimeDelta::FromMilliseconds(-1);
-      elapsed_time_since_last_change_to_default_match =
-          base::TimeDelta::FromMilliseconds(-1);
-    }
-    DCHECK_NE(OmniboxPopupModel::kNoMatch, index);
-    OmniboxLog log(
-        input_text,
-        just_deleted_text_,
-        input_.type(),
-        index,
-        -1,  // don't yet know tab ID; set later if appropriate
-        ClassifyPage(),
-        elapsed_time_since_user_first_modified_omnibox,
-        match.inline_autocompletion.length(),
-        elapsed_time_since_last_change_to_default_match,
-        result());
-
-    DCHECK(user_input_in_progress_ || (match.provider &&
-           (match.provider->type() == AutocompleteProvider::TYPE_ZERO_SUGGEST)))
-        << "We didn't get here through the expected series of calls. "
-        << "time_user_first_modified_omnibox_ is not set correctly and other "
-        << "things may be wrong. Match provider: "
-        << (match.provider ? match.provider->GetName() : "NULL");
-    DCHECK(log.elapsed_time_since_user_first_modified_omnibox >=
-           log.elapsed_time_since_last_change_to_default_match)
-        << "We should've got the notification that the user modified the "
-        << "omnibox text at same time or before the most recent time the "
-        << "default match changed.";
-
-    if ((disposition == CURRENT_TAB) && delegate_->CurrentPageExists()) {
-      // If we know the destination is being opened in the current tab,
-      // we can easily get the tab ID.  (If it's being opened in a new
-      // tab, we don't know the tab ID yet.)
-      log.tab_id = delegate_->GetSessionID().id();
-    }
-    autocomplete_controller()->AddProvidersInfo(&log.providers_info);
-    content::NotificationService::current()->Notify(
-        chrome::NOTIFICATION_OMNIBOX_OPENED_URL,
-        content::Source<Profile>(profile_),
-        content::Details<OmniboxLog>(&log));
-    HISTOGRAM_ENUMERATION("Omnibox.EventCount", 1, 2);
-    DCHECK(!last_omnibox_focus_.is_null())
-        << "An omnibox focus should have occurred before opening a match.";
-    UMA_HISTOGRAM_TIMES(kFocusToOpenTimeHistogram, now - last_omnibox_focus_);
+  base::TimeDelta elapsed_time_since_last_change_to_default_match(
+      now - autocomplete_controller()->last_time_default_match_changed());
+  // These elapsed times don't really make sense for ZeroSuggest matches
+  // (because the user does not modify the omnibox for ZeroSuggest), so for
+  // those we set the elapsed times to something that will be ignored by
+  // metrics_log.cc.  They also don't necessarily make sense if the omnibox
+  // dropdown is closed or the user used a paste-and-go action.  (In most
+  // cases when this happens, the user never modified the omnibox.)
+  if ((match.provider &&
+       (match.provider->type() == AutocompleteProvider::TYPE_ZERO_SUGGEST)) ||
+      !popup_model()->IsOpen() || !pasted_text.empty()) {
+    const base::TimeDelta default_time_delta =
+        base::TimeDelta::FromMilliseconds(-1);
+    elapsed_time_since_user_first_modified_omnibox = default_time_delta;
+    elapsed_time_since_last_change_to_default_match = default_time_delta;
+  }
+  // If the popup is closed or this is a paste-and-go action (meaning the
+  // contents of the dropdown are ignored regardless), we record for logging
+  // purposes a selected_index of 0 and a suggestion list as having a single
+  // entry of the match used.
+  ACMatches fake_single_entry_matches;
+  fake_single_entry_matches.push_back(match);
+  AutocompleteResult fake_single_entry_result;
+  fake_single_entry_result.AppendMatches(fake_single_entry_matches);
+  OmniboxLog log(
+      input_text,
+      just_deleted_text_,
+      input_.type(),
+      popup_model()->IsOpen(),
+      (!popup_model()->IsOpen() || !pasted_text.empty()) ? 0 : index,
+      !pasted_text.empty(),
+      -1,  // don't yet know tab ID; set later if appropriate
+      ClassifyPage(),
+      elapsed_time_since_user_first_modified_omnibox,
+      match.inline_autocompletion.length(),
+      elapsed_time_since_last_change_to_default_match,
+      (!popup_model()->IsOpen() || !pasted_text.empty()) ?
+          fake_single_entry_result : result());
+  DCHECK(!popup_model()->IsOpen() || !pasted_text.empty() ||
+         (log.elapsed_time_since_user_first_modified_omnibox >=
+          log.elapsed_time_since_last_change_to_default_match))
+      << "We should've got the notification that the user modified the "
+      << "omnibox text at same time or before the most recent time the "
+      << "default match changed.";
+
+  if ((disposition == CURRENT_TAB) && delegate_->CurrentPageExists()) {
+    // If we know the destination is being opened in the current tab,
+    // we can easily get the tab ID.  (If it's being opened in a new
+    // tab, we don't know the tab ID yet.)
+    log.tab_id = delegate_->GetSessionID().id();
   }
+  autocomplete_controller()->AddProvidersInfo(&log.providers_info);
+  content::NotificationService::current()->Notify(
+      chrome::NOTIFICATION_OMNIBOX_OPENED_URL,
+      content::Source<Profile>(profile_),
+      content::Details<OmniboxLog>(&log));
+  HISTOGRAM_ENUMERATION("Omnibox.EventCount", 1, 2);
+  DCHECK(!last_omnibox_focus_.is_null())
+      << "An omnibox focus should have occurred before opening a match.";
+  UMA_HISTOGRAM_TIMES(kFocusToOpenTimeHistogram, now - last_omnibox_focus_);
 
   TemplateURL* template_url = match.GetTemplateURL(profile_, false);
   if (template_url) {
diff --git a/chrome/common/metrics/proto/omnibox_event.proto b/chrome/common/metrics/proto/omnibox_event.proto
index f94c63d..a630973 100644
--- a/chrome/common/metrics/proto/omnibox_event.proto
+++ b/chrome/common/metrics/proto/omnibox_event.proto
@@ -10,7 +10,7 @@ option optimize_for = LITE_RUNTIME;
 
 package metrics;
 
-// Next tag: 15
+// Next tag: 17
 message OmniboxEventProto {
   // The timestamp for the event, in seconds since the epoch.
   optional int64 time = 1;
@@ -39,6 +39,19 @@ message OmniboxEventProto {
   // dropdown.
   optional bool is_top_result_hidden_in_dropdown = 14;
 
+  // Whether the omnibox popup is open.  It can be closed if, for instance,
+  // the user clicks in the omnibox and hits return to reload the same page.
+  // If the popup is closed, the suggestion list will contain only one item
+  // and selected_index will be 0 (pointing to that single item).  Because
+  // paste-and-search/paste-and-go actions ignore the current content of the
+  // omnibox dropdown (if it is open) when they happen, we pretend the
+  // dropdown is closed when logging these.
+  optional bool is_popup_open = 15;
+
+  // True if this is a paste-and-search or paste-and-go action.  (The codebase
+  // refers to both these types as paste-and-go.)
+  optional bool is_paste_and_go = 16;
+
   // The length of the inline autocomplete text in the omnibox.
   // The sum |typed_length| + |completed_length| gives the full length of the
   // user-visible text in the omnibox.