Move ChromeOS network connection initialization from BrowserTestBase to ContentBrowserTest

In ChromeOS chrome/ browser tests, a NetworkChangeManagerClient gets
created, which initializes the network connection state. This
initialization was also happening in BrowserTestBase, which meant
that for chrome/ tests it was happening twice. This CL moves the
initialization logic into ContentBrowserTest, which isn't used in
chrome/ tests.

Bug: 927293
Change-Id: If506d90b1fe074e49b795a2e206b9ced68c5b9ba
Reviewed-on: https://chromium-review.googlesource.com/c/1457478Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Commit-Queue: Robbie McElrath <rmcelrath@chromium.org>
Cr-Commit-Position: refs/heads/master@{#630181}

content/public/test/browser_test_base.cc

@@ -66,12 +66,6 @@
 #include "base/process/process_handle.h"
 #endif
 
-#if defined(OS_CHROMEOS)
-#include "content/public/browser/network_service_instance.h"
-#include "net/base/network_change_notifier.h"
-#include "net/base/network_change_notifier_chromeos.h"
-#endif
-
 #if defined(USE_AURA)
 #include "content/browser/compositor/image_transport_factory.h"
 #include "ui/aura/test/event_generator_delegate_aura.h"  // nogncheck
@@ -393,32 +387,6 @@ void BrowserTestBase::ProxyRunTestOnMainThreadLoop() {
     signal(SIGTERM, DumpStackTraceSignalHandler);
 #endif  // defined(OS_POSIX)
 
-#if defined(OS_CHROMEOS)
-  // Manually set the connection type since ChromeOS's NetworkChangeNotifier
-  // implementation relies on some other class controlling it (normally
-  // NetworkChangeManagerClient), which may not be set up in all browser tests.
-  net::NetworkChangeNotifierChromeos* network_change_notifier =
-      static_cast<net::NetworkChangeNotifierChromeos*>(
-          content::GetNetworkChangeNotifier());
-  network_change_notifier->OnConnectionChanged(
-      net::NetworkChangeNotifier::CONNECTION_ETHERNET);
-  // If the network service is enabled, set the connection type for its
-  // NetworkChangeNotifier instance as well.
-  if (base::FeatureList::IsEnabled(network::features::kNetworkService) &&
-      !IsNetworkServiceRunningInProcess()) {
-    network::mojom::NetworkChangeManagerPtr manager_ptr;
-    network::mojom::NetworkChangeManagerRequest request(
-        mojo::MakeRequest(&manager_ptr));
-    GetNetworkService()->GetNetworkChangeManager(std::move(request));
-    manager_ptr->OnNetworkChanged(
-        /*dns_changed=*/false, /*ip_address_changed=*/false,
-        /*connection_type_changed=*/true,
-        network::mojom::ConnectionType::CONNECTION_ETHERNET,
-        /*connection_subtype_changed=*/false,
-        network::mojom::ConnectionSubtype::SUBTYPE_UNKNOWN);
-  }
-#endif
-
   if (base::CommandLine::ForCurrentProcess()->HasSwitch(
           switches::kEnableTracing)) {
     base::trace_event::TraceConfig trace_config(

content/public/test/content_browser_test.cc

@@ -33,6 +33,10 @@
 #include "ui/base/ime/input_method_initializer.h"
 #endif
 
+#if defined(OS_CHROMEOS)
+#include "content/public/test/network_connection_change_simulator.h"
+#endif
+
 #if defined(USE_AURA) && defined(TOOLKIT_VIEWS)
 #include "ui/views/test/widget_test_api.h"  // nogncheck
 #endif
@@ -116,6 +120,11 @@ void ContentBrowserTest::TearDown() {
 }
 
 void ContentBrowserTest::PreRunTestOnMainThread() {
+#if defined(OS_CHROMEOS)
+  NetworkConnectionChangeSimulator network_change_simulator;
+  network_change_simulator.InitializeChromeosConnectionType();
+#endif
+
   if (!switches::IsRunWebTestsSwitchPresent()) {
     CHECK_EQ(Shell::windows().size(), 1u);
     shell_ = Shell::windows()[0];

content/public/test/network_connection_change_simulator.cc

@@ -4,18 +4,23 @@
 
 #include "content/public/test/network_connection_change_simulator.h"
 
+#include <utility>
+
 #include "base/bind.h"
-#include "base/feature_list.h"
 #include "base/run_loop.h"
 #include "content/public/browser/network_service_instance.h"
+#include "content/public/common/network_service_util.h"
 #include "content/public/common/service_manager_connection.h"
 #include "content/public/common/service_names.mojom.h"
-#include "content/public/test/browser_test_utils.h"
 #include "net/base/network_change_notifier.h"
-#include "services/network/public/cpp/features.h"
 #include "services/network/public/mojom/network_service_test.mojom.h"
 #include "services/service_manager/public/cpp/connector.h"
 
+#if defined(OS_CHROMEOS)
+#include "net/base/network_change_notifier_chromeos.h"
+#include "services/network/public/mojom/network_service.mojom.h"
+#endif
+
 namespace content {
 
 // SetConnectionType will block until the network connection changes, and
@@ -30,6 +35,33 @@ constexpr base::RunLoop::Type kRunLoopType =
 NetworkConnectionChangeSimulator::NetworkConnectionChangeSimulator() = default;
 NetworkConnectionChangeSimulator::~NetworkConnectionChangeSimulator() = default;
 
+#if defined(OS_CHROMEOS)
+void NetworkConnectionChangeSimulator::InitializeChromeosConnectionType() {
+  // Manually set the connection type since ChromeOS's NetworkChangeNotifier
+  // implementation relies on some other class controlling it (normally
+  // NetworkChangeManagerClient), which isn't used on content/.
+  net::NetworkChangeNotifierChromeos* network_change_notifier =
+      static_cast<net::NetworkChangeNotifierChromeos*>(
+          content::GetNetworkChangeNotifier());
+  network_change_notifier->OnConnectionChanged(
+      net::NetworkChangeNotifier::CONNECTION_ETHERNET);
+  // If the network service is enabled, set the connection type for its
+  // NetworkChangeNotifier instance as well.
+  if (IsOutOfProcessNetworkService()) {
+    network::mojom::NetworkChangeManagerPtr manager_ptr;
+    network::mojom::NetworkChangeManagerRequest request(
+        mojo::MakeRequest(&manager_ptr));
+    GetNetworkService()->GetNetworkChangeManager(std::move(request));
+    manager_ptr->OnNetworkChanged(
+        /*dns_changed=*/false, /*ip_address_changed=*/false,
+        /*connection_type_changed=*/true,
+        network::mojom::ConnectionType::CONNECTION_ETHERNET,
+        /*connection_subtype_changed=*/false,
+        network::mojom::ConnectionSubtype::SUBTYPE_UNKNOWN);
+  }
+}
+#endif
+
 void NetworkConnectionChangeSimulator::SetConnectionType(
     network::mojom::ConnectionType type) {
   network::NetworkConnectionTracker* network_connection_tracker =
@@ -60,8 +92,7 @@ void NetworkConnectionChangeSimulator::SetConnectionType(
 // static
 void NetworkConnectionChangeSimulator::SimulateNetworkChange(
     network::mojom::ConnectionType type) {
-  if (base::FeatureList::IsEnabled(network::features::kNetworkService) &&
-      !IsNetworkServiceRunningInProcess()) {
+  if (IsOutOfProcessNetworkService()) {
     network::mojom::NetworkServiceTestPtr network_service_test;
     ServiceManagerConnection::GetForProcess()->GetConnector()->BindInterface(
         mojom::kNetworkServiceName, &network_service_test);

content/public/test/network_connection_change_simulator.h

@@ -21,6 +21,12 @@ class NetworkConnectionChangeSimulator
   NetworkConnectionChangeSimulator();
   ~NetworkConnectionChangeSimulator() override;
 
+#if defined(OS_CHROMEOS)
+  // Initializes the ChromeOS network connection type.
+  // This should be used in tests that don't have a DBus set up.
+  void InitializeChromeosConnectionType();
+#endif
+
   // Synchronously sets the connection type.
   void SetConnectionType(network::mojom::ConnectionType connection_type);
 

extensions/shell/test/shell_test.cc

@@ -16,6 +16,10 @@
 #include "extensions/shell/browser/shell_extension_system.h"
 #include "ui/base/ui_base_features.h"
 
+#if defined(OS_CHROMEOS)
+#include "content/public/test/network_connection_change_simulator.h"
+#endif
+
 namespace extensions {
 
 AppShellTest::AppShellTest()
@@ -39,6 +43,11 @@ void AppShellTest::SetUp() {
 }
 
 void AppShellTest::PreRunTestOnMainThread() {
+#if defined(OS_CHROMEOS)
+  content::NetworkConnectionChangeSimulator network_change_simulator;
+  network_change_simulator.InitializeChromeosConnectionType();
+#endif
+
   browser_context_ = ShellContentBrowserClient::Get()->GetBrowserContext();
 
   extension_system_ = static_cast<ShellExtensionSystem*>(