Migrate SpellingServiceClient to SimpleURLLoader

Bug: 844964
Change-Id: I1b47ddad2856b8a92b4f8bd6f41b1d01b8706bac
Reviewed-on: https://chromium-review.googlesource.com/1097196Reviewed-by: Matt Menke <mmenke@chromium.org>
Reviewed-by: Rachel Blum <groby@chromium.org>
Commit-Queue: Mark Pilgrim <pilgrim@chromium.org>
Cr-Commit-Position: refs/heads/master@{#567326}

components/spellcheck/browser/BUILD.gn

@@ -37,6 +37,7 @@ source_set("browser") {
     "//content/public/common",
     "//google_apis",
     "//net",
+    "//services/network/public/cpp",
   ]
 
   if (is_android) {
@@ -60,6 +61,8 @@ source_set("unit_tests") {
     "//content/test:test_support",
     "//mojo/public/cpp/bindings",
     "//net:test_support",
+    "//services/network:test_support",
+    "//services/network/public/cpp",
     "//testing/gtest",
   ]
 }

components/spellcheck/browser/DEPS

@@ -10,4 +10,6 @@ include_rules = [
   "+jni",
   "+mojo/public/cpp/bindings",
   "+net",
+  "+services/network/public/cpp",
+  "+services/network/test",
 ]

components/spellcheck/browser/spelling_service_client.cc

@@ -28,7 +28,9 @@
 #include "content/public/browser/storage_partition.h"
 #include "google_apis/google_api_keys.h"
 #include "net/base/load_flags.h"
-#include "net/url_request/url_fetcher.h"
+#include "services/network/public/cpp/resource_request.h"
+#include "services/network/public/cpp/shared_url_loader_factory.h"
+#include "services/network/public/cpp/simple_url_loader.h"
 #include "url/gurl.h"
 
 namespace {
@@ -48,9 +50,9 @@ const char* const kValidLanguages[] = {"en", "es", "fi", "da"};
 
 }  // namespace
 
-SpellingServiceClient::SpellingServiceClient() {}
+SpellingServiceClient::SpellingServiceClient() = default;
 
-SpellingServiceClient::~SpellingServiceClient() {}
+SpellingServiceClient::~SpellingServiceClient() = default;
 
 bool SpellingServiceClient::RequestTextCheck(
     content::BrowserContext* context,
@@ -62,7 +64,6 @@ bool SpellingServiceClient::RequestTextCheck(
     std::move(callback).Run(false, text, std::vector<SpellCheckResult>());
     return false;
   }
-
   const PrefService* pref = user_prefs::UserPrefs::Get(context);
   DCHECK(pref);
 
@@ -138,19 +139,32 @@ bool SpellingServiceClient::RequestTextCheck(
           }
         })");
 
-  net::URLFetcher* fetcher =
-      CreateURLFetcher(url, traffic_annotation).release();
-  data_use_measurement::DataUseUserData::AttachToFetcher(
-      fetcher, data_use_measurement::DataUseUserData::SPELL_CHECKER);
-  fetcher->SetRequestContext(
-      content::BrowserContext::GetDefaultStoragePartition(context)
-          ->GetURLRequestContext());
-  fetcher->SetUploadData("application/json", request);
-  fetcher->SetLoadFlags(net::LOAD_DO_NOT_SEND_COOKIES |
-                        net::LOAD_DO_NOT_SAVE_COOKIES);
-  spellcheck_fetchers_[fetcher] = std::make_unique<TextCheckCallbackData>(
-      base::WrapUnique(fetcher), std::move(callback), text);
-  fetcher->Start();
+  auto resource_request = std::make_unique<network::ResourceRequest>();
+  resource_request->url = url;
+  resource_request->load_flags =
+      net::LOAD_DO_NOT_SEND_COOKIES | net::LOAD_DO_NOT_SAVE_COOKIES;
+  resource_request->method = "POST";
+  std::unique_ptr<network::SimpleURLLoader> simple_url_loader =
+      network::SimpleURLLoader::Create(std::move(resource_request),
+                                       traffic_annotation);
+  simple_url_loader->AttachStringForUpload(request, "application/json");
+  auto it = spellcheck_loaders_.insert(
+      spellcheck_loaders_.begin(),
+      std::make_unique<TextCheckCallbackData>(std::move(simple_url_loader),
+                                              std::move(callback), text));
+  network::SimpleURLLoader* loader = it->get()->simple_url_loader.get();
+  auto url_loader_factory =
+      url_loader_factory_for_testing_
+          ? url_loader_factory_for_testing_
+          : content::BrowserContext::GetDefaultStoragePartition(context)
+                ->GetURLLoaderFactoryForBrowserProcess();
+  // TODO(https://crbug.com/808498): Re-add data use measurement once
+  // SimpleURLLoader supports it.
+  // ID=data_use_measurement::DataUseUserData::SPELL_CHECKER
+  loader->DownloadToStringOfUnboundedSizeUntilCrashAndDie(
+      url_loader_factory.get(),
+      base::BindOnce(&SpellingServiceClient::OnSimpleLoaderComplete,
+                     base::Unretained(this), std::move(it)));
   return true;
 }
 
