Move tests for StoragePartitition and fix potential threading issue

The original tests pass non-thread-safe objects across threads which
could easily break.

This CL:
1. Adds 'storage_partition_test_utils.h/cc'.
2. Adds |IOThreadSharedURLLoaderFactoryOwner| which lives on UI thread
   and owns a |SharedURLLoaderFactory| on IO thread.
3. Moves related tests into 'storage_partition_impl_browsertest.cc' to
   make sure they run w/ and w/o Network Service.

This patch doesn't have behavior change.

Bug: 826869
Change-Id: I23d636b079d72627f8944dd61892d72f175e0c7d
Reviewed-on: https://chromium-review.googlesource.com/1033806
Commit-Queue: Chong Zhang <chongz@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Cr-Commit-Position: refs/heads/master@{#556136}

content/browser/storage_partition_impl_browsertest.cc

@@ -12,6 +12,8 @@
 #include "content/public/browser/web_contents.h"
 #include "content/public/test/content_browser_test.h"
 #include "content/shell/browser/shell.h"
+#include "content/shell/browser/shell_browser_context.h"
+#include "content/test/storage_partition_test_utils.h"
 #include "mojo/public/cpp/bindings/interface_request.h"
 #include "net/http/http_response_headers.h"
 #include "net/test/embedded_test_server/embedded_test_server.h"
@@ -42,6 +44,12 @@ class StoragePartititionImplBrowsertest
   }
   ~StoragePartititionImplBrowsertest() override {}
 
+  GURL GetTestURL() const {
+    // Use '/echoheader' instead of '/echo' to avoid a disk_cache bug.
+    // See https://crbug.com/792255.
+    return embedded_test_server()->GetURL("/echoheader");
+  }
+
  private:
   base::test::ScopedFeatureList feature_list_;
 };
@@ -80,6 +88,71 @@ IN_PROC_BROWSER_TEST_P(StoragePartititionImplBrowsertest, NetworkContext) {
   EXPECT_EQ("bar", foo_header_value);
 }
 
+// Make sure the factory info returned from
+// |StoragePartition::GetURLLoaderFactoryForBrowserProcessIOThread()| works.
+IN_PROC_BROWSER_TEST_P(StoragePartititionImplBrowsertest,
+                       GetURLLoaderFactoryForBrowserProcessIOThread) {
+  ASSERT_TRUE(embedded_test_server()->Start());
+
+  base::ScopedAllowBlockingForTesting allow_blocking;
+  auto shared_url_loader_factory_info =
+      BrowserContext::GetDefaultStoragePartition(
+          shell()->web_contents()->GetBrowserContext())
+          ->GetURLLoaderFactoryForBrowserProcessIOThread();
+
+  auto factory_owner = IOThreadSharedURLLoaderFactoryOwner::Create(
+      std::move(shared_url_loader_factory_info));
+
+  EXPECT_EQ(net::OK, factory_owner->LoadBasicRequestOnIOThread(GetTestURL()));
+}
+
+// Make sure the factory info returned from
+// |StoragePartition::GetURLLoaderFactoryForBrowserProcessIOThread()| doesn't
+// crash if it's called after the StoragePartition is deleted.
+IN_PROC_BROWSER_TEST_P(StoragePartititionImplBrowsertest,
+                       BrowserIOFactoryInfoAfterStoragePartitionGone) {
+  ASSERT_TRUE(embedded_test_server()->Start());
+
+  base::ScopedAllowBlockingForTesting allow_blocking;
+  std::unique_ptr<ShellBrowserContext> browser_context =
+      std::make_unique<ShellBrowserContext>(true, nullptr);
+  auto* partition =
+      BrowserContext::GetDefaultStoragePartition(browser_context.get());
+  auto shared_url_loader_factory_info =
+      partition->GetURLLoaderFactoryForBrowserProcessIOThread();
+
+  browser_context.reset();
+
+  auto factory_owner = IOThreadSharedURLLoaderFactoryOwner::Create(
+      std::move(shared_url_loader_factory_info));
+
+  EXPECT_EQ(net::ERR_FAILED,
+            factory_owner->LoadBasicRequestOnIOThread(GetTestURL()));
+}
+
+// Make sure the factory constructed from
+// |StoragePartition::GetURLLoaderFactoryForBrowserProcessIOThread()| doesn't
+// crash if it's called after the StoragePartition is deleted.
+IN_PROC_BROWSER_TEST_P(StoragePartititionImplBrowsertest,
+                       BrowserIOFactoryAfterStoragePartitionGone) {
+  ASSERT_TRUE(embedded_test_server()->Start());
+
+  base::ScopedAllowBlockingForTesting allow_blocking;
+  std::unique_ptr<ShellBrowserContext> browser_context =
+      std::make_unique<ShellBrowserContext>(true, nullptr);
+  auto* partition =
+      BrowserContext::GetDefaultStoragePartition(browser_context.get());
+  auto factory_owner = IOThreadSharedURLLoaderFactoryOwner::Create(
+      partition->GetURLLoaderFactoryForBrowserProcessIOThread());
+
+  EXPECT_EQ(net::OK, factory_owner->LoadBasicRequestOnIOThread(GetTestURL()));
+
+  browser_context.reset();
+
+  EXPECT_EQ(net::ERR_FAILED,
+            factory_owner->LoadBasicRequestOnIOThread(GetTestURL()));
+}
+
 // NetworkServiceState::kEnabled currently DCHECKs on Android, as Android isn't
 // expected to create extra processes.
 #if defined(OS_ANDROID)

