Convert all connect code to use NetworkHandler instead of NetworkLibrary

chrome/browser/chromeos/options/vpn_config_view.h

Index: chrome/browser/chromeos/options/vpn_config_view.h
diff --git a/chrome/browser/chromeos/options/vpn_config_view.h b/chrome/browser/chromeos/options/vpn_config_view.h
index cb5f7cec1a82a5c9eced12ca6ae6a8415362d8bf..cc3ec322c726c9dab92459cea5de80e0df664e9d 100644
--- a/chrome/browser/chromeos/options/vpn_config_view.h
+++ b/chrome/browser/chromeos/options/vpn_config_view.h
@@ -18,13 +18,20 @@
 #include "ui/views/controls/textfield/textfield_controller.h"
 #include "ui/views/view.h"
 
+namespace base {
+class DictionaryValue;
+}
+
 namespace views {
 class Checkbox;
+class GridLayout;
 class Label;
 }
 
 namespace chromeos {
 
+class NetworkState;
+
 namespace internal {
 class ProviderTypeComboboxModel;
 class VpnServerCACertComboboxModel;
@@ -36,10 +43,10 @@ class VPNConfigView : public ChildNetworkConfigView,
                       public views::TextfieldController,
                       public views::ButtonListener,
                       public views::ComboboxListener,
-                      public CertLibrary::Observer {
+                      public CertLibrary::Observer,
+                      public base::SupportsWeakPtr<VPNConfigView> {

[pneubeck (no reviews), 2013/08/06 15:45:19]

please use the WeakPtrFactory, since it's not used from the outside.

similarly for the other *ConfigViews.

[stevenjb, 2013/08/06 20:23:55]

Meh. There are no member dependencies and it's more typing, but fine.
Done.

  public:
-  VPNConfigView(NetworkConfigView* parent, VirtualNetwork* vpn);
-  explicit VPNConfigView(NetworkConfigView* parent);
+  VPNConfigView(NetworkConfigView* parent, const std::string& service_path);
   virtual ~VPNConfigView();
 
   // views::TextfieldController:
@@ -68,7 +75,18 @@ class VPNConfigView : public ChildNetworkConfigView,
 
  private:
   // Initializes data members and create UI controls.

[pneubeck (no reviews), 2013/08/06 15:45:19]

separate the comment.

[stevenjb, 2013/08/06 20:23:55]

Combined the functions, the separation ended up being kind of arbitrary.

-  void Init(VirtualNetwork* vpn);
+  void Init();
+  void InitControls();

[pneubeck (no reviews), 2013/08/06 15:45:19]

comment that this sets up all controls but without any values.

[stevenjb, 2013/08/06 20:23:55]

Ditto

+
+  // Callback to initialize fields from uncached network properties.
+  void InitFromProperties(const std::string& service_path,
+                          const base::DictionaryValue& dictionary);
+  void ParseUIProperties(const NetworkState* vpn, int provider_type_index);
+  void GetPropertiesError(const std::string& error_name,
+                          scoped_ptr<base::DictionaryValue> error_data);
+
+  // Helper function to set credential properties.
+  void SetCredentials(base::DictionaryValue* properties);
 
   // Set and update all control values.
   void Refresh();
@@ -81,9 +99,6 @@ class VPNConfigView : public ChildNetworkConfigView,
   // Update state of the Login button.
   void UpdateCanLogin();
 
-  // Returns true if the provider type requires a user certificate.
-  bool UserCertRequired() const;
-
   // Returns true if there is at least one user certificate installed.
   bool HaveUserCerts() const;
 
@@ -109,21 +124,22 @@ class VPNConfigView : public ChildNetworkConfigView,
   const std::string GetServerCACertPEM() const;
   const std::string GetUserCertID() const;
   bool GetSaveCredentials() const;
+  int GetProviderTypeIndex() const;
+  std::string GetProviderTypeString() const;
 
   // Parses a VPN UI |property| from the given |network|. |key| is the property
   // name within the type-specific VPN subdictionary named |dict_key|.
-  void ParseVPNUIProperty(NetworkPropertyUIData* property_ui_data,
-                          Network* network,
+  void ParseVPNUIProperty(const NetworkState* network,
                           const std::string& dict_key,
-                          const std::string& key);
+                          const std::string& key,
+                          NetworkPropertyUIData* property_ui_data);
 
-  std::string server_hostname_;
   string16 service_name_from_server_;
   bool service_text_modified_;
 
   // Initialized in Init():
 
-  ProviderType provider_type_;
+  std::string provider_type_;

[pneubeck (no reviews), 2013/08/06 15:45:19]

comment that it's only set if modifying an existing network.
(maybe move to "Cached VPN properties" too).

also, looking at the implementation, it would be much easier to understand, if
you would immediately translate the provider type into ProviderTypeComboboxIndex
in InitFromProperties and store that instead of the original Shill string.

[stevenjb, 2013/08/06 20:23:55]

Done.

 
   bool enable_psk_passphrase_;
   bool enable_user_cert_;
@@ -141,6 +157,7 @@ class VPNConfigView : public ChildNetworkConfigView,
 
   int title_;
 
+  views::GridLayout* layout_;
   views::Textfield* server_textfield_;
   views::Label* service_text_;
   views::Textfield* service_textfield_;
@@ -165,6 +182,10 @@ class VPNConfigView : public ChildNetworkConfigView,
   views::Checkbox* save_credentials_checkbox_;
   views::Label* error_label_;
 
+  // Cached VPN properties

[pneubeck (no reviews), 2013/08/06 15:45:19]

mention, that it's only set if modifying an existing network.

[stevenjb, 2013/08/06 20:23:55]

Done.

+  std::string ca_cert_pem_;
+  std::string client_cert_id_;
+
   DISALLOW_COPY_AND_ASSIGN(VPNConfigView);
 };