Fix SyncTest on ChromeOS with network service.

All tests based on SyncTest create a NetworkServiceConfigTestUtil, which
needs a URLRequestContext. This CL adds a NetworkServiceConfigTestUtil
constructor that takes a NetworkContext instead, and updates the test to
call the right constructor based on whether the network service is
enabled.

Bug: 924250
Change-Id: I211fe1d8c01008fb8bf3648146be1e5f95870eaf
Reviewed-on: https://chromium-review.googlesource.com/c/1449165
Reviewed-by: Sergey Ulanov <[email protected]>
Reviewed-by: Marc Treib <[email protected]>
Reviewed-by: Maks Orlovich <[email protected]>
Commit-Queue: Robbie McElrath <[email protected]>
Cr-Commit-Position: refs/heads/master@{#628829}

chrome/browser/sync/test/integration/sync_test.cc

@@ -82,6 +82,7 @@
 #include "content/public/browser/navigation_entry.h"
 #include "content/public/browser/network_service_instance.h"
 #include "content/public/browser/notification_service.h"
+#include "content/public/browser/storage_partition.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/test/test_browser_thread.h"
 #include "google_apis/gaia/gaia_urls.h"
@@ -94,6 +95,7 @@
 #include "net/traffic_annotation/network_traffic_annotation_test_helper.h"
 #include "net/url_request/test_url_fetcher_factory.h"
 #include "net/url_request/url_fetcher.h"
+#include "services/network/public/cpp/features.h"
 #include "services/network/public/cpp/shared_url_loader_factory.h"
 #include "services/network/public/cpp/simple_url_loader.h"
 #include "services/network/public/cpp/weak_wrapper_shared_url_loader_factory.h"
@@ -183,9 +185,17 @@ std::unique_ptr<KeyedService> BuildP2PProfileInvalidationProvider(
     content::BrowserContext* context,
     syncer::P2PNotificationTarget notification_target) {
   Profile* profile = static_cast<Profile*>(context);
-  auto config_helper =
-      std::make_unique<jingle_glue::NetworkServiceConfigTestUtil>(
-          profile->GetRequestContext());
+  std::unique_ptr<jingle_glue::NetworkServiceConfigTestUtil> config_helper;
+  if (base::FeatureList::IsEnabled(network::features::kNetworkService)) {
+    content::StoragePartition* storage_partition =
+        content::BrowserContext::GetDefaultStoragePartition(profile);
+    config_helper = std::make_unique<jingle_glue::NetworkServiceConfigTestUtil>(
+        base::BindRepeating(&content::StoragePartition::GetNetworkContext,
+                            base::Unretained(storage_partition)));
+  } else {
+    config_helper = std::make_unique<jingle_glue::NetworkServiceConfigTestUtil>(
+        profile->GetRequestContext());
+  }
   return std::make_unique<invalidation::ProfileInvalidationProvider>(
       std::make_unique<invalidation::P2PInvalidationService>(
           std::move(config_helper), content::GetNetworkConnectionTracker(),

jingle/glue/network_service_config_test_util.cc

@@ -6,8 +6,12 @@
 
 #include "jingle/glue/network_service_config_test_util.h"
 
+#include <memory>
+#include <utility>
+
 #include "base/bind.h"
 #include "base/synchronization/waitable_event.h"
+#include "base/task/post_task.h"
 #include "base/threading/sequenced_task_runner_handle.h"
 #include "base/threading/thread_restrictions.h"
 
@@ -38,6 +42,13 @@ NetworkServiceConfigTestUtil::NetworkServiceConfigTestUtil(
   }
 }
 
+NetworkServiceConfigTestUtil::NetworkServiceConfigTestUtil(
+    NetworkContextGetter network_context_getter)
+    : net_runner_(base::CreateSingleThreadTaskRunnerWithTraits({})),
+      mojo_runner_(base::SequencedTaskRunnerHandle::Get()),
+      network_context_getter_(network_context_getter),
+      weak_ptr_factory_(this) {}
+
 NetworkServiceConfigTestUtil::~NetworkServiceConfigTestUtil() {
   if (!net_runner_->BelongsToCurrentThread()) {
     base::ScopedAllowBaseSyncPrimitivesForTesting permission;
@@ -81,7 +92,12 @@ void NetworkServiceConfigTestUtil::RequestSocket(
 void NetworkServiceConfigTestUtil::RequestSocketOnMojoRunner(
     base::WeakPtr<NetworkServiceConfigTestUtil> instance,
     network::mojom::ProxyResolvingSocketFactoryRequest request) {
-  if (instance) {
+  if (!instance)
+    return;
+  if (instance->network_context_getter_) {
+    instance->network_context_getter_.Run()->CreateProxyResolvingSocketFactory(
+        std::move(request));
+  } else {
     instance->network_context_ptr_->CreateProxyResolvingSocketFactory(
         std::move(request));
   }

jingle/glue/network_service_config_test_util.h

@@ -18,17 +18,28 @@ namespace base {
 class WaitableEvent;
 }
 
+namespace network {
+namespace mojom {
+class NetworkContext;
+}
+}  // namespace network
+
 namespace jingle_glue {
 
 class NetworkServiceConfigTestUtil {
  public:
+  using NetworkContextGetter =
+      base::RepeatingCallback<network::mojom::NetworkContext*()>;
+
   // All public methods must be called on the thread this is created on,
   // but the callback returned by MakeSocketFactoryCallback() is expected to be
   // run on |url_request_context_getter->GetNetworkTaskRunner()|, which can be,
   // but does not have to be, a separare thread. The constructor and destructor
   // can block, but will not spin the event loop.
   explicit NetworkServiceConfigTestUtil(
       scoped_refptr<net::URLRequestContextGetter> url_request_context_getter);
+  explicit NetworkServiceConfigTestUtil(
+      NetworkContextGetter network_context_getter);
   ~NetworkServiceConfigTestUtil();
 
   // Configures |config| to run the result of MakeSocketFactoryCallback()
@@ -54,6 +65,7 @@ class NetworkServiceConfigTestUtil {
   scoped_refptr<base::SingleThreadTaskRunner> net_runner_;
   scoped_refptr<base::SequencedTaskRunner> mojo_runner_;
   scoped_refptr<net::URLRequestContextGetter> url_request_context_getter_;
+  NetworkContextGetter network_context_getter_;
   std::unique_ptr<network::NetworkContext>
       network_context_;  // lives on |net_runner_|
   network::mojom::NetworkContextPtr