Migrate remoting::ChromiumUrlRequest to SimpleURLLoader

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

BUG=773295

Change-Id: Ib8c567eb3271174f7b1fb802321af478f529f5c7
Reviewed-on: https://chromium-review.googlesource.com/1194042
Commit-Queue: Antonio Gomes <tonikitoo@igalia.com>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Reviewed-by: Joe Downing <joedow@chromium.org>
Cr-Commit-Position: refs/heads/master@{#586848}

remoting/base/chromium_url_request.cc

@@ -8,78 +8,93 @@
 
 #include "base/callback_helpers.h"
 #include "net/base/load_flags.h"
-#include "net/url_request/url_fetcher.h"
-#include "net/url_request/url_request_context_getter.h"
+#include "services/network/public/cpp/shared_url_loader_factory.h"
+#include "services/network/public/cpp/simple_url_loader.h"
 
 namespace remoting {
 
 ChromiumUrlRequest::ChromiumUrlRequest(
-    scoped_refptr<net::URLRequestContextGetter> url_context,
+    scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory,
     UrlRequest::Type type,
     const std::string& url,
-    const net::NetworkTrafficAnnotationTag& traffic_annotation) {
-  net::URLFetcher::RequestType request_type = net::URLFetcher::GET;
+    const net::NetworkTrafficAnnotationTag& traffic_annotation)
+    : traffic_annotation_(traffic_annotation) {
+  url_loader_factory_ = url_loader_factory;
+
+  std::string request_type = "GET";
   switch (type) {
     case Type::GET:
-      request_type = net::URLFetcher::GET;
       break;
     case Type::POST:
-      request_type = net::URLFetcher::POST;
+      request_type = "POST";
       break;
   }
-  url_fetcher_ = net::URLFetcher::Create(GURL(url), request_type, this,
-                                         traffic_annotation);
-  url_fetcher_->SetRequestContext(url_context.get());
-  url_fetcher_->SetReferrer("https://chrome.google.com/remotedesktop");
-  url_fetcher_->SetLoadFlags(net::LOAD_DO_NOT_SAVE_COOKIES |
-                             net::LOAD_DO_NOT_SEND_COOKIES);
+  resource_request_ = std::make_unique<network::ResourceRequest>();
+  resource_request_->url = GURL(url);
+  resource_request_->method = request_type;
+  resource_request_->load_flags =
+      net::LOAD_DO_NOT_SAVE_COOKIES | net::LOAD_DO_NOT_SEND_COOKIES;
+  resource_request_->referrer = GURL("https://chrome.google.com/remotedesktop");
 }
 
 ChromiumUrlRequest::~ChromiumUrlRequest() = default;
 
 void ChromiumUrlRequest::AddHeader(const std::string& value) {
-  url_fetcher_->AddExtraRequestHeader(value);
+  resource_request_->headers.AddHeaderFromString(value);
 }
 
 void ChromiumUrlRequest::SetPostData(const std::string& content_type,
                                      const std::string& data) {
-  url_fetcher_->SetUploadData(content_type, data);
+  post_data_content_type_ = content_type;
+  post_data_ = data;
 }
 
 void ChromiumUrlRequest::Start(const OnResultCallback& on_result_callback) {
   DCHECK(!on_result_callback.is_null());
   DCHECK(on_result_callback_.is_null());
 
   on_result_callback_ = on_result_callback;
-  url_fetcher_->Start();
-}
 
-void ChromiumUrlRequest::OnURLFetchComplete(
-    const net::URLFetcher* url_fetcher) {
-  DCHECK_EQ(url_fetcher, url_fetcher_.get());
+  std::string method = resource_request_->method;
+
+  url_loader_ = network::SimpleURLLoader::Create(std::move(resource_request_),
+                                                 traffic_annotation_);
+  if (method == "POST")
+    url_loader_->AttachStringForUpload(post_data_, post_data_content_type_);
 
+  url_loader_->DownloadToStringOfUnboundedSizeUntilCrashAndDie(
+      url_loader_factory_.get(),
+      base::BindOnce(&ChromiumUrlRequest::OnURLLoadComplete,
+                     base::Unretained(this)));
+}
+
+void ChromiumUrlRequest::OnURLLoadComplete(
+    std::unique_ptr<std::string> response_body) {
   Result result;
-  result.success =
-      url_fetcher_->GetResponseCode() != net::URLFetcher::RESPONSE_CODE_INVALID;
+  result.success = !!response_body;
   if (result.success) {
-    result.status = url_fetcher_->GetResponseCode();
-    url_fetcher_->GetResponseAsString(&result.response_body);
+    result.status = -1;
+    result.response_body = std::move(*response_body);
+  }
+
+  if (url_loader_->ResponseInfo() && url_loader_->ResponseInfo()->headers) {
+    result.status = url_loader_->ResponseInfo()->headers->response_code();
   }
 
   DCHECK(!on_result_callback_.is_null());
   base::ResetAndReturn(&on_result_callback_).Run(result);
 }
 
 ChromiumUrlRequestFactory::ChromiumUrlRequestFactory(
-    scoped_refptr<net::URLRequestContextGetter> url_context)
-    : url_context_(url_context) {}
+    scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory)
+    : url_loader_factory_(url_loader_factory) {}
 ChromiumUrlRequestFactory::~ChromiumUrlRequestFactory() = default;
 
 std::unique_ptr<UrlRequest> ChromiumUrlRequestFactory::CreateUrlRequest(
     UrlRequest::Type type,
     const std::string& url,
     const net::NetworkTrafficAnnotationTag& traffic_annotation) {
-  return std::make_unique<ChromiumUrlRequest>(url_context_, type, url,
+  return std::make_unique<ChromiumUrlRequest>(url_loader_factory_, type, url,
                                               traffic_annotation);
 }
 

remoting/base/chromium_url_request.h

@@ -10,20 +10,21 @@
 #include "base/callback.h"
 #include "base/memory/ref_counted.h"
 #include "net/traffic_annotation/network_traffic_annotation.h"
-#include "net/url_request/url_fetcher_delegate.h"
 #include "remoting/base/url_request.h"
 
-namespace net {
-class URLRequestContextGetter;
-}  // namespace net
+namespace network {
+class SharedURLLoaderFactory;
+class SimpleURLLoader;
+struct ResourceRequest;
+}  // namespace network
 
 namespace remoting {
 
-// UrlRequest implementation based on net::URLFetcher.
-class ChromiumUrlRequest : public UrlRequest, public net::URLFetcherDelegate {
+// UrlRequest implementation based on network::SimpleURLLoader.
+class ChromiumUrlRequest : public UrlRequest {
  public:
   ChromiumUrlRequest(
-      scoped_refptr<net::URLRequestContextGetter> url_context,
+      scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory,
       UrlRequest::Type type,
       const std::string& url,
       const net::NetworkTrafficAnnotationTag& traffic_annotation);
@@ -36,17 +37,23 @@ class ChromiumUrlRequest : public UrlRequest, public net::URLFetcherDelegate {
   void Start(const OnResultCallback& on_result_callback) override;
 
  private:
-  // net::URLFetcherDelegate interface.
-  void OnURLFetchComplete(const net::URLFetcher* url_fetcher) override;
+  void OnURLLoadComplete(std::unique_ptr<std::string> response_body);
+
+  std::unique_ptr<network::SimpleURLLoader> url_loader_;
+  scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory_;
+  std::unique_ptr<network::ResourceRequest> resource_request_;
+
+  const net::NetworkTrafficAnnotationTag traffic_annotation_;
+  std::string post_data_content_type_;
+  std::string post_data_;
 
-  std::unique_ptr<net::URLFetcher> url_fetcher_;
   OnResultCallback on_result_callback_;
 };
 
 class ChromiumUrlRequestFactory : public UrlRequestFactory {
  public:
   ChromiumUrlRequestFactory(
-      scoped_refptr<net::URLRequestContextGetter> url_context);
+      scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory);
   ~ChromiumUrlRequestFactory() override;
 
   // UrlRequestFactory interface.
@@ -56,7 +63,7 @@ class ChromiumUrlRequestFactory : public UrlRequestFactory {
       const net::NetworkTrafficAnnotationTag& traffic_annotation) override;
 
  private:
-  scoped_refptr<net::URLRequestContextGetter> url_context_;
+  scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory_;
 };
 
 }  // namespace remoting

remoting/client/BUILD.gn

@@ -6,8 +6,6 @@ static_library("client") {
   sources = [
     "chromoting_client.cc",
     "chromoting_client.h",
-    "chromoting_session.cc",
-    "chromoting_session.h",
     "client_context.cc",
     "client_context.h",
     "client_telemetry_logger.cc",
@@ -55,6 +53,8 @@ static_library("client") {
     sources += [
       "chromoting_client_runtime.cc",
       "chromoting_client_runtime.h",
+      "chromoting_session.cc",
+      "chromoting_session.h",
       "dual_buffer_frame_consumer.cc",
       "dual_buffer_frame_consumer.h",
       "oauth_token_getter_proxy.cc",
@@ -63,6 +63,7 @@ static_library("client") {
     deps += [
       "//remoting/client/input",
       "//remoting/client/ui",
+      "//services/network/public/mojom",
     ]
 
     if (!is_chromeos) {
@@ -73,9 +74,7 @@ static_library("client") {
         "gesture_interpreter.h",
       ]
 
-      deps += [
-        "//remoting/client/display",
-      ]
+      deps += [ "//remoting/client/display" ]
     }
   }
   if (is_android) {

remoting/client/chromoting_client_runtime.cc

@@ -86,7 +86,7 @@ void ChromotingClientRuntime::Init(
   delegate_ = delegate;
   log_writer_ = std::make_unique<TelemetryLogWriter>(
       kTelemetryBaseUrl,
-      std::make_unique<ChromiumUrlRequestFactory>(url_requester()),
+      std::make_unique<ChromiumUrlRequestFactory>(url_loader_factory()),
       CreateOAuthTokenGetter());
 }
 

remoting/client/chromoting_session.cc

@@ -36,6 +36,7 @@
 #include "remoting/protocol/transport_context.h"
 #include "remoting/protocol/video_renderer.h"
 #include "remoting/signaling/server_log_entry.h"
+#include "services/network/public/cpp/shared_url_loader_factory.h"
 #include "ui/events/keycodes/dom/keycode_converter.h"
 
 namespace remoting {
@@ -501,7 +502,7 @@ void ChromotingSession::Core::ConnectOnNetworkThread() {
           signaling_.get(),
           std::make_unique<protocol::ChromiumPortAllocatorFactory>(),
           std::make_unique<ChromiumUrlRequestFactory>(
-              runtime_->url_requester()),
+              runtime_->url_loader_factory()),
           protocol::NetworkSettings(
               protocol::NetworkSettings::NAT_TRAVERSAL_FULL),
           protocol::TransportRole::CLIENT);

remoting/host/it2me/it2me_host.cc

@@ -40,6 +40,7 @@
 #include "remoting/protocol/validating_authenticator.h"
 #include "remoting/signaling/jid_util.h"
 #include "remoting/signaling/server_log_entry.h"
+#include "services/network/public/cpp/shared_url_loader_factory.h"
 
 namespace remoting {
 
@@ -170,7 +171,7 @@ void It2MeHost::ConnectOnNetworkThread(const std::string& username,
           signal_strategy_.get(),
           base::WrapUnique(new protocol::ChromiumPortAllocatorFactory()),
           base::WrapUnique(new ChromiumUrlRequestFactory(
-              host_context_->url_request_context_getter())),
+              host_context_->url_loader_factory())),
           network_settings, protocol::TransportRole::SERVER);
   transport_context->set_turn_ice_config(ice_config);
 

remoting/host/it2me/it2me_host_unittest.cc

@@ -17,6 +17,7 @@
 #include "base/memory/weak_ptr.h"
 #include "base/message_loop/message_loop.h"
 #include "base/run_loop.h"
+#include "base/test/scoped_task_environment.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "components/policy/policy_constants.h"
 #include "remoting/base/auto_thread_task_runner.h"
@@ -185,7 +186,8 @@ class It2MeHostTest : public testing::Test, public It2MeHost::Observer {
  private:
   void StartupHostStateHelper(const base::Closure& quit_closure);
 
-  std::unique_ptr<base::MessageLoop> message_loop_;
+  base::test::ScopedTaskEnvironment scoped_task_environment_;
+
   std::unique_ptr<base::RunLoop> run_loop_;
   std::unique_ptr<FakeSignalStrategy> fake_bot_signal_strategy_;
 
@@ -207,7 +209,6 @@ void It2MeHostTest::SetUp() {
   // network thread. base::GetLinuxDistro() caches the result.
   base::GetLinuxDistro();
 #endif
-  message_loop_.reset(new base::MessageLoop());
   run_loop_.reset(new base::RunLoop());
 
   host_context_ = ChromotingHostContext::Create(new AutoThreadTaskRunner(

remoting/host/remoting_me2me_host.cc

@@ -1513,7 +1513,7 @@ void HostProcess::StartHost() {
           signal_strategy_.get(),
           std::make_unique<protocol::ChromiumPortAllocatorFactory>(),
           std::make_unique<ChromiumUrlRequestFactory>(
-              context_->url_request_context_getter()),
+              context_->url_loader_factory()),
           network_settings, protocol::TransportRole::SERVER);
   transport_context->set_ice_config_url(
       ServiceUrls::GetInstance()->ice_config_url(), oauth_token_getter_.get());

remoting/protocol/http_ice_config_request.cc

@@ -103,15 +103,15 @@ void HttpIceConfigRequest::SendRequest() {
 void HttpIceConfigRequest::OnResponse(const UrlRequest::Result& result) {
   DCHECK(!on_ice_config_callback_.is_null());
 
-  if (!result.success) {
-    LOG(ERROR) << "Failed to fetch " << url_;
+  if (result.status != -1 && result.status != 200) {
+    LOG(ERROR) << "Received status code " << result.status << " from " << url_
+               << ": " << result.response_body;
     base::ResetAndReturn(&on_ice_config_callback_).Run(IceConfig());
     return;
   }
 
-  if (result.status != 200) {
-    LOG(ERROR) << "Received status code " << result.status << " from " << url_
-               << ": " << result.response_body;
+  if (!result.success) {
+    LOG(ERROR) << "Failed to fetch " << url_;
     base::ResetAndReturn(&on_ice_config_callback_).Run(IceConfig());
     return;
   }

remoting/test/BUILD.gn

@@ -79,6 +79,7 @@ static_library("test_support") {
     "//remoting/codec:encoder",
     "//remoting/protocol",
     "//remoting/signaling",
+    "//services/network:test_support",
     "//third_party/webrtc/modules/desktop_capture",
     "//ui/gfx",
   ]

remoting/test/test_chromoting_client.cc

@@ -28,6 +28,7 @@
 #include "remoting/signaling/xmpp_signal_strategy.h"
 #include "remoting/test/connection_setup_info.h"
 #include "remoting/test/test_video_renderer.h"
+#include "services/network/test/test_shared_url_loader_factory.h"
 
 namespace remoting {
 namespace test {
@@ -85,6 +86,9 @@ void TestChromotingClient::StartConnection(
   request_context_getter =
       new URLRequestContextGetter(base::ThreadTaskRunnerHandle::Get());
 
+  auto test_shared_url_loader_factory =
+      base::MakeRefCounted<network::TestSharedURLLoaderFactory>();
+
   client_context_.reset(new ClientContext(base::ThreadTaskRunnerHandle::Get()));
 
   // Check to see if the user passed in a customized video renderer.
@@ -124,7 +128,8 @@ void TestChromotingClient::StartConnection(
       new protocol::TransportContext(
           signal_strategy_.get(),
           std::make_unique<protocol::ChromiumPortAllocatorFactory>(),
-          std::make_unique<ChromiumUrlRequestFactory>(request_context_getter),
+          std::make_unique<ChromiumUrlRequestFactory>(
+              test_shared_url_loader_factory),
           network_settings, protocol::TransportRole::CLIENT));
 
   protocol::ClientAuthenticationConfig client_auth_config;