S13n: port GaiaCookieManagerService to SimpleURLLoader from URLFetcher

URLFetcher will stop working with advent of Network Service, and
SimpleURLLoader is the replacement API for most clients.

Note that this only covers the code immediately here; the underlying
google_apis/gaia code is unchanged.

Bug: 844963
Change-Id: I9ba41c0c7cbb3a996db49d437b421d65f7e9c8ee
Reviewed-on: https://chromium-review.googlesource.com/1087412
Commit-Queue: Maks Orlovich <morlovich@chromium.org>
Reviewed-by: David Roger <droger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#569271}

components/signin/core/browser/gaia_cookie_manager_service.cc

@@ -26,13 +26,15 @@
 #include "net/base/load_flags.h"
 #include "net/http/http_status_code.h"
 #include "net/traffic_annotation/network_traffic_annotation.h"
-#include "net/url_request/url_fetcher.h"
-#include "net/url_request/url_fetcher_delegate.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"
 
 namespace {
 
-// In case of an error while fetching using the GaiaAuthFetcher or URLFetcher,
-// retry with exponential backoff. Try up to 7 times within 15 minutes.
+// In case of an error while fetching using the GaiaAuthFetcher or
+// SimpleURLLoader, retry with exponential backoff. Try up to 7 times within 15
+// minutes.
 const net::BackoffEntry::Policy kBackoffPolicy = {
     // Number of initial errors (in sequence) to ignore before applying
     // exponential back-off rules.
@@ -140,7 +142,7 @@ void GaiaCookieManagerService::ExternalCcResultFetcher::Start() {
 }
 
 bool GaiaCookieManagerService::ExternalCcResultFetcher::IsRunning() {
-  return helper_->gaia_auth_fetcher_ || fetchers_.size() > 0u ||
+  return helper_->gaia_auth_fetcher_ || loaders_.size() > 0u ||
          timer_.IsRunning();
 }
 
@@ -174,9 +176,9 @@ void GaiaCookieManagerService::ExternalCcResultFetcher::
       if (dict->GetString("carryBackToken", &token) &&
           dict->GetString("url", &url)) {
         results_[token] = "null";
-        net::URLFetcher* fetcher = CreateFetcher(GURL(url)).release();
-        fetchers_[fetcher->GetOriginalURL()] = std::make_pair(token, fetcher);
-        fetcher->Start();
+        network::SimpleURLLoader* loader =
+            CreateAndStartLoader(GURL(url)).release();
+        loaders_[loader] = token;
       }
     }
   }
@@ -199,8 +201,8 @@ void GaiaCookieManagerService::ExternalCcResultFetcher::
   GetCheckConnectionInfoCompleted(false);
 }
 
