This factors out all of the parsing code from the network library

chrome/browser/chromeos/cros/network_library.h

 // Copyright (c) 2011 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.
 
 #ifndef CHROME_BROWSER_CHROMEOS_CROS_NETWORK_LIBRARY_H_
 #define CHROME_BROWSER_CHROMEOS_CROS_NETWORK_LIBRARY_H_
 #pragma once
 
 #include <string>
 #include <vector>
 
 #include "base/gtest_prod_util.h"
 #include "base/memory/scoped_vector.h"
 #include "base/memory/singleton.h"
 #include "base/observer_list.h"
+#include "base/scoped_ptr.h"
 #include "base/string16.h"
 #include "base/timer.h"
 #include "third_party/cros/chromeos_network.h"
 
 namespace base {
 class DictionaryValue;
 class Value;
 }
 
 namespace chromeos {
 
+class NativeNetworkParser;
+class NativeNetworkDeviceParser;
+
+// This is the list of all implementation classes that are allowed
+// access to the internals of the network library classes.
+#define NETWORK_LIBRARY_IMPL_FRIENDS            \
+  friend class NetworkLibraryImplBase;          \
+  friend class NetworkLibraryImplCros;          \
+  friend class NetworkLibraryImplStub;
+
+// This is the list of all parsers that are allowed access to the
+// internals of all the network library classes.  All parsers should
+// still access the internals through the set_ functions to allow
+// these classes to evolve without also having to change all of the
+// parsers.  If you add a parser subclass, then you'll need to add it
+// to this list.  Subclasses that only need access to a particular
+// class (e.g. WifiNetwork) should go under that class.
+#define NETWORK_LIBRARY_PARSER_FRIENDS          \
+  friend class NetworkParser;                   \
+  friend class NetworkDeviceParser;             \
+  friend class NativeNetworkParser;             \
+  friend class NativeNetworkDeviceParser;
+
+// This enumerates the various property indices that can be found in a
+// dictionary being parsed.
+enum PropertyIndex {
+  PROPERTY_INDEX_ACTIVATION_STATE,
+  PROPERTY_INDEX_ACTIVE_PROFILE,
+  PROPERTY_INDEX_ARP_GATEWAY,
+  PROPERTY_INDEX_AUTO_CONNECT,
+  PROPERTY_INDEX_AVAILABLE_TECHNOLOGIES,
+  PROPERTY_INDEX_CARRIER,
+  PROPERTY_INDEX_CELLULAR_ALLOW_ROAMING,
+  PROPERTY_INDEX_CELLULAR_APN,
+  PROPERTY_INDEX_CELLULAR_APN_LIST,
+  PROPERTY_INDEX_CELLULAR_LAST_GOOD_APN,
+  PROPERTY_INDEX_CHECK_PORTAL_LIST,
+  PROPERTY_INDEX_CONNECTABLE,
+  PROPERTY_INDEX_CONNECTED_TECHNOLOGIES,
+  PROPERTY_INDEX_CONNECTIVITY_STATE,
+  PROPERTY_INDEX_DEFAULT_TECHNOLOGY,
+  PROPERTY_INDEX_DEVICE,
+  PROPERTY_INDEX_DEVICES,
+  PROPERTY_INDEX_EAP_ANONYMOUS_IDENTITY,
+  PROPERTY_INDEX_EAP_CA_CERT,
+  PROPERTY_INDEX_EAP_CA_CERT_ID,
+  PROPERTY_INDEX_EAP_CA_CERT_NSS,
+  PROPERTY_INDEX_EAP_CERT_ID,
+  PROPERTY_INDEX_EAP_CLIENT_CERT,
+  PROPERTY_INDEX_EAP_CLIENT_CERT_NSS,
+  PROPERTY_INDEX_EAP_IDENTITY,
+  PROPERTY_INDEX_EAP_KEY_ID,
+  PROPERTY_INDEX_EAP_KEY_MGMT,
+  PROPERTY_INDEX_EAP_METHOD,
+  PROPERTY_INDEX_EAP_PASSWORD,
+  PROPERTY_INDEX_EAP_PHASE_2_AUTH,
+  PROPERTY_INDEX_EAP_PIN,
+  PROPERTY_INDEX_EAP_PRIVATE_KEY,
+  PROPERTY_INDEX_EAP_PRIVATE_KEY_PASSWORD,
+  PROPERTY_INDEX_EAP_USE_SYSTEM_CAS,
+  PROPERTY_INDEX_ENABLED_TECHNOLOGIES,
+  PROPERTY_INDEX_ERROR,
+  PROPERTY_INDEX_ESN,
+  PROPERTY_INDEX_FAVORITE,
+  PROPERTY_INDEX_FIRMWARE_REVISION,
+  PROPERTY_INDEX_FOUND_NETWORKS,
+  PROPERTY_INDEX_GUID,
+  PROPERTY_INDEX_HARDWARE_REVISION,
+  PROPERTY_INDEX_HOME_PROVIDER,
+  PROPERTY_INDEX_HOST,
+  PROPERTY_INDEX_IDENTITY,
+  PROPERTY_INDEX_IMEI,
+  PROPERTY_INDEX_IMSI,
+  PROPERTY_INDEX_IS_ACTIVE,
+  PROPERTY_INDEX_L2TPIPSEC_CA_CERT_NSS,
+  PROPERTY_INDEX_L2TPIPSEC_CLIENT_CERT_ID,
+  PROPERTY_INDEX_L2TPIPSEC_CLIENT_CERT_SLOT,
+  PROPERTY_INDEX_L2TPIPSEC_PASSWORD,
+  PROPERTY_INDEX_L2TPIPSEC_PIN,
+  PROPERTY_INDEX_L2TPIPSEC_PSK,
+  PROPERTY_INDEX_L2TPIPSEC_USER,
+  PROPERTY_INDEX_MANUFACTURER,
+  PROPERTY_INDEX_MDN,
+  PROPERTY_INDEX_MEID,
+  PROPERTY_INDEX_MIN,
+  PROPERTY_INDEX_MODE,
+  PROPERTY_INDEX_MODEL_ID,
+  PROPERTY_INDEX_NAME,
+  PROPERTY_INDEX_NETWORKS,
+  PROPERTY_INDEX_NETWORK_TECHNOLOGY,
+  PROPERTY_INDEX_OFFLINE_MODE,
+  PROPERTY_INDEX_OPERATOR_CODE,
+  PROPERTY_INDEX_OPERATOR_NAME,
+  PROPERTY_INDEX_PASSPHRASE,
+  PROPERTY_INDEX_PASSPHRASE_REQUIRED,
+  PROPERTY_INDEX_PAYMENT_URL,
+  PROPERTY_INDEX_PORTAL_URL,
+  PROPERTY_INDEX_POWERED,
+  PROPERTY_INDEX_PRIORITY,
+  PROPERTY_INDEX_PRL_VERSION,
+  PROPERTY_INDEX_PROFILE,
+  PROPERTY_INDEX_PROFILES,
+  PROPERTY_INDEX_PROVIDER,
+  PROPERTY_INDEX_PROXY_CONFIG,
+  PROPERTY_INDEX_ROAMING_STATE,
+  PROPERTY_INDEX_SAVE_CREDENTIALS,
+  PROPERTY_INDEX_SCANNING,
+  PROPERTY_INDEX_SECURITY,
+  PROPERTY_INDEX_SELECTED_NETWORK,
+  PROPERTY_INDEX_SERVICES,
+  PROPERTY_INDEX_SERVICE_WATCH_LIST,
+  PROPERTY_INDEX_SERVING_OPERATOR,
+  PROPERTY_INDEX_SIGNAL_STRENGTH,
+  PROPERTY_INDEX_SIM_LOCK,
+  PROPERTY_INDEX_STATE,
+  PROPERTY_INDEX_SUPPORT_NETWORK_SCAN,
+  PROPERTY_INDEX_TECHNOLOGY_FAMILY,
+  PROPERTY_INDEX_TYPE,
+  PROPERTY_INDEX_UNKNOWN,
+  PROPERTY_INDEX_USAGE_URL,
+  PROPERTY_INDEX_WIFI_AUTH_MODE,
+  PROPERTY_INDEX_WIFI_FREQUENCY,
+  PROPERTY_INDEX_WIFI_HEX_SSID,
+  PROPERTY_INDEX_WIFI_HIDDEN_SSID,
+  PROPERTY_INDEX_WIFI_PHY_MODE,
+};
+
 // Connection enums (see flimflam/include/service.h)
 enum ConnectionType {
   TYPE_UNKNOWN   = 0,
   TYPE_ETHERNET  = 1,
   TYPE_WIFI      = 2,
   TYPE_WIMAX     = 3,
   TYPE_BLUETOOTH = 4,
   TYPE_CELLULAR  = 5,
   TYPE_VPN       = 6,
 };
@@ skipping 57 matching lines @@
   ROAMING_STATE_ROAMING = 2,
 };
 
 // Device enums (see flimflam/include/device.h)
 enum TechnologyFamily {
   TECHNOLOGY_FAMILY_UNKNOWN = 0,
   TECHNOLOGY_FAMILY_CDMA    = 1,
   TECHNOLOGY_FAMILY_GSM     = 2
 };
 
