Fixes bug where keyword editor would end up prefixing all keyword urls

with "http://" if they didn't have one. This proves problematic as for
the google search url we don't have http:// and don't want it. The fix
is to only add http:// if the url field is editable. If the url field
isn't editable, we know the user hasn't editted it and don't need to
try and fix it up.

BUG=13282
TEST=see bug, but also make sure you don't run into any other problems
with the keyword editor.

Review URL: http://codereview.chromium.org/126052

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

6 files changed, 38 insertions, 37 deletions

diff --git a/chrome/browser/search_engines/template_url.cc b/chrome/browser/search_engines/template_url.cc
index 9e9a5c9..e749e89 100644
--- a/chrome/browser/search_engines/template_url.cc
+++ b/chrome/browser/search_engines/template_url.cc
@@ -230,17 +230,17 @@ void TemplateURLRef::ParseHostAndSearchTermKey() const {
   }
 }
 
-GURL TemplateURLRef::ReplaceSearchTerms(
+std::wstring TemplateURLRef::ReplaceSearchTerms(
     const TemplateURL& host,
     const std::wstring& terms,
     int accepted_suggestion,
     const std::wstring& original_query_for_suggestion) const {
   ParseIfNecessary();
   if (!valid_)
-    return GURL();
+    return std::wstring();
 
   if (replacements_.empty())
-    return GURL(WideToUTF8(parsed_url_));
+    return parsed_url_;
 
   // Encode the search terms so that we know the encoding.
   const std::vector<std::string>& encodings = host.input_encodings();
@@ -330,7 +330,7 @@ GURL TemplateURLRef::ReplaceSearchTerms(
     }
   }
 
-  return GURL(WideToUTF8(url));
+  return url;
 }
 
 bool TemplateURLRef::SupportsReplacement() const {
diff --git a/chrome/browser/search_engines/template_url.h b/chrome/browser/search_engines/template_url.h
index d59a420..5abcb14 100644
--- a/chrome/browser/search_engines/template_url.h
+++ b/chrome/browser/search_engines/template_url.h
@@ -63,7 +63,7 @@ class TemplateURLRef {
   // returns false), an empty string is returned.
   //
   // The TemplateURL is used to determine the input encoding for the term.
-  GURL ReplaceSearchTerms(
+  std::wstring ReplaceSearchTerms(
       const TemplateURL& host,
       const std::wstring& terms,
       int accepted_suggestion,
diff --git a/chrome/browser/search_engines/template_url_model.cc b/chrome/browser/search_engines/template_url_model.cc
index 5a36b0e..8953673 100644
--- a/chrome/browser/search_engines/template_url_model.cc
+++ b/chrome/browser/search_engines/template_url_model.cc
@@ -196,10 +196,9 @@ GURL TemplateURLModel::GenerateSearchURL(const TemplateURL* t_url) {
   if (!search_ref->SupportsReplacement())
     return GURL(WideToUTF8(search_ref->url()));
 
-  return search_ref->ReplaceSearchTerms(
-      *t_url,
-      kReplacementTerm,
-      TemplateURLRef::NO_SUGGESTIONS_AVAILABLE, std::wstring());
+  return GURL(WideToUTF8(search_ref->ReplaceSearchTerms(
+      *t_url, kReplacementTerm, TemplateURLRef::NO_SUGGESTIONS_AVAILABLE,
+      std::wstring())));
 }
 
 bool TemplateURLModel::CanReplaceKeyword(
diff --git a/chrome/browser/search_engines/template_url_model_unittest.cc b/chrome/browser/search_engines/template_url_model_unittest.cc
index 4a4bace..1889262 100644
--- a/chrome/browser/search_engines/template_url_model_unittest.cc
+++ b/chrome/browser/search_engines/template_url_model_unittest.cc
@@ -632,8 +632,8 @@ TEST_F(TemplateURLModelTest, ChangeGoogleBaseValue) {
   EXPECT_TRUE(model_->GetTemplateURLForHost("google.com") == NULL);
   EXPECT_EQ("foo.com", t_url->url()->GetHost());
   EXPECT_EQ(L"foo.com", t_url->keyword());
-  EXPECT_EQ("http://foo.com/?q=x", t_url->url()->ReplaceSearchTerms(*t_url,
-      L"x", TemplateURLRef::NO_SUGGESTIONS_AVAILABLE, std::wstring()).spec());
+  EXPECT_EQ(L"http://foo.com/?q=x", t_url->url()->ReplaceSearchTerms(*t_url,
+      L"x", TemplateURLRef::NO_SUGGESTIONS_AVAILABLE, std::wstring()));
 }
 
 struct QueryHistoryCallbackImpl {
@@ -669,7 +669,8 @@ TEST_F(TemplateURLModelTest, GenerateVisitOnKeyword) {
   HistoryService* history =
       profile_->GetHistoryService(Profile::EXPLICIT_ACCESS);
   history->AddPage(
-      t_url->url()->ReplaceSearchTerms(*t_url, L"blah", 0, std::wstring()),
+      GURL(WideToUTF8(t_url->url()->ReplaceSearchTerms(*t_url, L"blah", 0,
+                                                       std::wstring()))),
       NULL, 0, GURL(), PageTransition::KEYWORD, HistoryService::RedirectList());
 
   // Wait for history to finish processing the request.
diff --git a/chrome/browser/search_engines/template_url_prepopulate_data.cc b/chrome/browser/search_engines/template_url_prepopulate_data.cc
index 414eb0e..1c97eec 100644
--- a/chrome/browser/search_engines/template_url_prepopulate_data.cc
+++ b/chrome/browser/search_engines/template_url_prepopulate_data.cc
@@ -3185,7 +3185,7 @@ void RegisterUserPrefs(PrefService* prefs) {
 }
 
 int GetDataVersion() {
-  return 21;  // Increment this if you change the above data in ways that mean
+  return 22;  // Increment this if you change the above data in ways that mean
               // users with existing data should get a new version.
 }
 
diff --git a/chrome/browser/search_engines/template_url_unittest.cc b/chrome/browser/search_engines/template_url_unittest.cc
index 7a44b6c..5aa9ce5b 100644
--- a/chrome/browser/search_engines/template_url_unittest.cc
+++ b/chrome/browser/search_engines/template_url_unittest.cc
@@ -43,8 +43,8 @@ TEST_F(TemplateURLTest, URLRefTestSearchTerms) {
   ASSERT_TRUE(ref.IsValid());
 
   ASSERT_TRUE(ref.SupportsReplacement());
-  GURL result = ref.ReplaceSearchTerms(t_url, L"search",
-      TemplateURLRef::NO_SUGGESTIONS_AVAILABLE, std::wstring());
+  GURL result = GURL(WideToUTF8(ref.ReplaceSearchTerms(t_url, L"search",
+      TemplateURLRef::NO_SUGGESTIONS_AVAILABLE, std::wstring())));
   ASSERT_TRUE(result.is_valid());
   ASSERT_EQ("http://foosearch/", result.spec());
 }
@@ -54,8 +54,8 @@ TEST_F(TemplateURLTest, URLRefTestCount) {
   TemplateURLRef ref(L"http://foo{searchTerms}{count?}", 0, 0);
   ASSERT_TRUE(ref.IsValid());
   ASSERT_TRUE(ref.SupportsReplacement());
-  GURL result = ref.ReplaceSearchTerms(t_url, L"X",
-      TemplateURLRef::NO_SUGGESTIONS_AVAILABLE, std::wstring());
+  GURL result = GURL(WideToUTF8(ref.ReplaceSearchTerms(t_url, L"X",
+      TemplateURLRef::NO_SUGGESTIONS_AVAILABLE, std::wstring())));
   ASSERT_TRUE(result.is_valid());
   ASSERT_EQ("http://foox/", result.spec());
 }
@@ -65,8 +65,8 @@ TEST_F(TemplateURLTest, URLRefTestCount2) {
   TemplateURLRef ref(L"http://foo{searchTerms}{count}", 0, 0);
   ASSERT_TRUE(ref.IsValid());
   ASSERT_TRUE(ref.SupportsReplacement());
-  GURL result = ref.ReplaceSearchTerms(t_url, L"X",
-      TemplateURLRef::NO_SUGGESTIONS_AVAILABLE, std::wstring());
+  GURL result = GURL(WideToUTF8(ref.ReplaceSearchTerms(t_url, L"X",
+      TemplateURLRef::NO_SUGGESTIONS_AVAILABLE, std::wstring())));
   ASSERT_TRUE(result.is_valid());
   ASSERT_EQ("http://foox10/", result.spec());
 }
@@ -77,8 +77,8 @@ TEST_F(TemplateURLTest, URLRefTestIndices) {
                      1, 2);
   ASSERT_TRUE(ref.IsValid());
   ASSERT_TRUE(ref.SupportsReplacement());
-  GURL result = ref.ReplaceSearchTerms(t_url, L"X",
-      TemplateURLRef::NO_SUGGESTIONS_AVAILABLE, std::wstring());
+  GURL result = GURL(WideToUTF8(ref.ReplaceSearchTerms(t_url, L"X",
+      TemplateURLRef::NO_SUGGESTIONS_AVAILABLE, std::wstring())));
   ASSERT_TRUE(result.is_valid());
   ASSERT_EQ("http://fooxxy/", result.spec());
 }
@@ -88,8 +88,8 @@ TEST_F(TemplateURLTest, URLRefTestIndices2) {
   TemplateURLRef ref(L"http://foo{searchTerms}x{startIndex}y{startPage}", 1, 2);
   ASSERT_TRUE(ref.IsValid());
   ASSERT_TRUE(ref.SupportsReplacement());
-  GURL result = ref.ReplaceSearchTerms(t_url, L"X",
-      TemplateURLRef::NO_SUGGESTIONS_AVAILABLE, std::wstring());
+  GURL result = GURL(WideToUTF8(ref.ReplaceSearchTerms(t_url, L"X",
+      TemplateURLRef::NO_SUGGESTIONS_AVAILABLE, std::wstring())));
   ASSERT_TRUE(result.is_valid());
   ASSERT_EQ("http://fooxx1y2/", result.spec());
 }
@@ -100,8 +100,8 @@ TEST_F(TemplateURLTest, URLRefTestEncoding) {
       L"http://foo{searchTerms}x{inputEncoding?}y{outputEncoding?}a", 1, 2);
   ASSERT_TRUE(ref.IsValid());
   ASSERT_TRUE(ref.SupportsReplacement());
-  GURL result = ref.ReplaceSearchTerms(t_url, L"X",
-      TemplateURLRef::NO_SUGGESTIONS_AVAILABLE, std::wstring());
+  GURL result = GURL(WideToUTF8(ref.ReplaceSearchTerms(t_url, L"X",
+      TemplateURLRef::NO_SUGGESTIONS_AVAILABLE, std::wstring())));
   ASSERT_TRUE(result.is_valid());
   ASSERT_EQ("http://fooxxutf-8ya/", result.spec());
 }
@@ -112,8 +112,8 @@ TEST_F(TemplateURLTest, InputEncodingBeforeSearchTerm) {
       L"http://foox{inputEncoding?}a{searchTerms}y{outputEncoding?}b", 1, 2);
   ASSERT_TRUE(ref.IsValid());
   ASSERT_TRUE(ref.SupportsReplacement());
-  GURL result = ref.ReplaceSearchTerms(t_url, L"X",
-      TemplateURLRef::NO_SUGGESTIONS_AVAILABLE, std::wstring());
+  GURL result = GURL(WideToUTF8(ref.ReplaceSearchTerms(t_url, L"X",
+      TemplateURLRef::NO_SUGGESTIONS_AVAILABLE, std::wstring())));
   ASSERT_TRUE(result.is_valid());
   ASSERT_EQ("http://fooxutf-8axyb/", result.spec());
 }
@@ -124,8 +124,8 @@ TEST_F(TemplateURLTest, URLRefTestEncoding2) {
       L"http://foo{searchTerms}x{inputEncoding}y{outputEncoding}a", 1, 2);
   ASSERT_TRUE(ref.IsValid());
   ASSERT_TRUE(ref.SupportsReplacement());
-  GURL result = ref.ReplaceSearchTerms(t_url, L"X",
-      TemplateURLRef::NO_SUGGESTIONS_AVAILABLE, std::wstring());
+  GURL result = GURL(WideToUTF8(ref.ReplaceSearchTerms(t_url, L"X",
+      TemplateURLRef::NO_SUGGESTIONS_AVAILABLE, std::wstring())));
   ASSERT_TRUE(result.is_valid());
   ASSERT_EQ("http://fooxxutf-8yutf-8a/", result.spec());
 }
@@ -239,8 +239,8 @@ TEST_F(TemplateURLTest, ReplaceSearchTerms) {
     std::string expected_result = data[i].expected_result;
     ReplaceSubstringsAfterOffset(&expected_result, 0, "{language}",
         WideToASCII(g_browser_process->GetApplicationLocale()));
-    GURL result = ref.ReplaceSearchTerms(turl, L"X",
-        TemplateURLRef::NO_SUGGESTIONS_AVAILABLE, std::wstring());
+    GURL result = GURL(WideToUTF8(ref.ReplaceSearchTerms(turl, L"X",
+        TemplateURLRef::NO_SUGGESTIONS_AVAILABLE, std::wstring())));
     EXPECT_TRUE(result.is_valid());
     EXPECT_EQ(expected_result, result.spec());
   }
@@ -265,8 +265,9 @@ TEST_F(TemplateURLTest, ReplaceArbitrarySearchTerms) {
     TemplateURL turl;
     turl.add_input_encoding(data[i].encoding);
     TemplateURLRef ref(data[i].url, 1, 2);
-    GURL result = ref.ReplaceSearchTerms(turl, data[i].search_term,
-        TemplateURLRef::NO_SUGGESTIONS_AVAILABLE, std::wstring());
+    GURL result = GURL(WideToUTF8(ref.ReplaceSearchTerms(turl,
+        data[i].search_term, TemplateURLRef::NO_SUGGESTIONS_AVAILABLE,
+        std::wstring())));
     EXPECT_TRUE(result.is_valid());
     EXPECT_EQ(data[i].expected_result, result.spec());
   }
@@ -296,8 +297,8 @@ TEST_F(TemplateURLTest, Suggestions) {
   ASSERT_TRUE(ref.IsValid());
   ASSERT_TRUE(ref.SupportsReplacement());
   for (size_t i = 0; i < ARRAYSIZE_UNSAFE(data); ++i) {
-    GURL result = ref.ReplaceSearchTerms(turl, L"foobar",
-        data[i].accepted_suggestion, data[i].original_query_for_suggestion);
+    GURL result = GURL(WideToUTF8(ref.ReplaceSearchTerms(turl, L"foobar",
+        data[i].accepted_suggestion, data[i].original_query_for_suggestion)));
     EXPECT_TRUE(result.is_valid());
     EXPECT_EQ(data[i].expected_result, result.spec());
   }
@@ -314,8 +315,8 @@ TEST_F(TemplateURLTest, RLZ) {
   TemplateURLRef ref(L"http://bar/?{google:RLZ}{searchTerms}", 1, 2);
   ASSERT_TRUE(ref.IsValid());
   ASSERT_TRUE(ref.SupportsReplacement());
-  GURL result = ref.ReplaceSearchTerms(t_url, L"x",
-      TemplateURLRef::NO_SUGGESTIONS_AVAILABLE, std::wstring());
+  GURL result = GURL(WideToUTF8(ref.ReplaceSearchTerms(t_url, L"x",
+      TemplateURLRef::NO_SUGGESTIONS_AVAILABLE, std::wstring())));
   ASSERT_TRUE(result.is_valid());
   std::string expected_url = "http://bar/?";
   if (!rlz_string.empty()) {