Protector adds the default prepopulated engine if it was removed.

*) If the backup default search provider was deleted from the keywords, it is taken from the prepopulated data array.
*) If a search provider with same URLs already exists, it is updated; otherwise, the prepopulated SP is added.

BUG=105423
TEST=TemplateURLServiceTest.*; For Protector: manual with sqlite3

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

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

chrome/browser/protector/default_search_provider_change.cc

@@ -5,11 +5,13 @@
 #include "base/basictypes.h"
 #include "base/compiler_specific.h"
 #include "base/logging.h"
+#include "base/memory/scoped_ptr.h"
 #include "base/metrics/histogram.h"
 #include "chrome/browser/protector/base_setting_change.h"
 #include "chrome/browser/protector/histograms.h"
 #include "chrome/browser/protector/protector.h"
 #include "chrome/browser/search_engines/template_url.h"
+#include "chrome/browser/search_engines/template_url_prepopulate_data.h"
 #include "chrome/browser/search_engines/template_url_service.h"
 #include "chrome/browser/search_engines/template_url_service_observer.h"
 #include "chrome/browser/webdata/keyword_table.h"
@@ -26,6 +28,60 @@ namespace {
 // Maximum length of the search engine name to be displayed.
 const size_t kMaxDisplayedNameLength = 10;
 
+// Predicate that matches a TemplateURL with given ID.
+class TemplateURLHasId {
+ public:
+  explicit TemplateURLHasId(TemplateURLID id) : id_(id) {
+  }
+
+  bool operator()(const TemplateURL* url) {
+    return url->id() == id_;
+  }
+
+ private:
+  TemplateURLID id_;
+};
+
+// Matches TemplateURL with all fields set from the prepopulated data equal
+// to fields in another TemplateURL.
+class TemplateURLIsSame {
+ public:
+  // Creates a matcher based on |other|.
+  explicit TemplateURLIsSame(const TemplateURL* other) : other_(other) {
+  }
+
+  // Returns true if both |other| and |url| are NULL or have same field values.
+  bool operator()(const TemplateURL* url) {
+    if (url == other_ )
+      return true;
+    if (!url || !other_)
+      return false;
+    return url->short_name() == other_->short_name() &&
+        AreKeywordsSame(url, other_) &&
+        TemplateURLRef::SameUrlRefs(url->url(), other_->url()) &&
+        TemplateURLRef::SameUrlRefs(url->suggestions_url(),
+                                    other_->suggestions_url()) &&
+        TemplateURLRef::SameUrlRefs(url->instant_url(),
+                                    other_->instant_url()) &&
+        url->GetFaviconURL() == other_->GetFaviconURL() &&
+        url->safe_for_autoreplace() == other_->safe_for_autoreplace() &&
+        url->show_in_default_list() == other_->show_in_default_list() &&
+        url->input_encodings() == other_->input_encodings() &&
+        url->logo_id() == other_->logo_id() &&
+        url->prepopulate_id() == other_->prepopulate_id();
+  }
+
+ private:
+  // Returns true if both |url1| and |url2| have autogenerated keywords
+  // or if their keywords are identical.
+  bool AreKeywordsSame(const TemplateURL* url1, const TemplateURL* url2) {
+    return (url1->autogenerate_keyword() && url2->autogenerate_keyword()) ||
+        url1->keyword() == url2->keyword();
+  }
+
+  const TemplateURL* other_;
+};
+
 }  // namespace
 
 class DefaultSearchProviderChange : public BaseSettingChange,