+// Type of a pending SIM operation.
+enum SimOperationType {
+  SIM_OPERATION_NONE               = 0,
+  SIM_OPERATION_CHANGE_PIN         = 1,
+  SIM_OPERATION_CHANGE_REQUIRE_PIN = 2,
+  SIM_OPERATION_ENTER_PIN          = 3,
+  SIM_OPERATION_UNBLOCK_PIN        = 4,
+};
+
 // SIMLock states (see gobi-cromo-plugin/gobi_gsm_modem.cc)
-enum SIMLockState {
+enum SimLockState {
   SIM_UNKNOWN    = 0,
   SIM_UNLOCKED   = 1,
   SIM_LOCKED_PIN = 2,
   SIM_LOCKED_PUK = 3,  // also when SIM is blocked, then retries = 0.
 };
 
 // SIM PinRequire states. Since PinRequire current state is not exposed as a
-// cellular property, we initialize it's value based on the SIMLockState
+// cellular property, we initialize it's value based on the SimLockState
 // initial value.
-// SIM_PIN_REQUIRE_UNKNOWN - SIM card is absent or SIMLockState initial value
+// SIM_PIN_REQUIRE_UNKNOWN - SIM card is absent or SimLockState initial value
 //                           hasn't been received yet.
 // SIM_PIN_REQUIRED - SIM card is locked when booted/wake from sleep and
 //                    requires user to enter PIN.
 // SIM_PIN_NOT_REQUIRED - SIM card is unlocked all the time and requires PIN
 // only on certain operations, such as ChangeRequirePin, ChangePin, EnterPin.
-enum SIMPinRequire {
+enum SimPinRequire {
   SIM_PIN_REQUIRE_UNKNOWN = 0,
   SIM_PIN_NOT_REQUIRED    = 1,
   SIM_PIN_REQUIRED        = 2,
 };
 
 // Any PIN operation result (EnterPin, UnblockPin etc.).
 enum PinOperationError {
   PIN_ERROR_NONE           = 0,
   PIN_ERROR_UNKNOWN        = 1,
   PIN_ERROR_INCORRECT_CODE = 2,  // Either PIN/PUK specified is incorrect.
@@ skipping 38 matching lines @@
   EAP_PHASE_2_AUTH_CHAP     = 5
 };
 
 // Misc enums
 enum NetworkProfileType {
   PROFILE_NONE,    // Not in any profile (not remembered).
   PROFILE_SHARED,  // In the local profile, shared by all users on device.
   PROFILE_USER     // In the user provile, not visible to other users.
 };
 
+// Virtual Network provider type.
+enum ProviderType {
+  PROVIDER_TYPE_L2TP_IPSEC_PSK,
+  PROVIDER_TYPE_L2TP_IPSEC_USER_CERT,
+  PROVIDER_TYPE_OPEN_VPN,
+  // Add new provider types before PROVIDER_TYPE_MAX.
+  PROVIDER_TYPE_MAX,
+};
+
 // Simple wrapper for property Cellular.FoundNetworks.
 struct FoundCellularNetwork {
   FoundCellularNetwork();
   ~FoundCellularNetwork();
 
   std::string status;
   std::string network_id;
   std::string short_name;
   std::string long_name;
   std::string technology;
@@ skipping 38 matching lines @@
 // e.g. ethernet, wifi, cellular.
 // TODO(dpolukhin): refactor to make base class and device specific derivatives.
 class NetworkDevice {
  public:
   explicit NetworkDevice(const std::string& device_path);
   ~NetworkDevice();
 
   // Device info.
   const std::string& device_path() const { return device_path_; }
   const std::string& name() const { return name_; }
+  const std::string& unique_id() const { return unique_id_; }
   ConnectionType type() const { return type_; }
   bool scanning() const { return scanning_; }
-  const std::string& meid() const { return MEID_; }
-  const std::string& imei() const { return IMEI_; }
-  const std::string& imsi() const { return IMSI_; }
-  const std::string& esn() const { return ESN_; }
-  const std::string& mdn() const { return MDN_; }
-  const std::string& min() const { return MIN_; }
+  const std::string& meid() const { return meid_; }
+  const std::string& imei() const { return imei_; }
+  const std::string& imsi() const { return imsi_; }
+  const std::string& esn() const { return esn_; }
+  const std::string& mdn() const { return mdn_; }
+  const std::string& min() const { return min_; }
   const std::string& model_id() const { return model_id_; }
   const std::string& manufacturer() const { return manufacturer_; }
-  SIMLockState sim_lock_state() const { return sim_lock_state_; }
+  SimLockState sim_lock_state() const { return sim_lock_state_; }
   bool is_sim_locked() const {
     return sim_lock_state_ == SIM_LOCKED_PIN ||
         sim_lock_state_ == SIM_LOCKED_PUK;
   }
   const int sim_retries_left() const { return sim_retries_left_; }
-  SIMPinRequire sim_pin_required() const { return sim_pin_required_; }
+  SimPinRequire sim_pin_required() const { return sim_pin_required_; }
   const std::string& firmware_revision() const { return firmware_revision_; }
   const std::string& hardware_revision() const { return hardware_revision_; }
-  const unsigned int prl_version() const { return PRL_version_; }
+  const unsigned int prl_version() const { return prl_version_; }
   const std::string& home_provider() const { return home_provider_; }
   const std::string& home_provider_code() const { return home_provider_code_; }
   const std::string& home_provider_country() const {
     return home_provider_country_;
   }
   const std::string& home_provider_id() const { return home_provider_id_; }
   const std::string& home_provider_name() const { return home_provider_name_; }
   const std::string& selected_cellular_network() const {
     return selected_cellular_network_;
   }
   const CellularNetworkList& found_cellular_networks() const {
     return found_cellular_networks_;
   }
   bool data_roaming_allowed() const { return data_roaming_allowed_; }
   bool support_network_scan() const { return support_network_scan_; }
   enum TechnologyFamily technology_family() const { return technology_family_; }
   const CellularApnList& provider_apn_list() const {
     return provider_apn_list_;
   }
 
+  // Updates the property specified by |key| with the contents of
+  // |value|.  Returns false on failure.  Upon success, returns the
+  // PropertyIndex that was updated in |index|.  |index| may be NULL
+  // if not needed.
+  virtual bool UpdateStatus(const std::string& key,
+                            const Value& value,
+                            PropertyIndex *index);
+
+  NativeNetworkDeviceParser* device_parser() { return device_parser_.get(); }
+
+ protected:
+  void set_unique_id(const std::string& unique_id) { unique_id_ = unique_id; }
+
  private:
-  bool ParseValue(int index, const base::Value* value);
-  void ParseInfo(const base::DictionaryValue* info);
+  // This allows NetworkDeviceParser and its subclasses access to
+  // device privates so that they can be reconstituted during parsing.
+  // The parsers only access things through the private set_ functions
+  // so that this class can evolve without having to change all the
+  // parsers.
+  NETWORK_LIBRARY_PARSER_FRIENDS;
+  friend class NativeEthernetNetworkParser;
+  friend class NativeWirelessNetworkParser;
+  friend class NativeWifiNetworkParser;
+  friend class NativeCellularNetworkParser;
+  friend class NativeVirtualNetworkParser;

[stevenjb, 2011/08/12 21:32:37]

I am curious why network parsers are modifying NetworkDevice, or accessing
private data? It seems like we should change the interface rather than having so
many friends.

[Greg Spencer (Chromium), 2011/08/12 23:45:19]

OK, turns out they're not.  I've fixed all of these classes to just use the
minimal set of friends required, and in most cases it's just one or two.

[stevenjb, 2011/08/13 00:01:45]

Awesome, this looks much better to me.

+
+  // This allows the implementation classes access to privates in the
+  // same fashion.
+  NETWORK_LIBRARY_IMPL_FRIENDS;
+
+  // Use these functions at your peril.  They are used by the various
+  // parsers to set state, and really shouldn't be used by anyone
+  // else.
+  void set_device_path(const std::string& device_path) {
+    device_path_ = device_path;
+  }
+  void set_name(const std::string& name) { name_ = name; }
+  void set_type(ConnectionType type) { type_ = type; }
+  void set_scanning(bool scanning) { scanning_ = scanning; }
+  void set_meid(const std::string& meid) { meid_ = meid; }
+  void set_imei(const std::string& imei) { imei_ = imei; }
+  void set_imsi(const std::string& imsi) { imsi_ = imsi; }
+  void set_esn(const std::string& esn) { esn_ = esn; }
+  void set_mdn(const std::string& mdn) { mdn_ = mdn; }
+  void set_min(const std::string& min) { min_ = min; }
+  void set_technology_family(TechnologyFamily technology_family) {
+    technology_family_ = technology_family;
+  }
+  void set_carrier(const std::string& carrier) { carrier_ = carrier; }
+  void set_home_provider(const std::string& home_provider) {
+    home_provider_ = home_provider;
+  }
+  void set_home_provider_code(const std::string& home_provider_code) {
+    home_provider_code_ = home_provider_code;
+  }
+  void set_home_provider_country(const std::string& home_provider_country) {
+    home_provider_country_ = home_provider_country;
+  }
+  void set_home_provider_id(const std::string& home_provider_id) {
+    home_provider_id_ = home_provider_id;
+  }
+  void set_home_provider_name(const std::string& home_provider_name) {
+    home_provider_name_ = home_provider_name;
+  }
+  void set_model_id(const std::string& model_id) { model_id_ = model_id; }
+  void set_manufacturer(const std::string& manufacturer) {
+    manufacturer_ = manufacturer;
+  }
+  void set_prl_version(int prl_version) {
+    prl_version_ = prl_version;
+  }
+  void set_sim_lock_state(SimLockState sim_lock_state) {
+    sim_lock_state_ = sim_lock_state;
+  }
+  void set_sim_retries_left(int sim_retries_left) {
+    sim_retries_left_ = sim_retries_left;
+  }
+  void set_sim_pin_required(SimPinRequire sim_pin_required) {
+    sim_pin_required_ = sim_pin_required;
+  }
+  void set_firmware_revision(const std::string& firmware_revision) {
+    firmware_revision_ = firmware_revision;
+  }
+  void set_hardware_revision(const std::string& hardware_revision) {
+    hardware_revision_ = hardware_revision;
+  }
+  void set_selected_cellular_network(
+      const std::string& selected_cellular_network) {
+    selected_cellular_network_ = selected_cellular_network;
+  }
+  void set_found_cellular_networks(
+      const CellularNetworkList& found_cellular_networks) {
+    found_cellular_networks_ = found_cellular_networks;
+  }
+  void set_data_roaming_allowed(bool data_roaming_allowed) {
+    data_roaming_allowed_ = data_roaming_allowed;
+  }
+  void set_support_network_scan(bool support_network_scan) {
+    support_network_scan_ = support_network_scan;
+  }
+  void set_provider_apn_list(const CellularApnList& provider_apn_list) {
+    provider_apn_list_ = provider_apn_list;
+  }
+
+  void ParseInfo(const base::DictionaryValue& info);
 
   // General device info.
   std::string device_path_;
   std::string name_;
+  std::string unique_id_;
   ConnectionType type_;
   bool scanning_;
   // Cellular specific device info.
   TechnologyFamily technology_family_;
   std::string carrier_;
   std::string home_provider_;
   std::string home_provider_code_;
   std::string home_provider_country_;
   std::string home_provider_id_;
   std::string home_provider_name_;
-  std::string MEID_;
-  std::string IMEI_;
-  std::string IMSI_;
-  std::string ESN_;
-  std::string MDN_;
-  std::string MIN_;
+  std::string meid_;
+  std::string imei_;
+  std::string imsi_;
+  std::string esn_;
+  std::string mdn_;
+  std::string min_;
   std::string model_id_;
   std::string manufacturer_;
-  SIMLockState sim_lock_state_;
+  SimLockState sim_lock_state_;
   int sim_retries_left_;
-  SIMPinRequire sim_pin_required_;
+  SimPinRequire sim_pin_required_;
   std::string firmware_revision_;
   std::string hardware_revision_;
-  int PRL_version_;
+  int prl_version_;
   std::string selected_cellular_network_;
   CellularNetworkList found_cellular_networks_;
   bool data_roaming_allowed_;
   bool support_network_scan_;
   CellularApnList provider_apn_list_;
 
-  friend class NetworkLibraryImplBase;
-  friend class NetworkLibraryImplCros;
-  friend class NetworkLibraryImplStub;
+  // This is the parser we use to parse messages from the native
+  // network layer.
+  scoped_ptr<NativeNetworkDeviceParser> device_parser_;
+
   DISALLOW_COPY_AND_ASSIGN(NetworkDevice);
 };
 
 // Contains data common to all network service types.
 class Network {
  public:
   virtual ~Network();
 
   const std::string& service_path() const { return service_path_; }
   const std::string& name() const { return name_; }
@@ skipping 69 matching lines @@
             state == STATE_CARRIER);
   }
   static bool IsDisconnectedState(ConnectionState state) {
     return (state == STATE_UNKNOWN ||
             state == STATE_IDLE ||
             state == STATE_DISCONNECT ||
             state == STATE_FAILURE ||
             state == STATE_ACTIVATION_FAILURE);
   }
 
+  virtual bool UpdateStatus(const std::string& key,
+                            const Value& value,
+                            PropertyIndex* index);
+
  protected:
-  Network(const std::string& service_path, ConnectionType type);
+  Network(const std::string& service_path,
+          ConnectionType type,
+          NativeNetworkParser* parser);
+
+  // Set the state and update flags if necessary.
+  void SetState(ConnectionState state);
 
   // Parse name/value pairs from libcros.
-  virtual bool ParseValue(int index, const base::Value* value);
-  virtual void ParseInfo(const base::DictionaryValue* info);
+  virtual void ParseInfo(const base::DictionaryValue& info);
 
   // Erase cached credentials, used when "Save password" is unchecked.
   virtual void EraseCredentials();
 
   // Calculate a unique identifier for the network.
   virtual void CalculateUniqueId();
 
   // Methods to asynchronously set network service properties
   virtual void SetStringProperty(const char* prop, const std::string& str,
                                  std::string* dest);
   virtual void SetBooleanProperty(const char* prop, bool b, bool* dest);
   virtual void SetIntegerProperty(const char* prop, int i, int* dest);
   virtual void SetValueProperty(const char* prop, base::Value* val);
   virtual void ClearProperty(const char* prop);
 
   // This will clear the property if string is empty. Otherwise, it will set it.
   virtual void SetOrClearStringProperty(const char* prop,
                                         const std::string& str,
                                         std::string* dest);
 
-  std::string device_path_;
-  std::string name_;
-  std::string ip_address_;
-  ConnectionMode mode_;
-  ConnectionState state_;
-  ConnectionError error_;
-  bool connectable_;
-  bool is_active_;
-  int priority_;  // determines order in network list.
-  bool auto_connect_;
-  bool save_credentials_;  // save passphrase and EAP credentials to disk.
-  std::string proxy_config_;
+  NativeNetworkParser* network_parser() { return network_parser_.get(); }
 
-  // Unique identifier, set the first time the network is parsed.
-  std::string unique_id_;
+  void set_unique_id(const std::string& unique_id) { unique_id_ = unique_id; }
 
  private:
+  // This allows NetworkParser and its subclasses access to device
+  // privates so that they can be reconstituted during parsing.  The
+  // parsers only access things through the private set_ functions so
+  // that this class can evolve without having to change all the
+  // parsers.
+  NETWORK_LIBRARY_PARSER_FRIENDS;
+  friend class NativeEthernetNetworkParser;
+  friend class NativeWirelessNetworkParser;
+  friend class NativeWifiNetworkParser;
+  friend class NativeCellularNetworkParser;
+  friend class NativeVirtualNetworkParser;

[stevenjb, 2011/08/12 21:32:37]

In theory, it seems like only NativeNetworkParser() should need to be a friend,
and anything that modifies a shared parameter should be in the base class. I
realize that may not be pragmatic however.

[Greg Spencer (Chromium), 2011/08/12 23:45:19]

I tried that approach, and it leads to a lot of protected functions in the base
class that look like:

static void set_network_name(Network* network, const std::string& name) {
network->set_name(name); }

and the call sites aren't as simple either, instead of:

network->set_name(name);

you get:

NativeNetworkParser::set_network_name(network, name);

It's not horrible, but it also meant that for every attribute added to the
network class, you have to go out to all to any parser that needs to set it and
add a function like this, so I opted for something more centralized: all the
accessors are next to the variables they set in the network class.

This way, you *could* set the private var directly, but using the private setter
gets you most of the benefits of having a more public setter, while restricting
use to classes that need it, and still you have the independence of changing the
representation of the private var without changing all the call sites if needed.

I could certainly be convinced that another method was better, but at this point
I've tried making all the setters public (with no friends needed), making the
subclasses all go through the base class, and just making the subclasses friends
where necessary, and the last one appears to be the best, so that's why I picked
it.

[stevenjb, 2011/08/13 00:01:45]

This is fine I think now that we are down to a more minimal set.

+
+  // This allows the implementation classes access to privates in the
+  // same fashion.
+  NETWORK_LIBRARY_IMPL_FRIENDS;
+
+  // Use these functions at your peril.  They are used by the various
+  // parsers to set state, and really shouldn't be used by anything else
+  // because they don't do the error checking and sending to the
+  // network layer that the other setters do.
+  void set_device_path(const std::string& device_path) {
+    device_path_ = device_path;
+  }
   void set_name(const std::string& name) { name_ = name; }
+  void set_mode(ConnectionMode mode) { mode_ = mode; }
   void set_connecting(bool connecting) {
     state_ = (connecting ? STATE_ASSOCIATION : STATE_IDLE);
   }
   void set_connected(bool connected) {
     state_ = (connected ? STATE_ONLINE : STATE_IDLE);
   }
   void set_connectable(bool connectable) { connectable_ = connectable; }
-  void set_active(bool is_active) { is_active_ = is_active; }
+  void set_is_active(bool is_active) { is_active_ = is_active; }
   void set_error(ConnectionError error) { error_ = error; }
   void set_added(bool added) { added_ = added; }
+  void set_auto_connect(bool auto_connect) { auto_connect_ = auto_connect; }
+  void set_save_credentials(bool save_credentials) {
+    save_credentials_ = save_credentials;
+  }
   void set_profile_path(const std::string& path) { profile_path_ = path; }
   void set_profile_type(NetworkProfileType type) { profile_type_ = type; }
 
-  // Set the state and update flags if necessary.
-  void SetState(ConnectionState state);
+  std::string device_path_;
+  std::string name_;
+  std::string ip_address_;
+  ConnectionMode mode_;
+  ConnectionState state_;
+  ConnectionError error_;
+  bool connectable_;
+  bool is_active_;
+  int priority_;  // determines order in network list.
+  bool auto_connect_;
+  bool save_credentials_;  // save passphrase and EAP credentials to disk.
+  std::string proxy_config_;
+
+  // Unique identifier, set the first time the network is parsed.
+  std::string unique_id_;
 
   // Set the profile path and update the flimfalm property.
   void SetProfilePath(const std::string& profile_path);
 
   // Initialize the IP address field
   void InitIPAddress();
 
   // Priority value, corresponds to index in list from flimflam (0 = first)
   int priority_order_;
 
   // Set to true if the UI requested this as a new network.
   bool added_;
 
   // Set to true when a new connection failure occurs; cleared when observers
   // are notified.
   bool notify_failure_;
 
   // Profile path for networks.
   std::string profile_path_;
 
   // Set to profile type based on profile_path_.
   NetworkProfileType profile_type_;
 
   // These must not be modified after construction.
   std::string service_path_;
   ConnectionType type_;
 
-  friend class NetworkLibraryImplBase;
-  friend class NetworkLibraryImplCros;
-  friend class NetworkLibraryImplStub;
+  // This is the parser we use to parse messages from the native
+  // network layer.
+  scoped_ptr<NativeNetworkParser> network_parser_;
+
   DISALLOW_COPY_AND_ASSIGN(Network);
 };
 
 // Class for networks of TYPE_ETHERNET.
 class EthernetNetwork : public Network {
  public:
-  explicit EthernetNetwork(const std::string& service_path) :
-      Network(service_path, TYPE_ETHERNET) {
-  }
+  explicit EthernetNetwork(const std::string& service_path);
  private:
-  friend class NetworkLibraryImplBase;
-  friend class NetworkLibraryImplCros;
-  friend class NetworkLibraryImplStub;
+  // This allows NetworkParser and its subclasses access to device
+  // privates so that they can be reconstituted during parsing.  The
+  // parsers only access things through the private set_ functions so
+  // that this class can evolve without having to change all the
+  // parsers.
+  NETWORK_LIBRARY_PARSER_FRIENDS;
+  friend class NativeEthernetNetworkParser;
+
+  // This allows the implementation classes access to privates in the
+  // same fashion.
+  NETWORK_LIBRARY_IMPL_FRIENDS;
+
   DISALLOW_COPY_AND_ASSIGN(EthernetNetwork);
 };
 
 // Class for networks of TYPE_VPN.
 class VirtualNetwork : public Network {
  public:
-  enum ProviderType {
-    PROVIDER_TYPE_L2TP_IPSEC_PSK,
-    PROVIDER_TYPE_L2TP_IPSEC_USER_CERT,
-    PROVIDER_TYPE_OPEN_VPN,
-    // Add new provider types before PROVIDER_TYPE_MAX.
-    PROVIDER_TYPE_MAX,
-  };
-
   explicit VirtualNetwork(const std::string& service_path);
   virtual ~VirtualNetwork();
 
   const std::string& server_hostname() const { return server_hostname_; }
   ProviderType provider_type() const { return provider_type_; }
   const std::string& ca_cert_nss() const { return ca_cert_nss_; }
   const std::string& psk_passphrase() const { return psk_passphrase_; }
   const std::string& client_cert_id() const { return client_cert_id_; }
   const std::string& username() const { return username_; }
   const std::string& user_passphrase() const { return user_passphrase_; }
@@ skipping 11 matching lines @@
   std::string GetProviderTypeString() const;
 
   // Public setters.
   void SetCACertNSS(const std::string& ca_cert_nss);
   void SetPSKPassphrase(const std::string& psk_passphrase);
   void SetClientCertID(const std::string& cert_id);
   void SetUsername(const std::string& username);
   void SetUserPassphrase(const std::string& user_passphrase);
 
  private:
-  // Network overrides.
-  virtual bool ParseValue(int index, const base::Value* value);
-  virtual void ParseInfo(const base::DictionaryValue* info);
-  virtual void EraseCredentials();
-  virtual void CalculateUniqueId();
+  // This allows NetworkParser and its subclasses access to device
+  // privates so that they can be reconstituted during parsing.  The
+  // parsers only access things through the private set_ functions so
+  // that this class can evolve without having to change all the
+  // parsers.
+  NETWORK_LIBRARY_PARSER_FRIENDS;
+  friend class NativeVirtualNetworkParser;
 
-  // VirtualNetwork private methods.
-  bool ParseProviderValue(int index, const base::Value* value);
+  // This allows the implementation classes access to privates in the
+  // same fashion.
+  NETWORK_LIBRARY_IMPL_FRIENDS;
 
+  // Use these functions at your peril.  They are used by the various
+  // parsers to set state, and really shouldn't be used by anything else
+  // because they don't do the error checking and sending to the
+  // network layer that the other setters do.
   void set_server_hostname(const std::string& server_hostname) {
     server_hostname_ = server_hostname;
   }
   void set_provider_type(ProviderType provider_type) {
     provider_type_ = provider_type;
   }
   void set_ca_cert_nss(const std::string& ca_cert_nss) {
     ca_cert_nss_ = ca_cert_nss;
   }
   void set_psk_passphrase(const std::string& psk_passphrase) {
     psk_passphrase_ = psk_passphrase;
   }
   void set_client_cert_id(const std::string& client_cert_id) {
     client_cert_id_ = client_cert_id;
   }
-  void set_username(const std::string& username) {
-    username_ = username;
-  }
+  void set_username(const std::string& username) { username_ = username; }
   void set_user_passphrase(const std::string& user_passphrase) {
     user_passphrase_ = user_passphrase;
   }
 
+  // Network overrides.
+  virtual void EraseCredentials() OVERRIDE;
+  virtual void CalculateUniqueId() OVERRIDE;
+
+  // VirtualNetwork private methods.
+  bool ParseProviderValue(int index, const base::Value* value);
+
   std::string server_hostname_;
   ProviderType provider_type_;
   // NSS nickname for server CA certificate.
   std::string ca_cert_nss_;
   std::string psk_passphrase_;
   // PKCS#11 ID for client certificate.
   std::string client_cert_id_;
   std::string username_;
   std::string user_passphrase_;
-
-  friend class NetworkLibraryImplBase;
-  friend class NetworkLibraryImplCros;
-  friend class NetworkLibraryImplStub;
   DISALLOW_COPY_AND_ASSIGN(VirtualNetwork);
 };
 typedef std::vector<VirtualNetwork*> VirtualNetworkVector;
 
 // Base class for networks of TYPE_WIFI or TYPE_CELLULAR.
 class WirelessNetwork : public Network {
  public:
   int strength() const { return strength_; }
 
  protected:
-  WirelessNetwork(const std::string& service_path, ConnectionType type)
-      : Network(service_path, type),
-        strength_(0) {}
+  WirelessNetwork(const std::string& service_path,
+                  ConnectionType type,
+                  NativeNetworkParser* parser)
+      : Network(service_path, type, parser), strength_(0) {}
+ private:
+  // This allows NetworkParser and its subclasses access to device
+  // privates so that they can be reconstituted during parsing.  The
+  // parsers only access things through the private set_ functions so
+  // that this class can evolve without having to change all the
+  // parsers.
+  NETWORK_LIBRARY_PARSER_FRIENDS;
+  friend class NativeWirelessNetworkParser;
+  friend class NativeWifiNetworkParser;
+  friend class NativeCellularNetworkParser;
+
+  // This allows the implementation classes access to privates in the
+  // same fashion.
+  NETWORK_LIBRARY_IMPL_FRIENDS;
+
+  // The friend parsers use this.
+  void set_strength(int strength) { strength_ = strength; }
+
   int strength_;  // 0-100
 
-  // Network overrides.
-  virtual bool ParseValue(int index, const base::Value* value);
-
- private:
-  void set_strength(int strength) { strength_ = strength; }
-
-  friend class NetworkLibraryImplBase;
-  friend class NetworkLibraryImplCros;
-  friend class NetworkLibraryImplStub;
   DISALLOW_COPY_AND_ASSIGN(WirelessNetwork);
 };
 
 // Class for networks of TYPE_CELLULAR.
 class CellularDataPlan;
 
 class CellularNetwork : public WirelessNetwork {
  public:
   enum DataLeft {
     DATA_UNKNOWN,
@@ skipping 27 matching lines @@
         && connected() && activated();
   }
   const std::string& operator_name() const { return operator_name_; }
   const std::string& operator_code() const { return operator_code_; }
   const std::string& operator_country() const { return operator_country_; }
   const std::string& payment_url() const { return payment_url_; }
   const std::string& usage_url() const { return usage_url_; }
   DataLeft data_left() const { return data_left_; }
   const CellularApn& apn() const { return apn_; }
   const CellularApn& last_good_apn() const { return last_good_apn_; }
+
   // Sets the APN to use in establishing data connections. Only
   // the fields of the APN that are needed for making connections
   // are passed to flimflam. The name, localized_name, and language
   // fields are ignored.
   void SetApn(const CellularApn& apn);
 
   // Returns true if network supports activation.
   // Current implementation returns same as SupportsDataPlan().
   bool SupportsActivation() const;
 
   // Returns true if one of the usage_url_ / payment_url_ (or both) is defined.
   bool SupportsDataPlan() const;
 
   // Return a string representation of network technology.
   std::string GetNetworkTechnologyString() const;
   // Return a string representation of activation state.
   std::string GetActivationStateString() const;
   // Return a string representation of roaming state.
   std::string GetRoamingStateString() const;
 
   // Return a string representation of |activation_state|.
   static std::string ActivationStateToString(ActivationState activation_state);
 
- protected:
-  // WirelessNetwork overrides.
-  virtual bool ParseValue(int index, const base::Value* value);
+ private:
+  // This allows NetworkParser and its subclasses access to device
+  // privates so that they can be reconstituted during parsing.  The
+  // parsers only access things through the private set_ functions so
+  // that this class can evolve without having to change all the
+  // parsers.
+  NETWORK_LIBRARY_PARSER_FRIENDS;
+  friend class NativeWirelessNetworkParser;
+  friend class NativeCellularNetworkParser;
+
+  // This allows the implementation classes access to privates in the
+  // same fashion.
+  NETWORK_LIBRARY_IMPL_FRIENDS;
+
+  // Use these functions at your peril.  They are used by the various
+  // parsers to set state, and really shouldn't be used by anything else
+  // because they don't do the error checking and sending to the
+  // network layer that the other setters do.
+  void set_activation_state(ActivationState activation_state) {
+    activation_state_ = activation_state;
+  }
+  void set_network_technology(NetworkTechnology network_technology) {
+    network_technology_ = network_technology;
+  }
+  void set_roaming_state(NetworkRoamingState roaming_state) {
+    roaming_state_ = roaming_state;
+  }
+  void set_operator_name(const std::string& operator_name) {
+    operator_name_ = operator_name;
+  }
+  void set_operator_code(const std::string& operator_code) {
+    operator_code_ = operator_code;
+  }
+  void set_operator_country(const std::string& operator_country) {
+    operator_country_ = operator_country;
+  }
+  void set_payment_url(const std::string& payment_url) {
+    payment_url_ = payment_url;
+  }
+  void set_usage_url(const std::string& usage_url) { usage_url_ = usage_url; }
+  void set_data_left(DataLeft data_left) { data_left_ = data_left; }
+  void set_apn(const DictionaryValue& apn) { apn_.Set(apn); }
+  void set_last_good_apn(const DictionaryValue& last_good_apn) {
+    last_good_apn_.Set(last_good_apn);
+  }
 
   ActivationState activation_state_;
   NetworkTechnology network_technology_;
   NetworkRoamingState roaming_state_;
   // Carrier Info
   std::string operator_name_;
   std::string operator_code_;
   std::string operator_country_;
   std::string payment_url_;
   std::string usage_url_;
   // Cached values
   DataLeft data_left_;  // Updated when data plans are updated.
   CellularApn apn_;
   CellularApn last_good_apn_;
 
- private:
-  void set_activation_state(ActivationState state) {
-    activation_state_ = state;
-  }
-  void set_payment_url(const std::string& url) { payment_url_ = url; }
-  void set_usage_url(const std::string& url) { usage_url_ = url; }
-  void set_network_technology(NetworkTechnology technology) {
-    network_technology_ = technology;
-  }
-  void set_roaming_state(NetworkRoamingState state) { roaming_state_ = state; }
-  void set_data_left(DataLeft data_left) { data_left_ = data_left; }
-  void set_apn(const CellularApn& apn) { apn_ = apn; }
-  void set_last_good_apn(const CellularApn& apn) { last_good_apn_ = apn; }
-
-  friend class NetworkLibraryImplBase;
-  friend class NetworkLibraryImplCros;
-  friend class NetworkLibraryImplStub;
   DISALLOW_COPY_AND_ASSIGN(CellularNetwork);
 };
 typedef std::vector<CellularNetwork*> CellularNetworkVector;
 
 // Class for networks of TYPE_WIFI.
 class WifiNetwork : public WirelessNetwork {
  public:
   explicit WifiNetwork(const std::string& service_path);
   virtual ~WifiNetwork();
 
@@ skipping 39 matching lines @@
   virtual bool RequiresUserProfile() const;
 
   // Return a string representation of the encryption code.
   // This not translated and should be only used for debugging purposes.
   std::string GetEncryptionString() const;
 
   // Return true if a passphrase or other input is required to connect.
   bool IsPassphraseRequired() const;
 
  private:
-  // Network overrides.
-  virtual void EraseCredentials();
-  virtual void CalculateUniqueId();
+  // This allows NetworkParser and its subclasses access to device
+  // privates so that they can be reconstituted during parsing.  The
+  // parsers only access things through the private set_ functions so
+  // that this class can evolve without having to change all the
+  // parsers.
+  NETWORK_LIBRARY_PARSER_FRIENDS;
+  friend class NativeWirelessNetworkParser;
+  friend class NativeWifiNetworkParser;
 
-  // WirelessNetwork overrides.
-  virtual bool ParseValue(int index, const base::Value* value);
+  // This allows the implementation classes access to privates in the
+  // same fashion.
+  NETWORK_LIBRARY_IMPL_FRIENDS;
 
+  // Use these functions at your peril.  They are used by the various
+  // parsers to set state, and really shouldn't be used by anything else
+  // because they don't do the error checking and sending to the
+  // network layer that the other setters do.
   void set_encryption(ConnectionSecurity encryption) {
     encryption_ = encryption;
   }
   void set_passphrase(const std::string& passphrase) {
     passphrase_ = passphrase;
+    user_passphrase_ = passphrase;
   }
   void set_passphrase_required(bool passphrase_required) {
     passphrase_required_ = passphrase_required;
   }
   void set_identity(const std::string& identity) {
     identity_ = identity;
   }
+  void set_eap_method(EAPMethod eap_method) { eap_method_ = eap_method; }
+  void set_eap_phase_2_auth(EAPPhase2Auth eap_phase_2_auth) {
+    eap_phase_2_auth_ = eap_phase_2_auth;
+  }
+  void set_eap_server_ca_cert_nss_nickname(
+      const std::string& eap_server_ca_cert_nss_nickname) {
+    eap_server_ca_cert_nss_nickname_ = eap_server_ca_cert_nss_nickname;
+  }
+  void set_eap_client_cert_pkcs11_id(
+      const std::string& eap_client_cert_pkcs11_id) {
+    eap_client_cert_pkcs11_id_ = eap_client_cert_pkcs11_id;
+  }
+  void set_eap_use_system_cas(bool eap_use_system_cas) {
+    eap_use_system_cas_ = eap_use_system_cas;
+  }
+  void set_eap_identity(const std::string& eap_identity) {
+    eap_identity_ = eap_identity;
+  }
+  void set_eap_anonymous_identity(const std::string& eap_anonymous_identity) {
+    eap_anonymous_identity_ = eap_anonymous_identity;
+  }
+  void set_eap_passphrase(const std::string& eap_passphrase) {
+    eap_passphrase_ = eap_passphrase;
+  }
+
+  // Network overrides.
+  virtual void EraseCredentials() OVERRIDE;
+  virtual void CalculateUniqueId() OVERRIDE;
+
   ConnectionSecurity encryption_;
   std::string passphrase_;
   bool passphrase_required_;
   std::string identity_;
 
   EAPMethod eap_method_;
   EAPPhase2Auth eap_phase_2_auth_;
   std::string eap_server_ca_cert_nss_nickname_;
   std::string eap_client_cert_pkcs11_id_;
   bool eap_use_system_cas_;
   std::string eap_identity_;
   std::string eap_anonymous_identity_;
   std::string eap_passphrase_;
 
   // Internal state (not stored in flimflam).
   // Passphrase set by user (stored for UI).
   std::string user_passphrase_;
 
-  friend class NetworkLibraryImplBase;
-  friend class NetworkLibraryImplCros;
-  friend class NetworkLibraryImplStub;
   DISALLOW_COPY_AND_ASSIGN(WifiNetwork);
 };
 typedef std::vector<WifiNetwork*> WifiNetworkVector;
 
 // Cellular Data Plan management.
 class CellularDataPlan {
  public:
   CellularDataPlan();
   explicit CellularDataPlan(const CellularDataPlanInfo &plan);
   ~CellularDataPlan();
@@ skipping 464 matching lines @@
   virtual void SwitchToPreferredNetwork() = 0;
 
   // Factory function, creates a new instance and returns ownership.
   // For normal usage, access the singleton via CrosLibrary::Get().
   static NetworkLibrary* GetImpl(bool stub);
 };
 
 }  // namespace chromeos
 
 #endif  // CHROME_BROWSER_CHROMEOS_CROS_NETWORK_LIBRARY_H_