Remove CrosNetworkConfigTestHelper from ShellBrowserMainParts

CrosNetworkConfigTestHelper was intended as a convenience class for
tests that require specific network configuration setup. It relies
on running the message loop to ensure complete setup before tests run.

For ShellBrowserMainParts, we just need a CorsNetworkConfig instance.
The easiest way to provide this is to initialize the fake shill clients
and NetworkHandler, then create an owned CrosNetworkConfig instance,
without the additional complexity in the test helper.

Bug: 980951
Change-Id: I4b4a72ec7258f5010e0e24750e2f37f6bd884138
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1726932
Commit-Queue: Steven Bennetts <stevenjb@chromium.org>
Reviewed-by: Mitsuru Oshima <oshima@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>
Cr-Commit-Position: refs/heads/master@{#682458}

ash/BUILD.gn

@@ -1541,8 +1541,11 @@ static_library("ash_shell_lib_with_content") {
     "//chromeos/constants",
     "//chromeos/dbus/biod",
     "//chromeos/dbus/power",
+    "//chromeos/dbus/shill",
+    "//chromeos/network",
+    "//chromeos/services/network_config",
     "//chromeos/services/network_config/public/cpp:manifest",
-    "//chromeos/services/network_config/public/cpp:test_support",
+    "//chromeos/services/network_config/public/mojom",
     "//chromeos/system",
     "//components/discardable_memory/public/mojom",
     "//components/services/font:lib",

ash/shell/content/client/DEPS

 include_rules = [
   "+ash/components/shortcut_viewer",
   "+chromeos/dbus/biod",
+  "+chromeos/services/network_config",
   "+components/discardable_memory/public/interfaces",
   "+content/public",
   "+content/shell",

ash/shell/content/client/shell_browser_main_parts.cc

@@ -27,7 +27,10 @@
 #include "base/command_line.h"
 #include "base/run_loop.h"
 #include "chromeos/dbus/biod/biod_client.h"
-#include "chromeos/services/network_config/public/cpp/cros_network_config_test_helper.h"
+#include "chromeos/dbus/shill/shill_clients.h"
+#include "chromeos/network/network_handler.h"
+#include "chromeos/services/network_config/cros_network_config.h"
+#include "chromeos/services/network_config/public/mojom/constants.mojom.h"
 #include "components/exo/file_helper.h"
 #include "content/public/browser/context_factory.h"
 #include "content/public/browser/system_connector.h"
@@ -65,6 +68,12 @@ ShellBrowserMainParts::~ShellBrowserMainParts() {
   main_parts = nullptr;
 }
 
+void ShellBrowserMainParts::PostEarlyInitialization() {
+  content::BrowserMainParts::PostEarlyInitialization();
+  chromeos::shill_clients::InitializeFakes();
+  chromeos::NetworkHandler::Initialize();
+}
+
 void ShellBrowserMainParts::PreMainMessageLoopStart() {}
 
 void ShellBrowserMainParts::PostMainMessageLoopStart() {
@@ -79,10 +88,10 @@ void ShellBrowserMainParts::ToolkitInitialized() {
 void ShellBrowserMainParts::PreMainMessageLoopRun() {
   browser_context_.reset(new content::ShellBrowserContext(false));
 
+  // CrosNetworkConfig is required for the system tray UI.
+  InitializeCrosNetworkConfig();
+
   ash_test_helper_ = std::make_unique<AshTestHelper>();
-  network_config_helper_ =
-      std::make_unique<chromeos::network_config::CrosNetworkConfigTestHelper>(
-          content::GetSystemConnector());
 
   AshTestHelper::InitParams init_params;
   // TODO(oshima): Separate the class for ash_shell to reduce the test binary
@@ -139,6 +148,19 @@ void ShellBrowserMainParts::PreMainMessageLoopRun() {
   }
 }
 
+bool ShellBrowserMainParts::MainMessageLoopRun(int* result_code) {
+  if (parameters_.ui_task) {
+    parameters_.ui_task->Run();
+    delete parameters_.ui_task;
+  } else {
+    base::RunLoop run_loop;
+    example_session_controller_client_->set_quit_closure(
+        run_loop.QuitWhenIdleClosure());
+    run_loop.Run();
+  }
+  return true;
+}
+
 void ShellBrowserMainParts::PostMainMessageLoopRun() {
   window_watcher_.reset();
   example_app_list_client_.reset();
@@ -149,7 +171,7 @@ void ShellBrowserMainParts::PostMainMessageLoopRun() {
 
   views_delegate_.reset();
 
-  network_config_helper_.reset();
+  cros_network_config_.reset();
 
   // The keyboard may have created a WebContents. The WebContents is destroyed
   // with the UI, and it needs the BrowserContext to be alive during its
@@ -158,17 +180,33 @@ void ShellBrowserMainParts::PostMainMessageLoopRun() {
   browser_context_.reset();
 }
 
-bool ShellBrowserMainParts::MainMessageLoopRun(int* result_code) {
-  if (parameters_.ui_task) {
-    parameters_.ui_task->Run();
-    delete parameters_.ui_task;
-  } else {
-    base::RunLoop run_loop;
-    example_session_controller_client_->set_quit_closure(
-        run_loop.QuitWhenIdleClosure());
-    run_loop.Run();
-  }
-  return true;
+void ShellBrowserMainParts::PostDestroyThreads() {
+  chromeos::NetworkHandler::Shutdown();
+  chromeos::shill_clients::Shutdown();
+  content::BrowserMainParts::PostDestroyThreads();
+}
+
+void ShellBrowserMainParts::InitializeCrosNetworkConfig() {
+  chromeos::NetworkHandler* network_handler = chromeos::NetworkHandler::Get();
+  cros_network_config_ =
+      std::make_unique<chromeos::network_config::CrosNetworkConfig>(
+          network_handler->network_state_handler(),
+          network_handler->network_device_handler(),
+          network_handler->managed_network_configuration_handler());
+  // Use a test override for the mojo binding.
+  content::GetSystemConnector()->OverrideBinderForTesting(
+      service_manager::ServiceFilter::ByName(
+          chromeos::network_config::mojom::kServiceName),
+      chromeos::network_config::mojom::CrosNetworkConfig::Name_,
+      base::BindRepeating(&ShellBrowserMainParts::AddNetworkConfigBinding,
+                          base::Unretained(this)));
+}
+
+void ShellBrowserMainParts::AddNetworkConfigBinding(
+    mojo::ScopedMessagePipeHandle handle) {
+  cros_network_config_->BindRequest(
+      chromeos::network_config::mojom::CrosNetworkConfigRequest(
+          std::move(handle)));
 }
 
 }  // namespace shell

ash/shell/content/client/shell_browser_main_parts.h

@@ -10,10 +10,11 @@
 #include "base/macros.h"
 #include "content/public/browser/browser_main_parts.h"
 #include "content/public/common/main_function_params.h"
+#include "mojo/public/cpp/system/message_pipe.h"
 
 namespace chromeos {
 namespace network_config {
-class CrosNetworkConfigTestHelper;
+class CrosNetworkConfig;
 }  // namespace network_config
 }  // namespace chromeos
 
@@ -43,16 +44,21 @@ class ShellBrowserMainParts : public content::BrowserMainParts {
   ~ShellBrowserMainParts() override;
 
   // Overridden from content::BrowserMainParts:
+  void PostEarlyInitialization() override;
   void PreMainMessageLoopStart() override;
   void PostMainMessageLoopStart() override;
   void ToolkitInitialized() override;
   void PreMainMessageLoopRun() override;
   bool MainMessageLoopRun(int* result_code) override;
   void PostMainMessageLoopRun() override;
+  void PostDestroyThreads() override;
 
   content::BrowserContext* browser_context() { return browser_context_.get(); }
 
  private:
+  void InitializeCrosNetworkConfig();
+  void AddNetworkConfigBinding(mojo::ScopedMessagePipeHandle handle);
+
   std::unique_ptr<content::BrowserContext> browser_context_;
   std::unique_ptr<views::ViewsDelegate> views_delegate_;
   std::unique_ptr<WindowWatcher> window_watcher_;
@@ -60,8 +66,8 @@ class ShellBrowserMainParts : public content::BrowserMainParts {
       example_session_controller_client_;
   std::unique_ptr<ExampleAppListClient> example_app_list_client_;
   std::unique_ptr<ash::AshTestHelper> ash_test_helper_;
-  std::unique_ptr<chromeos::network_config::CrosNetworkConfigTestHelper>
-      network_config_helper_;
+  std::unique_ptr<chromeos::network_config::CrosNetworkConfig>
+      cros_network_config_;
   std::unique_ptr<ShellNewWindowDelegate> new_window_delegate_;
   content::MainFunctionParams parameters_;
 

chromeos/network/network_handler.h

@@ -85,6 +85,7 @@ class COMPONENT_EXPORT(CHROMEOS_NETWORK) NetworkHandler {
 
   // Global network configuration services.
   UIProxyConfigService* ui_proxy_config_service();
+  bool has_ui_proxy_config_service() { return ui_proxy_config_service_.get(); }
 
  private:
   NetworkHandler();

chromeos/services/network_config/cros_network_config.cc

@@ -185,7 +185,8 @@ mojom::NetworkStatePropertiesPtr NetworkStateToMojo(const NetworkState* network,
 
   // NetworkHandler and UIProxyConfigService may not exist in tests.
   UIProxyConfigService* ui_proxy_config_service =
-      NetworkHandler::IsInitialized()
+      NetworkHandler::IsInitialized() &&
+              NetworkHandler::Get()->has_ui_proxy_config_service()
           ? NetworkHandler::Get()->ui_proxy_config_service()
           : nullptr;
   result->proxy_mode =