Enable cros_network_config.mojom in Settings UI

This is an initial CL that enables mojo for Settings ands includes a
small isolated useage.

The network_config mojo API will be used in parallel with the
networkingPrivate extension API while the code is transitioned.

Forthcoming CLs will transition the rest of Internet Settings to use
the mojo API, eventually fully replacing the extension API.

Adding mojo bindings to the Settings page resulted in no measurable
load time regressions. See issue for details.

This CL:
* Modifies SettingsUI to derive from MojoWebUIContriller
* Binds network_config mojo service to SettingsUI and OSSettingsUI
* Implements device enable API call in internet_page using Mojo

Bug: 853953
Change-Id: I99c3d2064361abb51368acc3811c45341f2db56b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1652802
Commit-Queue: Steven Bennetts <stevenjb@chromium.org>
Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#669033}

chrome/browser/resources/settings/internet_page/BUILD.gn

@@ -25,11 +25,14 @@ js_library("internet_page") {
     ":internet_page_browser_proxy",
     "..:route",
     "../settings_page:settings_animated_pages",
+    "//chromeos/services/network_config/public/mojom:mojom_js_library_for_compile",
+    "//ui/webui/resources/cr_components/chromeos/network:mojo_interface_provider",
     "//ui/webui/resources/cr_elements/chromeos/network:cr_network_listener_behavior",
     "//ui/webui/resources/cr_elements/chromeos/network:cr_onc_types",
     "//ui/webui/resources/js:assert",
     "//ui/webui/resources/js:i18n_behavior",
     "//ui/webui/resources/js:web_ui_listener_behavior",
+    "//ui/webui/resources/js/chromeos:onc_mojo",
   ]
   externs_list = [
     "$externs_path/chrome_send.js",

chrome/browser/resources/settings/internet_page/internet_page.html

 <link rel="import" href="chrome://resources/html/polymer.html">
 
+<link rel="import" href="chrome://resources/cr_components/chromeos/network/mojo_interface_provider.html">
 <link rel="import" href="chrome://resources/cr_elements/chromeos/network/cr_network_listener_behavior.html">
 <link rel="import" href="chrome://resources/cr_elements/chromeos/network/cr_onc_types.html">
 <link rel="import" href="chrome://resources/cr_elements/cr_expand_button/cr_expand_button.html">
 <link rel="import" href="chrome://resources/cr_elements/cr_icon_button/cr_icon_button.html">
 <link rel="import" href="chrome://resources/cr_elements/icons.html">
+<link rel="import" href="chrome://resources/html/chromeos/onc_mojo.html">
 <link rel="import" href="chrome://resources/html/i18n_behavior.html">
 <link rel="import" href="chrome://resources/html/web_ui_listener_behavior.html">
 <link rel="import" href="chrome://resources/polymer/v1_0/iron-icon/iron-icon.html">

chrome/browser/resources/settings/internet_page/internet_page.js

@@ -174,12 +174,23 @@ Polymer({
   /** @private {Function} */
   onExtensionDisabledListener_: null,
 
-  /** @private  {settings.InternetPageBrowserProxy} */
+  /** @private  {?settings.InternetPageBrowserProxy} */
   browserProxy_: null,
 
+  /**
+   * This UI will use both the networkingPrivate extension API and the
+   * networkConfig mojo API until we provide all of the required functionality
+   * in networkConfig. TODO(stevenjb): Remove use of networkingPrivate api.
+   * @private {?chromeos.networkConfig.mojom.CrosNetworkConfigProxy}
+   */
+  networkConfigProxy_: null,
+
   /** @override */
   created: function() {
     this.browserProxy_ = settings.InternetPageBrowserProxyImpl.getInstance();
+    this.networkConfigProxy_ =
+        network_config.MojoInterfaceProviderImpl.getInstance()
+            .getMojoServiceProxy();
   },
 
   /** @override */
@@ -295,11 +306,9 @@ Polymer({
    * @private
    */
   onDeviceEnabledToggled_: function(event) {
-    if (event.detail.enabled) {
-      this.networkingPrivate.enableNetworkType(event.detail.type);
-    } else {
-      this.networkingPrivate.disableNetworkType(event.detail.type);
-    }
+    this.networkConfigProxy_.setNetworkTypeEnabledState(
+        OncMojo.getNetworkTypeFromString(event.detail.type),
+        event.detail.enabled);
   },
 
   /**
@@ -595,8 +604,7 @@ Polymer({
    * @return {string}
    */
   getAddThirdPartyVpnLabel_: function(provider) {
-    return this.i18n(
-        'internetAddThirdPartyVPN', provider.ProviderName || '');
+    return this.i18n('internetAddThirdPartyVPN', provider.ProviderName || '');
   },
 
   /**

chrome/browser/sessions/session_restore_browsertest.cc

@@ -1142,16 +1142,16 @@ IN_PROC_BROWSER_TEST_F(SessionRestoreTest, MAYBE_RestoreWebUISettings) {
   ui_test_utils::NavigateToURL(browser(), webui_url);
   content::WebContents* old_tab =
       browser()->tab_strip_model()->GetActiveWebContents();
-  EXPECT_EQ(content::BINDINGS_POLICY_WEB_UI,
-            old_tab->GetMainFrame()->GetEnabledBindings());
+  EXPECT_TRUE(old_tab->GetMainFrame()->GetEnabledBindings() &
+              content::BINDINGS_POLICY_WEB_UI);
 
   Browser* new_browser = QuitBrowserAndRestore(browser(), 1);
   ASSERT_EQ(1u, active_browser_list_->size());
   content::WebContents* new_tab =
       new_browser->tab_strip_model()->GetActiveWebContents();
   EXPECT_EQ(webui_url, new_tab->GetURL());
-  EXPECT_EQ(content::BINDINGS_POLICY_WEB_UI,
-            new_tab->GetMainFrame()->GetEnabledBindings());
+  EXPECT_TRUE(new_tab->GetMainFrame()->GetEnabledBindings() &
+              content::BINDINGS_POLICY_WEB_UI);
 }
 
 IN_PROC_BROWSER_TEST_F(SessionRestoreTest, RestoresForwardAndBackwardNavs) {

chrome/browser/ui/webui/settings/chromeos/os_settings_ui.cc

@@ -38,9 +38,11 @@
 #include "chrome/grit/generated_resources.h"
 #include "chrome/grit/os_settings_resources.h"
 #include "chrome/grit/os_settings_resources_map.h"
+#include "chromeos/services/network_config/public/mojom/constants.mojom.h"
 #include "components/password_manager/core/common/password_manager_features.h"
 #include "components/unified_consent/feature.h"
 #include "content/public/browser/web_ui_data_source.h"
+#include "services/service_manager/public/cpp/connector.h"
 
 #if defined(FULL_SAFE_BROWSING)
 #include "chrome/browser/safe_browsing/chrome_password_protection_service.h"
@@ -51,7 +53,7 @@ namespace chromeos {
 namespace settings {
 
 OSSettingsUI::OSSettingsUI(content::WebUI* web_ui)
-    : content::WebUIController(web_ui) {
+    : ui::MojoWebUIController(web_ui, /*enable_chrome_send =*/true) {
   Profile* profile = Profile::FromWebUI(web_ui);
   content::WebUIDataSource* html_source =
       content::WebUIDataSource::Create(chrome::kChromeUIOSSettingsHost);
@@ -147,9 +149,12 @@ OSSettingsUI::OSSettingsUI(content::WebUI* web_ui)
 
   content::WebUIDataSource::Add(web_ui->GetWebContents()->GetBrowserContext(),
                                 html_source);
+
+  AddHandlerToRegistry(base::BindRepeating(&OSSettingsUI::BindCrosNetworkConfig,
+                                           base::Unretained(this)));
 }
 
-OSSettingsUI::~OSSettingsUI() {}
+OSSettingsUI::~OSSettingsUI() = default;
 
 void OSSettingsUI::AddSettingsPageUIHandler(
     std::unique_ptr<content::WebUIMessageHandler> handler) {
@@ -157,5 +162,12 @@ void OSSettingsUI::AddSettingsPageUIHandler(
   web_ui()->AddMessageHandler(std::move(handler));
 }
 
+void OSSettingsUI::BindCrosNetworkConfig(
+    network_config::mojom::CrosNetworkConfigRequest request) {
+  content::BrowserContext::GetConnectorFor(
+      web_ui()->GetWebContents()->GetBrowserContext())
+      ->BindInterface(network_config::mojom::kServiceName, std::move(request));
+}
+
 }  // namespace settings
 }  // namespace chromeos

chrome/browser/ui/webui/settings/chromeos/os_settings_ui.h

@@ -8,7 +8,9 @@
 #include <memory>
 
 #include "base/macros.h"
+#include "chromeos/services/network_config/public/mojom/cros_network_config.mojom.h"
 #include "content/public/browser/web_ui_controller.h"
+#include "ui/webui/mojo_web_ui_controller.h"
 
 namespace content {
 class WebUIMessageHandler;
@@ -18,7 +20,7 @@ namespace chromeos {
 namespace settings {
 
 // The WebUI handler for chrome://settings.
-class OSSettingsUI : public content::WebUIController {
+class OSSettingsUI : public ui::MojoWebUIController {
  public:
   explicit OSSettingsUI(content::WebUI* web_ui);
   ~OSSettingsUI() override;
@@ -26,6 +28,8 @@ class OSSettingsUI : public content::WebUIController {
  private:
   void AddSettingsPageUIHandler(
       std::unique_ptr<content::WebUIMessageHandler> handler);
+  void BindCrosNetworkConfig(
+      network_config::mojom::CrosNetworkConfigRequest request);
 
   // TODO(crbug/950007): Create load histograms and embed WebuiLoadTimer.
 

chrome/browser/ui/webui/settings/settings_ui.cc

@@ -119,8 +119,10 @@
 #include "chromeos/constants/chromeos_pref_names.h"
 #include "chromeos/constants/chromeos_switches.h"
 #include "chromeos/services/multidevice_setup/public/cpp/prefs.h"
+#include "chromeos/services/network_config/public/mojom/constants.mojom.h"
 #include "components/arc/arc_util.h"
 #include "components/prefs/pref_service.h"
+#include "services/service_manager/public/cpp/connector.h"
 #include "ui/base/ui_base_features.h"
 #include "ui/chromeos/resources/grit/ui_chromeos_resources.h"
 #else  // !defined(OS_CHROMEOS)
@@ -147,6 +149,24 @@
 
 namespace settings {
 
+namespace {
+
+#if defined(OS_CHROMEOS)
+bool ShouldShowParentalControls(Profile* profile) {
+  // Show Parental controls for regular and child accounts that are the
+  // primary profile.  Do not show it to any secondary profiles, managed
+  // accounts that aren't child accounts (i.e. enterprise and EDU accounts),
+  // OTR accounts, or legacy supervised user accounts.
+  return chromeos::switches::IsParentalControlsSettingsEnabled() &&
+         profile == ProfileManager::GetPrimaryUserProfile() &&
+         !profile->IsLegacySupervised() && !profile->IsGuestSession() &&
+         (profile->IsChild() ||
+          !profile->GetProfilePolicyConnector()->IsManaged());
+}
+#endif  // defined(OS_CHROMEOS)
+
+}  // namespace
+
 // static
 void SettingsUI::RegisterProfilePrefs(
     user_prefs::PrefRegistrySyncable* registry) {
@@ -158,7 +178,11 @@ void SettingsUI::RegisterProfilePrefs(
 }
 
 SettingsUI::SettingsUI(content::WebUI* web_ui)
+#if defined(OS_CHROMEOS)
+    : ui::MojoWebUIController(web_ui, /*enable_chrome_send =*/true),
+#else
     : content::WebUIController(web_ui),
+#endif
       webui_load_timer_(web_ui->GetWebContents(),
                         "Settings.LoadDocumentTime.MD",
                         "Settings.LoadCompletedTime.MD") {
@@ -321,29 +345,14 @@ SettingsUI::SettingsUI(content::WebUI* web_ui)
 
   content::WebUIDataSource::Add(web_ui->GetWebContents()->GetBrowserContext(),
                                 html_source);
-}
-
-SettingsUI::~SettingsUI() {}
 
 #if defined(OS_CHROMEOS)
-bool SettingsUI::ShouldShowParentalControls(Profile* profile) {
-  // Show Parental controls for regular and child accounts that are the
-  // primary profile.  Do not show it to any secondary profiles, managed
-  // accounts that aren't child accounts (i.e. enterprise and EDU accounts),
-  // OTR accounts, or legacy supervised user accounts.
-  return chromeos::switches::IsParentalControlsSettingsEnabled() &&
-         profile == ProfileManager::GetPrimaryUserProfile() &&
-         !profile->IsLegacySupervised() && !profile->IsGuestSession() &&
-         (profile->IsChild() ||
-          !profile->GetProfilePolicyConnector()->IsManaged());
+  AddHandlerToRegistry(base::BindRepeating(&SettingsUI::BindCrosNetworkConfig,
+                                           base::Unretained(this)));
+#endif  // defined (OS_CHROMEOS)
 }
-#endif
 
-void SettingsUI::AddSettingsPageUIHandler(
-    std::unique_ptr<content::WebUIMessageHandler> handler) {
-  DCHECK(handler);
-  web_ui()->AddMessageHandler(std::move(handler));
-}
+SettingsUI::~SettingsUI() = default;
 
 #if defined(OS_CHROMEOS)
 // static
@@ -518,4 +527,20 @@ void SettingsUI::InitOSWebUIHandlers(Profile* profile,
 }
 #endif  // defined(OS_CHROMEOS)
 
+void SettingsUI::AddSettingsPageUIHandler(
+    std::unique_ptr<content::WebUIMessageHandler> handler) {
+  DCHECK(handler);
+  web_ui()->AddMessageHandler(std::move(handler));
+}
+
+#if defined(OS_CHROMEOS)
+void SettingsUI::BindCrosNetworkConfig(
+    chromeos::network_config::mojom::CrosNetworkConfigRequest request) {
+  content::BrowserContext::GetConnectorFor(
+      web_ui()->GetWebContents()->GetBrowserContext())
+      ->BindInterface(chromeos::network_config::mojom::kServiceName,
+                      std::move(request));
+}
+#endif  // defined(OS_CHROMEOS)
+
 }  // namespace settings

chrome/browser/ui/webui/settings/settings_ui.h

@@ -8,7 +8,13 @@
 #include "base/macros.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/webui/webui_load_timer.h"
+
+#if defined(OS_CHROMEOS)
+#include "chromeos/services/network_config/public/mojom/cros_network_config.mojom.h"
+#include "ui/webui/mojo_web_ui_controller.h"
+#else
 #include "content/public/browser/web_ui_controller.h"
+#endif
 
 class Profile;
 
@@ -24,7 +30,13 @@ class PrefRegistrySyncable;
 namespace settings {
 
 // The WebUI handler for chrome://settings.
-class SettingsUI : public content::WebUIController {
+class SettingsUI
+#if defined(OS_CHROMEOS)
+    : public ui::MojoWebUIController
+#else
+    : public content::WebUIController
+#endif
+{
  public:
   static void RegisterProfilePrefs(user_prefs::PrefRegistrySyncable* registry);
 
@@ -36,13 +48,15 @@ class SettingsUI : public content::WebUIController {
   static void InitOSWebUIHandlers(Profile* profile,
                                   content::WebUI* web_ui,
                                   content::WebUIDataSource* html_source);
-
-  static bool ShouldShowParentalControls(Profile* profile);
 #endif  // defined(OS_CHROMEOS)
 
  private:
   void AddSettingsPageUIHandler(
       std::unique_ptr<content::WebUIMessageHandler> handler);
+#if defined(OS_CHROMEOS)
+  void BindCrosNetworkConfig(
+      chromeos::network_config::mojom::CrosNetworkConfigRequest request);
+#endif
 
   WebuiLoadTimer webui_load_timer_;
 

chrome/test/data/webui/chromeos/fake_network_config_mojom.js

+// Copyright 2019 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.
+
+/**
+ * @fileoverview Fake implementation of CrosNetworkConfig for testing.
+ * Currently this is built on top of FakeNetworkingPrivate to avoid
+ * inconsistencies between fakes in tests.
+ */
+
+// TODO(stevenjb): Include cros_network_config.mojom.js and extend
+// CrosNetworkConfigInterface
+class FakeNetworkConfig {
+  /** @param {!NetworkingPrivate} extensionApi */
+  constructor(extensionApi) {
+    this.extensionApi_ = extensionApi;
+  }
+
+  /**
+   * @param {!chromeos.networkConfig.mojom.NetworkType} type
+   * @param {boolean} enabled
+   * @return {!Promise<{success: boolean}>}
+   */
+  setNetworkTypeEnabledState(type, enabled) {
+    if (enabled) {
+      this.extensionApi_.enableNetworkType(OncMojo.getNetworkTypeString(type));
+    } else {
+      this.extensionApi_.disableNetworkType(OncMojo.getNetworkTypeString(type));
+    }
+    return Promise.resolve(true);
+  }
+}

chrome/test/data/webui/settings/cr_settings_browsertest.js

@@ -1639,6 +1639,7 @@ CrSettingsInternetPageTest.prototype = {
     '//ui/webui/resources/js/promise_resolver.js',
     '//ui/webui/resources/js/assert.js',
     '../fake_chrome_event.js',
+    '../chromeos/fake_network_config_mojom.js',
     '../chromeos/fake_networking_private.js',
     '../chromeos/cr_onc_strings.js',
     'internet_page_tests.js',

chrome/test/data/webui/settings/internet_page_tests.js

@@ -3,15 +3,18 @@
 // found in the LICENSE file.
 
 suite('InternetPage', function() {
-  /** @type {InternetPageElement} */
+  /** @type {?InternetPageElement} */
   let internetPage = null;
 
-  /** @type {NetworkSummaryElement} */
+  /** @type {?NetworkSummaryElement} */
   let networkSummary_ = null;
 
-  /** @type {NetworkingPrivate} */
+  /** @type {?NetworkingPrivate} */
   let api_;
 
+  /** @type {?chromeos.networkConfig.mojom.CrosNetworkConfigProxy} */
+  let mojoApi_;
+
   suiteSetup(function() {
     loadTimeData.overrideValues({
       internetAddConnection: 'internetAddConnection',
@@ -43,6 +46,8 @@ suite('InternetPage', function() {
     };
 
     api_ = new chrome.FakeNetworkingPrivate();
+    mojoApi_ = new FakeNetworkConfig(api_);
+    network_config.MojoInterfaceProviderImpl.getInstance().proxy_ = mojoApi_;
 
     // Disable animations so sub-pages open within one event loop.
     testing.Test.disableAnimationsAndTransitions();
@@ -83,7 +88,7 @@ suite('InternetPage', function() {
 
   teardown(function() {
     internetPage.remove();
-    delete internetPage;
+    internetPage = null;
     // Navigating to the details page changes the Route state.
     settings.resetRouteForTesting();
   });