Omnibox: Change |inline_autocomplete_offset| to |inline_autocompletion|

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 6ba8740707a2043a1534829e5670972b48e02585..c1022bb81859a3e9d35d44fb799f35a458d68f64 100644
--- a/chrome/browser/ui/omnibox/omnibox_edit_model.cc
+++ b/chrome/browser/ui/omnibox/omnibox_edit_model.cc
@@ -628,7 +628,7 @@ void OmniboxEditModel::OpenMatch(const AutocompleteMatch& match,
         delegate_->CurrentPageExists() ? ClassifyPage(delegate_->GetURL()) :
             metrics::OmniboxEventProto_PageClassification_OTHER,
         elapsed_time_since_user_first_modified_omnibox,
-        string16::npos,  // completed_length; possibly set later
+        match.inline_autocompletion.length(),
         elapsed_time_since_last_change_to_default_match,
         result());
 
@@ -646,12 +646,6 @@ void OmniboxEditModel::OpenMatch(const AutocompleteMatch& match,
 
     if (index != OmniboxPopupModel::kNoMatch)
       log.selected_index = index;
-    if (match.inline_autocomplete_offset != string16::npos) {
-      DCHECK_GE(match.fill_into_edit.length(),
-                match.inline_autocomplete_offset);
-      log.completed_length =
-          match.fill_into_edit.length() - match.inline_autocomplete_offset;
-    }
 
     if ((disposition == CURRENT_TAB) && delegate_->CurrentPageExists()) {
       // If we know the destination is being opened in the current tab,
@@ -1100,6 +1094,8 @@ bool OmniboxEditModel::OnAfterPossibleChange(const string16& old_text,
            MaybeAcceptKeywordBySpace(user_text_));
 }
 
+// TODO(beaudoin): Merge OnPopupDataChanged with this method once the popup
+// handling has completely migrated to omnibox_controller.
 void OmniboxEditModel::OnCurrentMatchChanged() {
   has_temporary_text_ = false;
 
@@ -1110,17 +1106,15 @@ void OmniboxEditModel::OnCurrentMatchChanged() {
   string16 keyword;
   bool is_keyword_hint;
   match.GetKeywordUIState(profile_, &keyword, &is_keyword_hint);
-  string16 inline_autocomplete_text;
-  if (match.inline_autocomplete_offset < match.fill_into_edit.length()) {
-    // We have blue text, go through OnPopupDataChanged.
-    // TODO(beaudoin): Merge OnPopupDataChanged with this method once the
-    // popup handling has completely migrated to omnibox_controller.
-    inline_autocomplete_text =
-        match.fill_into_edit.substr(match.inline_autocomplete_offset);
-  }
   popup_model()->OnResultChanged();
-  OnPopupDataChanged(inline_autocomplete_text, NULL, keyword,
-                     is_keyword_hint);
+  // We must pass a copy of |match.inline_autocompletion| to
+  // OnPopupDataChanged() because OnPopupDataChanged() resets the current
+  // match, clearing its inline_autocompletion.  OnPopupDataChanged() takes
+  // its first argument as a reference, meaning that when it resets the
+  // current match, if we didn't so this we would lose knowledge of what
+  // the inline_autocompletion originally was.

[Peter Kasting, 2013/07/16 21:48:50]

Nit: Typo + confusing, how about:

OnPopupDataChanged() resets OmniboxController's |current_match_| early on. 
Therefore, copy match.inline_autocompletion to a temp to preserve its value
across the entire call.

[Mark P, 2013/07/16 23:34:08]

Done.

(You're a good editor!)

+  const string16 inline_autocompletion = match.inline_autocompletion;

[Peter Kasting, 2013/07/16 21:48:50]

Tiny nit: I slightly prefer constructor-style init for non-POD types.

[Mark P, 2013/07/16 23:34:08]

Done.

+  OnPopupDataChanged(inline_autocompletion, NULL, keyword, is_keyword_hint);
 }
 
 string16 OmniboxEditModel::GetViewText() const {