Implement networkingPrivate.getNetworks

chrome/common/extensions/api/networking_private.json

Index: chrome/common/extensions/api/networking_private.json
diff --git a/chrome/common/extensions/api/networking_private.json b/chrome/common/extensions/api/networking_private.json
index db603ccf76241100603be8fa2a5cd8c4010b39b6..91b605d186ee046159e7003d12626f8aeb6fe8fb 100644
--- a/chrome/common/extensions/api/networking_private.json
+++ b/chrome/common/extensions/api/networking_private.json
@@ -58,7 +58,7 @@
       {
         "id": "NetworkType",
         "type": "string",
-        "enum": ["Ethernet", "WiFi", "Cellular"]
+        "enum": ["All", "Bluetooth", "Cellular", "Ethernet", "VPN", "Wireless", "WiFi", "Wimax"]
       },
       {
         "id": "CaptivePortalStatus",
@@ -113,7 +113,7 @@
       },
       {
         "name": "getState",
-        "description": "Gets the cached read-only properties of the network with id networkGuid.  This is meant to be a higher performance function than getProperties, which requires a round trip to query the networking subsystem.  It only returns a subset of the properties returned by getProperties.",
+        "description": "Gets the cached read-only properties of the network with id networkGuid. This is meant to be a higher performance function than getProperties, which requires a round trip to query the networking subsystem.  The following properties are returned for all networks: GUID, Type, Name, WiFi.Security. Additional properties are provided for visible networks: ConnectionState, ErrorState, WiFi.SignalStrength, Cellular.NetworkTechnology, Cellular.ActivationState, Cellular.RoamingState, Cellular.OutOfCredits.",

[asargent_no_longer_on_chrome, 2014/05/16 19:32:58]

Instead of having one big bag o' properties that the schema validation system
doesn't know about with some degree of scoping provded by dot-seprated string
prefix on property names, had you considered using actual objects with named
fields? E.g. instead of just:

------------------------------------------
{
  "id": "NetworkProperties",
  "type": "object",
  "additionalProperties": { "type": "any" }
},
{
  "id": "NetworkType",
  "type": "string",
  "enum": ["Ethernet", "WiFi", "Cellular"]
},
------------------------------------------

you'd have:

------------------------------------------
{
  "id": "NetworkType",
  "type": "string",
  "enum": ["Ethernet", "WiFi", "Cellular"]
},

{
  "id": "WifiProperties",
  "type": "object",
  "properties": {
    "Security": { "type": "string", "enum": ["None", "WPA2"] },
    "SignalStrength": { "type": "integer" }
    ... etc. ...
  }
}

{
  "id": "CellularProperties",
  "type": "object",
  "properties": {
    "NetworkTechnology": { "type": "string", "enum": [ <whatever values> ]  },
    "ActivationState": { "type": "string", "enum": [ <whatever values> ] }
    ... etc. ...
  }
}

{
  "id": "NetworkProperties",
  "type": "object",
  "properties": {
    "Type" : {
      "$ref": "NetworkType"
    },
    "Wifi": {
      "$ref": "WifiProperties",
      "optional": true,
      "description": "Wifi properties, set if Type is 'Wifi'"
    },
    "Cellular": {
      "$ref": "CellularProperties",
      "optional": true,
      "description": "Cellular properties, set if Type is 'Cellular'"
    }
  }
  "additionalProperties": { "type": "any" }
},
------------------------------------------


Then when dealing with this in the callback, instead of doing:


chrome.networkingPrivate.getState(<id>, function(props) {
  if (props.Type == "Wifi") {
    console.log(props["Wifi.Security"]);
  }
});

you could do:

chrome.networkingPrivate.getState(<id>, function(props) {
  if (props.Type == "Wifi") {
    console.log(props.Wifi.Security);
  }
});

The big advantage though is actually that then you get schema validation of the
results. 

[stevenjb, 2014/05/16 19:44:44]

The result is an ONC (Open Network Configuration) object, and we're changing it
pretty rapidly at this point, but there would definitely be value to validating
the results. I'll discuss this with Philip and consider doing this (in a
separate CL).

         "parameters": [
           {
             "name": "networkGuid",
@@ -127,7 +127,7 @@
               {
                 "name": "properties",
                 "$ref": "NetworkProperties",
-                "description": "Results of the query for network properties."
+                "description": "Results of the query for network state properties."
               }
             ]
           }
