cros_network_config.mojom: Use unions for ConfigProperties

This CL uses a unuion for type specific properties in ConfigProperties.

Since the config properties are typically generated on the fly and
passed from UI -> the CrosNetworkConfig implementation, the union
type is used to specify the network type (i.e. no explicit type is
required).

Bug: 853953
Change-Id: I08ae054f8d5554727c4d9422ca325b284c3927e8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1850724
Commit-Queue: Steven Bennetts <stevenjb@chromium.org>
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#704740}

chrome/browser/resources/chromeos/internet_detail_dialog/internet_detail_dialog.js

@@ -231,7 +231,7 @@ Polymer({
    * @private
    */
   getDefaultConfigProperties_: function() {
-    return {type: this.managedProperties_.type};
+    return OncMojo.getDefaultConfigProperties(this.managedProperties_.type);
   },
 
   /**
@@ -489,7 +489,7 @@ Polymer({
     }
     const config = this.getDefaultConfigProperties_();
     const apn = event.detail;
-    config.cellular = {apn: apn};
+    config.typeConfig.cellular = {apn: apn};
     this.setMojoNetworkProperties_(config);
   },
 

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

@@ -551,7 +551,7 @@ Polymer({
    * @private
    */
   getDefaultConfigProperties_: function() {
-    return {type: this.managedProperties_.type};
+    return OncMojo.getDefaultConfigProperties(this.managedProperties_.type);
   },
 
   /**
@@ -1169,14 +1169,14 @@ Polymer({
     OncMojo.setConfigProperty(config, field, value);
     // Ensure that any required configuration properties for partial
     // configurations are set.
-    if (config.vpn) {
-      config.vpn.type = this.managedProperties_.typeProperties.vpn.type;
-      if (config.vpn.openVpn &&
-          config.vpn.openVpn.saveCredentials == undefined) {
-        config.vpn.openVpn.saveCredentials = false;
+    const vpnConfig = config.typeConfig.vpn;
+    if (vpnConfig) {
+      vpnConfig.type = this.managedProperties_.typeProperties.vpn.type;
+      if (vpnConfig.openVpn && vpnConfig.openVpn.saveCredentials == undefined) {
+        vpnConfig.openVpn.saveCredentials = false;
       }
-      if (config.vpn.l2tp && config.vpn.l2tp.saveCredentials == undefined) {
-        config.vpn.l2tp.saveCredentials = false;
+      if (vpnConfig.l2tp && vpnConfig.l2tp.saveCredentials == undefined) {
+        vpnConfig.l2tp.saveCredentials = false;
       }
     }
     this.setMojoNetworkProperties_(config);
@@ -1192,7 +1192,7 @@ Polymer({
     }
     const config = this.getDefaultConfigProperties_();
     const apn = event.detail;
-    config.cellular = {apn: apn};
+    config.typeConfig.cellular = {apn: apn};
     this.setMojoNetworkProperties_(config);
   },
 

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

@@ -191,14 +191,18 @@ Polymer({
   /** @private */
   onRemovePreferredTap_: function() {
     assert(this.networkType !== undefined);
-    this.setProperties_({type: this.networkType, priority: {value: 0}});
+    const config = OncMojo.getDefaultConfigProperties(this.networkType);
+    config.priority = {value: 0};
+    this.setProperties_(config);
     /** @type {!CrActionMenuElement} */ (this.$.dotsMenu).close();
   },
 
   /** @private */
   onAddPreferredTap_: function() {
     assert(this.networkType !== undefined);
-    this.setProperties_({type: this.networkType, priority: {value: 1}});
+    const config = OncMojo.getDefaultConfigProperties(this.networkType);
+    config.priority = {value: 1};
+    this.setProperties_(config);
     /** @type {!CrActionMenuElement} */ (this.$.dotsMenu).close();
   },
 

chrome/test/data/webui/cr_components/network_config_test.js

@@ -234,6 +234,10 @@ suite('network-config', function() {
             'PEAP',
             networkConfig.managedProperties.typeProperties.ethernet.eap.outer
                 .activeValue);
+        assertEquals(
+            'PEAP',
+            networkConfig.configProperties_.typeConfig.ethernet.eap.outer);
+        assertEquals('PEAP', networkConfig.eapProperties_.outer);
         let outer = networkConfig.$$('#outer');
         assertTrue(!!outer);
         assertTrue(!outer.disabled);

chromeos/services/network_config/cros_network_config_unittest.cc

@@ -740,7 +740,8 @@ TEST_F(CrosNetworkConfigTest, SetProperties) {
 
   // Set priority.
   auto config = mojom::ConfigProperties::New();
-  config->type = mojom::NetworkType::kWiFi;
+  config->type_config = mojom::NetworkTypeConfigProperties::NewWifi(
+      mojom::WiFiConfigProperties::New());
   config->priority = mojom::PriorityConfig::New();
   config->priority->value = 1;
   bool success = SetProperties(kGUID, std::move(config));
@@ -757,7 +758,8 @@ TEST_F(CrosNetworkConfigTest, SetProperties) {
 
   // Set auto connect only. Priority should remain unchanged.
   config = mojom::ConfigProperties::New();
-  config->type = mojom::NetworkType::kWiFi;
+  config->type_config = mojom::NetworkTypeConfigProperties::NewWifi(
+      mojom::WiFiConfigProperties::New());
   config->auto_connect = mojom::AutoConnectConfig::New();
   config->auto_connect->value = true;
   success = SetProperties(kGUID, std::move(config));
@@ -780,9 +782,10 @@ TEST_F(CrosNetworkConfigTest, ConfigureNetwork) {
   const std::string ssid = "new_wifi_ssid";
   // Configure a new wifi network.
   auto config = mojom::ConfigProperties::New();
-  config->type = mojom::NetworkType::kWiFi;
-  config->wifi = mojom::WiFiConfigProperties::New();
-  config->wifi->ssid = ssid;
+  auto wifi = mojom::WiFiConfigProperties::New();
+  wifi->ssid = ssid;
+  config->type_config =
+      mojom::NetworkTypeConfigProperties::NewWifi(std::move(wifi));
   std::string guid = ConfigureNetwork(std::move(config), shared);
   EXPECT_FALSE(guid.empty());
 

chromeos/services/network_config/public/mojom/cros_network_config.mojom

@@ -758,10 +758,15 @@ struct WiFiConfigProperties {
   SecurityType security = kNone;
 };
 
+union NetworkTypeConfigProperties {
+  CellularConfigProperties cellular;
+  EthernetConfigProperties ethernet;
+  VPNConfigProperties vpn;
+  WiFiConfigProperties wifi;
+};
+
 struct ConfigProperties {
   AutoConnectConfig? auto_connect;
-  CellularConfigProperties? cellular;
-  EthernetConfigProperties? ethernet;
   string? ip_address_config_type;
   // Name is only required for new configurations.
   string? name;
@@ -769,10 +774,8 @@ struct ConfigProperties {
   PriorityConfig? priority;
   ProxySettings? proxy_settings;
   IPConfigProperties? static_ip_config;
-  // |type| must match the existing configuration when used with SetProperties.
-  NetworkType type;
-  VPNConfigProperties? vpn;
-  WiFiConfigProperties? wifi;
+  // NetworkType is inferred from |type_config|.
+  NetworkTypeConfigProperties type_config;
 };
 
 // Properties provided to SetCellularSimState.

ui/webui/resources/cr_components/chromeos/network/network_config.html

@@ -35,7 +35,7 @@
       <!-- SSID (WiFi) -->
       <template is="dom-if" if="[[isWiFi_(mojoType_)]]" restamp>
         <network-config-input id="ssid" label="[[i18n('OncWiFi-SSID')]]"
-            value="{{configProperties_.wifi.ssid}}"
+            value="{{configProperties_.typeConfig.wifi.ssid}}"
             readonly="[[hasGuid_(guid)]]"
             on-enter="onEnterPressedInInput_">
         </network-config-input>
@@ -58,7 +58,7 @@
           if="[[configRequiresPassphrase_(mojoType_, securityType)]]">
         <network-password-input id="wifi-passphrase"
             label="[[i18n('OncWiFi-Passphrase')]]"
-            value="{{configProperties_.wifi.passphrase}}"
+            value="{{configProperties_.typeConfig.wifi.passphrase}}"
             on-enter="onEnterPressedInInput_"
             property="[[managedProperties_.typeProperties.wifi.passphrase]]">
         </network-password-input>
@@ -67,7 +67,7 @@
       <!-- VPN -->
       <template is="dom-if" if="[[showVpn_]]" restamp>
         <network-config-input label="[[i18n('OncVPN-Host')]]"
-            value="{{configProperties_.vpn.host}}"
+            value="{{configProperties_.typeConfig.vpn.host}}"
             property="[[managedProperties_.typeProperties.vpn.host]]">
         </network-config-input>
         <network-config-input label="[[i18n('OncName')]]"
@@ -81,37 +81,37 @@
         </network-config-select>
         <template is="dom-if" if="[[!showVpn_.OpenVPN]]">
           <network-config-input label="[[i18n('OncVPN-L2TP-Username')]]"
-              value="{{configProperties_.vpn.l2tp.username}}"
+              value="{{configProperties_.typeConfig.vpn.l2tp.username}}"
               property="[[managedProperties_.typeProperties.vpn.l2tp.username]]"
           >
           </network-config-input>
           <network-password-input label="[[i18n('OncVPN-L2TP-Password')]]"
-              value="{{configProperties_.vpn.l2tp.password}}"
+              value="{{configProperties_.typeConfig.vpn.l2tp.password}}"
               property="[[managedProperties_.typeProperties.vpn.l2tp.password]]"
           >
           </network-password-input>
           <network-config-input label="[[i18n('OncVPN-IPsec-Group')]]"
-              value="{{configProperties_.vpn.ipSec.group}}"
+              value="{{configProperties_.typeConfig.vpn.ipSec.group}}"
               property="[[managedProperties_.typeProperties.vpn.ipSec.group]]">
           </network-config-input>
           <template is="dom-if" if="[[!showVpn_.Cert]]">
             <network-password-input label="[[i18n('OncVPN-IPsec-PSK')]]"
-                value="{{configProperties_.vpn.ipSec.psk}}"
+                value="{{configProperties_.typeConfig.vpn.ipSec.psk}}"
                 property="[[managedProperties_.typeProperties.vpn.ipSec.psk]]">
             </network-password-input>
           </template>
         </template>
         <template is="dom-if" if="[[showVpn_.OpenVPN]]">
           <network-config-input label="[[i18n('OncVPN-OpenVPN-Username')]]"
-              value="{{configProperties_.vpn.openVpn.username}}"
+              value="{{configProperties_.typeConfig.vpn.openVpn.username}}"
               property="[[managedProperties_.typeProperties.vpn.openVpn.username]]">
           </network-config-input>
           <network-password-input label="[[i18n('OncVPN-OpenVPN-Password')]]"
-              value="{{configProperties_.vpn.openVpn.password}}"
+              value="{{configProperties_.typeConfig.vpn.openVpn.password}}"
               property="[[managedProperties_.typeProperties.vpn.openVpn.password]]">
           </network-password-input>
           <network-config-input label="[[i18n('OncVPN-OpenVPN-OTP')]]"
-              value="{{configProperties_.vpn.openVpn.otp}}"
+              value="{{configProperties_.typeConfig.vpn.openVpn.otp}}"
               property="[[managedProperties_.typeProperties.vpn.openVpn.otp]]">
           </network-config-input>
         </template>

ui/webui/resources/js/chromeos/onc_mojo.js

@@ -663,7 +663,7 @@ class OncMojo {
    * @param {!chromeos.networkConfig.mojom.NetworkType} type
    * @param {string} guid
    * @param {string} name
-   * @return {chromeos.networkConfig.mojom.ManagedProperties}
+   * @return {!chromeos.networkConfig.mojom.ManagedProperties}
    */
   static getDefaultManagedProperties(type, guid, name) {
     const mojom = chromeos.networkConfig.mojom;
@@ -728,6 +728,34 @@ class OncMojo {
     return result;
   }
 
+  /**
+   * Returns a ConfigProperties object with a default networkType struct
+   * based on |type|.
+   * @param {!chromeos.networkConfig.mojom.NetworkType} type
+   * @return {!chromeos.networkConfig.mojom.ConfigProperties}
+   */
+  static getDefaultConfigProperties(type) {
+    const mojom = chromeos.networkConfig.mojom;
+    switch (type) {
+      case mojom.NetworkType.kCellular:
+        return {typeConfig: {cellular: {}}};
+        break;
+      case mojom.NetworkType.kEthernet:
+        return {typeConfig: {ethernet: {authentication: 'None'}}};
+        break;
+      case mojom.NetworkType.kVPN:
+        return {typeConfig: {vpn: {type: mojom.VpnType.kOpenVPN}}};
+        break;
+      case mojom.NetworkType.kWiFi:
+        return {
+          typeConfig: {wifi: {ssid: '', security: mojom.SecurityType.kNone}}
+        };
+        break;
+    }
+    assertNotReached('Unexpected type: ' + type.toString());
+    return {typeConfig: {}};
+  }
+
   /**
    * Sets the value of a property in an mojo config dictionary.
    * @param {!chromeos.networkConfig.mojom.ConfigProperties} config
@@ -931,7 +959,7 @@ class OncMojo {
     }
 
     // Set ONC IP config properties to existing values + new values.
-    const config = {type: managedProperties.type};
+    const config = OncMojo.getDefaultConfigProperties(managedProperties.type);
     config.ipAddressConfigType = ipConfigType;
     config.nameServersConfigType = nsConfigType;
     if (ipConfigType == 'Static') {