Cursor jumps to end of omnibox when activating keyword mode via inserted space

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 8232aae8b8de4a26080de0f111343c22d7212a14..19f59cf82bc907b98757584a38a1276ae96727b5 100644
--- a/chrome/browser/ui/omnibox/omnibox_edit_model.cc
+++ b/chrome/browser/ui/omnibox/omnibox_edit_model.cc
@@ -883,10 +883,16 @@ void OmniboxEditModel::OnPopupDataChanged(
 
   bool call_controller_onchanged = true;
   inline_autocomplete_text_ = text;
-  if (view_->OnInlineAutocompleteTextMaybeChanged(
-      DisplayTextFromUserText(user_text_ + inline_autocomplete_text_),
-      DisplayTextFromUserText(user_text_).length()))
+  if (KeywordIsSelected() &&
+      view_->GetText() != DisplayTextFromUserText(user_text_)) {

[Peter Kasting, 2012/10/02 07:37:12]

Does this do the right thing in cases where we have (new or existing) inline
autocompletion in keyword mode?  Or when we're tabbing through the dropdown and
thus have temporary text?  I'm worried about those causing cases where the
display text does not match the user text and thus erroneously triggering this
block.

[Joe Thomas, 2012/10/02 18:39:23]

When keyword search mode is activated, AFAIK there is no more inline
autocompletion. OmniboxEditModel::OnPopupDataChanged is always called with  NULL
value for 'text' parameter while 'keyword' search mode is active. And this sets
the existing inline_autocomplete_text_ to NULL in
OmniboxEditModel::OnPopupDataChanged.
This case is handled in the block just above and it returns from there.

[Peter Kasting, 2012/10/02 18:42:35]

We can definitely inline autocomplete while in keyword search mode.  I just
tested now: tab-to-search google.com and type "test"; then after the page loads,
do the same thing, and "test" will inline autocomplete as soon as you type the
't'.

[Joe Thomas, 2012/10/02 18:57:24]

Yes, you are right. The patch breaks that usecase. I will re-work on the patch.
I did not know that there is an inline autocompletion available for search
strings. Sorry for my ignorance.

+    view_->SetWindowTextAndCaretPos(DisplayTextFromUserText(user_text_), 0,
+        false, true);
+    call_controller_onchanged = false;
+  } else if (view_->OnInlineAutocompleteTextMaybeChanged(
+                DisplayTextFromUserText(user_text_ + inline_autocomplete_text_),
+                DisplayTextFromUserText(user_text_).length())) {
     call_controller_onchanged = false;
+  }
 
   // If |has_temporary_text_| is true, then we previously had a manual selection
   // but now don't (or |destination_for_temporary_text_change| would have been