Omnibox Logging Clarifications

chrome/browser/ui/omnibox/omnibox_edit_model.cc

Index: chrome/browser/ui/omnibox/omnibox_edit_model.cc
diff --git a/chrome/browser/ui/omnibox/omnibox_edit_model.cc b/chrome/browser/ui/omnibox/omnibox_edit_model.cc
index 433e3bb9b88328817fc9286b750773c57b001570..30145c18a10cbcc19011fd84932607a535bcf313 100644
--- a/chrome/browser/ui/omnibox/omnibox_edit_model.cc
+++ b/chrome/browser/ui/omnibox/omnibox_edit_model.cc
@@ -673,9 +673,10 @@ void OmniboxEditModel::AcceptInput(WindowOpenDisposition disposition,
   const TemplateURL* template_url = match.GetTemplateURL(profile_, false);
   if (template_url && template_url->url_ref().HasGoogleBaseURLs())
     GoogleURLTracker::GoogleURLSearchCommitted(profile_);
-

[Peter Kasting, 2014/04/08 22:52:56]

Nit: Don't remove this blank line

[Mark P, 2014/04/08 22:58:32]

Done.

-  view_->OpenMatch(match, disposition, alternate_nav_url, base::string16(),
-                   OmniboxPopupModel::kNoMatch);
+  view_->OpenMatch(
+      match, disposition, alternate_nav_url, base::string16(),
+      popup_model() && popup_model()->IsOpen() ?

[Peter Kasting, 2014/04/08 22:52:56]

It doesn't seem like we need to check this.  If the popup isn't open, the value
we pass will be ignored.

[Mark P, 2014/04/08 22:58:32]

Done.

+          popup_model()->selected_line() : OmniboxPopupModel::kNoMatch);
 }
 
 void OmniboxEditModel::OpenMatch(AutocompleteMatch match,
@@ -715,11 +716,12 @@ void OmniboxEditModel::OpenMatch(AutocompleteMatch match,
       elapsed_time_since_last_change_to_default_match =
           base::TimeDelta::FromMilliseconds(-1);
     }
+    DCHECK_NE(OmniboxPopupModel::kNoMatch, index);
     OmniboxLog log(
         input_text,
         just_deleted_text_,
         autocomplete_controller()->input().type(),
-        popup_model()->selected_line(),
+        index,
         -1,  // don't yet know tab ID; set later if appropriate
         ClassifyPage(),
         elapsed_time_since_user_first_modified_omnibox,
@@ -739,9 +741,6 @@ void OmniboxEditModel::OpenMatch(AutocompleteMatch match,
         << "omnibox text at same time or before the most recent time the "
         << "default match changed.";
 
-    if (index != OmniboxPopupModel::kNoMatch)
-      log.selected_index = index;
-
     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