diff options
author | ivankr@chromium.org <ivankr@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2011-12-02 09:00:25 +0000 |
---|---|---|
committer | ivankr@chromium.org <ivankr@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2011-12-02 09:00:25 +0000 |
commit | 5bff94bb94ea8a1c774c4069de28acfdc21a13d3 (patch) | |
tree | 2b03b285da3f3180042382465b68047b9a58d878 /chrome/browser/protector | |
parent | e3b8f1dd30c6fe7ee4fed720e662b71b713e8f5c (diff) | |
download | chromium_src-5bff94bb94ea8a1c774c4069de28acfdc21a13d3.zip chromium_src-5bff94bb94ea8a1c774c4069de28acfdc21a13d3.tar.gz chromium_src-5bff94bb94ea8a1c774c4069de28acfdc21a13d3.tar.bz2 |
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
Diffstat (limited to 'chrome/browser/protector')
-rw-r--r-- | chrome/browser/protector/default_search_provider_change.cc | 129 | ||||
-rw-r--r-- | chrome/browser/protector/histograms.cc | 6 | ||||
-rw-r--r-- | chrome/browser/protector/histograms.h | 9 |
3 files changed, 122 insertions, 22 deletions
diff --git a/chrome/browser/protector/default_search_provider_change.cc b/chrome/browser/protector/default_search_provider_change.cc index a9ade1d..feb2210 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; } diff --git a/chrome/browser/protector/histograms.cc b/chrome/browser/protector/histograms.cc index 2669d69..b4b416c 100644 --- a/chrome/browser/protector/histograms.cc +++ b/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"; diff --git a/chrome/browser/protector/histograms.h b/chrome/browser/protector/histograms.h index c27c379..5dc301e 100644 --- a/chrome/browser/protector/histograms.h +++ b/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[]; |