Replace chrome://network implementation with networkConfig API

chrome/browser/resources/chromeos/network/network_config.js

Index: chrome/browser/resources/chromeos/network/network_config.js
diff --git a/chrome/browser/resources/chromeos/network/network_config.js b/chrome/browser/resources/chromeos/network/network_config.js
new file mode 100644
index 0000000000000000000000000000000000000000..20836cfee4c6d2da057c7fb2ef142f581dc2a75f
--- /dev/null
+++ b/chrome/browser/resources/chromeos/network/network_config.js
@@ -0,0 +1,122 @@
+// Copyright 2014 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.
+
+'use strict';
+
+// This object provides a similar API to chrome.networkingPrivate. It simulates
+// the extension callback model by storing callbacks in a member object and
+// invoking them when the corresponding method is called by Chrome in response
+// to a chrome.send call.
+
+var networkConfig = {
+  // ONC types
+  Type: {
+    All: 'All',
+    Cellular: 'Cellular',
+    Ethernet: 'Ethernet',
+    VPN: 'VPN',
+    WiFi: 'WiFi'
+  },
+
+  // ONC Service properties
+  Service: {

[pneubeck (no reviews), 2014/05/02 09:43:46]

The term 'Service' doesn't appear in ONC (so far).
Maybe s/Service/NetworkConfiguration/

[stevenjb, 2014/05/02 22:07:38]

Or 'ONC'?

[pneubeck (no reviews), 2014/05/08 09:21:43]

Fine with me, but then there should at least be a comment referring to ONC's
type 'NetworkConfiguration' (as mentioned in the onc_spec.html)

+    Address: 'Address',
+    Cellular: {
+      NetworkTechnology: 'Cellular.NetworkTechnology',

[pneubeck (no reviews), 2014/05/02 09:43:46]

optional:

since an access by path of a property dictionary will require splitting at the
'.', you might also consider using an array of keys instead.
I.e.:

Service: {
  ...
  Cellular: {
     NetworkTechnology: ['Cellular', 'NetworkTechnology']
  }
}

[stevenjb, 2014/05/02 22:07:38]

I think the implementation of that would be more complicated than we need. This
is a convenient shorthand for our 2-level dictionary. I did move the code to
access the sub-dictionaries to this object.

+      ActivationState: 'Cellular.ActivationState',
+      RoamingState: 'Cellular.RoamingState',
+      OutOfCredits: 'Cellular.OutOfCredits'
+    },
+    Connectable: 'Connectable',
+    ConnectionState: 'ConnectionState',
+    EAP: {
+      EAP: 'EAP.EAP'
+    },
+    ErrorState: 'ErrorState',
+    GUID: 'GUID',
+    Name: 'Name',
+    OncSource: 'onc_source',

[pneubeck (no reviews), 2014/05/02 09:43:46]

add a comment: 'temporary'

in the future we should something like 'Policies' being a subset of
{DevicePolicy, UserPolicy}

[stevenjb, 2014/05/02 22:07:38]

Done.

+    Profile: 'Profile',
+    WiFi: {
+      Security: 'WiFi.Security',
+      SignalStrength: 'WiFi.SignalStrength'
+    },
+    ServicePath: 'ServicePath',
+    Type: 'Type'
+  },
+
+  /**
+   * Associate the callback with |id|
+   *
+   * @param {function} callback The associated callback
+   * @return {integer} The id of the callback
+   */
+  callbackId: 1,
+  callbackMap: {},
+  rememberCallback: function(callback) {
+    var id = networkConfig.callbackId++;
+    networkConfig.callbackMap[id] = callback;
+    return id;
+  },
+
+  /**
+   * Return the callback associated with |id|
+   *
+   * @param {integer} id The id of the callback
+   * @return {function} The associated callback
+   */
+  getCallback: function(id) {

[pneubeck (no reviews), 2014/05/02 09:43:46]

get -> pop/take/remove

[stevenjb, 2014/05/02 22:07:38]

remember/get -> store/retrieve

+    var callback = networkConfig.callbackMap[id];
+    delete networkConfig.callbackMap[id];
+    return callback;
+  },
+
+  /**
+   * Callback invoked by Chrome
+   *
+   * @param {string} data A list of arguments passed to the callback

[pneubeck (no reviews), 2014/05/02 09:43:46]

optional: mention, first entry of list is callbackID

[stevenjb, 2014/05/02 22:07:38]

Done.

+   */
+  chromeCallback: function(data) {

[pneubeck (no reviews), 2014/05/02 09:43:46]

unify the naming of chromeCallback vs. chromeError.
Maybe onSuccess/onError?

[stevenjb, 2014/05/02 22:07:38]

chromeCallbackSuccess / chromeCallbackError

+    var callbackId = data.shift();
+    var callback = networkConfig.getCallback(callbackId);
+    if (callback)
+      callback(data);

[pneubeck (no reviews), 2014/05/02 09:43:46]

callback.apply(null, data)

[stevenjb, 2014/05/02 22:07:38]

Interesting. I'll need to change the callbacks to take individual args instead
of an array, but that does seem better. Done.

+    else
+      console.log('Callback not found for id: ' + callbackId);

[pneubeck (no reviews), 2014/05/02 09:43:46]

log -> error

[stevenjb, 2014/05/02 22:07:38]

Done.

+  },
+
+  /**
+   * Error Callback invoked by Chrome
+   *
+   * @param {string} data A list of arguments passed to the callback

[pneubeck (no reviews), 2014/05/02 09:43:46]

optional: mention, first entry of list is callbackID

[stevenjb, 2014/05/02 22:07:38]

Done.

+   */
+  chromeError: function(data) {
+    var callbackId = data.shift();
+    var error = data.shift();
+    // Get the callback so that it is deleted from the map.
+    networkConfig.getCallback(callbackId);
+    console.log('Callback error: '' + error + '' for id: ' + callbackId);

[pneubeck (no reviews), 2014/05/02 09:43:46]

log -> error

In case of an error, ext. APIs usually set chrome.runtime.lastError and call the
callback. See chrome/renderer/resources/extensions/last_error.js

[stevenjb, 2014/05/02 22:07:38]

Hmm, I'd assumed that was an 'Extensions' mechanism. I'm not sure I really like
using a global here, it seems too easy for multiple errors to stomp over each
other. Do you know of any examples using this mechanism outside of extensions?
It's not clear to me looking at last_error.js how one would ensure that it is
set on error or cleared on success outside of an extension.

[pneubeck (no reviews), 2014/05/08 09:21:43]

The global is only for reporting an error to the caller and not for internal
errors (it should be fine to do this anyways).
It's the idiomatic way for extension APIs and the networking API does/will do it
that way.

I just wanted to mention it, so that you can make this interface similar enough
that the transition to networkingAPI is unproblematic. E.g. you could use a
global like networkConfig.lastError.

+  },
+
+  /**
+   * Implement networkingPrivate.getProperties
+   *
+   * @param {string} type The type of networks to return
+   */
+  getProperties: function(guid, callback) {
+    var callbackId = networkConfig.rememberCallback(callback);
+    chrome.send('networkConfig.getProperties', [callbackId, guid]);
+  },
+
+  /**
+   * Implement networkingPrivate.getVisibleNetworks
+   *
+   * @param {string} type The type of networks to return
+   */
+  getVisibleNetworks: function(type, callback) {
+    var callbackId = networkConfig.rememberCallback(callback);
+    chrome.send('networkConfig.getVisibleNetworks', [callbackId, type]);
+  }
+
+};