Migrate PrefetchRequestFetcher to using SimpleURLLoader

URLFetcher will stop working with advent of Network Service, and
SimpleURLLoader is the replacement API for most clients.
This CL migrates PrefetchRequestFetcher et al and the
respective unittests away from URLFetcher.

Bug: 773295,879776
Change-Id: Ibcfc2a7d72b6efe1b372233ade95b58d95fbf62b
Reviewed-on: https://chromium-review.googlesource.com/1215367
Reviewed-by: Justin DeWitt <[email protected]>
Reviewed-by: Matt Menke <[email protected]>
Commit-Queue: Antonio Gomes <[email protected]>
Cr-Commit-Position: refs/heads/master@{#590048}

chrome/browser/offline_pages/prefetch/prefetch_service_factory.cc

@@ -33,6 +33,7 @@
 #include "components/offline_pages/core/prefetch/store/prefetch_store.h"
 #include "components/offline_pages/core/prefetch/suggested_articles_observer.h"
 #include "content/public/browser/browser_context.h"
+#include "services/network/public/cpp/shared_url_loader_factory.h"
 
 namespace offline_pages {
 
@@ -75,7 +76,7 @@ KeyedService* PrefetchServiceFactory::BuildServiceInstanceFor(
 
   auto prefetch_network_request_factory =
       std::make_unique<PrefetchNetworkRequestFactoryImpl>(
-          profile->GetRequestContext(), chrome::GetChannel(), GetUserAgent());
+          profile->GetURLLoaderFactory(), chrome::GetChannel(), GetUserAgent());
 
   scoped_refptr<base::SequencedTaskRunner> background_task_runner =
       base::CreateSequencedTaskRunnerWithTraits({base::MayBlock()});

components/offline_pages/core/prefetch/BUILD.gn

@@ -111,6 +111,7 @@ static_library("prefetch") {
     "//components/version_info",
     "//google_apis",
     "//net:net",
+    "//services/network/public/cpp:cpp",
     "//sql:sql",
     "//url",
   ]
@@ -168,6 +169,7 @@ static_library("test_support") {
     "//components/prefs:test_support",
     "//components/version_info:channel",
     "//net:test_support",
+    "//services/network:test_support",
     "//sql:sql",
     "//testing/gmock",
     "//url",
@@ -248,6 +250,7 @@ source_set("unit_tests") {
     "//components/variations:test_support",
     "//components/version_info:channel",
     "//net:test_support",
+    "//services/network:test_support",
     "//sql:sql",
     "//testing/gmock",
     "//testing/gtest",

components/offline_pages/core/prefetch/DEPS

@@ -7,5 +7,7 @@ include_rules = [
   "+components/ntp_snippets",
   "+components/version_info",
   "+net",
+  "+services/network/public/cpp",
+  "+services/network/test",
   "+sql",
 ]

components/offline_pages/core/prefetch/generate_page_bundle_request.cc

@@ -11,7 +11,7 @@
 #include "components/offline_pages/core/prefetch/prefetch_request_fetcher.h"
 #include "components/offline_pages/core/prefetch/prefetch_server_urls.h"
 #include "components/offline_pages/core/prefetch/proto/offline_pages.pb.h"
-#include "net/url_request/url_request_context_getter.h"
+#include "services/network/public/cpp/shared_url_loader_factory.h"
 #include "url/gurl.h"
 
 namespace offline_pages {
@@ -22,7 +22,7 @@ GeneratePageBundleRequest::GeneratePageBundleRequest(
     int max_bundle_size_bytes,
     const std::vector<std::string>& page_urls,
     version_info::Channel channel,
-    net::URLRequestContextGetter* request_context_getter,
+    scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory,
     PrefetchRequestFinishedCallback callback)
     : callback_(std::move(callback)), requested_urls_(page_urls) {
   proto::GeneratePageBundleRequest request;
@@ -41,8 +41,7 @@ GeneratePageBundleRequest::GeneratePageBundleRequest(
   request.SerializeToString(&upload_data);
 
   fetcher_ = PrefetchRequestFetcher::CreateForPost(
-      GeneratePageBundleRequestURL(channel), upload_data,
-      request_context_getter,
+      GeneratePageBundleRequestURL(channel), upload_data, url_loader_factory,
       base::BindOnce(&GeneratePageBundleRequest::OnCompleted,
                      // Fetcher is owned by this instance.
                      base::Unretained(this)));

components/offline_pages/core/prefetch/generate_page_bundle_request.h

@@ -13,8 +13,8 @@
 #include "components/offline_pages/core/prefetch/prefetch_types.h"
 #include "components/version_info/channel.h"
 
-namespace net {
-class URLRequestContextGetter;
+namespace network {
+class SharedURLLoaderFactory;
 }
 
 namespace offline_pages {
@@ -29,7 +29,7 @@ class GeneratePageBundleRequest {
       int max_bundle_size_bytes,
       const std::vector<std::string>& page_urls,
       version_info::Channel channel,
-      net::URLRequestContextGetter* request_context_getter,
+      scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory,
       PrefetchRequestFinishedCallback callback);
   ~GeneratePageBundleRequest();
 

components/offline_pages/core/prefetch/generate_page_bundle_request_unittest.cc

@@ -10,6 +10,7 @@
 #include "components/offline_pages/core/prefetch/proto/offline_pages.pb.h"
 #include "net/http/http_status_code.h"
 #include "net/url_request/url_request_status.h"
+#include "services/network/test/test_utils.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "url/gurl.h"
 #include "url/url_constants.h"
@@ -43,7 +44,7 @@ class GeneratePageBundleRequestTest : public PrefetchRequestTestBase {
     return std::unique_ptr<GeneratePageBundleRequest>(
         new GeneratePageBundleRequest(
             kTestUserAgent, kTestGCMID, kTestMaxBundleSize, page_urls,
-            kTestChannel, request_context(), std::move(callback)));
+            kTestChannel, shared_url_loader_factory(), std::move(callback)));
   }
 };
 
@@ -56,19 +57,26 @@ TEST_F(GeneratePageBundleRequestTest, RequestData) {
   EXPECT_THAT(request->requested_urls(), Contains(kTestURL));
   EXPECT_THAT(request->requested_urls(), Contains(kTestURL2));
 
-  net::TestURLFetcher* fetcher = GetRunningFetcher();
-  EXPECT_TRUE(fetcher->GetOriginalURL().SchemeIs(url::kHttpsScheme));
-  EXPECT_TRUE(base::StartsWith(fetcher->GetOriginalURL().query(), "key",
+  network::TestURLLoaderFactory::PendingRequest* pending_request =
+      GetPendingRequest();
+  DCHECK(pending_request);
+  GURL request_url = pending_request->request.url;
+  EXPECT_TRUE(request_url.SchemeIs(url::kHttpsScheme));
+  EXPECT_TRUE(base::StartsWith(request_url.query(), "key",
                                base::CompareCase::SENSITIVE));
 
-  EXPECT_FALSE(fetcher->upload_content_type().empty());
-  EXPECT_FALSE(fetcher->upload_data().empty());
+  std::string upload_content_type;
+  pending_request->request.headers.GetHeader(
+      net::HttpRequestHeaders::kContentType, &upload_content_type);
+  EXPECT_FALSE(upload_content_type.empty());
+  EXPECT_FALSE(network::GetUploadData(pending_request->request).empty());
 
   // Experiment header should not be set.
-  EXPECT_EQ("", GetExperiementHeaderValue(fetcher));
+  EXPECT_EQ("", GetExperiementHeaderValue(pending_request));
 
   proto::GeneratePageBundleRequest bundle_data;
-  ASSERT_TRUE(bundle_data.ParseFromString(fetcher->upload_data()));
+  ASSERT_TRUE(bundle_data.ParseFromString(
+      network::GetUploadData(pending_request->request)));
   EXPECT_EQ(kTestUserAgent, bundle_data.user_agent());
   EXPECT_EQ(proto::FORMAT_MHTML, bundle_data.output_format());
   EXPECT_EQ(kTestMaxBundleSize, bundle_data.max_bundle_size_bytes());
@@ -87,11 +95,13 @@ TEST_F(GeneratePageBundleRequestTest, ExperimentHeaderInRequestData) {
   base::MockCallback<PrefetchRequestFinishedCallback> callback;
   std::unique_ptr<GeneratePageBundleRequest> request(
       CreateRequest(callback.Get()));
-  net::TestURLFetcher* fetcher = GetRunningFetcher();
+  network::TestURLLoaderFactory::PendingRequest* pending_request =
+      GetPendingRequest();
+  DCHECK(pending_request);
 
   // Experiment header should be set.
   EXPECT_EQ(kExperimentValueSetInFieldTrial,
-            GetExperiementHeaderValue(fetcher));
+            GetExperiementHeaderValue(pending_request));
 }
 
 TEST_F(GeneratePageBundleRequestTest, EmptyResponse) {
@@ -106,6 +116,7 @@ TEST_F(GeneratePageBundleRequestTest, EmptyResponse) {
       .WillOnce(DoAll(SaveArg<0>(&status), SaveArg<1>(&operation_name),
                       SaveArg<2>(&pages)));
   RespondWithData("");
+  RunUntilIdle();
 
   EXPECT_EQ(PrefetchRequestStatus::SHOULD_RETRY_WITH_BACKOFF, status);
   EXPECT_TRUE(operation_name.empty());
@@ -124,6 +135,7 @@ TEST_F(GeneratePageBundleRequestTest, InvalidResponse) {
       .WillOnce(DoAll(SaveArg<0>(&status), SaveArg<1>(&operation_name),
                       SaveArg<2>(&pages)));
   RespondWithData("Some invalid data");
+  RunUntilIdle();
 
   EXPECT_EQ(PrefetchRequestStatus::SHOULD_RETRY_WITH_BACKOFF, status);
   EXPECT_TRUE(operation_name.empty());

components/offline_pages/core/prefetch/generate_page_bundle_task_unittest.cc

@@ -19,6 +19,7 @@
 #include "components/offline_pages/core/prefetch/test_prefetch_dispatcher.h"
 #include "components/offline_pages/core/prefetch/test_prefetch_gcm_handler.h"
 #include "components/offline_pages/task/task.h"
+#include "services/network/test/test_utils.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -127,7 +128,7 @@ TEST_F(GeneratePageBundleTaskTest, TaskMakesNetworkRequest) {
   EXPECT_THAT(*requested_urls, Not(Contains(item3.url.spec())));
 
   std::string upload_data =
-      url_fetcher_factory()->GetFetcherByID(0)->upload_data();
+      network::GetUploadData(GetPendingRequest(0 /*index*/)->request);
   EXPECT_THAT(upload_data, HasSubstr(MockPrefetchItemGenerator::kUrlPrefix));
 
   EXPECT_EQ(3, store_util()->CountPrefetchItems());

components/offline_pages/core/prefetch/get_operation_request.cc

@@ -10,19 +10,19 @@
 #include "components/offline_pages/core/prefetch/prefetch_proto_utils.h"
 #include "components/offline_pages/core/prefetch/prefetch_request_fetcher.h"
 #include "components/offline_pages/core/prefetch/prefetch_server_urls.h"
-#include "net/url_request/url_request_context_getter.h"
+#include "services/network/public/cpp/shared_url_loader_factory.h"
 #include "url/gurl.h"
 
 namespace offline_pages {
 
 GetOperationRequest::GetOperationRequest(
     const std::string& name,
     version_info::Channel channel,
-    net::URLRequestContextGetter* request_context_getter,
+    scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory,
     PrefetchRequestFinishedCallback callback)
     : callback_(std::move(callback)) {
   fetcher_ = PrefetchRequestFetcher::CreateForGet(
-      GetOperationRequestURL(name, channel), request_context_getter,
+      GetOperationRequestURL(name, channel), url_loader_factory,
       base::BindOnce(&GetOperationRequest::OnCompleted,
                      // Fetcher is owned by this instance.
                      base::Unretained(this), name));

components/offline_pages/core/prefetch/get_operation_request.h

@@ -12,8 +12,8 @@
 #include "components/offline_pages/core/prefetch/prefetch_types.h"
 #include "components/version_info/channel.h"
 
-namespace net {
-class URLRequestContextGetter;
+namespace network {
+class SharedURLLoaderFactory;
 }
 
 namespace offline_pages {
@@ -27,10 +27,11 @@ class GetOperationRequest {
   // |name| identifies the operation triggered by the GeneratePageBundleRequest.
   // It is retrieved from the operation data returned in the
   // GeneratePageBundleRequest response.
-  GetOperationRequest(const std::string& name,
-                      version_info::Channel channel,
-                      net::URLRequestContextGetter* request_context_getter,
-                      PrefetchRequestFinishedCallback callback);
+  GetOperationRequest(
+      const std::string& name,
+      version_info::Channel channel,
+      scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory,
+      PrefetchRequestFinishedCallback callback);
   ~GetOperationRequest();
 
  private:

components/offline_pages/core/prefetch/get_operation_request_unittest.cc

@@ -10,6 +10,7 @@
 #include "components/offline_pages/core/prefetch/proto/offline_pages.pb.h"
 #include "net/http/http_status_code.h"
 #include "net/url_request/url_request_status.h"
+#include "services/network/test/test_utils.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "url/gurl.h"
 #include "url/url_constants.h"
@@ -39,35 +40,38 @@ class GetOperationRequestTest : public PrefetchRequestTestBase {
  public:
   std::unique_ptr<GetOperationRequest> CreateRequest(
       PrefetchRequestFinishedCallback callback) {
-    return std::unique_ptr<GetOperationRequest>(
-        new GetOperationRequest(kTestOperationName, kTestChannel,
-                                request_context(), std::move(callback)));
+    return std::unique_ptr<GetOperationRequest>(new GetOperationRequest(
+        kTestOperationName, kTestChannel, shared_url_loader_factory(),
+        std::move(callback)));
   }
 };
 
 TEST_F(GetOperationRequestTest, RequestData) {
   base::MockCallback<PrefetchRequestFinishedCallback> callback;
   std::unique_ptr<GetOperationRequest> request(CreateRequest(callback.Get()));
 
-  net::TestURLFetcher* fetcher = GetRunningFetcher();
-  GURL fetcher_url = fetcher->GetOriginalURL();
-  EXPECT_TRUE(fetcher_url.SchemeIs(url::kHttpsScheme));
-  EXPECT_EQ(kServerPathForTestOperation, fetcher_url.path());
-  EXPECT_TRUE(base::StartsWith(fetcher_url.query(), "key",
+  auto* pending_request = GetPendingRequest();
+  DCHECK(pending_request);
+  GURL request_url = pending_request->request.url;
+  EXPECT_TRUE(request_url.SchemeIs(url::kHttpsScheme));
+  EXPECT_EQ(kServerPathForTestOperation, request_url.path());
+  EXPECT_TRUE(base::StartsWith(request_url.query(), "key",
                                base::CompareCase::SENSITIVE));
 
-  net::HttpRequestHeaders headers;
-  fetcher->GetExtraRequestHeaders(&headers);
+  net::HttpRequestHeaders headers = pending_request->request.headers;
   std::string content_type_header;
   headers.GetHeader(net::HttpRequestHeaders::kContentType,
                     &content_type_header);
   EXPECT_EQ("application/x-protobuf", content_type_header);
 
   // Experiment header should not be set.
-  EXPECT_EQ("", GetExperiementHeaderValue(fetcher));
+  EXPECT_EQ("", GetExperiementHeaderValue(pending_request));
 
-  EXPECT_TRUE(fetcher->upload_content_type().empty());
-  EXPECT_TRUE(fetcher->upload_data().empty());
+  std::string upload_content_type;
+  pending_request->request.headers.GetHeader(
+      net::HttpRequestHeaders::kContentType, &upload_content_type);
+  EXPECT_FALSE(upload_content_type.empty());
+  EXPECT_TRUE(network::GetUploadData(pending_request->request).empty());
 }
 
 TEST_F(GetOperationRequestTest, ExperimentHeaderInRequestData) {
@@ -76,11 +80,12 @@ TEST_F(GetOperationRequestTest, ExperimentHeaderInRequestData) {
 
   base::MockCallback<PrefetchRequestFinishedCallback> callback;
   std::unique_ptr<GetOperationRequest> request(CreateRequest(callback.Get()));
-  net::TestURLFetcher* fetcher = GetRunningFetcher();
+  auto* pending_request = GetPendingRequest();
+  DCHECK(pending_request);
 
   // Experiment header should be set.
   EXPECT_EQ(kExperimentValueSetInFieldTrial,
-            GetExperiementHeaderValue(fetcher));
+            GetExperiementHeaderValue(pending_request));
 }
 
 TEST_F(GetOperationRequestTest, EmptyResponse) {
@@ -94,6 +99,7 @@ TEST_F(GetOperationRequestTest, EmptyResponse) {
       .WillOnce(DoAll(SaveArg<0>(&status), SaveArg<1>(&operation_name),
                       SaveArg<2>(&pages)));
   RespondWithData("");
+  RunUntilIdle();
 
   EXPECT_EQ(PrefetchRequestStatus::SHOULD_RETRY_WITH_BACKOFF, status);
   EXPECT_EQ(std::string(kTestOperationName), operation_name);
@@ -111,6 +117,7 @@ TEST_F(GetOperationRequestTest, InvalidResponse) {
       .WillOnce(DoAll(SaveArg<0>(&status), SaveArg<1>(&operation_name),
                       SaveArg<2>(&pages)));
   RespondWithData("Some invalid data");
+  RunUntilIdle();
 
   EXPECT_EQ(PrefetchRequestStatus::SHOULD_RETRY_WITH_BACKOFF, status);
   EXPECT_EQ(std::string(kTestOperationName), operation_name);

components/offline_pages/core/prefetch/get_operation_task_unittest.cc

@@ -53,8 +53,7 @@ TEST_F(GetOperationTaskTest, NormalOperationTask) {
 
   EXPECT_NE(nullptr, prefetch_request_factory()->FindGetOperationRequestByName(
                          kOperationName));
-  std::string path =
-      url_fetcher_factory()->GetFetcherByID(0)->GetOriginalURL().path();
+  std::string path = GetPendingRequest(0 /*index*/)->request.url.path();
   EXPECT_THAT(path, HasSubstr(kOperationName));
   EXPECT_EQ(1, store_util()->LastCommandChangeCount());
   ASSERT_NE(nullptr, store_util()->GetPrefetchItem(id));

components/offline_pages/core/prefetch/prefetch_dispatcher_impl_unittest.cc

@@ -115,6 +115,10 @@ class TestPrefetchConfiguration : public PrefetchConfiguration {
 class FakePrefetchNetworkRequestFactory
     : public TestPrefetchNetworkRequestFactory {
  public:
+  FakePrefetchNetworkRequestFactory(
+      scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory)
+      : TestPrefetchNetworkRequestFactory(url_loader_factory) {}
+
   void MakeGeneratePageBundleRequest(
       const std::vector<std::string>& prefetch_urls,
       const std::string& gcm_registration_id,
@@ -254,7 +258,8 @@ void PrefetchDispatcherTest::SetUp() {
   ASSERT_TRUE(archive_directory_.CreateUniqueTempDir());
 
   dispatcher_ = new PrefetchDispatcherImpl();
-  network_request_factory_ = new FakePrefetchNetworkRequestFactory();
+  network_request_factory_ =
+      new FakePrefetchNetworkRequestFactory(shared_url_loader_factory());
   taco_.reset(new PrefetchServiceTestTaco);
   store_util_.BuildStore();
   taco_->SetPrefetchStore(store_util_.ReleaseStore());
@@ -544,7 +549,7 @@ TEST_F(PrefetchDispatcherTest, NoNetworkRequestsAfterNewURLs) {
   RunUntilIdle();
 
   // We should not have started GPB
-  EXPECT_EQ(nullptr, GetRunningFetcher());
+  EXPECT_EQ(nullptr, GetPendingRequest());
 }
 
 TEST_F(PrefetchDispatcherTest, ThumbnailFetchFailure_ItemDownloaded) {