Add a test helper method in services/network to get the body from a ResourceRequest.

Also remove the custom URLLoaderFactory in content_hash_fetcher_unittest. This isn't needed anymore as TestURLLoaderFactory now supports Clone.

Cq-Include-Trybots: luci.chromium.try:linux_mojo
Change-Id: Id70ec605886a6b0f2cb9bf7392fac4eff85fc1b3
Reviewed-on: https://chromium-review.googlesource.com/1115346Reviewed-by: Doug Turner <dougt@chromium.org>
Commit-Queue: John Abd-El-Malek <jam@chromium.org>
Cr-Commit-Position: refs/heads/master@{#570813}

chrome/browser/profile_resetter/profile_resetter_unittest.cc

@@ -50,6 +50,7 @@
 #include "net/http/http_util.h"
 #include "services/network/public/cpp/resource_request.h"
 #include "services/network/test/test_url_loader_factory.h"
+#include "services/network/test/test_utils.h"
 #include "url/gurl.h"
 
 #if defined(OS_WIN)
@@ -182,17 +183,6 @@ class ConfigParserTest : public testing::Test {
   ConfigParserTest();
   ~ConfigParserTest() override;
 
-  std::string GetBodyFromRequest(const network::ResourceRequest& request) {
-    auto body = request.request_body;
-    if (!body)
-      return std::string();
-
-    CHECK_EQ(1u, body->elements()->size());
-    auto& element = body->elements()->at(0);
-    CHECK_EQ(network::DataElement::TYPE_BYTES, element.type());
-    return std::string(element.bytes(), element.length());
-  }
-
   std::unique_ptr<BrandcodeConfigFetcher> WaitForRequest(const GURL& url);
 
   network::TestURLLoaderFactory& test_url_loader_factory() {
@@ -224,7 +214,7 @@ std::unique_ptr<BrandcodeConfigFetcher> ConfigParserTest::WaitForRequest(
   std::string upload_data;
   test_url_loader_factory_.SetInterceptor(
       base::BindLambdaForTesting([&](const network::ResourceRequest& request) {
-        upload_data = GetBodyFromRequest(request);
+        upload_data = network::GetUploadData(request);
       }));
   std::unique_ptr<BrandcodeConfigFetcher> fetcher(new BrandcodeConfigFetcher(
       &test_url_loader_factory_,

chrome/browser/safe_browsing/certificate_reporting_service_test_utils.cc

@@ -15,6 +15,7 @@
 #include "net/base/upload_data_stream.h"
 #include "net/http/http_response_headers.h"
 #include "net/url_request/url_request_filter.h"
+#include "services/network/test/test_utils.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "third_party/boringssl/src/include/openssl/curve25519.h"
 
@@ -23,18 +24,9 @@ namespace {
 static const char kHkdfLabel[] = "certificate report";
 const uint32_t kServerPublicKeyTestVersion = 16;
 
-std::string GetUploadData(const network::ResourceRequest& request) {
-  auto body = request.request_body;
-  CHECK(body.get());
-  CHECK_EQ(1u, body->elements()->size());
-  const auto& element = body->elements()->at(0);
-  CHECK_EQ(network::DataElement::TYPE_BYTES, element.type());
-  return std::string(element.bytes(), element.length());
-}
-
 std::string GetReportContents(const network::ResourceRequest& request,
                               const uint8_t* server_private_key) {
-  std::string serialized_report(GetUploadData(request));
+  std::string serialized_report(network::GetUploadData(request));
   encrypted_messages::EncryptedMessage encrypted_message;
   EXPECT_TRUE(encrypted_message.ParseFromString(serialized_report));
   EXPECT_EQ(kServerPublicKeyTestVersion,

chrome/browser/safe_browsing/download_protection/download_protection_service_unittest.cc

@@ -69,6 +69,7 @@
 #include "net/url_request/url_request_test_util.h"
 #include "services/network/public/cpp/weak_wrapper_shared_url_loader_factory.h"
 #include "services/network/test/test_url_loader_factory.h"
+#include "services/network/test/test_utils.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "third_party/zlib/google/zip.h"
@@ -207,17 +208,6 @@ class MockBinaryFeatureExtractor : public BinaryFeatureExtractor {
 
 using NiceMockDownloadItem = NiceMock<download::MockDownloadItem>;
 
-std::string GetBodyFromRequest(const network::ResourceRequest& request) {
-  auto body = request.request_body;
-  if (!body)
-    return std::string();
-
-  CHECK_EQ(1u, body->elements()->size());
-  auto& element = body->elements()->at(0);
-  CHECK_EQ(network::DataElement::TYPE_BYTES, element.type());
-  return std::string(element.bytes(), element.length());
-}
-
 }  // namespace
 
 ACTION_P(SetCertificateContents, contents) {
@@ -1875,7 +1865,7 @@ TEST_F(DownloadProtectionServiceTest,
     sb_service_->test_url_loader_factory()->SetInterceptor(
         base::BindLambdaForTesting(
             [&](const network::ResourceRequest& request) {
-              upload_data = GetBodyFromRequest(request);
+              upload_data = network::GetUploadData(request);
               if (!upload_data.empty())
                 interceptor_run_loop.Quit();
             }));
@@ -1933,7 +1923,7 @@ TEST_F(DownloadProtectionServiceTest,
     sb_service_->test_url_loader_factory()->SetInterceptor(
         base::BindLambdaForTesting(
             [&](const network::ResourceRequest& request) {
-              upload_data = GetBodyFromRequest(request);
+              upload_data = network::GetUploadData(request);
               if (!upload_data.empty())
                 interceptor_run_loop.Quit();
             }));
@@ -2488,7 +2478,7 @@ TEST_F(DownloadProtectionServiceTest, PPAPIDownloadRequest_Payload) {
   std::string upload_data;
   sb_service_->test_url_loader_factory()->SetInterceptor(
       base::BindLambdaForTesting([&](const network::ResourceRequest& request) {
-        upload_data = GetBodyFromRequest(request);
+        upload_data = network::GetUploadData(request);
       }));
 
   base::FilePath default_file_path(FILE_PATH_LITERAL("/foo/bar/test.crx"));

chrome/browser/safe_browsing/protocol_manager_unittest.cc

@@ -23,6 +23,7 @@
 #include "net/base/net_errors.h"
 #include "services/network/public/cpp/weak_wrapper_shared_url_loader_factory.h"
 #include "services/network/test/test_url_loader_factory.h"
+#include "services/network/test/test_utils.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gmock_mutant.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -113,17 +114,6 @@ class SafeBrowsingProtocolManagerTest : public testing::Test {
     last_request_ = request;
   }
 
-  std::string GetBodyFromRequest(const network::ResourceRequest& request) {
-    auto body = request.request_body;
-    if (!body)
-      return std::string();
-
-    CHECK_EQ(1u, body->elements()->size());
-    auto& element = body->elements()->at(0);
-    CHECK_EQ(network::DataElement::TYPE_BYTES, element.type());
-    return std::string(element.bytes(), element.length());
-  }
-
   void ValidateUpdateFetcherRequest(const std::string& expected_prefix,
                                     const std::string& expected_suffix) {
     EXPECT_EQ(net::LOAD_DISABLE_CACHE, last_request_.load_flags);
@@ -131,7 +121,7 @@ class SafeBrowsingProtocolManagerTest : public testing::Test {
     std::string expected_lists(base::StringPrintf("%s;\n%s;\n",
                                                   kDefaultPhishList,
                                                   kDefaultMalwareList));
-    EXPECT_EQ(expected_lists, GetBodyFromRequest(last_request_));
+    EXPECT_EQ(expected_lists, network::GetUploadData(last_request_));
     EXPECT_EQ(GURL(expected_prefix +
                    "/downloads?client=unittest&appver=1.0"
                    "&pver=3.0" +
@@ -145,7 +135,7 @@ class SafeBrowsingProtocolManagerTest : public testing::Test {
 
   void ValidateRedirectFetcherRequest(const std::string& expected_url) {
     EXPECT_EQ(net::LOAD_DISABLE_CACHE, last_request_.load_flags);
-    EXPECT_EQ("", GetBodyFromRequest(last_request_));
+    EXPECT_EQ("", network::GetUploadData(last_request_));
     EXPECT_EQ(GURL(expected_url), last_request_.url);
   }
 
@@ -491,7 +481,7 @@ TEST_F(SafeBrowsingProtocolManagerTest, ExistingDatabase) {
                                "unknown_list;a:adds_unknown:s:subs_unknown\n"
                                "%s;\n",
                                kDefaultPhishList, kDefaultMalwareList),
-            GetBodyFromRequest(last_request_));
+            network::GetUploadData(last_request_));
   EXPECT_EQ(GURL("https://prefix.com/foo/downloads?client=unittest&appver=1.0"
                  "&pver=3.0" +
                  key_param_ + "&ext=0"),

chrome/browser/sessions/better_session_restore_browsertest.cc

@@ -54,6 +54,7 @@
 #include "net/base/upload_data_stream.h"
 #include "net/url_request/url_request.h"
 #include "services/network/public/cpp/features.h"
+#include "services/network/test/test_utils.h"
 
 #if defined(OS_MACOSX)
 #include "base/mac/scoped_nsautorelease_pool.h"
@@ -136,14 +137,8 @@ class BetterSessionRestoreTest : public InProcessBrowserTest {
               }
               if (path == path_prefix + "posted.php") {
                 last_upload_bytes_.clear();
-                if (params->url_request.request_body) {
-                  auto* elements = params->url_request.request_body->elements();
-                  DCHECK_EQ(elements->size(), 1u);
-                  auto& element = (*elements)[0];
-                  DCHECK_EQ(element.type(), network::DataElement::TYPE_BYTES);
-                  last_upload_bytes_ =
-                      std::string(element.bytes(), element.length());
-                }
+                last_upload_bytes_ =
+                    network::GetUploadData(params->url_request);
                 content::URLLoaderInterceptor::WriteResponse(
                     kTestHeaders,
                     "<html><head><title>PASS</title></head><body>Data posted"

chrome/browser/spellchecker/spelling_service_client_unittest.cc

@@ -27,6 +27,7 @@
 #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 "services/network/test/test_utils.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace {
@@ -113,17 +114,6 @@ class SpellingServiceClientTest : public testing::Test {
     return false;
   }
 
-  std::string GetBodyFromRequest(const network::ResourceRequest& request) {
-    auto body = request.request_body;
-    if (!body)
-      return std::string();
-
-    CHECK_EQ(1u, body->elements()->size());
-    auto& element = body->elements()->at(0);
-    CHECK_EQ(network::DataElement::TYPE_BYTES, element.type());
-    return std::string(element.bytes(), element.length());
-  }
-
   content::TestBrowserThreadBundle thread_bundle_;
   TestingSpellingServiceClient client_;
   TestingProfile profile_;
@@ -249,7 +239,7 @@ TEST_F(SpellingServiceClientTest, RequestTextCheck) {
         base::BindLambdaForTesting(
             [&](const network::ResourceRequest& request) {
               intercepted_headers = request.headers;
-              intercepted_body = GetBodyFromRequest(request);
+              intercepted_body = network::GetUploadData(request);
             }));
     client_.SetExpectedTextCheckResult(kTests[i].success,
                                        kTests[i].sanitized_request_text,

chrome/browser/ssl/certificate_error_reporter_unittest.cc

@@ -24,6 +24,7 @@
 #include "services/network/public/cpp/resource_request.h"
 #include "services/network/public/cpp/weak_wrapper_shared_url_loader_factory.h"
 #include "services/network/test/test_url_loader_factory.h"
+#include "services/network/test/test_utils.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "third_party/boringssl/src/include/openssl/curve25519.h"
 
@@ -70,11 +71,7 @@ TEST_F(ErrorReporterTest, ExtendedReportingSendReport) {
         latest_report_uri = request.url;
         request.headers.GetHeader(net::HttpRequestHeaders::kContentType,
                                   &latest_content_type);
-        auto body = request.request_body;
-        CHECK_EQ(1u, body->elements()->size());
-        auto& element = body->elements()->at(0);
-        CHECK_EQ(network::DataElement::TYPE_BYTES, element.type());
-        latest_report = std::string(element.bytes(), element.length());
+        latest_report = network::GetUploadData(request);
       }));
 
   // Data should not be encrypted when sent to an HTTPS URL.

components/ntp_snippets/breaking_news/subscription_json_request_unittest.cc

@@ -14,6 +14,7 @@
 #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 "services/network/test/test_utils.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -77,17 +78,6 @@ class SubscriptionJsonRequestTest : public testing::Test {
     base::RunLoop().RunUntilIdle();
   }
 
-  std::string GetBodyFromRequest(const network::ResourceRequest& request) {
-    auto body = request.request_body;
-    if (!body)
-      return std::string();
-
-    CHECK_EQ(1u, body->elements()->size());
-    auto& element = body->elements()->at(0);
-    CHECK_EQ(network::DataElement::TYPE_BYTES, element.type());
-    return std::string(element.bytes(), element.length());
-  }
-
  private:
   base::MessageLoop message_loop_;
   network::TestURLLoaderFactory test_url_loader_factory_;
@@ -117,7 +107,7 @@ TEST_F(SubscriptionJsonRequestTest, BuildRequest) {
         EXPECT_TRUE(request.headers.GetHeader(
             net::HttpRequestHeaders::kContentType, &header));
         EXPECT_EQ(header, "application/json; charset=UTF-8");
-        actual_body = GetBodyFromRequest(request);
+        actual_body = network::GetUploadData(request);
         EXPECT_THAT(actual_body, EqualsJSON(expected_body));
       }));
 

components/sync/driver/sync_stopped_reporter_unittest.cc

@@ -15,6 +15,7 @@
 #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 "services/network/test/test_utils.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace syncer {
@@ -64,17 +65,6 @@ class SyncStoppedReporterTest : public testing::Test {
     return request_result_;
   }
 
-  std::string GetBodyFromRequest(const network::ResourceRequest& request) {
-    auto body = request.request_body;
-    if (!body)
-      return std::string();
-
-    CHECK_EQ(1u, body->elements()->size());
-    auto& element = body->elements()->at(0);
-    CHECK_EQ(network::DataElement::TYPE_BYTES, element.type());
-    return std::string(element.bytes(), element.length());
-  }
-
   network::TestURLLoaderFactory* url_loader_factory() {
     return &test_url_loader_factory_;
   }
@@ -132,7 +122,7 @@ TEST_F(SyncStoppedReporterTest, FetcherConfiguration) {
       base::BindLambdaForTesting([&](const network::ResourceRequest& request) {
         intercepted_url = request.url;
         intercepted_headers = request.headers;
-        intercepted_body = GetBodyFromRequest(request);
+        intercepted_body = network::GetUploadData(request);
       }));
   SyncStoppedReporter ssr(test_url(), user_agent(), shared_url_loader_factory(),
                           callback());

extensions/browser/content_hash_fetcher_unittest.cc

@@ -95,22 +95,6 @@ class ContentHashWaiter {
   DISALLOW_COPY_AND_ASSIGN(ContentHashWaiter);
 };
 
-// Custom network::TestURLLoaderFactory that re-implements ::Clone.
-class ClonableTestURLLoaderFactory : public network::TestURLLoaderFactory {
- public:
-  ClonableTestURLLoaderFactory() = default;
-  ~ClonableTestURLLoaderFactory() override = default;
-
-  void Clone(network::mojom::URLLoaderFactoryRequest request) override {
-    bindings_.AddBinding(this, std::move(request));
-  }
-
- private:
-  mojo::BindingSet<network::mojom::URLLoaderFactory> bindings_;
-
-  DISALLOW_COPY_AND_ASSIGN(ClonableTestURLLoaderFactory);
-};
-
 // Installs and tests various functionality of an extension loaded without
 // verified_contents.json file.
 class ContentHashFetcherTest : public ExtensionsTest {
@@ -227,7 +211,7 @@ class ContentHashFetcherTest : public ExtensionsTest {
   }
 
   scoped_refptr<network::SharedURLLoaderFactory> test_shared_loader_factory_;
-  ClonableTestURLLoaderFactory test_url_loader_factory_;
+  network::TestURLLoaderFactory test_url_loader_factory_;
 
   base::ScopedTempDir temp_dir_;
 

services/network/BUILD.gn

@@ -278,6 +278,8 @@ source_set("test_support") {
     "test/test_url_loader_client.h",
     "test/test_url_loader_factory.cc",
     "test/test_url_loader_factory.h",
+    "test/test_utils.cc",
+    "test/test_utils.h",
     "udp_socket_test_util.cc",
     "udp_socket_test_util.h",
   ]

services/network/test/test_utils.cc

+// Copyright 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "services/network/test/test_utils.h"
+
+#include "services/network/public/cpp/resource_request.h"
+
+namespace network {
+
+std::string GetUploadData(const network::ResourceRequest& request) {
+  auto body = request.request_body;
+  if (!body)
+    return std::string();
+
+  CHECK_EQ(1u, body->elements()->size());
+  const auto& element = body->elements()->at(0);
+  CHECK_EQ(network::DataElement::TYPE_BYTES, element.type());
+  return std::string(element.bytes(), element.length());
+}
+
+}  // namespace network

services/network/test/test_utils.h

+// Copyright 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef SERVICES_NETWORK_TEST_TEST_UTILS_H_
+#define SERVICES_NETWORK_TEST_TEST_UTILS_H_
+
+#include <string>
+
+namespace network {
+struct ResourceRequest;
+
+// Helper method to read the upload bytes from a ResourceRequest with a body.
+std::string GetUploadData(const network::ResourceRequest& request);
+
+}  // namespace network
+
+#endif  // SERVICES_NETWORK_TEST_TEST_UTILS_H_