Fix cursor position for default provider searches in keyword mode.

chrome/browser/autocomplete/keyword_provider.cc

Index: chrome/browser/autocomplete/keyword_provider.cc
diff --git a/chrome/browser/autocomplete/keyword_provider.cc b/chrome/browser/autocomplete/keyword_provider.cc
index c37da20e228f36040e821a071aecc4df86b70668..a5e1907716b9b659154f5174682c48e97003bffd 100644
--- a/chrome/browser/autocomplete/keyword_provider.cc
+++ b/chrome/browser/autocomplete/keyword_provider.cc
@@ -8,6 +8,7 @@
 #include <vector>
 
 #include "base/string16.h"
+#include "base/string_util.h"
 #include "base/utf_string_conversions.h"
 #include "chrome/browser/autocomplete/autocomplete_match.h"
 #include "chrome/browser/autocomplete/autocomplete_provider_listener.h"
@@ -136,25 +137,34 @@ string16 KeywordProvider::SplitReplacementStringFromInput(
 
 // static
 const TemplateURL* KeywordProvider::GetSubstitutingTemplateURLForInput(
-    Profile* profile,
-    const AutocompleteInput& input,
-    string16* remaining_input) {
-  if (!input.allow_exact_keyword_match())
+    TemplateURLService* model,
+    AutocompleteInput* input) {
+  if (!input->allow_exact_keyword_match())
     return NULL;
 
-  string16 keyword;
-  if (!ExtractKeywordFromInput(input, &keyword, remaining_input))
+  string16 keyword, remaining_input;
+  if (!ExtractKeywordFromInput(*input, &keyword, &remaining_input))
     return NULL;
 
-  // Make sure the model is loaded. This is cheap and quickly bails out if
-  // the model is already loaded.
-  TemplateURLService* model = TemplateURLServiceFactory::GetForProfile(profile);
   DCHECK(model);
-  model->Load();
-
   const TemplateURL* template_url = model->GetTemplateURLForKeyword(keyword);
-  return (template_url && template_url->SupportsReplacement()) ?
-      template_url : NULL;
+  if (template_url && template_url->SupportsReplacement()) {
+    // Adjust cursor position iff it was set before, otherwise leave it as is.
+    size_t cursor_position = input->cursor_position();
+    // The adjustment assumes that the keyword was stripped from the beginning
+    // of the original input.
+    if (cursor_position != string16::npos &&
+        EndsWith(input->text(), remaining_input, false)) {

[Mark P, 2013/01/25 21:19:15]

Why you being case insensitive here?

[Mark P, 2013/01/25 21:19:15]

Under what conditions does this EndsWith fail?

[Bart N., 2013/01/29 21:28:17]

Actually, the original reason is moot here. I've changed it as well as updated
the unit test to reflect the change.

[Bart N., 2013/01/29 21:28:17]

When there is a trailing space in the original input. This part is something I
don't really like and I hope to fix in subsequent CLs (see our discussion).

+      int offset = input->text().length() - cursor_position;
+      cursor_position = offset <= 0 ? remaining_input.length() :

[Mark P, 2013/01/25 21:19:15]

Perhaps DCHECK that offset >= 0 before this line?

And for that matter DCHECK its <= input->text().length()

[Bart N., 2013/01/29 21:28:17]

Done.

+          (offset > static_cast<int>(remaining_input.length()) ? 0u :

[Mark P, 2013/01/25 21:19:15]

I think I understand why offset can be greater than  remaining_input.length but
I think this should be commented here.  And comment about the consequences of
what you're doing.

[Peter Kasting, 2013/01/28 00:04:22]

Please do not nest ?:s within a single statement.

[Bart N., 2013/01/29 21:28:17]

Done.

[Bart N., 2013/01/29 21:28:17]

Done.

+           remaining_input.length() - offset);
+    }
+    input->UpdateText(remaining_input, cursor_position, input->parts());
+    return template_url;
+  }
+
+  return NULL;
 }
 
 string16 KeywordProvider::GetKeywordForText(