-std::unique_ptr<net::URLFetcher>
-GaiaCookieManagerService::ExternalCcResultFetcher::CreateFetcher(
+std::unique_ptr<network::SimpleURLLoader>
+GaiaCookieManagerService::ExternalCcResultFetcher::CreateAndStartLoader(
     const GURL& url) {
   net::NetworkTrafficAnnotationTag traffic_annotation =
       net::DefineNetworkTrafficAnnotation(
@@ -229,47 +231,63 @@ GaiaCookieManagerService::ExternalCcResultFetcher::CreateFetcher(
               "support for child accounts). It makes sense to control top "
               "level features that use the GaiaCookieManager."
           })");
-  std::unique_ptr<net::URLFetcher> fetcher = net::URLFetcher::Create(
-      0, url, net::URLFetcher::GET, this, traffic_annotation);
-  fetcher->SetRequestContext(helper_->request_context());
-  fetcher->SetLoadFlags(net::LOAD_DO_NOT_SEND_COOKIES |
-                        net::LOAD_DO_NOT_SAVE_COOKIES);
-  data_use_measurement::DataUseUserData::AttachToFetcher(
-      fetcher.get(), data_use_measurement::DataUseUserData::SIGNIN);
+
+  auto request = std::make_unique<network::ResourceRequest>();
+  request->url = url;
+  request->load_flags =
+      net::LOAD_DO_NOT_SEND_COOKIES | net::LOAD_DO_NOT_SAVE_COOKIES;
+  // TODO(https://crbug.com/808498) re-add data use measurement once
+  // SimpleURLLoader supports it: data_use_measurement::DataUseUserData::SIGNIN
+
+  std::unique_ptr<network::SimpleURLLoader> loader =
+      network::SimpleURLLoader::Create(std::move(request), traffic_annotation);
 
   // Fetchers are sometimes cancelled because a network change was detected,
   // especially at startup and after sign-in on ChromeOS.
-  fetcher->SetAutomaticallyRetryOnNetworkChanges(1);
-  return fetcher;
-}
-
-void GaiaCookieManagerService::ExternalCcResultFetcher::OnURLFetchComplete(
-    const net::URLFetcher* source) {
-  const GURL& url = source->GetOriginalURL();
-  const net::URLRequestStatus& status = source->GetStatus();
-  int response_code = source->GetResponseCode();
-  if (status.is_success() && response_code == net::HTTP_OK &&
-      fetchers_.count(url) > 0) {
-    std::string data;
-    source->GetResponseAsString(&data);
-    // Only up to the first 16 characters of the response are important to GAIA.
-    // Truncate if needed to keep amount data sent back to GAIA down.
-    if (data.size() > 16)
-      data.resize(16);
-    results_[fetchers_[url].first] = data;
-
-    // Clean up tracking of this fetcher.  The rest will be cleaned up after
-    // the timer expires in CleanupTransientState().
-    DCHECK_EQ(source, fetchers_[url].second);
-    fetchers_.erase(url);
-    delete source;
-
-    // If all expected responses have been received, cancel the timer and
-    // report the result.
-    if (fetchers_.empty()) {
-      CleanupTransientState();
-      GetCheckConnectionInfoCompleted(true);
-    }
+  loader->SetRetryOptions(1, network::SimpleURLLoader::RETRY_ON_NETWORK_CHANGE);
+
+  loader->DownloadToStringOfUnboundedSizeUntilCrashAndDie(
+      helper_->GetURLLoaderFactory().get(),
+      base::BindOnce(&ExternalCcResultFetcher::OnURLLoadComplete,
+                     base::Unretained(this), loader.get()));
+
+  return loader;
+}
+
+void GaiaCookieManagerService::ExternalCcResultFetcher::OnURLLoadComplete(
+    const network::SimpleURLLoader* source,
+    std::unique_ptr<std::string> body) {
+  if (source->NetError() != net::OK || !source->ResponseInfo() ||
+      !source->ResponseInfo()->headers ||
+      source->ResponseInfo()->headers->response_code() != net::HTTP_OK) {
+    return;
+  }
+
+  LoaderToToken::iterator it = loaders_.find(source);
+  if (it == loaders_.end())
+    return;
+
+  std::string data;
+  if (body)
+    data = std::move(*body);
+
+  // Only up to the first 16 characters of the response are important to GAIA.
+  // Truncate if needed to keep amount data sent back to GAIA down.
+  if (data.size() > 16)
+    data.resize(16);
+  results_[it->second] = data;
+
+  // Clean up tracking of this fetcher.  The rest will be cleaned up after
+  // the timer expires in CleanupTransientState().
+  DCHECK_EQ(source, it->first);
+  loaders_.erase(it);
+  delete source;
+
+  // If all expected responses have been received, cancel the timer and
+  // report the result.
+  if (loaders_.empty()) {
+    CleanupTransientState();
+    GetCheckConnectionInfoCompleted(true);
   }
 }
 
@@ -284,11 +302,10 @@ void GaiaCookieManagerService::ExternalCcResultFetcher::
   timer_.Stop();
   helper_->gaia_auth_fetcher_.reset();
 
-  for (URLToTokenAndFetcher::const_iterator it = fetchers_.begin();
-       it != fetchers_.end(); ++it) {
-    delete it->second.second;
+  for (const auto& loader_token_pair : loaders_) {
+    delete loader_token_pair.first;
   }
-  fetchers_.clear();
+  loaders_.clear();
 }
 
 void GaiaCookieManagerService::ExternalCcResultFetcher::
@@ -490,6 +507,11 @@ void GaiaCookieManagerService::CancelAll() {
   fetcher_timer_.Stop();
 }
 
+scoped_refptr<network::SharedURLLoaderFactory>
+GaiaCookieManagerService::GetURLLoaderFactory() {
+  return signin_client_->GetURLLoaderFactory();
+}
+
 std::string GaiaCookieManagerService::GetSourceForRequest(
     const GaiaCookieManagerService::GaiaCookieRequest& request) {
   std::string source = request.source().empty() ? GetDefaultSourceForRequest()

components/signin/core/browser/gaia_cookie_manager_service.h

@@ -20,15 +20,15 @@
 #include "google_apis/gaia/gaia_auth_util.h"
 #include "google_apis/gaia/ubertoken_fetcher.h"
 #include "net/base/backoff_entry.h"
-#include "net/url_request/url_fetcher_delegate.h"
 
 class GaiaAuthFetcher;
 class GaiaCookieRequest;
 class GoogleServiceAuthError;
 class OAuth2TokenService;
 
-namespace net {
-class URLFetcher;
+namespace network {
+class SharedURLLoaderFactory;
+class SimpleURLLoader;
 }
 
 // Merges a Google account known to Chrome into the cookie jar.  When merging
@@ -108,13 +108,11 @@ class GaiaCookieManagerService : public KeyedService,
 
   // Class to retrieve the external connection check results from gaia.
   // Declared publicly for unit tests.
-  class ExternalCcResultFetcher : public GaiaAuthConsumer,
-                                  public net::URLFetcherDelegate {
+  class ExternalCcResultFetcher : public GaiaAuthConsumer {
    public:
-    // Maps connection URLs, as returned by StartGetCheckConnectionInfo() to
-    // token and URLFetcher used to fetch the URL.
-    typedef std::map<GURL, std::pair<std::string, net::URLFetcher*>>
-        URLToTokenAndFetcher;
+    // Maps connection check SimpleURLLoader to corresponding token.
+    typedef std::map<const network::SimpleURLLoader*, std::string>
+        LoaderToToken;
 
     // Maps tokens to the fetched result for that token.
     typedef std::map<std::string, std::string> ResultMap;
@@ -132,8 +130,8 @@ class GaiaCookieManagerService : public KeyedService,
     // Are external URLs still being checked?
     bool IsRunning();
 
-    // Returns a copy of the internal token to fetcher map.
-    URLToTokenAndFetcher get_fetcher_map_for_testing() { return fetchers_; }
+    // Returns a copy of the internal loader to token map.
+    LoaderToToken get_loader_map_for_testing() { return loaders_; }
 
     // Simulate a timeout for tests.
     void TimeoutForTests();
@@ -144,11 +142,13 @@ class GaiaCookieManagerService : public KeyedService,
     void OnGetCheckConnectionInfoError(
         const GoogleServiceAuthError& error) override;
 
-    // Creates and initializes a URL fetcher for doing a connection check.
-    std::unique_ptr<net::URLFetcher> CreateFetcher(const GURL& url);
+    // Creates and initializes a loader for doing a connection check.
+    std::unique_ptr<network::SimpleURLLoader> CreateAndStartLoader(
+        const GURL& url);
 
-    // Overridden from URLFetcherDelgate.
-    void OnURLFetchComplete(const net::URLFetcher* source) override;
+    // Called back from SimpleURLLoader.
+    void OnURLLoadComplete(const network::SimpleURLLoader* source,
+                           std::unique_ptr<std::string> body);
 
     // Any fetches still ongoing after this call are considered timed out.
     void Timeout();
@@ -159,7 +159,7 @@ class GaiaCookieManagerService : public KeyedService,
 
     GaiaCookieManagerService* helper_;
     base::OneShotTimer timer_;
-    URLToTokenAndFetcher fetchers_;
+    LoaderToToken loaders_;
     ResultMap results_;
     base::Time m_external_cc_result_start_time_;
 
@@ -237,6 +237,8 @@ class GaiaCookieManagerService : public KeyedService,
     return signin_client_->GetURLRequestContext();
   }
 
+  scoped_refptr<network::SharedURLLoaderFactory> GetURLLoaderFactory();
+
   // Returns the source value to use for GaiaFetcher requests.  This is
   // virtual to allow tests and fake classes to override.
   virtual std::string GetSourceForRequest(

components/signin/core/browser/gaia_cookie_manager_service_unittest.cc

@@ -28,6 +28,7 @@
 #include "google_apis/gaia/gaia_urls.h"
 #include "net/url_request/test_url_fetcher_factory.h"
 #include "net/url_request/url_request_test_util.h"
+#include "services/network/test/test_url_loader_factory.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -164,14 +165,15 @@ class GaiaCookieManagerServiceTest : public testing::Test {
     fetcher->delegate()->OnURLFetchComplete(fetcher);
   }
 
-  void SimulateGetCheckConnctionInfoResult(net::URLFetcher* fetcher,
-                                           const std::string& result) {
-    net::TestURLFetcher* test_fetcher =
-        static_cast<net::TestURLFetcher*>(fetcher);
-    test_fetcher->set_status(net::URLRequestStatus());
-    test_fetcher->set_response_code(200);
-    test_fetcher->SetResponseString(result);
-    test_fetcher->delegate()->OnURLFetchComplete(fetcher);
+  void SimulateGetCheckConnectionInfoResult(const std::string& url,
+                                            const std::string& result) {
+    signin_client_->test_url_loader_factory()->AddResponse(url, result);
+    base::RunLoop().RunUntilIdle();
+  }
+
+  bool IsLoadPending(const std::string& url) {
+    return signin_client_->test_url_loader_factory()->IsPending(
+        GURL(url).spec());
   }
 
   const GoogleServiceAuthError& no_error() { return no_error_; }
@@ -713,18 +715,16 @@ TEST_F(GaiaCookieManagerServiceTest, ExternalCcResultFetcher) {
       " {\"carryBackToken\": \"bl\", \"url\": \"http://www.bl.com\"}]");
 
   // Simulate responses for the two connection URLs.
-  GaiaCookieManagerService::ExternalCcResultFetcher::URLToTokenAndFetcher
-      fetchers = result_fetcher.get_fetcher_map_for_testing();
-  ASSERT_EQ(2u, fetchers.size());
-  ASSERT_EQ(1u, fetchers.count(GURL("http://www.yt.com")));
-  ASSERT_EQ(1u, fetchers.count(GURL("http://www.bl.com")));
+  GaiaCookieManagerService::ExternalCcResultFetcher::LoaderToToken loaders =
+      result_fetcher.get_loader_map_for_testing();
+  ASSERT_EQ(2u, loaders.size());
+  ASSERT_TRUE(IsLoadPending("http://www.yt.com"));
+  ASSERT_TRUE(IsLoadPending("http://www.bl.com"));
 
   ASSERT_EQ("bl:null,yt:null", result_fetcher.GetExternalCcResult());
-  SimulateGetCheckConnctionInfoResult(
-      fetchers[GURL("http://www.yt.com")].second, "yt_result");
+  SimulateGetCheckConnectionInfoResult("http://www.yt.com", "yt_result");
   ASSERT_EQ("bl:null,yt:yt_result", result_fetcher.GetExternalCcResult());
-  SimulateGetCheckConnctionInfoResult(
-      fetchers[GURL("http://www.bl.com")].second, "bl_result");
+  SimulateGetCheckConnectionInfoResult("http://www.bl.com", "bl_result");
   ASSERT_EQ("bl:bl_result,yt:yt_result", result_fetcher.GetExternalCcResult());
 }
 
@@ -742,23 +742,22 @@ TEST_F(GaiaCookieManagerServiceTest, ExternalCcResultFetcherTimeout) {
       "[{\"carryBackToken\": \"yt\", \"url\": \"http://www.yt.com\"},"
       " {\"carryBackToken\": \"bl\", \"url\": \"http://www.bl.com\"}]");
 
-  GaiaCookieManagerService::ExternalCcResultFetcher::URLToTokenAndFetcher
-      fetchers = result_fetcher.get_fetcher_map_for_testing();
-  ASSERT_EQ(2u, fetchers.size());
-  ASSERT_EQ(1u, fetchers.count(GURL("http://www.yt.com")));
-  ASSERT_EQ(1u, fetchers.count(GURL("http://www.bl.com")));
+  GaiaCookieManagerService::ExternalCcResultFetcher::LoaderToToken loaders =
+      result_fetcher.get_loader_map_for_testing();
+  ASSERT_EQ(2u, loaders.size());
+  ASSERT_TRUE(IsLoadPending("http://www.yt.com"));
+  ASSERT_TRUE(IsLoadPending("http://www.bl.com"));
 
   // Simulate response only for "yt".
   ASSERT_EQ("bl:null,yt:null", result_fetcher.GetExternalCcResult());
-  SimulateGetCheckConnctionInfoResult(
-      fetchers[GURL("http://www.yt.com")].second, "yt_result");
+  SimulateGetCheckConnectionInfoResult("http://www.yt.com", "yt_result");
   ASSERT_EQ("bl:null,yt:yt_result", result_fetcher.GetExternalCcResult());
 
   // Now timeout.
   result_fetcher.TimeoutForTests();
   ASSERT_EQ("bl:null,yt:yt_result", result_fetcher.GetExternalCcResult());
-  fetchers = result_fetcher.get_fetcher_map_for_testing();
-  ASSERT_EQ(0u, fetchers.size());
+  loaders = result_fetcher.get_loader_map_for_testing();
+  ASSERT_EQ(0u, loaders.size());
 }
 
 TEST_F(GaiaCookieManagerServiceTest, ExternalCcResultFetcherTruncate) {
@@ -774,14 +773,14 @@ TEST_F(GaiaCookieManagerServiceTest, ExternalCcResultFetcherTruncate) {
       fetcher,
       "[{\"carryBackToken\": \"yt\", \"url\": \"http://www.yt.com\"}]");
 
-  GaiaCookieManagerService::ExternalCcResultFetcher::URLToTokenAndFetcher
-      fetchers = result_fetcher.get_fetcher_map_for_testing();
-  ASSERT_EQ(1u, fetchers.size());
-  ASSERT_EQ(1u, fetchers.count(GURL("http://www.yt.com")));
+  GaiaCookieManagerService::ExternalCcResultFetcher::LoaderToToken loaders =
+      result_fetcher.get_loader_map_for_testing();
+  ASSERT_EQ(1u, loaders.size());
+  ASSERT_TRUE(IsLoadPending("http://www.yt.com"));
 
   // Simulate response for "yt" with a string that is too long.
-  SimulateGetCheckConnctionInfoResult(
-      fetchers[GURL("http://www.yt.com")].second, "1234567890123456trunc");
+  SimulateGetCheckConnectionInfoResult("http://www.yt.com",
+                                       "1234567890123456trunc");
   ASSERT_EQ("yt:1234567890123456", result_fetcher.GetExternalCcResult());
 }
 
@@ -802,10 +801,10 @@ TEST_F(GaiaCookieManagerServiceTest, UbertokenSuccessFetchesExternalCC) {
       "[{\"carryBackToken\": \"bl\", \"url\": \"http://www.bl.com\"}]");
   GaiaCookieManagerService::ExternalCcResultFetcher* result_fetcher =
       helper.external_cc_result_fetcher_for_testing();
-  GaiaCookieManagerService::ExternalCcResultFetcher::URLToTokenAndFetcher
-      fetchers = result_fetcher->get_fetcher_map_for_testing();
-  ASSERT_EQ(1u, fetchers.size());
-  ASSERT_EQ(1u, fetchers.count(GURL("http://www.bl.com")));
+  GaiaCookieManagerService::ExternalCcResultFetcher::LoaderToToken loaders =
+      result_fetcher->get_loader_map_for_testing();
+  ASSERT_EQ(1u, loaders.size());
+  ASSERT_TRUE(IsLoadPending("http://www.bl.com"));
 }
 
 TEST_F(GaiaCookieManagerServiceTest, UbertokenSuccessFetchesExternalCCOnce) {