@@ -156,7 +156,7 @@
       },
       {
         "name": "createNetwork",
-        "description": "Creates a new network configuration from propeties. If a matching configured network already exists, this will fail. Otherwise returns the guid of the new network.",
+        "description": "Creates a new network configuration from properties. If a matching configured network already exists, this will fail. Otherwise returns the guid of the new network.",
         "parameters": [
           {
             "name": "shared",
@@ -181,13 +181,56 @@
         ]
       },
       {
+        "name": "getNetworks",
+        "description": "Returns a list of network objects with the same properties provided by getState. A filter is provided to specify the type of networks returned and to limit the number of networks. Networks are ordered by the system based on their priority, with connected or connecting networks listed first.",
+        "parameters": [
+          {
+            "name": "filter",
+            "type": "object",
+            "properties": {
+              "networkType": {
+                "$ref": "NetworkType",
+                "description": "The type of networks to return."

[asargent_no_longer_on_chrome, 2014/05/16 19:32:58]

Instead of making callers pass "All" here, you could just skip having the "All"
enum value altogether and instead let this be an optional property, and the
absence of it implies all. 

[stevenjb, 2014/05/16 19:44:44]

I think I prefer requiring an explicit type, but could be convinced otherwise.
Philip, thoughts?

[pneubeck (no reviews), 2014/05/18 18:36:55]

Similar to the other filter options, I think it's logical that omitting
NetworkType means 'any network type'.
Anyways, I'm fine with both.

+              },
+              "visible": {
+                "type": "boolean",
+                "optional": true,
+                "description": "If true, only include visible (physically connected or in-range) networks. Defaults to 'false'."
+              },
+              "configured": {
+                "type": "boolean",
+                "optional": true,
+                "description": "If true, only include configured (saved) networks. Defaults to 'false'."
+              },
+              "limit": {
+                "type": "integer",
+                "optional": true,
+                "description": "Maximum number of networks to return. Defaults to 1000. Use 0 for no limit."
+              }
+            }
+          },
+          {
+            "name": "callback",
+            "type": "function",
+            "optional": true,
+            "parameters": [
+              {
+                "name": "networkList",
+                "type": "array",
+                "items": { "$ref": "NetworkProperties" }
+              }
+            ]
+          }
+        ]
+      },
+      {
         "name": "getVisibleNetworks",
-        "description": "Returns a list of visible network objects with the following ONC properties: GUID, Type, Name, ConnectionState, ErrorState, WiFi.Security, WiFi.SignalStrength, Cellular.NetworkTechnology, Cellular.ActivationState, Cellular.RoamingState. Cellular.OutOfCredits",
+        "deprecated": "Please use $(ref:networkingPrivate.getNetworks) with filter.visble = true.",
+        "description": "Returns a list of visible network objects with the same properties provided by getState.",
         "parameters": [
           {
-            "name": "type",
-            "type": "string",
-            "enum": ["Ethernet", "WiFi", "Bluetooth", "Cellular", "VPN", "All"],
+            "name": "networkType",
+            "$ref": "NetworkType",
             "description": "The type of networks to return."
           },
           {
@@ -206,7 +249,7 @@
       },
       {
         "name": "getEnabledNetworkTypes",
-        "description": "Returns a list of the enabled network type.",
+        "description": "Returns a list of the enabled network types. Note: this only returns discrete types that can be enabled or disabled: Cellular, Ethernet, WiFi, Wimax.",
         "parameters": [
           {
             "name": "callback",
@@ -214,7 +257,7 @@
             "optional": true,
             "parameters": [
               {
-                "name": "networkList",
+                "name": "enabledTypes",
                 "type": "array",
                 "items": { "$ref": "NetworkType" }
               }
@@ -224,7 +267,7 @@
       },
       {
         "name": "enableNetworkType",
-        "description": "Enable the specified network type.",
+        "description": "Enable the specified network type. Note, the type might represent multiple.",

[asargent_no_longer_on_chrome, 2014/05/16 19:32:58]

nit: did you mean "multiple types"? "multiple networks"? 

[stevenjb, 2014/05/16 19:44:44]

I meant 'multiple network types', thanks! Done.

         "parameters": [
           {
             "name": "networkType",