@@ -279,35 +293,30 @@ bool SpellingServiceClient::ParseResponse(
 }
 
 SpellingServiceClient::TextCheckCallbackData::TextCheckCallbackData(
-    std::unique_ptr<net::URLFetcher> fetcher,
+    std::unique_ptr<network::SimpleURLLoader> simple_url_loader,
     TextCheckCompleteCallback callback,
     base::string16 text)
-    : fetcher(std::move(fetcher)), callback(std::move(callback)), text(text) {}
+    : simple_url_loader(std::move(simple_url_loader)),
+      callback(std::move(callback)),
+      text(text) {}
 
 SpellingServiceClient::TextCheckCallbackData::~TextCheckCallbackData() {}
 
-void SpellingServiceClient::OnURLFetchComplete(const net::URLFetcher* source) {
-  DCHECK(base::ContainsKey(spellcheck_fetchers_, source));
-  std::unique_ptr<TextCheckCallbackData> callback_data =
-      std::move(spellcheck_fetchers_[source]);
-  spellcheck_fetchers_.erase(source);
-
+void SpellingServiceClient::OnSimpleLoaderComplete(
+    SpellCheckLoaderList::iterator it,
+    std::unique_ptr<std::string> response_body) {
+  TextCheckCompleteCallback callback = std::move(it->get()->callback);
+  base::string16 text = it->get()->text;
   bool success = false;
   std::vector<SpellCheckResult> results;
-  if (source->GetResponseCode() / 100 == 2) {
-    std::string data;
-    source->GetResponseAsString(&data);
-    success = ParseResponse(data, &results);
-  }
-
-  // The callback may release the last (transitive) dependency on |this|. It
-  // MUST be the last function called.
-  std::move(callback_data->callback).Run(success, callback_data->text, results);
+  if (response_body)
+    success = ParseResponse(*response_body, &results);
+  spellcheck_loaders_.erase(it);
+  std::move(callback).Run(success, text, results);
 }
 
-std::unique_ptr<net::URLFetcher> SpellingServiceClient::CreateURLFetcher(
-    const GURL& url,
-    net::NetworkTrafficAnnotationTag traffic_annotation) {
-  return net::URLFetcher::Create(url, net::URLFetcher::POST, this,
-                                 traffic_annotation);
+void SpellingServiceClient::SetURLLoaderFactoryForTesting(
+    scoped_refptr<network::SharedURLLoaderFactory>
+        url_loader_factory_for_testing) {
+  url_loader_factory_for_testing_ = std::move(url_loader_factory_for_testing);
 }

components/spellcheck/browser/spelling_service_client.h

@@ -5,7 +5,7 @@
 #ifndef COMPONENTS_SPELLCHECK_BROWSER_SPELLING_SERVICE_CLIENT_H_
 #define COMPONENTS_SPELLCHECK_BROWSER_SPELLING_SERVICE_CLIENT_H_
 
-#include <map>
+#include <list>
 #include <memory>
 #include <string>
 #include <vector>
@@ -14,18 +14,17 @@
 #include "base/compiler_specific.h"
 #include "base/strings/string16.h"
 #include "net/traffic_annotation/network_traffic_annotation.h"
-#include "net/url_request/url_fetcher_delegate.h"
 
-class GURL;
 struct SpellCheckResult;
 
 namespace content {
 class BrowserContext;
 }
 
-namespace net {
-class URLFetcher;
-}  // namespace net
+namespace network {
+class SharedURLLoaderFactory;
+class SimpleURLLoader;
+}  // namespace network
 
 // A class that encapsulates a JSON-RPC call to the Spelling service to check
 // text there. This class creates a JSON-RPC request, sends the request to the
@@ -57,7 +56,7 @@ class URLFetcher;
 //     std::unique_ptr<SpellingServiceClient> client_;
 //   };
 //
-class SpellingServiceClient : public net::URLFetcherDelegate {
+class SpellingServiceClient {
  public:
   // Service types provided by the Spelling service. The Spelling service
   // consists of a couple of backends:
@@ -76,7 +75,7 @@ class SpellingServiceClient : public net::URLFetcherDelegate {
       TextCheckCompleteCallback;
 
   SpellingServiceClient();
-  ~SpellingServiceClient() override;
+  ~SpellingServiceClient();
 
   // Sends a text-check request to the Spelling service. When we send a request
   // to the Spelling service successfully, this function returns true. (This
@@ -90,6 +89,11 @@ class SpellingServiceClient : public net::URLFetcherDelegate {
   // Returns whether the specified service is available for the given context.
   static bool IsAvailable(content::BrowserContext* context, ServiceType type);
 
+  // Set the URL loader factory for tests.
+  void SetURLLoaderFactoryForTesting(
+      scoped_refptr<network::SharedURLLoaderFactory>
+          url_loader_factory_for_testing);
+
  protected:
   // Parses a JSON-RPC response from the Spelling service.
   bool ParseResponse(const std::string& data,
@@ -98,13 +102,14 @@ class SpellingServiceClient : public net::URLFetcherDelegate {
  private:
   struct TextCheckCallbackData {
    public:
-    TextCheckCallbackData(std::unique_ptr<net::URLFetcher> fetcher,
-                          TextCheckCompleteCallback callback,
-                          base::string16 text);
+    TextCheckCallbackData(
+        std::unique_ptr<network::SimpleURLLoader> simple_url_loader,
+        TextCheckCompleteCallback callback,
+        base::string16 text);
     ~TextCheckCallbackData();
 
-    // The fetcher used.
-    std::unique_ptr<net::URLFetcher> fetcher;
+    // The URL loader used.
+    std::unique_ptr<network::SimpleURLLoader> simple_url_loader;
 
     // The callback function to be called when we receive a response from the
     // Spelling service and parse it.
@@ -117,19 +122,18 @@ class SpellingServiceClient : public net::URLFetcherDelegate {
     DISALLOW_COPY_AND_ASSIGN(TextCheckCallbackData);
   };
 
-  // net::URLFetcherDelegate implementation.
-  void OnURLFetchComplete(const net::URLFetcher* source) override;
+  using SpellCheckLoaderList =
+      std::list<std::unique_ptr<TextCheckCallbackData>>;
+
+  void OnSimpleLoaderComplete(SpellCheckLoaderList::iterator it,
+                              std::unique_ptr<std::string> response_body);
 
-  // Creates a URLFetcher object used for sending a JSON-RPC request. This
-  // function is overridden by unit tests to prevent them from actually sending
-  // requests to the Spelling service.
-  virtual std::unique_ptr<net::URLFetcher> CreateURLFetcher(
-      const GURL& url,
-      net::NetworkTrafficAnnotationTag traffic_annotation);
+  // List of loaders in use.
+  SpellCheckLoaderList spellcheck_loaders_;
 
-  // The URLFetcher object used for sending a JSON-RPC request.
-  std::map<const net::URLFetcher*, std::unique_ptr<TextCheckCallbackData>>
-      spellcheck_fetchers_;
+  // URL loader factory to use for fake network requests during testing.
+  scoped_refptr<network::SharedURLLoaderFactory>
+      url_loader_factory_for_testing_;
 };
 
 #endif  // COMPONENTS_SPELLCHECK_BROWSER_SPELLING_SERVICE_CLIENT_H_