Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(673)

Unified Diff: components/search_engines/template_url_fetcher.cc

Issue 1951153002: Remove AddSearchProvider (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: fix unit test Created 4 years, 7 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: components/search_engines/template_url_fetcher.cc
diff --git a/components/search_engines/template_url_fetcher.cc b/components/search_engines/template_url_fetcher.cc
index 96888e41b30775f245291e23f711a639014d1cbf..db7562c07ef678f45b7231f88c234b7e67ac37fe 100644
--- a/components/search_engines/template_url_fetcher.cc
+++ b/components/search_engines/template_url_fetcher.cc
@@ -26,9 +26,7 @@ class TemplateURLFetcher::RequestDelegate : public net::URLFetcherDelegate {
const base::string16& keyword,
const GURL& osdd_url,
const GURL& favicon_url,
- const URLFetcherCustomizeCallback& url_fetcher_customize_callback,
- const ConfirmAddSearchProviderCallback& confirm_add_callback,
- ProviderType provider_type);
+ const URLFetcherCustomizeCallback& url_fetcher_customize_callback);
// net::URLFetcherDelegate:
// If data contains a valid OSDD, a TemplateURL is created and added to
@@ -41,9 +39,6 @@ class TemplateURLFetcher::RequestDelegate : public net::URLFetcherDelegate {
// Keyword to use.
base::string16 keyword() const { return keyword_; }
- // The type of search provider being fetched.
- ProviderType provider_type() const { return provider_type_; }
-
private:
void OnLoaded();
void AddSearchProvider();
@@ -54,8 +49,6 @@ class TemplateURLFetcher::RequestDelegate : public net::URLFetcherDelegate {
base::string16 keyword_;
const GURL osdd_url_;
const GURL favicon_url_;
- const ProviderType provider_type_;
- ConfirmAddSearchProviderCallback confirm_add_callback_;
std::unique_ptr<TemplateURLService::Subscription> template_url_subscription_;
@@ -67,17 +60,13 @@ TemplateURLFetcher::RequestDelegate::RequestDelegate(
const base::string16& keyword,
const GURL& osdd_url,
const GURL& favicon_url,
- const URLFetcherCustomizeCallback& url_fetcher_customize_callback,
- const ConfirmAddSearchProviderCallback& confirm_add_callback,
- ProviderType provider_type)
+ const URLFetcherCustomizeCallback& url_fetcher_customize_callback)
: url_fetcher_(
net::URLFetcher::Create(osdd_url, net::URLFetcher::GET, this)),
fetcher_(fetcher),
keyword_(keyword),
osdd_url_(osdd_url),
- favicon_url_(favicon_url),
- provider_type_(provider_type),
- confirm_add_callback_(confirm_add_callback) {
+ favicon_url_(favicon_url) {
TemplateURLService* model = fetcher_->template_url_service_;
DCHECK(model); // TemplateURLFetcher::ScheduleDownload verifies this.
@@ -133,7 +122,7 @@ void TemplateURLFetcher::RequestDelegate::OnURLFetchComplete(
return;
}
- if (provider_type_ != AUTODETECTED_PROVIDER || keyword_.empty()) {
+ if (keyword_.empty()) {
// Use the parser-generated new keyword from the URL in the OSDD for the
// non-autodetected case. The existing |keyword_| was generated from the
// URL that hosted the OSDD, which results in the wrong keyword when the
@@ -158,15 +147,15 @@ void TemplateURLFetcher::RequestDelegate::AddSearchProvider() {
DCHECK(model->loaded());
TemplateURL* existing_url = NULL;
- if (model->CanAddAutogeneratedKeyword(keyword_, GURL(template_url_->url()),
- &existing_url)) {
- if (existing_url)
- model->Remove(existing_url);
- } else if (provider_type_ == AUTODETECTED_PROVIDER) {
+ if (!model->CanAddAutogeneratedKeyword(keyword_, GURL(template_url_->url()),
+ &existing_url)) {
fetcher_->RequestCompleted(this); // WARNING: Deletes us!
return;
}
+ if (existing_url)
+ model->Remove(existing_url);
+
// The short name is what is shown to the user. We preserve original names
// since it is better when generated keyword in many cases.
TemplateURLData data(template_url_->data());
@@ -177,27 +166,9 @@ void TemplateURLFetcher::RequestDelegate::AddSearchProvider() {
if (!data.favicon_url.is_valid())
data.favicon_url = favicon_url_;
- switch (provider_type_) {
- case AUTODETECTED_PROVIDER:
- // Mark the keyword as replaceable so it can be removed if necessary.
- data.safe_for_autoreplace = true;
- model->Add(new TemplateURL(data));
- break;
-
- case EXPLICIT_PROVIDER:
- // Confirm addition and allow user to edit default choices. It's ironic
- // that only *non*-autodetected additions get confirmed, but the user
- // expects feedback that his action did something.
- // The source WebContents' delegate takes care of adding the URL to the
- // model, which takes ownership, or of deleting it if the add is
- // cancelled.
- confirm_add_callback_.Run(base::WrapUnique(new TemplateURL(data)));
- break;
-
- default:
- NOTREACHED();
- break;
- }
+ // Mark the keyword as replaceable so it can be removed if necessary.
+ data.safe_for_autoreplace = true;
+ model->Add(new TemplateURL(data));
fetcher_->RequestCompleted(this);
// WARNING: RequestCompleted deletes us.
@@ -219,42 +190,31 @@ void TemplateURLFetcher::ScheduleDownload(
const base::string16& keyword,
const GURL& osdd_url,
const GURL& favicon_url,
- const URLFetcherCustomizeCallback& url_fetcher_customize_callback,
- const ConfirmAddSearchProviderCallback& confirm_add_callback,
- ProviderType provider_type) {
+ const URLFetcherCustomizeCallback& url_fetcher_customize_callback) {
DCHECK(osdd_url.is_valid());
+ DCHECK(!keyword.empty());
- // For a JS-added OSDD, the provided keyword is irrelevant because we will
- // generate a keyword later from the OSDD content. For the autodetected case,
- // we need a valid keyword up front.
- if (provider_type == TemplateURLFetcher::AUTODETECTED_PROVIDER) {
- DCHECK(!keyword.empty());
-
- if (!template_url_service_->loaded()) {
- // We could try to set up a callback to this function again once the model
- // is loaded but since this is an auto-add case anyway, meh.
- template_url_service_->Load();
- return;
- }
-
- const TemplateURL* template_url =
- template_url_service_->GetTemplateURLForKeyword(keyword);
- if (template_url && (!template_url->safe_for_autoreplace() ||
- template_url->originating_url() == osdd_url))
- return;
+ if (!template_url_service_->loaded()) {
+ // We could try to set up a callback to this function again once the model
+ // is loaded but since this is an auto-add case anyway, meh.
Peter Kasting 2016/05/09 22:24:00 Nit: Comment should maybe be updates since there's
Evan Stade 2016/05/09 22:54:53 Done.
+ template_url_service_->Load();
+ return;
}
+ const TemplateURL* template_url =
+ template_url_service_->GetTemplateURLForKeyword(keyword);
+ if (template_url && (!template_url->safe_for_autoreplace() ||
+ template_url->originating_url() == osdd_url))
+ return;
+
// Make sure we aren't already downloading this request.
for (Requests::iterator i = requests_.begin(); i != requests_.end(); ++i) {
- if (((*i)->url() == osdd_url) ||
- ((provider_type == TemplateURLFetcher::AUTODETECTED_PROVIDER) &&
- ((*i)->keyword() == keyword)))
+ if (((*i)->url() == osdd_url) || (((*i)->keyword() == keyword)))
Peter Kasting 2016/05/09 22:23:59 Nit: Extra parens around second comparison
Evan Stade 2016/05/09 22:54:53 Done.
return;
}
requests_.push_back(new RequestDelegate(
- this, keyword, osdd_url, favicon_url, url_fetcher_customize_callback,
- confirm_add_callback, provider_type));
+ this, keyword, osdd_url, favicon_url, url_fetcher_customize_callback));
}
void TemplateURLFetcher::RequestCompleted(RequestDelegate* request) {

Powered by Google App Engine
This is Rietveld 408576698