Extend the default search provider sanity checks.

BUG=116952
TEST=TemplateURLServiceTest.*, DefaultSearchProviderChangeTest.*


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

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

2 files changed, 126 insertions, 5 deletions

diff --git a/chrome/browser/protector/default_search_provider_change.cc b/chrome/browser/protector/default_search_provider_change.cc
index 012657e..dbd0027 100644
--- a/chrome/browser/protector/default_search_provider_change.cc
+++ b/chrome/browser/protector/default_search_provider_change.cc
@@ -121,6 +121,10 @@ class DefaultSearchProviderChange : public BaseSettingChange,
   const TemplateURL* SetDefaultSearchProvider(
       scoped_ptr<TemplateURL>* search_provider);
 
+  // Returns true if |new_search_provider_| can be used as the default search
+  // provider.
+  bool NewSearchProviderValid() const;
+
   // Opens the Search engine settings page in a new tab.
   void OpenSearchEngineSettings(Browser* browser);
 
@@ -186,7 +190,8 @@ bool DefaultSearchProviderChange::Init(Profile* profile) {
   if (!BaseSettingChange::Init(profile))
     return false;
 
-  if (!backup_search_provider_.get()) {
+  if (!backup_search_provider_.get() ||
+      !backup_search_provider_->SupportsReplacement()) {
     // Fallback to a prepopulated default search provider, ignoring any
     // overrides in Prefs.
     backup_search_provider_.reset(
@@ -246,7 +251,7 @@ void DefaultSearchProviderChange::Apply(Browser* browser) {
       kProtectorMaxSearchProviderID);
 
   GetTemplateURLService()->RemoveObserver(this);
-  if (new_search_provider_) {
+  if (NewSearchProviderValid()) {
     GetTemplateURLService()->SetDefaultSearchProvider(new_search_provider_);
   } else {
     // Open settings page in case the new setting is invalid.
@@ -302,7 +307,7 @@ string16 DefaultSearchProviderChange::GetBubbleMessage() const {
 }
 
 string16 DefaultSearchProviderChange::GetApplyButtonText() const {
-  if (new_search_provider_) {
+  if (NewSearchProviderValid()) {
     // If backup search engine is lost and fallback was made to the current
     // search provider then there is no need to show this button.
     if (fallback_is_new_)
@@ -333,7 +338,7 @@ string16 DefaultSearchProviderChange::GetDiscardButtonText() const {
 
 BaseSettingChange::DisplayName
 DefaultSearchProviderChange::GetApplyDisplayName() const {
-  if (new_search_provider_ &&
+  if (NewSearchProviderValid() &&
       new_search_provider_->short_name().length() <= kMaxDisplayedNameLength) {
     return DisplayName(kDefaultSearchProviderChangeNamePriority,
                        new_search_provider_->short_name());
@@ -348,7 +353,7 @@ GURL DefaultSearchProviderChange::GetNewSettingURL() const {
     // and Discard behaviour is pretty different.
     return GURL();
   }
-  if (!new_search_provider_)
+  if (!NewSearchProviderValid())
     return GURL();
   return TemplateURLService::GenerateSearchURL(new_search_provider_);
 }
@@ -410,6 +415,10 @@ const TemplateURL* DefaultSearchProviderChange::SetDefaultSearchProvider(
   return new_default_provider;
 }
 
+bool DefaultSearchProviderChange::NewSearchProviderValid() const {
+  return new_search_provider_ && new_search_provider_->SupportsReplacement();
+}
+
 void DefaultSearchProviderChange::OpenSearchEngineSettings(Browser* browser) {
   ProtectorServiceFactory::GetForProfile(profile())->OpenTab(
       GURL(std::string(chrome::kChromeUISettingsURL) +
diff --git a/chrome/browser/protector/default_search_provider_change_browsertest.cc b/chrome/browser/protector/default_search_provider_change_browsertest.cc
index e5a63e7..5d8aca8 100644
--- a/chrome/browser/protector/default_search_provider_change_browsertest.cc
+++ b/chrome/browser/protector/default_search_provider_change_browsertest.cc
@@ -34,10 +34,14 @@ namespace {
 const string16 example_info = ASCIIToUTF16("Example.info");
 const string16 example_info_long = ASCIIToUTF16("ExampleSearchEngine.info");
 const std::string http_example_info = "http://example.info/?q={searchTerms}";
+const std::string http_example_info_no_search_terms =
+    "http://example.net/";
 const std::string example_info_domain = "example.info";
 const string16 example_com = ASCIIToUTF16("Example.com");
 const string16 example_com_long = ASCIIToUTF16("ExampleSearchEngine.com");
 const std::string http_example_com = "http://example.com/?q={searchTerms}";
+const std::string http_example_com_no_search_terms =
+    "http://example.com/";
 const std::string example_com_domain = "example.com";
 const string16 example_net = ASCIIToUTF16("Example.net");
 const std::string http_example_net = "http://example.net/?q={searchTerms}";
@@ -302,6 +306,63 @@ IN_PROC_BROWSER_TEST_F(DefaultSearchProviderChangeTest, BackupInvalid) {
             turl_service_->GetDefaultSearchProvider());
 }
 
+
+IN_PROC_BROWSER_TEST_F(DefaultSearchProviderChangeTest,
+                       BackupValidCannotBeDefault) {
+  // Backup is a valid keyword but cannot be the default search provider (has no
+  // search terms), current search provider exists, fallback to the prepopulated
+  // default search, which exists among keywords.
+  TemplateURL* backup_url =
+      MakeTemplateURL(example_info, ASCIIToUTF16("a"),
+                      http_example_info_no_search_terms);
+  int prepopulated_histogram_id =
+      protector::GetSearchProviderHistogramID(prepopulated_url_.get());
+  TemplateURL* current_url =
+      MakeTemplateURL(example_com, ASCIIToUTF16("b"), http_example_com);
+  int current_histogram_id =
+      protector::GetSearchProviderHistogramID(current_url);
+
+  AddAndSetDefault(current_url);
+
+  // Prepopulated default search must exist.
+  ASSERT_TRUE(FindTemplateURL(prepopulated_url_->url()));
+
+  scoped_ptr<BaseSettingChange> change(
+      CreateDefaultSearchProviderChange(current_url, backup_url));
+  ASSERT_TRUE(change.get());
+  ASSERT_TRUE(change->Init(browser()->profile()));
+
+  // Verify that the prepopulated default search is active.
+  EXPECT_EQ(FindTemplateURL(prepopulated_url_->url()),
+            turl_service_->GetDefaultSearchProvider());
+
+  // Verify histograms.
+  ExpectHistogramCount(kProtectorHistogramSearchProviderHijacked,
+                       current_histogram_id, 1);
+  ExpectHistogramCount(kProtectorHistogramSearchProviderRestored,
+                       prepopulated_histogram_id, 1);
+  ExpectHistogramCount(kProtectorHistogramSearchProviderFallback,
+                       prepopulated_histogram_id, 1);
+
+  // Verify text messages.
+  EXPECT_EQ(GetBubbleMessage(prepopulated_url_->short_name()),
+            change->GetBubbleMessage());
+  EXPECT_EQ(GetChangeSearchButtonText(example_com),
+            change->GetApplyButtonText());
+  EXPECT_EQ(GetOpenSettingsButtonText(), change->GetDiscardButtonText());
+
+  // Verify that search engine settings are opened by Discard.
+  ExpectSettingsOpened(chrome::kSearchEnginesSubPage);
+  change->Discard(browser());
+  EXPECT_EQ(FindTemplateURL(prepopulated_url_->url()),
+            turl_service_->GetDefaultSearchProvider());
+
+  // Verify that Apply switches back to |current_url|.
+  change->Apply(browser());
+  EXPECT_EQ(FindTemplateURL(http_example_com),
+            turl_service_->GetDefaultSearchProvider());
+}
+
 IN_PROC_BROWSER_TEST_F(DefaultSearchProviderChangeTest,
                        BackupInvalidFallbackRemoved) {
   // Backup is invalid, current search provider exists, fallback to the
@@ -408,6 +469,57 @@ IN_PROC_BROWSER_TEST_F(DefaultSearchProviderChangeTest,
 }
 
 IN_PROC_BROWSER_TEST_F(DefaultSearchProviderChangeTest,
+                       BackupValidCurrentCannotBeDefault) {
+  // Backup is valid, current search provider has no search terms.
+  TemplateURL* backup_url =
+      MakeTemplateURL(example_info, ASCIIToUTF16("a"), http_example_info);
+  int backup_histogram_id = protector::GetSearchProviderHistogramID(backup_url);
+  TemplateURL* current_url =
+      MakeTemplateURL(example_com, ASCIIToUTF16("b"),
+                      http_example_com_no_search_terms);
+  int current_histogram_id =
+      protector::GetSearchProviderHistogramID(current_url);
+
+  turl_service_->Add(new TemplateURL(backup_url->data()));
+  // TODO(ivankr): this may become unsupported soon.
+  AddAndSetDefault(current_url);
+
+  scoped_ptr<BaseSettingChange> change(
+      CreateDefaultSearchProviderChange(current_url, backup_url));
+  ASSERT_TRUE(change.get());
+  ASSERT_TRUE(change->Init(browser()->profile()));
+
+  // Verify that backup is active.
+  EXPECT_EQ(FindTemplateURL(http_example_info),
+            turl_service_->GetDefaultSearchProvider());
+
+  // Verify histograms.
+  ExpectHistogramCount(kProtectorHistogramSearchProviderHijacked,
+                       current_histogram_id, 1);
+  ExpectHistogramCount(kProtectorHistogramSearchProviderRestored,
+                       backup_histogram_id, 1);
+
+  // Verify text messages.
+  EXPECT_EQ(GetBubbleMessage(), change->GetBubbleMessage());
+  EXPECT_EQ(GetOpenSettingsButtonText(), change->GetApplyButtonText());
+  EXPECT_EQ(GetKeepSearchButtonText(example_info),
+            change->GetDiscardButtonText());
+  EXPECT_EQ(GURL(), change->GetNewSettingURL());
+  EXPECT_EQ(kNoDisplayName, change->GetApplyDisplayName());
+
+  // Discard does nothing - backup was already active.
+  change->Discard(browser());
+  EXPECT_EQ(FindTemplateURL(http_example_info),
+            turl_service_->GetDefaultSearchProvider());
+
+   // Verify that search engine settings are opened by Apply (no other changes).
+  ExpectSettingsOpened(chrome::kSearchEnginesSubPage);
+  change->Apply(browser());
+  EXPECT_EQ(FindTemplateURL(http_example_info),
+            turl_service_->GetDefaultSearchProvider());
+}
+
+IN_PROC_BROWSER_TEST_F(DefaultSearchProviderChangeTest,
                        BackupInvalidCurrentRemoved) {
   // Backup is invalid, no current search provider, fallback to the prepopulated
   // default search.