@@ -115,16 +171,26 @@ bool DefaultSearchProviderChange::Init(Protector* protector) {
   if (!default_search_provider_)
     return false;
 
+  int restored_histogram_id =
+      GetSearchProviderHistogramID(default_search_provider_);
+  UMA_HISTOGRAM_ENUMERATION(
+      kProtectorHistogramSearchProviderRestored,
+      restored_histogram_id,
+      kProtectorMaxSearchProviderID);
+
   if (!old_id_ || default_search_provider_->id() != old_id_) {
     // Old settings is lost or invalid, so we had to fall back to one of the
     // prepopulated search engines.
     fallback_id_ = default_search_provider_->id();
     fallback_name_ = default_search_provider_->short_name();
-    VLOG(1) << "Fallback to " << fallback_name_;
+
+    VLOG(1) << "Fallback to search provider: " << fallback_name_;
+    UMA_HISTOGRAM_ENUMERATION(
+        kProtectorHistogramSearchProviderFallback,
+        restored_histogram_id,
+        kProtectorMaxSearchProviderID);
   }
 
-  // This must be called after the initial |SetDefaultSearchProvider| call
-  // because the latter will remove the observer.
   protector->GetTemplateURLService()->AddObserver(this);
 
   return true;
@@ -136,6 +202,7 @@ void DefaultSearchProviderChange::Apply() {
       new_histogram_id_,
       kProtectorMaxSearchProviderID);
 
+  protector()->GetTemplateURLService()->RemoveObserver(this);
   if (!new_id_) {
     // Open settings page in case the new setting is invalid.
     OpenSearchEngineSettings();
@@ -150,6 +217,7 @@ void DefaultSearchProviderChange::Discard() {
       new_histogram_id_,
       kProtectorMaxSearchProviderID);
 
+  protector()->GetTemplateURLService()->RemoveObserver(this);
   if (!old_id_) {
     // Open settings page in case the old setting is invalid.
     OpenSearchEngineSettings();
@@ -214,10 +282,11 @@ string16 DefaultSearchProviderChange::GetDiscardButtonText() const {
 }
 
 void DefaultSearchProviderChange::OnTemplateURLServiceChanged() {
-  if (protector()->GetTemplateURLService()->GetDefaultSearchProvider() !=
-      default_search_provider_) {
-    default_search_provider_ = NULL;
+  TemplateURLService* url_service = protector()->GetTemplateURLService();
+  if (url_service->GetDefaultSearchProvider() != default_search_provider_) {
     VLOG(1) << "Default search provider has been changed by user";
+    default_search_provider_ = NULL;
+    url_service->RemoveObserver(this);
     // This will delete the Protector instance and |this|.
     protector()->DismissChange();
   }
@@ -231,29 +300,45 @@ const TemplateURL* DefaultSearchProviderChange::SetDefaultSearchProvider(
     NOTREACHED() << "Can't get TemplateURLService object.";
     return NULL;
   }
+
+  TemplateURLService::TemplateURLVector urls = url_service->GetTemplateURLs();
   const TemplateURL* url = NULL;
   if (id) {
-    const TemplateURLService::TemplateURLVector& urls =
-        url_service->GetTemplateURLs();
-    for (size_t i = 0; i < urls.size(); ++i) {
-      if (urls[i]->id() == id) {
-        url = urls[i];
-        break;
-      }
-    }
+    TemplateURLService::TemplateURLVector::const_iterator i =
+        find_if(urls.begin(), urls.end(), TemplateURLHasId(id));
+    if (i != urls.end())
+      url = *i;
   }
   if (!url && allow_fallback) {
-    url = url_service->FindNewDefaultSearchProvider();
-    DCHECK(url);
+    // Fallback to the prepopulated default search provider.
+    scoped_ptr<TemplateURL> new_url(
+        TemplateURLPrepopulateData::GetPrepopulatedDefaultSearch(
+            NULL  // Ignore any overrides in prefs.
+        ));
+    DCHECK(new_url.get());
+    VLOG(1) << "Prepopulated search provider: " << new_url->short_name();
+
+    // Check if this provider already exists and add it otherwise.
+    TemplateURLService::TemplateURLVector::const_iterator i =
+        find_if(urls.begin(), urls.end(), TemplateURLIsSame(new_url.get()));
+    if (i != urls.end()) {
+      VLOG(1) << "Provider already exists";
+      url = *i;
+    } else {
+      VLOG(1) << "No match, adding new provider";
+      url = new_url.get();
+      url_service->Add(new_url.release());
+      UMA_HISTOGRAM_ENUMERATION(
+          kProtectorHistogramSearchProviderMissing,
+          GetSearchProviderHistogramID(url),
+          kProtectorMaxSearchProviderID);
+    }
+    // TODO(ivankr): handle keyword conflicts with existing providers.
   }
+
   if (url) {
-    // Remove ourselves from the observer list to prevent from catching our own
-    // change. It is safe to do this multiple times or before adding ourselves.
-    url_service->RemoveObserver(this);
-    url_service->SetDefaultSearchProvider(url);
     VLOG(1) << "Default search provider set to: " << url->short_name();
-    // No need to re-add observer again because any further changes to the
-    // default search provider are of no interest.
+    url_service->SetDefaultSearchProvider(url);
   }
   return url;
 }

chrome/browser/protector/histograms.cc

@@ -25,6 +25,12 @@ const char kProtectorHistogramSearchProviderApplied[] =
     "Protector.SearchProvider.Applied";
 const char kProtectorHistogramSearchProviderDiscarded[] =
     "Protector.SearchProvider.Discarded";
+const char kProtectorHistogramSearchProviderFallback[] =
+    "Protector.SearchProvider.Fallback";
+const char kProtectorHistogramSearchProviderMissing[] =
+    "Protector.SearchProvider.Missing";
+const char kProtectorHistogramSearchProviderRestored[] =
+    "Protector.SearchProvider.Restored";
 const char kProtectorHistogramSearchProviderTimeout[] =
     "Protector.SearchProvider.Timeout";
 

chrome/browser/protector/histograms.h

@@ -37,6 +37,15 @@ extern const char kProtectorHistogramNewSearchProvider[];
 extern const char kProtectorHistogramSearchProviderApplied[];
 // Histogram name to report when user keeps previous default search provider.
 extern const char kProtectorHistogramSearchProviderDiscarded[];
+// Histogram name to report the fallback default search provider when the
+// backup value is invalid or doesn't match an existing provider.
+extern const char kProtectorHistogramSearchProviderFallback[];
+// Histogram name to report when the prepopulated default search provider was
+// missing and has been added for fallback.
+extern const char kProtectorHistogramSearchProviderMissing[];
+// Histogram name to report the default search provider restored by Protector
+// before showing user the bubble.
+extern const char kProtectorHistogramSearchProviderRestored[];
 // Histogram name to report when user ignores search provider change.
 extern const char kProtectorHistogramSearchProviderTimeout[];
 

chrome/browser/search_engines/template_url_prepopulate_data.h

@@ -35,6 +35,7 @@ void GetPrepopulatedEngines(PrefService* prefs,
 
 // Returns the default search provider specified by the prepopulate data.
 // The caller owns the returned value, which may be NULL.
+// If |prefs| is NULL, search provider overrides from preferences are not used.
 TemplateURL* GetPrepopulatedDefaultSearch(PrefService* prefs);
 
 // Returns a TemplateURL from the prepopulated data which has the same origin