content/test/BUILD.gn

@@ -226,6 +226,8 @@ jumbo_static_library("test_support") {
     "net/url_request_abort_on_end_job.h",
     "ppapi_unittest.cc",
     "ppapi_unittest.h",
+    "storage_partition_test_utils.cc",
+    "storage_partition_test_utils.h",
     "test_background_sync_context.cc",
     "test_background_sync_context.h",
     "test_background_sync_manager.cc",

content/test/storage_partition_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 "content/test/storage_partition_test_utils.h"
+
+#include "base/bind.h"
+#include "base/run_loop.h"
+#include "content/browser/url_loader_factory_getter.h"
+#include "content/public/test/simple_url_loader_test_helper.h"
+#include "net/traffic_annotation/network_traffic_annotation_test_helper.h"
+#include "services/network/public/cpp/simple_url_loader.h"
+
+namespace content {
+namespace {
+
+using SharedURLLoaderFactoryGetterCallback =
+    base::OnceCallback<scoped_refptr<network::SharedURLLoaderFactory>()>;
+
+void InitializeSharedFactoryOnIOThread(
+    SharedURLLoaderFactoryGetterCallback shared_url_loader_factory_getter,
+    scoped_refptr<network::SharedURLLoaderFactory>* out_shared_factory) {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  base::RunLoop run_loop;
+  BrowserThread::PostTaskAndReply(
+      BrowserThread::IO, FROM_HERE,
+      base::BindOnce(
+          [](SharedURLLoaderFactoryGetterCallback getter,
+             scoped_refptr<network::SharedURLLoaderFactory>*
+                 shared_factory_ptr) {
+            DCHECK(!shared_factory_ptr->get())
+                << "shared_url_loader_factory_ can only be initialized once.";
+            *shared_factory_ptr = std::move(getter).Run();
+          },
+          std::move(shared_url_loader_factory_getter),
+          base::Unretained(out_shared_factory)),
+      run_loop.QuitClosure());
+  run_loop.Run();
+}
+
+network::SimpleURLLoader::BodyAsStringCallback RunOnUIThread(
+    network::SimpleURLLoader::BodyAsStringCallback ui_callback) {
+  return base::BindOnce(
+      [](network::SimpleURLLoader::BodyAsStringCallback callback,
+         std::unique_ptr<std::string> response_body) {
+        DCHECK_CURRENTLY_ON(BrowserThread::IO);
+        BrowserThread::PostTask(
+            BrowserThread::UI, FROM_HERE,
+            base::BindOnce(std::move(callback), std::move(response_body)));
+      },
+      std::move(ui_callback));
+}
+
+}  // namespace
+
+// static
+IOThreadSharedURLLoaderFactoryOwner::IOThreadSharedURLLoaderFactoryOwnerPtr
+IOThreadSharedURLLoaderFactoryOwner::Create(
+    URLLoaderFactoryGetter* url_loader_factory_getter) {
+  return IOThreadSharedURLLoaderFactoryOwnerPtr(
+      new IOThreadSharedURLLoaderFactoryOwner(url_loader_factory_getter));
+}
+
+// static
+IOThreadSharedURLLoaderFactoryOwner::IOThreadSharedURLLoaderFactoryOwnerPtr
+IOThreadSharedURLLoaderFactoryOwner::Create(
+    std::unique_ptr<network::SharedURLLoaderFactoryInfo> info) {
+  return IOThreadSharedURLLoaderFactoryOwnerPtr(
+      new IOThreadSharedURLLoaderFactoryOwner(std::move(info)));
+}
+
+IOThreadSharedURLLoaderFactoryOwner::IOThreadSharedURLLoaderFactoryOwner(
+    URLLoaderFactoryGetter* url_loader_factory_getter) {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  InitializeSharedFactoryOnIOThread(
+      base::BindOnce(&URLLoaderFactoryGetter::GetNetworkFactory,
+                     base::Unretained(url_loader_factory_getter)),
+      &shared_url_loader_factory_);
+}
+
+IOThreadSharedURLLoaderFactoryOwner::IOThreadSharedURLLoaderFactoryOwner(
+    std::unique_ptr<network::SharedURLLoaderFactoryInfo> info) {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  InitializeSharedFactoryOnIOThread(
+      base::BindOnce(&network::SharedURLLoaderFactory::Create, std::move(info)),
+      &shared_url_loader_factory_);
+}
+
+IOThreadSharedURLLoaderFactoryOwner::~IOThreadSharedURLLoaderFactoryOwner() {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+}
+
+int IOThreadSharedURLLoaderFactoryOwner::LoadBasicRequestOnIOThread(
+    const GURL& url) {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  auto request = std::make_unique<network::ResourceRequest>();
+  request->url = url;
+
+  // |simple_loader_helper| lives on UI thread and shouldn't be accessed on
+  // other threads.
+  SimpleURLLoaderTestHelper simple_loader_helper;
+
+  std::unique_ptr<network::SimpleURLLoader> simple_loader =
+      network::SimpleURLLoader::Create(std::move(request),
+                                       TRAFFIC_ANNOTATION_FOR_TESTS);
+
+  BrowserThread::PostTask(
+      BrowserThread::IO, FROM_HERE,
+      base::BindOnce(
+          [](network::SimpleURLLoader* loader,
+             network::mojom::URLLoaderFactory* factory,
+             network::SimpleURLLoader::BodyAsStringCallback
+                 body_as_string_callback) {
+            loader->DownloadToStringOfUnboundedSizeUntilCrashAndDie(
+                factory, std::move(body_as_string_callback));
+          },
+          base::Unretained(simple_loader.get()),
+          base::Unretained(shared_url_loader_factory_.get()),
+          RunOnUIThread(simple_loader_helper.GetCallback())));
+
+  simple_loader_helper.WaitForCallback();
+  return simple_loader->NetError();
+}
+
+}  // namespace content

content/test/storage_partition_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 CONTENT_TEST_STORAGE_PARTITION_TEST_UTILS_H_
+#define CONTENT_TEST_STORAGE_PARTITION_TEST_UTILS_H_
+
+#include <memory>
+
+#include "base/macros.h"
+#include "base/memory/scoped_refptr.h"
+#include "content/public/browser/browser_thread.h"
+#include "services/network/public/cpp/shared_url_loader_factory.h"
+#include "services/network/public/mojom/url_loader_factory.mojom.h"
+
+namespace content {
+
+class URLLoaderFactoryGetter;
+
+// Class to own the SharedURLLoaderFactory for use on the IO thread.
+//
+// Created on the UI thread and destroyed on the IO thread.
+class IOThreadSharedURLLoaderFactoryOwner {
+ public:
+  using IOThreadSharedURLLoaderFactoryOwnerPtr =
+      std::unique_ptr<IOThreadSharedURLLoaderFactoryOwner,
+                      BrowserThread::DeleteOnIOThread>;
+
+  // To be called on the UI thread. Will block and finish initialization on the
+  // IO thread.
+  static IOThreadSharedURLLoaderFactoryOwnerPtr Create(
+      URLLoaderFactoryGetter* url_loader_factory_getter);
+  static IOThreadSharedURLLoaderFactoryOwnerPtr Create(
+      std::unique_ptr<network::SharedURLLoaderFactoryInfo> info);
+
+  // Load the given |url| with the internal |shared_url_loader_factory_| on IO
+  // thread and return the |net::Error| code.
+  int LoadBasicRequestOnIOThread(const GURL& url);
+
+ private:
+  friend struct BrowserThread::DeleteOnThread<BrowserThread::IO>;
+  friend class base::DeleteHelper<IOThreadSharedURLLoaderFactoryOwner>;
+
+  explicit IOThreadSharedURLLoaderFactoryOwner(
+      URLLoaderFactoryGetter* url_loader_factory_getter);
+  explicit IOThreadSharedURLLoaderFactoryOwner(
+      std::unique_ptr<network::SharedURLLoaderFactoryInfo> info);
+  ~IOThreadSharedURLLoaderFactoryOwner();
+
+  // Lives on the IO thread.
+  scoped_refptr<network::SharedURLLoaderFactory> shared_url_loader_factory_;
+
+  DISALLOW_COPY_AND_ASSIGN(IOThreadSharedURLLoaderFactoryOwner);
+};
+
+}  // namespace content
+
+#endif  // CONTENT_TEST_STORAGE_PARTITION_TEST_UTILS_H_