Migrate ntp_snippets::JsonRequest to SimpleURLLoader

Bug: 844942
Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: Ifda80b4277d7d4ad6f1336de6f437aa82ff75c29
Reviewed-on: https://chromium-review.googlesource.com/1087174Reviewed-by: Eric Noyau <noyau@chromium.org>
Reviewed-by: Bernhard Bauer <bauerb@chromium.org>
Commit-Queue: Mark Pilgrim <pilgrim@chromium.org>
Cr-Commit-Position: refs/heads/master@{#564875}

chrome/browser/ntp_snippets/content_suggestions_service_factory.cc

@@ -327,7 +327,7 @@ void RegisterArticleProviderIfEnabled(ContentSuggestionsService* service,
   }
 #endif  // BUILDFLAG(ENABLE_OFFLINE_PAGES)
   auto suggestions_fetcher = std::make_unique<RemoteSuggestionsFetcherImpl>(
-      identity_manager, request_context, pref_service, language_histogram,
+      identity_manager, url_loader_factory, pref_service, language_histogram,
       base::Bind(
           &data_decoder::SafeJsonParser::Parse,
           content::ServiceManagerConnection::GetForProcess()->GetConnector()),

components/ntp_snippets/remote/json_request.h

@@ -16,9 +16,7 @@
 #include "components/language/core/browser/url_language_histogram.h"
 #include "components/ntp_snippets/remote/request_params.h"
 #include "components/ntp_snippets/status.h"
-#include "net/http/http_request_headers.h"
-#include "net/url_request/url_fetcher_delegate.h"
-#include "net/url_request/url_request_context_getter.h"
+#include "services/network/public/cpp/resource_request.h"
 #include "url/gurl.h"
 
 namespace base {
@@ -26,6 +24,11 @@ class Value;
 class Clock;
 }  // namespace base
 
+namespace network {
+class SharedURLLoaderFactory;
+class SimpleURLLoader;
+}  // namespace network
+
 namespace ntp_snippets {
 class UserClassifier;
 
@@ -50,7 +53,7 @@ enum class FetchResult {
 
 // A single request to query remote suggestions. On success, the suggestions are
 // returned in parsed JSON form (base::Value).
-class JsonRequest : public net::URLFetcherDelegate {
+class JsonRequest {
  public:
   // A client can expect error_details only, if there was any error during the
   // fetching or parsing. In successful cases, it will be an empty string.
@@ -83,8 +86,8 @@ class JsonRequest : public net::URLFetcherDelegate {
     // It has to be alive until the request is destroyed.
     Builder& SetClock(base::Clock* clock);
     Builder& SetUrl(const GURL& url);
-    Builder& SetUrlRequestContextGetter(
-        const scoped_refptr<net::URLRequestContextGetter>& context_getter);
+    Builder& SetUrlLoaderFactory(
+        scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory);
     Builder& SetUserClassifier(const UserClassifier& user_classifier);
 
     // These preview methods allow to inspect the Request without exposing it
@@ -92,7 +95,9 @@ class JsonRequest : public net::URLFetcherDelegate {
     // TODO(fhorschig): Remove these when moving the Builder to
     // snippets::internal and trigger the request to intercept the request.
     std::string PreviewRequestBodyForTesting() { return BuildBody(); }
-    std::string PreviewRequestHeadersForTesting() { return BuildHeaders(); }
+    std::string PreviewRequestHeadersForTesting() {
+      return BuildResourceRequest()->headers.ToString();
+    }
     Builder& SetUserClassForTesting(const std::string& user_class) {
       user_class_ = user_class;
       return *this;
@@ -101,11 +106,9 @@ class JsonRequest : public net::URLFetcherDelegate {
     bool is_interactive_request() const { return params_.interactive_request; }
 
    private:
-    std::string BuildHeaders() const;
+    std::unique_ptr<network::ResourceRequest> BuildResourceRequest() const;
     std::string BuildBody() const;
-    std::unique_ptr<net::URLFetcher> BuildURLFetcher(
-        net::URLFetcherDelegate* request,
-        const std::string& headers,
+    std::unique_ptr<network::SimpleURLLoader> BuildURLLoader(
         const std::string& body) const;
 
     void PrepareLanguages(
@@ -118,7 +121,7 @@ class JsonRequest : public net::URLFetcherDelegate {
     RequestParams params_;
     ParseJSONCallback parse_json_callback_;
     GURL url_;
-    scoped_refptr<net::URLRequestContextGetter> url_request_context_getter_;
+    scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory_;
 
     // Optional properties.
     std::string obfuscated_gaia_id_;
@@ -132,10 +135,12 @@ class JsonRequest : public net::URLFetcherDelegate {
               base::Clock* clock,
               const ParseJSONCallback& callback);
   JsonRequest(JsonRequest&&);
-  ~JsonRequest() override;
+  ~JsonRequest();
 
   void Start(CompletedCallback callback);
 
+  static int Get5xxRetryCount(bool interactive_request);
+
   const base::Optional<Category>& exclusive_category() const {
     return exclusive_category_;
   }
@@ -144,16 +149,18 @@ class JsonRequest : public net::URLFetcherDelegate {
   std::string GetResponseString() const;
 
  private:
-  // URLFetcherDelegate implementation.
-  void OnURLFetchComplete(const net::URLFetcher* source) override;
+  void OnSimpleLoaderComplete(std::unique_ptr<std::string> response_body);
 
   void ParseJsonResponse();
   void OnJsonParsed(std::unique_ptr<base::Value> result);
   void OnJsonError(const std::string& error);
 
-  // The fetcher for downloading the snippets. Only non-null if a fetch is
+  // The loader for downloading the snippets. Only non-null if a load is
   // currently ongoing.
-  std::unique_ptr<net::URLFetcher> url_fetcher_;
+  std::unique_ptr<network::SimpleURLLoader> simple_url_loader_;
+
+  // The loader factory for downloading the snippets.
+  scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory_;
 
   // If set, only return results for this category.
   base::Optional<Category> exclusive_category_;
@@ -171,6 +178,9 @@ class JsonRequest : public net::URLFetcherDelegate {
   // The callback to notify when URLFetcher finished and results are available.
   CompletedCallback request_completed_callback_;
 
+  // The last response string
+  std::string last_response_string_;
+
   base::WeakPtrFactory<JsonRequest> weak_ptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(JsonRequest);

components/ntp_snippets/remote/json_request_unittest.cc

@@ -18,8 +18,9 @@
 #include "components/ntp_snippets/remote/request_params.h"
 #include "components/prefs/testing_pref_service.h"
 #include "components/variations/variations_params_manager.h"
-#include "net/url_request/test_url_fetcher_factory.h"
-#include "net/url_request/url_request_test_util.h"
+#include "services/network/public/cpp/shared_url_loader_factory.h"
+#include "services/network/public/cpp/weak_wrapper_shared_url_loader_factory.h"
+#include "services/network/test/test_url_loader_factory.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -65,8 +66,9 @@ class JsonRequestTest : public testing::Test {
             {ntp_snippets::kArticleSuggestionsFeature.name}),
         pref_service_(std::make_unique<TestingPrefServiceSimple>()),
         mock_task_runner_(new base::TestMockTimeTaskRunner()),
-        request_context_getter_(
-            new net::TestURLRequestContextGetter(mock_task_runner_.get())) {
+        test_shared_loader_factory_(
+            base::MakeRefCounted<network::WeakWrapperSharedURLLoaderFactory>(
+                &test_url_loader_factory_)) {
     language::UrlLanguageHistogram::RegisterProfilePrefs(
         pref_service_->registry());
   }
@@ -88,7 +90,7 @@ class JsonRequestTest : public testing::Test {
     JsonRequest::Builder builder;
     builder.SetUrl(GURL("http://valid-url.test"))
         .SetClock(mock_task_runner_->GetMockClock())
-        .SetUrlRequestContextGetter(request_context_getter_.get());
+        .SetUrlLoaderFactory(test_shared_loader_factory_);
     return builder;
   }
 
@@ -96,8 +98,8 @@ class JsonRequestTest : public testing::Test {
   variations::testing::VariationParamsManager params_manager_;
   std::unique_ptr<TestingPrefServiceSimple> pref_service_;
   scoped_refptr<base::TestMockTimeTaskRunner> mock_task_runner_;
-  scoped_refptr<net::TestURLRequestContextGetter> request_context_getter_;
-  net::TestURLFetcherFactory fetcher_factory_;
+  network::TestURLLoaderFactory test_url_loader_factory_;
+  scoped_refptr<network::SharedURLLoaderFactory> test_shared_loader_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(JsonRequestTest);
 };

components/ntp_snippets/remote/remote_suggestions_fetcher_impl.cc

@@ -18,15 +18,10 @@
 #include "components/ntp_snippets/user_classifier.h"
 #include "components/variations/variations_associated_data.h"
 #include "net/base/url_util.h"
-#include "net/url_request/url_fetcher.h"
-#include "net/url_request/url_request_status.h"
 #include "services/identity/public/cpp/identity_manager.h"
+#include "services/network/public/cpp/shared_url_loader_factory.h"
 
 using language::UrlLanguageHistogram;
-using net::HttpRequestHeaders;
-using net::URLFetcher;
-using net::URLRequestContextGetter;
-using net::URLRequestStatus;
 
 namespace ntp_snippets {
 
@@ -154,7 +149,7 @@ void FilterCategories(FetchedCategoriesVector* categories,
 
 RemoteSuggestionsFetcherImpl::RemoteSuggestionsFetcherImpl(
     identity::IdentityManager* identity_manager,
-    scoped_refptr<URLRequestContextGetter> url_request_context_getter,
+    scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory,
     PrefService* pref_service,
     UrlLanguageHistogram* language_histogram,
     const ParseJSONCallback& parse_json_callback,
@@ -162,7 +157,7 @@ RemoteSuggestionsFetcherImpl::RemoteSuggestionsFetcherImpl(
     const std::string& api_key,
     const UserClassifier* user_classifier)
     : identity_manager_(identity_manager),
-      url_request_context_getter_(std::move(url_request_context_getter)),
+      url_loader_factory_(std::move(url_loader_factory)),
       language_histogram_(language_histogram),
       parse_json_callback_(parse_json_callback),
       fetch_url_(api_endpoint),
@@ -208,7 +203,7 @@ void RemoteSuggestionsFetcherImpl::FetchSnippets(
       .SetParams(params)
       .SetParseJsonCallback(parse_json_callback_)
       .SetClock(clock_)
-      .SetUrlRequestContextGetter(url_request_context_getter_)
+      .SetUrlLoaderFactory(url_loader_factory_)
       .SetUserClassifier(*user_classifier_);
 
   if (identity_manager_->HasPrimaryAccount()) {

components/ntp_snippets/remote/remote_suggestions_fetcher_impl.h

@@ -18,7 +18,6 @@
 #include "components/ntp_snippets/remote/json_to_categories.h"
 #include "components/ntp_snippets/remote/remote_suggestions_fetcher.h"
 #include "components/ntp_snippets/remote/request_params.h"
-#include "net/url_request/url_request_context_getter.h"
 #include "services/identity/public/cpp/primary_account_access_token_fetcher.h"
 
 class PrefService;
@@ -36,6 +35,10 @@ namespace language {
 class UrlLanguageHistogram;
 }  // namespace language
 
+namespace network {
+class SharedURLLoaderFactory;
+}  // namespace network
+
 namespace ntp_snippets {
 
 class UserClassifier;
@@ -44,7 +47,7 @@ class RemoteSuggestionsFetcherImpl : public RemoteSuggestionsFetcher {
  public:
   RemoteSuggestionsFetcherImpl(
       identity::IdentityManager* identity_manager,
-      scoped_refptr<net::URLRequestContextGetter> url_request_context_getter,
+      scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory,
       PrefService* pref_service,
       language::UrlLanguageHistogram* language_histogram,
       const ParseJSONCallback& parse_json_callback,
@@ -97,8 +100,8 @@ class RemoteSuggestionsFetcherImpl : public RemoteSuggestionsFetcher {
 
   std::unique_ptr<identity::PrimaryAccountAccessTokenFetcher> token_fetcher_;
 
-  // Holds the URL request context.
-  scoped_refptr<net::URLRequestContextGetter> url_request_context_getter_;
+  // Holds the URL loader factory
+  scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory_;
 
   // Stores requests that wait for an access token.
   base::queue<

ios/chrome/browser/ntp_snippets/ios_chrome_content_suggestions_service_factory_util.cc

@@ -166,6 +166,9 @@ void RegisterRemoteSuggestionsProvider(ContentSuggestionsService* service,
       IdentityManagerFactory::GetForBrowserState(chrome_browser_state);
   scoped_refptr<net::URLRequestContextGetter> request_context =
       browser_state->GetRequestContext();
+  scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory =
+      browser_state->GetSharedURLLoaderFactory();
+
   base::FilePath database_dir(
       browser_state->GetStatePath().Append(ntp_snippets::kDatabaseFolder));
 
@@ -178,7 +181,7 @@ void RegisterRemoteSuggestionsProvider(ContentSuggestionsService* service,
                                 : google_apis::GetNonStableAPIKey();
   }
   auto suggestions_fetcher = std::make_unique<RemoteSuggestionsFetcherImpl>(
-      identity_manager, request_context, prefs, nullptr,
+      identity_manager, url_loader_factory, prefs, nullptr,
       base::BindRepeating(&ParseJson), GetFetchEndpoint(GetChannel()), api_key,
       service->user_classifier());