Index: chrome/browser/protector/default_search_provider_change.cc |
diff --git a/chrome/browser/protector/default_search_provider_change.cc b/chrome/browser/protector/default_search_provider_change.cc |
index a9ade1d2c9cbadf24c5cf3774c2636717b020070..feb221021856491b05bd78ee474be17e58c85586 100644 |
--- a/chrome/browser/protector/default_search_provider_change.cc |
+++ b/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; |
} |