No content dependencies in TemplateURLFetcher

BUG=371535

Review URL: https://codereview.chromium.org/371333003

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

chrome/browser/search_engines/template_url_fetcher.cc

@@ -11,10 +11,6 @@
 #include "chrome/browser/search_engines/template_url_parser.h"
 #include "components/search_engines/template_url.h"
 #include "components/search_engines/template_url_service.h"
-#include "content/public/browser/render_frame_host.h"
-#include "content/public/browser/render_process_host.h"
-#include "content/public/browser/web_contents.h"
-#include "content/public/common/url_fetcher.h"
 #include "net/base/load_flags.h"
 #include "net/url_request/url_fetcher.h"
 #include "net/url_request/url_fetcher_delegate.h"
@@ -24,14 +20,14 @@
 // RequestDelegate ------------------------------------------------------------
 class TemplateURLFetcher::RequestDelegate : public net::URLFetcherDelegate {
  public:
-  // Takes ownership of |callbacks|.
-  RequestDelegate(TemplateURLFetcher* fetcher,
-                  const base::string16& keyword,
-                  const GURL& osdd_url,
-                  const GURL& favicon_url,
-                  content::WebContents* web_contents,
-                  const ConfirmAddSearchProviderCallback& callback,
-                  ProviderType provider_type);
+  RequestDelegate(
+      TemplateURLFetcher* fetcher,
+      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);
 
   // net::URLFetcherDelegate:
   // If data contains a valid OSDD, a TemplateURL is created and added to
@@ -58,7 +54,7 @@ class TemplateURLFetcher::RequestDelegate : public net::URLFetcherDelegate {
   const GURL osdd_url_;
   const GURL favicon_url_;
   const ProviderType provider_type_;
-  ConfirmAddSearchProviderCallback callback_;
+  ConfirmAddSearchProviderCallback confirm_add_callback_;
 
   scoped_ptr<TemplateURLService::Subscription> template_url_subscription_;
 
@@ -70,8 +66,8 @@ TemplateURLFetcher::RequestDelegate::RequestDelegate(
     const base::string16& keyword,
     const GURL& osdd_url,
     const GURL& favicon_url,
-    content::WebContents* web_contents,
-    const ConfirmAddSearchProviderCallback& callback,
+    const URLFetcherCustomizeCallback& url_fetcher_customize_callback,
+    const ConfirmAddSearchProviderCallback& confirm_add_callback,
     ProviderType provider_type)
     : url_fetcher_(net::URLFetcher::Create(
           osdd_url, net::URLFetcher::GET, this)),
@@ -80,7 +76,7 @@ TemplateURLFetcher::RequestDelegate::RequestDelegate(
       osdd_url_(osdd_url),
       favicon_url_(favicon_url),
       provider_type_(provider_type),
-      callback_(callback) {
+      confirm_add_callback_(confirm_add_callback) {
   TemplateURLService* model = fetcher_->template_url_service_;
   DCHECK(model);  // TemplateURLFetcher::ScheduleDownload verifies this.
 
@@ -92,16 +88,10 @@ TemplateURLFetcher::RequestDelegate::RequestDelegate(
     model->Load();
   }
 
-  url_fetcher_->SetRequestContext(fetcher->request_context_.get());
-  // Can be NULL during tests.
-  if (web_contents) {
-    content::AssociateURLFetcherWithRenderFrame(
-        url_fetcher_.get(),
-        web_contents->GetURL(),
-        web_contents->GetRenderProcessHost()->GetID(),
-        web_contents->GetMainFrame()->GetRoutingID());
-  }
+  if (!url_fetcher_customize_callback.is_null())
+    url_fetcher_customize_callback.Run(url_fetcher_.get());
 
+  url_fetcher_->SetRequestContext(fetcher->request_context_.get());
   url_fetcher_->Start();
 }
 
@@ -200,7 +190,7 @@ void TemplateURLFetcher::RequestDelegate::AddSearchProvider() {
       // 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.
-      callback_.Run(make_scoped_ptr(new TemplateURL(data)));
+      confirm_add_callback_.Run(make_scoped_ptr(new TemplateURL(data)));
       break;
 
     default:
@@ -228,8 +218,8 @@ void TemplateURLFetcher::ScheduleDownload(
     const base::string16& keyword,
     const GURL& osdd_url,
     const GURL& favicon_url,
-    content::WebContents* web_contents,
-    const ConfirmAddSearchProviderCallback& callback,
+    const URLFetcherCustomizeCallback& url_fetcher_customize_callback,
+    const ConfirmAddSearchProviderCallback& confirm_add_callback,
     ProviderType provider_type) {
   DCHECK(osdd_url.is_valid());
 
@@ -261,9 +251,9 @@ void TemplateURLFetcher::ScheduleDownload(
       return;
   }
 
-  requests_.push_back(
-      new RequestDelegate(this, keyword, osdd_url, favicon_url, web_contents,
-                          callback, provider_type));
+  requests_.push_back(new RequestDelegate(
+      this, keyword, osdd_url, favicon_url, url_fetcher_customize_callback,
+      confirm_add_callback, provider_type));
 }
 
 void TemplateURLFetcher::RequestCompleted(RequestDelegate* request) {

chrome/browser/search_engines/template_url_fetcher.h

@@ -16,11 +16,8 @@ class GURL;
 class TemplateURL;
 class TemplateURLService;
 
-namespace content {
-class WebContents;
-}
-
 namespace net {
+class URLFetcher;
 class URLRequestContextGetter;
 }
 
@@ -30,6 +27,8 @@ class URLRequestContextGetter;
 //
 class TemplateURLFetcher : public KeyedService {
  public:
+  typedef base::Callback<void(
+      net::URLFetcher* url_fetcher)> URLFetcherCustomizeCallback;
   typedef base::Callback<void(
       scoped_ptr<TemplateURL> template_url)> ConfirmAddSearchProviderCallback;
 
@@ -45,7 +44,7 @@ class TemplateURLFetcher : public KeyedService {
 
   // If TemplateURLFetcher is not already downloading the OSDD for osdd_url,
   // it is downloaded. If successful and the result can be parsed, a TemplateURL
-  // is added to the TemplateURLService. Takes ownership of |callbacks|.
+  // is added to the TemplateURLService.
   //
   // If |provider_type| is AUTODETECTED_PROVIDER, |keyword| must be non-empty,
   // and if there's already a non-replaceable TemplateURL in the model for
@@ -53,14 +52,16 @@ class TemplateURLFetcher : public KeyedService {
   // download is started.  If |provider_type| is EXPLICIT_PROVIDER, |keyword| is
   // ignored.
   //
-  // |web_contents| specifies which WebContents displays the page the OSDD is
-  // downloaded for. |web_contents| must not be NULL, except during tests.
-  void ScheduleDownload(const base::string16& keyword,
-                        const GURL& osdd_url,
-                        const GURL& favicon_url,
-                        content::WebContents* web_contents,
-                        const ConfirmAddSearchProviderCallback& callback,
-                        ProviderType provider_type);
+  // If |url_fetcher_customize_callback| is not null, it's run after a
+  // URLFetcher is created. This callback can be used to set additional
+  // parameters on the URLFetcher.
+  void 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);
 
   // The current number of outstanding requests.
   int requests_count() const { return requests_.size(); }

chrome/browser/search_engines/template_url_fetcher_unittest.cc

@@ -127,7 +127,8 @@ void TemplateURLFetcherTest::StartDownload(
 
   TemplateURLFetcherFactory::GetForProfile(
       test_util_.profile())->ScheduleDownload(
-          keyword, osdd_url, favicon_url, NULL,
+          keyword, osdd_url, favicon_url,
+          TemplateURLFetcher::URLFetcherCustomizeCallback(),
           base::Bind(&TemplateURLFetcherTest::ConfirmAddSearchProvider,
                      base::Unretained(this),
                      base::Owned(callback_destruction_notifier)),

chrome/browser/ui/search_engines/search_engine_tab_helper.cc

@@ -16,8 +16,11 @@
 #include "content/public/browser/favicon_status.h"
 #include "content/public/browser/navigation_controller.h"
 #include "content/public/browser/navigation_entry.h"
+#include "content/public/browser/render_frame_host.h"
+#include "content/public/browser/render_process_host.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/common/frame_navigate_params.h"
+#include "content/public/common/url_fetcher.h"
 
 using content::NavigationController;
 using content::NavigationEntry;
@@ -61,6 +64,15 @@ base::string16 GenerateKeywordFromNavigationEntry(
   return TemplateURL::GenerateKeyword(url);
 }
 
+void AssociateURLFetcherWithWebContents(content::WebContents* web_contents,
+                                        net::URLFetcher* url_fetcher) {
+  content::AssociateURLFetcherWithRenderFrame(
+      url_fetcher,
+      web_contents->GetURL(),
+      web_contents->GetRenderProcessHost()->GetID(),
+      web_contents->GetMainFrame()->GetRoutingID());
+}
+
 }  // namespace
 
 SearchEngineTabHelper::~SearchEngineTabHelper() {
@@ -134,7 +146,8 @@ void SearchEngineTabHelper::OnPageHasOSDD(
   // Download the OpenSearch description document. If this is successful, a
   // new keyword will be created when done.
   TemplateURLFetcherFactory::GetForProfile(profile)->ScheduleDownload(
-      keyword, osdd_url, entry->GetFavicon().url, web_contents(),
+      keyword, osdd_url, entry->GetFavicon().url,
+      base::Bind(&AssociateURLFetcherWithWebContents, web_contents()),
       base::Bind(&SearchEngineTabHelper::OnDownloadedOSDD,
                  weak_ptr_factory_.GetWeakPtr()),
       provider_type);