Fix race during NetworkChangeNotifier construction in unit tests.

See crbug.com/989225#c25 for more details.

Bug: 989225
Change-Id: Ib2b9ef82ea881cdc59d2940dfe54d197d53f67bf
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1742381Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Wez <wez@chromium.org>
Commit-Queue: Robbie McElrath <rmcelrath@chromium.org>
Cr-Commit-Position: refs/heads/master@{#687410}

chrome/browser/resource_coordinator/tab_manager_unittest.cc

@@ -1692,6 +1692,7 @@ TEST_F(TabManagerWithProactiveDiscardExperimentEnabledTest,
   TabStripModel* tab_strip = browser->tab_strip_model();
 
   // Simulate being offline.
+  net::NetworkChangeNotifier::DisableForTest net_change_notifier_disabler_;
   FakeOfflineNetworkChangeNotifier fake_offline_state;
 
   tab_strip->AppendWebContents(CreateWebContents(), /*foreground=*/true);

content/browser/network_service_instance_impl.cc

@@ -17,6 +17,7 @@
 #include "base/no_destructor.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
+#include "base/synchronization/waitable_event.h"
 #include "base/task/post_task.h"
 #include "base/threading/sequence_local_storage_slot.h"
 #include "build/build_config.h"
@@ -91,7 +92,9 @@ network::mojom::NetworkServiceParamsPtr CreateNetworkServiceParams() {
   return network_service_params;
 }
 
-void CreateNetworkServiceOnIO(network::mojom::NetworkServiceRequest request) {
+void CreateNetworkServiceOnIOForTesting(
+    network::mojom::NetworkServiceRequest request,
+    base::WaitableEvent* completion_event) {
   if (GetLocalNetworkService()) {
     GetLocalNetworkService()->Bind(std::move(request));
     return;
@@ -99,6 +102,8 @@ void CreateNetworkServiceOnIO(network::mojom::NetworkServiceRequest request) {
 
   GetLocalNetworkService() =
       std::make_unique<network::NetworkService>(nullptr, std::move(request));
+  if (completion_event)
+    completion_event->Signal();
 }
 
 void BindNetworkChangeManagerRequest(
@@ -157,16 +162,6 @@ net::NetLogCaptureMode GetNetCaptureModeFromCommandLine(
 }  // namespace
 
 network::mojom::NetworkService* GetNetworkService() {
-  service_manager::Connector* connector = nullptr;
-  if (GetSystemConnector() &&  // null in unit tests.
-      !g_force_create_network_service_directly) {
-    connector = GetSystemConnector();
-  }
-  return GetNetworkServiceFromConnector(connector);
-}
-
-CONTENT_EXPORT network::mojom::NetworkService* GetNetworkServiceFromConnector(
-    service_manager::Connector* connector) {
   if (!g_network_service_ptr)
     g_network_service_ptr = new network::mojom::NetworkServicePtr;
   static NetworkServiceClient* g_client;
@@ -181,16 +176,27 @@ CONTENT_EXPORT network::mojom::NetworkService* GetNetworkServiceFromConnector(
       auto request = mojo::MakeRequest(g_network_service_ptr);
       auto leaked_pipe = request.PassMessagePipe().release();
     } else {
-      if (connector) {
-        connector->BindInterface(mojom::kNetworkServiceName,
-                                 g_network_service_ptr);
+      if (GetSystemConnector() &&  // null in unit tests.
+          !g_force_create_network_service_directly) {
+        GetSystemConnector()->BindInterface(mojom::kNetworkServiceName,
+                                            g_network_service_ptr);
         g_network_service_ptr->set_connection_error_handler(
             base::BindOnce(&OnNetworkServiceCrash));
       } else {
-        base::PostTask(
-            FROM_HERE, {BrowserThread::IO},
-            base::BindOnce(CreateNetworkServiceOnIO,
-                           mojo::MakeRequest(g_network_service_ptr)));
+        // This should only be reached in unit tests.
+        if (BrowserThread::CurrentlyOn(BrowserThread::IO)) {
+          CreateNetworkServiceOnIOForTesting(
+              mojo::MakeRequest(g_network_service_ptr),
+              /*completion_event=*/nullptr);
+        } else {
+          base::WaitableEvent event;
+          base::PostTaskWithTraits(
+              FROM_HERE, {BrowserThread::IO},
+              base::BindOnce(CreateNetworkServiceOnIOForTesting,
+                             mojo::MakeRequest(g_network_service_ptr),
+                             base::Unretained(&event)));
+          event.Wait();
+        }
       }
 
       network::mojom::NetworkServiceClientPtr client_ptr;

content/public/browser/network_service_instance.h

@@ -28,10 +28,6 @@ class NetworkService;
 }
 }  // namespace network
 
-namespace service_manager {
-class Connector;
-}  // namespace service_manager
-
 namespace content {
 
 // Returns a pointer to the NetworkService, creating / re-creating it as needed.
@@ -42,14 +38,6 @@ namespace content {
 // This method can only be called on the UI thread.
 CONTENT_EXPORT network::mojom::NetworkService* GetNetworkService();
 
-// Similar to GetNetworkService(), but will create the NetworkService from a
-// service manager connector if needed. If network service is disabled,
-// |connector| will be ignored and this method is identical to
-// GetNetworkService().
-// This method can only be called on the UI thread.
-CONTENT_EXPORT network::mojom::NetworkService* GetNetworkServiceFromConnector(
-    service_manager::Connector* connector);
-
 // Only on ChromeOS since it's only used there.
 #if defined(OS_CHROMEOS)
 // Returns the global NetworkChangeNotifier instance.