Convert all connect code to use NetworkHandler instead of NetworkLibrary

chrome/browser/chromeos/options/vpn_config_view.cc

 // Copyright (c) 2012 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.
 
 #include "chrome/browser/chromeos/options/vpn_config_view.h"
 
+#include "ash/system/chromeos/network/network_connect.h"
 #include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
-#include "chrome/browser/chromeos/cros/network_library.h"
 #include "chrome/browser/chromeos/enrollment_dialog_view.h"
+#include "chrome/browser/chromeos/options/network_connect.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/common/net/x509_certificate_model.h"
+#include "chromeos/network/network_configuration_handler.h"
+#include "chromeos/network/network_event_log.h"
+#include "chromeos/network/network_state.h"
+#include "chromeos/network/network_state_handler.h"
 #include "chromeos/network/network_ui_data.h"
 #include "chromeos/network/onc/onc_constants.h"
 #include "grit/chromium_strings.h"
 #include "grit/generated_resources.h"
 #include "grit/locale_settings.h"
 #include "grit/theme_resources.h"
+#include "third_party/cros_system_api/dbus/service_constants.h"
 #include "ui/base/events/event.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/base/models/combobox_model.h"
 #include "ui/base/resource/resource_bundle.h"
 #include "ui/views/controls/button/checkbox.h"
 #include "ui/views/controls/combobox/combobox.h"
 #include "ui/views/controls/label.h"
 #include "ui/views/controls/textfield/textfield.h"
 #include "ui/views/layout/grid_layout.h"
 #include "ui/views/layout/layout_constants.h"
 #include "ui/views/widget/widget.h"
 #include "ui/views/window/dialog_client_view.h"
 
 namespace {
 
 // Root CA certificates that are built into Chrome use this token name.
 const char* const kRootCertificateTokenName = "Builtin Object Token";
 
-string16 ProviderTypeToString(chromeos::ProviderType type) {
-  switch (type) {
-    case chromeos::PROVIDER_TYPE_L2TP_IPSEC_PSK:
+enum ProviderTypeComboboxIndex {

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

You are using this even if the combobox doesn't exist.
In that case it's not really an index but only a type. That's confusing.

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

Removed 'Combobox'

+  PROVIDER_TYPE_INDEX_L2TP_IPSEC_PSK = 0,
+  PROVIDER_TYPE_INDEX_L2TP_IPSEC_USER_CERT = 1,
+  PROVIDER_TYPE_INDEX_OPEN_VPN = 2,
+  PROVIDER_TYPE_INDEX_MAX = 3,
+};
+
+string16 ProviderTypeIndexToString(int index) {
+  switch (index) {
+    case PROVIDER_TYPE_INDEX_L2TP_IPSEC_PSK:
       return l10n_util::GetStringUTF16(
           IDS_OPTIONS_SETTINGS_INTERNET_OPTIONS_L2TP_IPSEC_PSK);
-    case chromeos::PROVIDER_TYPE_L2TP_IPSEC_USER_CERT:
+    case PROVIDER_TYPE_INDEX_L2TP_IPSEC_USER_CERT:
       return l10n_util::GetStringUTF16(
           IDS_OPTIONS_SETTINGS_INTERNET_OPTIONS_L2TP_IPSEC_USER_CERT);
-    case chromeos::PROVIDER_TYPE_OPEN_VPN:
+    case PROVIDER_TYPE_INDEX_OPEN_VPN:
       return l10n_util::GetStringUTF16(
           IDS_OPTIONS_SETTINGS_INTERNET_OPTIONS_OPEN_VPN);
-    case chromeos::PROVIDER_TYPE_MAX:
-      break;
   }
   NOTREACHED();
   return string16();
 }
 
+ProviderTypeComboboxIndex ProviderTypeToIndex(
+    const std::string& provider_type,
+    const std::string& client_cert_id) {
+  if (provider_type == flimflam::kProviderL2tpIpsec) {
+    if (!client_cert_id.empty())
+      return PROVIDER_TYPE_INDEX_L2TP_IPSEC_USER_CERT;
+    else
+      return PROVIDER_TYPE_INDEX_L2TP_IPSEC_PSK;
+  } else {
+    DCHECK(provider_type == flimflam::kProviderOpenVpn);
+    return PROVIDER_TYPE_INDEX_OPEN_VPN;
+  }
+}
+
 // Translates the provider type to the name of the respective ONC dictionary
 // containing configuration data for the type.
-std::string ProviderTypeToONCDictKey(chromeos::ProviderType type) {
-  switch (type) {
-    case chromeos::PROVIDER_TYPE_L2TP_IPSEC_PSK:
-    case chromeos::PROVIDER_TYPE_L2TP_IPSEC_USER_CERT:
-      return chromeos::onc::vpn::kIPsec;
-    case chromeos::PROVIDER_TYPE_OPEN_VPN:
-      return chromeos::onc::vpn::kOpenVPN;
-    case chromeos::PROVIDER_TYPE_MAX:
-      break;
-  }
+std::string ProviderTypeToONCDictKey(const std::string& provider_type) {
+  if (provider_type == flimflam::kProviderL2tpIpsec)
+    return chromeos::onc::vpn::kIPsec;
+  if (provider_type == flimflam::kProviderOpenVpn)
+    return chromeos::onc::vpn::kOpenVPN;
+  NOTREACHED() << "Unhandled provider type " << provider_type;
+  return std::string();
+}
 
-  NOTREACHED() << "Unhandled provider type " << type;
-  return std::string();
+void ShillError(const std::string& function,
+                const std::string& error_name,
+                scoped_ptr<base::DictionaryValue> error_data) {
+  NET_LOG_ERROR("Shill Error from VpnConfigView: " + error_name, function);
 }
 
 }  // namespace
 
 namespace chromeos {
 
 namespace internal {
 
 class ProviderTypeComboboxModel : public ui::ComboboxModel {
  public:
@@ skipping 36 matching lines @@
 
 // ProviderTypeComboboxModel ---------------------------------------------------
 
 ProviderTypeComboboxModel::ProviderTypeComboboxModel() {
 }
 
 ProviderTypeComboboxModel::~ProviderTypeComboboxModel() {
 }
 
 int ProviderTypeComboboxModel::GetItemCount() const {
-  return PROVIDER_TYPE_MAX;
+  return PROVIDER_TYPE_INDEX_MAX;
 }
 
 string16 ProviderTypeComboboxModel::GetItemAt(int index) {
-  ProviderType type = static_cast<ProviderType>(index);
-  return ProviderTypeToString(type);
+  return ProviderTypeIndexToString(index);
 }
 
 // VpnServerCACertComboboxModel ------------------------------------------------
 
 VpnServerCACertComboboxModel::VpnServerCACertComboboxModel() {
 }
 
 VpnServerCACertComboboxModel::~VpnServerCACertComboboxModel() {
 }
 
@@ skipping 43 matching lines @@
   if (CertLibrary::Get()->NumCertificates(CertLibrary::CERT_TYPE_USER) == 0) {
     return l10n_util::GetStringUTF16(
         IDS_OPTIONS_SETTINGS_INTERNET_OPTIONS_USER_CERT_NONE_INSTALLED);
   }
   return CertLibrary::Get()->GetCertDisplayStringAt(
       CertLibrary::CERT_TYPE_USER, index);
 }
 
 }  // namespace internal
 
-VPNConfigView::VPNConfigView(NetworkConfigView* parent, VirtualNetwork* vpn)
-    : ChildNetworkConfigView(parent, vpn),
-      title_(0) {
-  Init(vpn);
-}
-
-VPNConfigView::VPNConfigView(NetworkConfigView* parent)
-    : ChildNetworkConfigView(parent),
-      title_(0) {
-  Init(NULL);
+VPNConfigView::VPNConfigView(NetworkConfigView* parent,
+                             const std::string& service_path)
+    : ChildNetworkConfigView(parent, service_path),
+      service_text_modified_(false),
+      enable_psk_passphrase_(false),
+      enable_user_cert_(false),
+      enable_server_ca_cert_(false),
+      enable_otp_(false),
+      enable_group_name_(false),
+      title_(0),
+      layout_(NULL),
+      server_textfield_(NULL),
+      service_text_(NULL),
+      service_textfield_(NULL),
+      provider_type_combobox_(NULL),
+      provider_type_text_label_(NULL),
+      psk_passphrase_label_(NULL),
+      psk_passphrase_textfield_(NULL),
+      user_cert_label_(NULL),
+      user_cert_combobox_(NULL),
+      server_ca_cert_label_(NULL),
+      server_ca_cert_combobox_(NULL),
+      username_textfield_(NULL),
+      user_passphrase_textfield_(NULL),
+      otp_label_(NULL),
+      otp_textfield_(NULL),
+      group_name_label_(NULL),
+      group_name_textfield_(NULL),
+      save_credentials_checkbox_(NULL),
+      error_label_(NULL) {
+  Init();
 }
 
 VPNConfigView::~VPNConfigView() {
   CertLibrary::Get()->RemoveObserver(this);
 }
 
 string16 VPNConfigView::GetTitle() const {
   DCHECK_NE(title_, 0);
   return l10n_util::GetStringUTF16(title_);
 }
@@ skipping 10 matching lines @@
     return psk_passphrase_textfield_;
   else if (user_cert_combobox_ && user_cert_combobox_->enabled())
     return user_cert_combobox_;
   else if (server_ca_cert_combobox_ && server_ca_cert_combobox_->enabled())
     return server_ca_cert_combobox_;
   else
     return NULL;
 }
 
 bool VPNConfigView::CanLogin() {
+  int provider_type_index = GetProviderTypeIndex();

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

move it to it's usage.

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

Done.

+
   // Username is always required.
   if (GetUsername().empty())
     return false;
 
   // TODO(stevenjb): min kMinPassphraseLen length?
   if (service_path_.empty() &&
       (GetService().empty() || GetServer().empty()))
     return false;
 
   // Block login if certs are required but user has none.
-  if (UserCertRequired() && (!HaveUserCerts() || !IsUserCertValid()))
+  bool cert_required =
+      provider_type_index == PROVIDER_TYPE_INDEX_L2TP_IPSEC_USER_CERT;
+  if (cert_required && (!HaveUserCerts() || !IsUserCertValid()))
     return false;
 
   return true;
 }
 
 void VPNConfigView::ContentsChanged(views::Textfield* sender,
                                     const string16& new_contents) {
   if (sender == server_textfield_ && !service_text_modified_) {
     // Set the service name to the server name up to '.', unless it has
     // been explicitly set by the user.
@@ skipping 19 matching lines @@
     parent_->GetDialogClientView()->AcceptWindow();
   }
   return false;
 }
 
 void VPNConfigView::ButtonPressed(views::Button* sender,
                                   const ui::Event& event) {
 }
 
 void VPNConfigView::OnSelectedIndexChanged(views::Combobox* combobox) {
-  if (combobox == provider_type_combobox_) {
-    provider_type_ = static_cast<ProviderType>(combobox->selected_index());
-    UpdateControls();
-  } else if (combobox == user_cert_combobox_ ||
-             combobox == server_ca_cert_combobox_) {
-    // Do nothing.
-  } else {
-    NOTREACHED();
-  }
+  UpdateControls();
   UpdateErrorLabel();
   UpdateCanLogin();
 }
 
 void VPNConfigView::OnCertificatesLoaded(bool initial_load) {
   Refresh();
 }
 
 bool VPNConfigView::Login() {
-  NetworkLibrary* cros = NetworkLibrary::Get();
   if (service_path_.empty()) {
-    NetworkLibrary::VPNConfigData config_data;
-    switch (provider_type_) {
-      case PROVIDER_TYPE_L2TP_IPSEC_PSK:
-        config_data.psk = GetPSKPassphrase();
-        config_data.username = GetUsername();
-        config_data.user_passphrase = GetUserPassphrase();
-        config_data.group_name = GetGroupName();
-        break;
-      case PROVIDER_TYPE_L2TP_IPSEC_USER_CERT: {
-        config_data.server_ca_cert_pem = GetServerCACertPEM();
-        config_data.client_cert_pkcs11_id = GetUserCertID();
-        config_data.username = GetUsername();
-        config_data.user_passphrase = GetUserPassphrase();
-        config_data.group_name = GetGroupName();
-        break;
-      }
-      case PROVIDER_TYPE_OPEN_VPN:
-        config_data.server_ca_cert_pem = GetServerCACertPEM();
-        config_data.client_cert_pkcs11_id = GetUserCertID();
-        config_data.username = GetUsername();
-        config_data.user_passphrase = GetUserPassphrase();
-        config_data.otp = GetOTP();
-        break;
-      case PROVIDER_TYPE_MAX:
-        break;
+    base::DictionaryValue properties;
+    // Identifying properties
+    properties.SetStringWithoutPathExpansion(
+        flimflam::kTypeProperty, flimflam::kTypeVPN);
+    properties.SetStringWithoutPathExpansion(
+        flimflam::kNameProperty, GetService());
+    properties.SetStringWithoutPathExpansion(
+        flimflam::kProviderHostProperty, GetServer());
+    properties.SetStringWithoutPathExpansion(
+        flimflam::kProviderTypeProperty, GetProviderTypeString());
+
+    SetCredentials(&properties);
+    ash::network_connect::CreateConfigurationAndConnect(
+        &properties, false /* shared */);
+  } else {
+    const NetworkState* vpn = NetworkHandler::Get()->network_state_handler()->
+        GetNetworkState(service_path_);
+    if (!vpn) {
+      // Shill no longer knows about this network (edge case).
+      // TODO(stevenjb): Add notification for this.
+      NET_LOG_ERROR("Network not found", service_path_);
+      return true;  // Close dialog
     }
-    config_data.save_credentials = GetSaveCredentials();
-    cros->ConnectToUnconfiguredVirtualNetwork(
-        GetService(), GetServer(), provider_type_, config_data);
-  } else {
-    VirtualNetwork* vpn = cros->FindVirtualNetworkByPath(service_path_);
-    if (!vpn) {
-      // TODO(stevenjb): Add notification for this.
-      LOG(WARNING) << "VPN no longer exists: " << service_path_;
-      return true;  // Close dialog.
-    }
-    switch (provider_type_) {
-      case PROVIDER_TYPE_L2TP_IPSEC_PSK:
-        vpn->SetL2TPIPsecPSKCredentials(GetPSKPassphrase(),
-                                        GetUsername(),
-                                        GetUserPassphrase(),
-                                        GetGroupName());
-        break;
-      case PROVIDER_TYPE_L2TP_IPSEC_USER_CERT: {
-        vpn->SetL2TPIPsecCertCredentials(GetUserCertID(),
-                                         GetUsername(),
-                                         GetUserPassphrase(),
-                                         GetGroupName());
-        break;
-      }
-      case PROVIDER_TYPE_OPEN_VPN: {
-        vpn->SetOpenVPNCredentials(GetUserCertID(),
-                                   GetUsername(),
-                                   GetUserPassphrase(),
-                                   GetOTP());
-        break;
-      }
-      case PROVIDER_TYPE_MAX:
-        break;
-    }
-    vpn->SetEnrollmentDelegate(
-        CreateEnrollmentDelegate(GetWidget()->GetNativeWindow(),
-                                 vpn->name(),
-                                 ProfileManager::GetLastUsedProfile()));
-    vpn->SetSaveCredentials(GetSaveCredentials());
-    cros->ConnectToVirtualNetwork(vpn);
+    base::DictionaryValue properties;
+    SetCredentials(&properties);
+    ash::network_connect::ConfigureNetworkAndConnect(service_path_, properties);
   }
-  // Connection failures are responsible for updating the UI, including
-  // reopening dialogs.
   return true;  // Close dialog.
 }
 
 void VPNConfigView::Cancel() {
 }
 
 void VPNConfigView::InitFocus() {
   views::View* view_to_focus = GetInitiallyFocusedView();
   if (view_to_focus)
     view_to_focus->RequestFocus();
 }
 
 const std::string VPNConfigView::GetService() const {
   if (service_textfield_ != NULL)
     return GetTextFromField(service_textfield_, true);
   return service_path_;
 }
 
 const std::string VPNConfigView::GetServer() const {
   if (server_textfield_ != NULL)
     return GetTextFromField(server_textfield_, true);
-  return server_hostname_;
+  return std::string();
 }
 
 const std::string VPNConfigView::GetPSKPassphrase() const {
   if (psk_passphrase_textfield_ &&
       enable_psk_passphrase_ &&
       psk_passphrase_textfield_->visible())
     return GetPassphraseFromField(psk_passphrase_textfield_);
   return std::string();
 }
 
@@ skipping 34 matching lines @@
     int index = user_cert_combobox_ ? user_cert_combobox_->selected_index() : 0;
     return CertLibrary::Get()->GetCertPkcs11IdAt(
         CertLibrary::CERT_TYPE_USER, index);
   }
 }
 
 bool VPNConfigView::GetSaveCredentials() const {
   return save_credentials_checkbox_->checked();
 }
 
-void VPNConfigView::Init(VirtualNetwork* vpn) {
-  if (vpn) {
-    ProviderType type = vpn->provider_type();
-    std::string type_dict_name = ProviderTypeToONCDictKey(type);
+int VPNConfigView::GetProviderTypeIndex() const {
+  if (!provider_type_.empty())

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

IMO, it would be less confusing if you would use service_path_.empty() instead.

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

Refactored

+    return ProviderTypeToIndex(provider_type_, client_cert_id_);
+  if (provider_type_combobox_)
+    return provider_type_combobox_->selected_index();
+  return PROVIDER_TYPE_INDEX_MAX;
+}
 
-    if (type == PROVIDER_TYPE_L2TP_IPSEC_PSK) {
-      ParseVPNUIProperty(&ca_cert_ui_data_, vpn, type_dict_name,
-                         onc::ipsec::kServerCARef);
-      ParseVPNUIProperty(&psk_passphrase_ui_data_, vpn, type_dict_name,
-                         onc::ipsec::kPSK);
-      ParseVPNUIProperty(&group_name_ui_data_, vpn, type_dict_name,
-                         onc::ipsec::kGroup);
-    } else { // OpenVPN
-      ParseVPNUIProperty(&ca_cert_ui_data_, vpn, type_dict_name,
-                         onc::openvpn::kServerCARef);
-    }
-    ParseVPNUIProperty(&user_cert_ui_data_, vpn, type_dict_name,
-                       onc::vpn::kClientCertRef);
+std::string VPNConfigView::GetProviderTypeString() const {
+  int index = GetProviderTypeIndex();
+  switch (index) {
+    case PROVIDER_TYPE_INDEX_L2TP_IPSEC_PSK:
+    case PROVIDER_TYPE_INDEX_L2TP_IPSEC_USER_CERT:
+      return flimflam::kProviderL2tpIpsec;
+    case PROVIDER_TYPE_INDEX_OPEN_VPN:
+      return flimflam::kProviderOpenVpn;
+  }
+  NOTREACHED();
+  return std::string();
+}
 
-    const std::string credentials_dict_name(
-        type == PROVIDER_TYPE_L2TP_IPSEC_PSK ?
-            onc::vpn::kL2TP : type_dict_name);
-    ParseVPNUIProperty(&username_ui_data_, vpn, credentials_dict_name,
-                       onc::vpn::kUsername);
-    ParseVPNUIProperty(&user_passphrase_ui_data_, vpn, credentials_dict_name,
-                       onc::vpn::kPassword);
-    ParseVPNUIProperty(&save_credentials_ui_data_, vpn, credentials_dict_name,
-                       onc::vpn::kSaveCredentials);
+void VPNConfigView::Init() {
+  const NetworkState* vpn = NULL;
+  if (!service_path_.empty()) {
+    vpn = NetworkHandler::Get()->network_state_handler()->
+        GetNetworkState(service_path_);
+    DCHECK(vpn->type() == flimflam::kTypeVPN);

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

DCHECK(vpn)

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

Done.

   }
-
-  views::GridLayout* layout = views::GridLayout::CreatePanel(this);
-  SetLayoutManager(layout);
+  layout_ = views::GridLayout::CreatePanel(this);
+  SetLayoutManager(layout_);
 
   // Observer any changes to the certificate list.
   CertLibrary::Get()->AddObserver(this);
 
-  const int column_view_set_id = 0;
-  views::ColumnSet* column_set = layout->AddColumnSet(column_view_set_id);
+  views::ColumnSet* column_set = layout_->AddColumnSet(0);
   // Label.
   column_set->AddColumn(views::GridLayout::LEADING, views::GridLayout::FILL, 1,
                         views::GridLayout::USE_PREF, 0, 0);
   column_set->AddPaddingColumn(0, views::kRelatedControlSmallHorizontalSpacing);
   // Textfield, combobox.
   column_set->AddColumn(views::GridLayout::FILL, views::GridLayout::FILL, 1,
                         views::GridLayout::USE_PREF, 0,
                         ChildNetworkConfigView::kInputFieldMinWidth);
   column_set->AddPaddingColumn(0, views::kRelatedControlSmallHorizontalSpacing);
   // Policy indicator.
   column_set->AddColumn(views::GridLayout::CENTER, views::GridLayout::CENTER, 0,
                         views::GridLayout::USE_PREF, 0, 0);
 
   // Initialize members.
   service_text_modified_ = false;
+  title_ = vpn ? IDS_OPTIONS_SETTINGS_JOIN_VPN : IDS_OPTIONS_SETTINGS_ADD_VPN;
+
+  // Set up initial layout.
+  InitControls();
+
   if (vpn) {
-    provider_type_ = vpn->provider_type();
-    // Sets enable_* based on the provider type which we use to control
-    // which controls to make visible.
-    UpdateControlsToEnable();
-  } else {
-    // Set the default provider type.
-    provider_type_ = PROVIDER_TYPE_L2TP_IPSEC_PSK;
-    // Provider Type is user selectable, so enable all controls during init.
-    enable_psk_passphrase_ = true;
-    enable_user_cert_ = true;
-    enable_server_ca_cert_ = true;
-    enable_otp_ = true;
-    enable_group_name_ = true;
-  }
-
-  // Initialize the title string ID used for the dialog.
-  title_ = vpn ? IDS_OPTIONS_SETTINGS_JOIN_VPN : IDS_OPTIONS_SETTINGS_ADD_VPN;
+    NetworkHandler::Get()->network_configuration_handler()->GetProperties(
+        service_path_,
+        base::Bind(&VPNConfigView::InitFromProperties, AsWeakPtr()),

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

The dialog will be visible before the callback is called.
How will the dialog look like in between? Is it flickering?
And if it ever fails the dialog ends up in a weird state.

Alternatively, fetching the properties before showing the dialog might be less
distracting. In that case, you could also de-duplicate this code:
InitFromProperties could be a virtual method in ChildNetworkConfigView.
and GetProperties could be called by Network

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

Views Layout is... challenging. This creates all of the controls first, only
filling in properties when received. I combined the Init methods to make that
more clear (the separation was essentially an intermediate step).

+        base::Bind(&VPNConfigView::GetPropertiesError, AsWeakPtr()));
+  }
+}
+
+void VPNConfigView::InitControls() {
+  // By default enable all controls.
+  enable_psk_passphrase_ = true;
+  enable_user_cert_ = true;
+  enable_server_ca_cert_ = true;
+  enable_otp_ = true;
+  enable_group_name_ = true;
 
   // Server label and input.
   // Only provide Server name when configuring a new VPN.
-  if (!vpn) {
-    layout->StartRow(0, column_view_set_id);
-    layout->AddView(new views::Label(l10n_util::GetStringUTF16(
+  if (service_path_.empty()) {
+    layout_->StartRow(0, 0);
+    layout_->AddView(new views::Label(l10n_util::GetStringUTF16(
       IDS_OPTIONS_SETTINGS_INTERNET_OPTIONS_VPN_SERVER_HOSTNAME)));
     server_textfield_ = new views::Textfield(views::Textfield::STYLE_DEFAULT);
     server_textfield_->SetController(this);
-    layout->AddView(server_textfield_);
-    layout->AddPaddingRow(0, views::kRelatedControlVerticalSpacing);
+    layout_->AddView(server_textfield_);
+    layout_->AddPaddingRow(0, views::kRelatedControlVerticalSpacing);
   } else {
     server_textfield_ = NULL;
   }
 
   // Service label and name or input.
-  layout->StartRow(0, column_view_set_id);
-  layout->AddView(new views::Label(l10n_util::GetStringUTF16(
+  layout_->StartRow(0, 0);
+  layout_->AddView(new views::Label(l10n_util::GetStringUTF16(
       IDS_OPTIONS_SETTINGS_INTERNET_OPTIONS_VPN_SERVICE_NAME)));
-  if (!vpn) {
+  if (service_path_.empty()) {
     service_textfield_ = new views::Textfield(views::Textfield::STYLE_DEFAULT);
     service_textfield_->SetController(this);
-    layout->AddView(service_textfield_);
+    layout_->AddView(service_textfield_);
     service_text_ = NULL;
   } else {
-    service_text_ = new views::Label(ASCIIToUTF16(vpn->name()));
+    service_text_ = new views::Label();
     service_text_->SetHorizontalAlignment(gfx::ALIGN_LEFT);
-    layout->AddView(service_text_);
+    layout_->AddView(service_text_);
     service_textfield_ = NULL;
   }
-  layout->AddPaddingRow(0, views::kRelatedControlVerticalSpacing);
+  layout_->AddPaddingRow(0, views::kRelatedControlVerticalSpacing);
 
   // Provider type label and select.
-  layout->StartRow(0, column_view_set_id);
-  layout->AddView(new views::Label(l10n_util::GetStringUTF16(
+  layout_->StartRow(0, 0);
+  layout_->AddView(new views::Label(l10n_util::GetStringUTF16(
       IDS_OPTIONS_SETTINGS_INTERNET_OPTIONS_VPN_PROVIDER_TYPE)));
-  if (!vpn) {
+  if (service_path_.empty()) {
     provider_type_combobox_model_.reset(
         new internal::ProviderTypeComboboxModel);
     provider_type_combobox_ = new views::Combobox(
         provider_type_combobox_model_.get());
     provider_type_combobox_->set_listener(this);
-    layout->AddView(provider_type_combobox_);
+    layout_->AddView(provider_type_combobox_);
     provider_type_text_label_ = NULL;
   } else {
-    provider_type_text_label_ =
-        new views::Label(ProviderTypeToString(provider_type_));
+    provider_type_text_label_ = new views::Label();
     provider_type_text_label_->SetHorizontalAlignment(gfx::ALIGN_LEFT);
-    layout->AddView(provider_type_text_label_);
+    layout_->AddView(provider_type_text_label_);
     provider_type_combobox_ = NULL;
   }
-  layout->AddPaddingRow(0, views::kRelatedControlVerticalSpacing);
+  layout_->AddPaddingRow(0, views::kRelatedControlVerticalSpacing);
 
   // PSK passphrase label, input and visible button.
-  if (enable_psk_passphrase_) {
-    layout->StartRow(0, column_view_set_id);
-    psk_passphrase_label_ =  new views::Label(l10n_util::GetStringUTF16(
-        IDS_OPTIONS_SETTINGS_INTERNET_OPTIONS_VPN_PSK_PASSPHRASE));
-    layout->AddView(psk_passphrase_label_);
-    bool has_psk_passphrase = vpn && !vpn->IsPSKPassphraseRequired();
-    psk_passphrase_textfield_ = new PassphraseTextfield(has_psk_passphrase);
-    psk_passphrase_textfield_->SetController(this);
-    layout->AddView(psk_passphrase_textfield_);
-    layout->AddView(
-        new ControlledSettingIndicatorView(psk_passphrase_ui_data_));
-    layout->AddPaddingRow(0, views::kRelatedControlVerticalSpacing);
-  } else {
-    psk_passphrase_label_ = NULL;
-    psk_passphrase_textfield_ = NULL;
-  }
+  layout_->StartRow(0, 0);
+  psk_passphrase_label_ =  new views::Label(l10n_util::GetStringUTF16(
+      IDS_OPTIONS_SETTINGS_INTERNET_OPTIONS_VPN_PSK_PASSPHRASE));
+  layout_->AddView(psk_passphrase_label_);
+  psk_passphrase_textfield_ = new PassphraseTextfield();
+  psk_passphrase_textfield_->SetController(this);
+  layout_->AddView(psk_passphrase_textfield_);
+  layout_->AddView(
+      new ControlledSettingIndicatorView(psk_passphrase_ui_data_));
+  layout_->AddPaddingRow(0, views::kRelatedControlVerticalSpacing);
 
   // Server CA certificate
-  // Only provide Server CA when configuring a new VPN.
-  if (!vpn) {
-    layout->StartRow(0, column_view_set_id);
-    server_ca_cert_label_ =
-        new views::Label(l10n_util::GetStringUTF16(
-            IDS_OPTIONS_SETTINGS_INTERNET_OPTIONS_CERT_SERVER_CA));
-    layout->AddView(server_ca_cert_label_);
+  if (service_path_.empty()) {
+    layout_->StartRow(0, 0);
+    server_ca_cert_label_ = new views::Label(l10n_util::GetStringUTF16(
+        IDS_OPTIONS_SETTINGS_INTERNET_OPTIONS_CERT_SERVER_CA));
+    layout_->AddView(server_ca_cert_label_);
     server_ca_cert_combobox_model_.reset(
         new internal::VpnServerCACertComboboxModel());
     server_ca_cert_combobox_ = new views::Combobox(
         server_ca_cert_combobox_model_.get());
-    layout->AddView(server_ca_cert_combobox_);
-    layout->AddView(new ControlledSettingIndicatorView(ca_cert_ui_data_));
-    layout->AddPaddingRow(0, views::kRelatedControlVerticalSpacing);
+    layout_->AddView(server_ca_cert_combobox_);
+    layout_->AddView(new ControlledSettingIndicatorView(ca_cert_ui_data_));
+    layout_->AddPaddingRow(0, views::kRelatedControlVerticalSpacing);
   } else {
     server_ca_cert_label_ = NULL;
     server_ca_cert_combobox_ = NULL;
   }
 
   // User certificate label and input.
-  if (enable_user_cert_) {
-    layout->StartRow(0, column_view_set_id);
-    user_cert_label_ = new views::Label(l10n_util::GetStringUTF16(
-        IDS_OPTIONS_SETTINGS_INTERNET_OPTIONS_VPN_USER_CERT));
-    layout->AddView(user_cert_label_);
-    user_cert_combobox_model_.reset(
-        new internal::VpnUserCertComboboxModel());
-    user_cert_combobox_ = new views::Combobox(user_cert_combobox_model_.get());
-    user_cert_combobox_->set_listener(this);
-    layout->AddView(user_cert_combobox_);
-    layout->AddView(new ControlledSettingIndicatorView(user_cert_ui_data_));
-    layout->AddPaddingRow(0, views::kRelatedControlVerticalSpacing);
-  } else {
-    user_cert_label_ = NULL;
-    user_cert_combobox_ = NULL;
-  }
+  layout_->StartRow(0, 0);
+  user_cert_label_ = new views::Label(l10n_util::GetStringUTF16(
+      IDS_OPTIONS_SETTINGS_INTERNET_OPTIONS_VPN_USER_CERT));
+  layout_->AddView(user_cert_label_);
+  user_cert_combobox_model_.reset(
+      new internal::VpnUserCertComboboxModel());
+  user_cert_combobox_ = new views::Combobox(user_cert_combobox_model_.get());
+  user_cert_combobox_->set_listener(this);
+  layout_->AddView(user_cert_combobox_);
+  layout_->AddView(new ControlledSettingIndicatorView(user_cert_ui_data_));
+  layout_->AddPaddingRow(0, views::kRelatedControlVerticalSpacing);
 
   // Username label and input.
-  layout->StartRow(0, column_view_set_id);
-  layout->AddView(new views::Label(l10n_util::GetStringUTF16(
+  layout_->StartRow(0, 0);
+  layout_->AddView(new views::Label(l10n_util::GetStringUTF16(
       IDS_OPTIONS_SETTINGS_INTERNET_OPTIONS_VPN_USERNAME)));
   username_textfield_ = new views::Textfield(views::Textfield::STYLE_DEFAULT);
   username_textfield_->SetController(this);
   username_textfield_->SetEnabled(username_ui_data_.IsEditable());
-  if (vpn && !vpn->username().empty())
-    username_textfield_->SetText(UTF8ToUTF16(vpn->username()));
-  layout->AddView(username_textfield_);
-  layout->AddView(new ControlledSettingIndicatorView(username_ui_data_));
-  layout->AddPaddingRow(0, views::kRelatedControlVerticalSpacing);
+  layout_->AddView(username_textfield_);
+  layout_->AddView(new ControlledSettingIndicatorView(username_ui_data_));
+  layout_->AddPaddingRow(0, views::kRelatedControlVerticalSpacing);
 
   // User passphrase label, input and visble button.
-  layout->StartRow(0, column_view_set_id);
-  layout->AddView(new views::Label(l10n_util::GetStringUTF16(
+  layout_->StartRow(0, 0);
+  layout_->AddView(new views::Label(l10n_util::GetStringUTF16(
       IDS_OPTIONS_SETTINGS_INTERNET_OPTIONS_VPN_USER_PASSPHRASE)));
-  bool has_user_passphrase = vpn && !vpn->IsUserPassphraseRequired();
-  user_passphrase_textfield_ = new PassphraseTextfield(has_user_passphrase);
+  user_passphrase_textfield_ = new PassphraseTextfield();

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

as feared, SetShowFake is never called the password would be always visible.

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

We never set the value of this to the passphrase. "Fake" isn't obscuring
anything, it's supposed to be an indicator that Shill knows the passphrase
so the user doesn't need to provide it. We don't know that until
InitFromProperties. If that never gets called somehow, this would just be blank.

   user_passphrase_textfield_->SetController(this);
   user_passphrase_textfield_->SetEnabled(user_passphrase_ui_data_.IsEditable());
-  layout->AddView(user_passphrase_textfield_);
-  layout->AddView(new ControlledSettingIndicatorView(user_passphrase_ui_data_));
-  layout->AddPaddingRow(0, views::kRelatedControlVerticalSpacing);
+  layout_->AddView(user_passphrase_textfield_);
+  layout_->AddView(
+      new ControlledSettingIndicatorView(user_passphrase_ui_data_));
+  layout_->AddPaddingRow(0, views::kRelatedControlVerticalSpacing);
 
   // OTP label and input.
-  if (enable_otp_) {
-    layout->StartRow(0, column_view_set_id);
-    otp_label_ = new views::Label(l10n_util::GetStringUTF16(
-        IDS_OPTIONS_SETTINGS_INTERNET_OPTIONS_VPN_OTP));
-    layout->AddView(otp_label_);
-    otp_textfield_ = new views::Textfield(views::Textfield::STYLE_DEFAULT);
-    otp_textfield_->SetController(this);
-    layout->AddView(otp_textfield_);
-    layout->AddPaddingRow(0, views::kRelatedControlVerticalSpacing);
-  } else {
-    otp_label_ = NULL;
-    otp_textfield_ = NULL;
-  }
+  layout_->StartRow(0, 0);
+  otp_label_ = new views::Label(l10n_util::GetStringUTF16(
+      IDS_OPTIONS_SETTINGS_INTERNET_OPTIONS_VPN_OTP));
+  layout_->AddView(otp_label_);
+  otp_textfield_ = new views::Textfield(views::Textfield::STYLE_DEFAULT);
+  otp_textfield_->SetController(this);
+  layout_->AddView(otp_textfield_);
+  layout_->AddPaddingRow(0, views::kRelatedControlVerticalSpacing);
 
   // Group Name label and input.
-  if (enable_group_name_) {
-    layout->StartRow(0, column_view_set_id);
-    group_name_label_ = new views::Label(l10n_util::GetStringUTF16(
-        IDS_OPTIONS_SETTINGS_INTERNET_OPTIONS_VPN_GROUP_NAME));
-    layout->AddView(group_name_label_);
-    group_name_textfield_ =
-        new views::Textfield(views::Textfield::STYLE_DEFAULT);
-    group_name_textfield_->SetController(this);
-    if (vpn && !vpn->group_name().empty())
-      group_name_textfield_->SetText(UTF8ToUTF16(vpn->group_name()));
-    layout->AddView(group_name_textfield_);
-    layout->AddView(new ControlledSettingIndicatorView(group_name_ui_data_));
-    layout->AddPaddingRow(0, views::kRelatedControlVerticalSpacing);
-  } else {
-    group_name_label_ = NULL;
-    group_name_textfield_ = NULL;
-  }
+  layout_->StartRow(0, 0);
+  group_name_label_ = new views::Label(l10n_util::GetStringUTF16(
+      IDS_OPTIONS_SETTINGS_INTERNET_OPTIONS_VPN_GROUP_NAME));
+  layout_->AddView(group_name_label_);
+  group_name_textfield_ =
+      new views::Textfield(views::Textfield::STYLE_DEFAULT);
+  group_name_textfield_->SetController(this);
+  layout_->AddView(group_name_textfield_);
+  layout_->AddView(new ControlledSettingIndicatorView(group_name_ui_data_));
+  layout_->AddPaddingRow(0, views::kRelatedControlVerticalSpacing);
 
   // Save credentials
-  layout->StartRow(0, column_view_set_id);
+  layout_->StartRow(0, 0);
   save_credentials_checkbox_ = new views::Checkbox(
       l10n_util::GetStringUTF16(
           IDS_OPTIONS_SETTINGS_INTERNET_OPTIONS_SAVE_CREDENTIALS));
   save_credentials_checkbox_->SetEnabled(
       save_credentials_ui_data_.IsEditable());
-  bool save_credentials = vpn ? vpn->save_credentials() : false;
-  save_credentials_checkbox_->SetChecked(save_credentials);
-  layout->SkipColumns(1);
-  layout->AddView(save_credentials_checkbox_);
-  layout->AddView(
+  layout_->SkipColumns(1);
+  layout_->AddView(save_credentials_checkbox_);
+  layout_->AddView(
       new ControlledSettingIndicatorView(save_credentials_ui_data_));
 
   // Error label.
-  layout->StartRow(0, column_view_set_id);
-  layout->SkipColumns(1);
+  layout_->StartRow(0, 0);
+  layout_->SkipColumns(1);
   error_label_ = new views::Label();
   error_label_->SetHorizontalAlignment(gfx::ALIGN_LEFT);
   error_label_->SetEnabledColor(SK_ColorRED);
-  layout->AddView(error_label_);
+  layout_->AddView(error_label_);
 
   // Set or hide the UI, update comboboxes and error labels.
   Refresh();

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

Shouldn't this be called from InitFromProperties?

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

It should really be called by both, done.

 }
 
+void VPNConfigView::InitFromProperties(
+    const std::string& service_path,
+    const base::DictionaryValue& service_properties) {
+  const NetworkState* vpn = NetworkHandler::Get()->network_state_handler()->
+      GetNetworkState(service_path);
+  if (!vpn) {
+    NET_LOG_ERROR("Shill Error getting properties VpnConfigView", service_path);
+    return;
+  }
+
+  std::string server_hostname, username, group_name;
+  bool psk_passphrase_required = false;
+  const base::DictionaryValue* provider_properties;
+  if (service_properties.GetDictionaryWithoutPathExpansion(
+          flimflam::kProviderProperty, &provider_properties)) {
+    provider_properties->GetStringWithoutPathExpansion(
+        flimflam::kTypeProperty, &provider_type_);
+    service_properties.GetStringWithoutPathExpansion(

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

provider_properties->

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

Bah, good catch, Done.

+        flimflam::kHostProperty, &server_hostname);
+  }
+  if (provider_type_ == flimflam::kProviderL2tpIpsec) {
+    service_properties.GetStringWithoutPathExpansion(

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

provider_properties->

most vpn properties (both l2tp and openvpn) are nested in the provider
properties when reading from Shill.

i didn't mark them below.

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

Done.

+        flimflam::kL2tpIpsecClientCertIdProperty, &client_cert_id_);
+    service_properties.GetStringWithoutPathExpansion(
+        shill::kL2tpIpsecCaCertPemProperty, &ca_cert_pem_);

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

CaCertPemProperty is a list, not a string.
You can only show the first entry of that list.

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

Gah. Done.

+    service_properties.GetBooleanWithoutPathExpansion(
+        flimflam::kL2tpIpsecPskRequiredProperty, &psk_passphrase_required);
+    service_properties.GetStringWithoutPathExpansion(
+        flimflam::kL2tpIpsecUserProperty, &username);
+    service_properties.GetStringWithoutPathExpansion(
+        shill::kL2tpIpsecTunnelGroupProperty, &group_name);
+  } else if (provider_type_ == flimflam::kProviderOpenVpn) {
+    service_properties.GetStringWithoutPathExpansion(
+        flimflam::kOpenVPNClientCertIdProperty, &client_cert_id_);
+    service_properties.GetStringWithoutPathExpansion(
+        shill::kOpenVPNCaCertPemProperty, &ca_cert_pem_);

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

CaCertPemProperty is a list, not a string.
You can only show the first entry of that list.

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

Done.

+    service_properties.GetStringWithoutPathExpansion(
+        flimflam::kOpenVPNUserProperty, &username);
+  }
+  bool save_credentials = false;
+  service_properties.GetBooleanWithoutPathExpansion(
+      flimflam::kSaveCredentialsProperty, &save_credentials);
+
+  ProviderTypeComboboxIndex provider_type_index =
+      ProviderTypeToIndex(provider_type_, client_cert_id_);
+  ParseUIProperties(vpn, provider_type_index);
+
+  if (service_text_)
+    service_text_->SetText(ASCIIToUTF16(vpn->name()));
+  if (provider_type_text_label_)
+    provider_type_text_label_->SetText(
+        ProviderTypeIndexToString(provider_type_index));
+
+  if (server_textfield_ && !server_hostname.empty())
+    server_textfield_->SetText(UTF8ToUTF16(server_hostname));
+  if (username_textfield_ && !username.empty())
+    username_textfield_->SetText(UTF8ToUTF16(username));
+  if (group_name_textfield_ && !group_name.empty())
+    group_name_textfield_->SetText(UTF8ToUTF16(group_name));
+  if (psk_passphrase_textfield_)
+    psk_passphrase_textfield_->SetShowFake(!psk_passphrase_required);
+  if (save_credentials_checkbox_)
+    save_credentials_checkbox_->SetChecked(save_credentials);
+}
+
+void VPNConfigView::ParseUIProperties(const NetworkState* vpn,
+                                      int provider_type_index) {
+    std::string type_dict_name = ProviderTypeToONCDictKey(provider_type_);

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

nit: indentation

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

that's again confusing: there is a provider_type_ and provider_type_index
can they ever describe different provider types?

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

Done.

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

Done.

+  if (provider_type_index == PROVIDER_TYPE_INDEX_L2TP_IPSEC_PSK) {
+    ParseVPNUIProperty(vpn, type_dict_name, onc::ipsec::kServerCARef,
+                       &ca_cert_ui_data_);
+    ParseVPNUIProperty(vpn, type_dict_name, onc::ipsec::kPSK,
+                       &psk_passphrase_ui_data_);
+    ParseVPNUIProperty(vpn, type_dict_name, onc::ipsec::kGroup,
+                       &group_name_ui_data_);
+  } else if (provider_type_index == PROVIDER_TYPE_INDEX_OPEN_VPN) {
+    ParseVPNUIProperty(vpn, type_dict_name, onc::openvpn::kServerCARef,
+                       &ca_cert_ui_data_);
+  }
+  ParseVPNUIProperty(vpn, type_dict_name, onc::vpn::kClientCertRef,
+                     &user_cert_ui_data_);
+
+  const std::string credentials_dict_name(
+      provider_type_index == PROVIDER_TYPE_INDEX_L2TP_IPSEC_PSK ?
+      onc::vpn::kL2TP : type_dict_name);
+  ParseVPNUIProperty(vpn, credentials_dict_name, onc::vpn::kUsername,
+                     &username_ui_data_);
+  ParseVPNUIProperty(vpn, credentials_dict_name, onc::vpn::kPassword,
+                     &user_passphrase_ui_data_);
+  ParseVPNUIProperty(vpn, credentials_dict_name, onc::vpn::kSaveCredentials,
+                     &save_credentials_ui_data_);
+}
+
+void VPNConfigView::GetPropertiesError(
+    const std::string& error_name,
+    scoped_ptr<base::DictionaryValue> error_data) {
+  NET_LOG_ERROR("Shill Error from VpnConfigView: " + error_name, "");
+}
+
+void VPNConfigView::SetCredentials(base::DictionaryValue* properties) {
+  int provider_type_index = GetProviderTypeIndex();
+  std::string user_passphrase = GetUserPassphrase();
+  std::string user_name = GetUsername();
+  std::string group_name = GetGroupName();

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

nit:
this is not a credential.
similarly CACerts are not credentials.
better call the function SetModifiableProperties

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

Named SetConfigProperties

+  switch (provider_type_index) {
+    case PROVIDER_TYPE_INDEX_L2TP_IPSEC_PSK: {
+      std::string psk_passphrase = GetPSKPassphrase();
+      if (!psk_passphrase.empty()) {
+        properties->SetStringWithoutPathExpansion(
+            flimflam::kL2tpIpsecPskProperty, GetPSKPassphrase());
+      }
+      if (!group_name.empty()) {
+        properties->SetStringWithoutPathExpansion(
+            shill::kL2tpIpsecTunnelGroupProperty, group_name);
+      }
+      if (!user_name.empty()) {
+        properties->SetStringWithoutPathExpansion(
+            flimflam::kL2tpIpsecUserProperty, user_name);
+      }
+      if (!user_passphrase.empty()) {
+        properties->SetStringWithoutPathExpansion(
+            flimflam::kL2tpIpsecPasswordProperty, user_passphrase);
+      }
+      break;
+    }
+    case PROVIDER_TYPE_INDEX_L2TP_IPSEC_USER_CERT: {
+      std::string ca_cert_pem = GetServerCACertPEM();
+      if (!ca_cert_pem.empty()) {
+        properties->SetStringWithoutPathExpansion(
+            shill::kL2tpIpsecCaCertPemProperty, ca_cert_pem);
+      }
+      properties->SetStringWithoutPathExpansion(
+          flimflam::kL2tpIpsecClientCertIdProperty, GetUserCertID());
+      if (!group_name.empty()) {
+        properties->SetStringWithoutPathExpansion(
+            shill::kL2tpIpsecTunnelGroupProperty, GetGroupName());
+      }
+      if (!user_name.empty()) {
+        properties->SetStringWithoutPathExpansion(
+            flimflam::kL2tpIpsecUserProperty, user_name);
+      }
+      if (!user_passphrase.empty()) {
+        properties->SetStringWithoutPathExpansion(
+            flimflam::kL2tpIpsecPasswordProperty, user_passphrase);
+      }
+      break;
+    }
+    case PROVIDER_TYPE_INDEX_OPEN_VPN: {
+      std::string ca_cert_pem = GetServerCACertPEM();
+      if (!ca_cert_pem.empty()) {
+        base::ListValue* pem_list = new base::ListValue;
+        pem_list->AppendString(GetServerCACertPEM());
+        properties->SetWithoutPathExpansion(
+            shill::kOpenVPNCaCertPemProperty, pem_list);
+      }
+      properties->SetStringWithoutPathExpansion(
+          flimflam::kOpenVPNClientCertIdProperty, GetUserCertID());
+      properties->SetStringWithoutPathExpansion(
+          flimflam::kOpenVPNUserProperty, GetUsername());
+      if (!user_passphrase.empty()) {
+        properties->SetStringWithoutPathExpansion(
+            flimflam::kOpenVPNPasswordProperty, user_passphrase);
+      }
+      std::string otp = GetOTP();
+      if (!otp.empty()) {
+        properties->SetStringWithoutPathExpansion(
+            flimflam::kOpenVPNOTPProperty, otp);
+      }
+      break;
+    }
+    case PROVIDER_TYPE_INDEX_MAX:
+      NOTREACHED();
+      break;
+  }
+  properties->SetBooleanWithoutPathExpansion(
+      flimflam::kSaveCredentialsProperty, GetSaveCredentials());
+}
+
 void VPNConfigView::Refresh() {
-  NetworkLibrary* cros = NetworkLibrary::Get();
-
   UpdateControls();
 
   // Set certificate combo boxes.
-  VirtualNetwork* vpn = cros->FindVirtualNetworkByPath(service_path_);
   if (server_ca_cert_combobox_) {
     server_ca_cert_combobox_->ModelChanged();
-    if (enable_server_ca_cert_ &&
-        (vpn && !vpn->ca_cert_pem().empty())) {
+    if (enable_server_ca_cert_ && !ca_cert_pem_.empty()) {
       // Select the current server CA certificate in the combobox.
       int cert_index = CertLibrary::Get()->GetCertIndexByPEM(
-          CertLibrary::CERT_TYPE_SERVER_CA, vpn->ca_cert_pem());
+          CertLibrary::CERT_TYPE_SERVER_CA, ca_cert_pem_);
       if (cert_index >= 0) {
         // Skip item for "Default"
         server_ca_cert_combobox_->SetSelectedIndex(1 + cert_index);
       } else {
         server_ca_cert_combobox_->SetSelectedIndex(0);
       }
     } else {
       server_ca_cert_combobox_->SetSelectedIndex(0);
     }
   }
 
   if (user_cert_combobox_) {
     user_cert_combobox_->ModelChanged();
-    if (enable_user_cert_ &&
-        (vpn && !vpn->client_cert_id().empty())) {
+    if (enable_user_cert_ && !client_cert_id_.empty()) {
       int cert_index = CertLibrary::Get()->GetCertIndexByPkcs11Id(
-          CertLibrary::CERT_TYPE_USER, vpn->client_cert_id());
+          CertLibrary::CERT_TYPE_USER, client_cert_id_);
       if (cert_index >= 0)
         user_cert_combobox_->SetSelectedIndex(cert_index);
       else
         user_cert_combobox_->SetSelectedIndex(0);
     } else {
       user_cert_combobox_->SetSelectedIndex(0);
     }
   }
 
   UpdateErrorLabel();
 }
 
 void VPNConfigView::UpdateControlsToEnable() {
-  // Set which controls are enabled.
   enable_psk_passphrase_ = false;
   enable_user_cert_ = false;
   enable_server_ca_cert_ = false;
   enable_otp_ = false;
   enable_group_name_ = false;
-  switch (provider_type_) {
-    case PROVIDER_TYPE_L2TP_IPSEC_PSK:
-      enable_psk_passphrase_ = true;
-      enable_group_name_ = true;
-      break;
-    case PROVIDER_TYPE_L2TP_IPSEC_USER_CERT:
-      enable_server_ca_cert_ = true;
-      enable_user_cert_ = HaveUserCerts();
-      enable_group_name_ = true;
-      break;
-    case PROVIDER_TYPE_OPEN_VPN:
-      enable_server_ca_cert_ = true;
-      enable_user_cert_ = HaveUserCerts();
-      enable_otp_ = true;
-      break;
-    default:
-      NOTREACHED();
-      break;
+  int provider_type_index = GetProviderTypeIndex();
+  if (provider_type_index == PROVIDER_TYPE_INDEX_L2TP_IPSEC_PSK) {
+    enable_psk_passphrase_ = true;
+    enable_group_name_ = true;
+  } else if (provider_type_index == PROVIDER_TYPE_INDEX_L2TP_IPSEC_USER_CERT) {
+    enable_server_ca_cert_ = true;
+    enable_user_cert_ = HaveUserCerts();
+    enable_group_name_ = true;
+  } else {  // PROVIDER_TYPE_INDEX_OPEN_VPN (default)
+    enable_server_ca_cert_ = true;
+    enable_user_cert_ = HaveUserCerts();
+    enable_otp_ = true;
   }
 }
 
 void VPNConfigView::UpdateControls() {
   UpdateControlsToEnable();
 
   if (psk_passphrase_label_)
     psk_passphrase_label_->SetEnabled(enable_psk_passphrase_);
   if (psk_passphrase_textfield_)
     psk_passphrase_textfield_->SetEnabled(enable_psk_passphrase_ &&
@@ skipping 17 matching lines @@
     otp_textfield_->SetEnabled(enable_otp_);
 
   if (group_name_label_)
     group_name_label_->SetEnabled(enable_group_name_);
   if (group_name_textfield_)
     group_name_textfield_->SetEnabled(enable_group_name_ &&
                                       group_name_ui_data_.IsEditable());
 }
 
 void VPNConfigView::UpdateErrorLabel() {
-  NetworkLibrary* cros = NetworkLibrary::Get();
-
   // Error message.
-  std::string error_msg;
-  if (UserCertRequired() && CertLibrary::Get()->CertificatesLoaded()) {
+  string16 error_msg;
+  bool cert_required =
+      GetProviderTypeIndex() == PROVIDER_TYPE_INDEX_L2TP_IPSEC_USER_CERT;
+  if (cert_required && CertLibrary::Get()->CertificatesLoaded()) {
     if (!HaveUserCerts()) {
-      error_msg = l10n_util::GetStringUTF8(
+      error_msg = l10n_util::GetStringUTF16(
           IDS_OPTIONS_SETTINGS_INTERNET_OPTIONS_PLEASE_INSTALL_USER_CERT);
     } else if (!IsUserCertValid()) {
-      error_msg = l10n_util::GetStringUTF8(
+      error_msg = l10n_util::GetStringUTF16(
           IDS_OPTIONS_SETTINGS_INTERNET_OPTIONS_REQUIRE_HARDWARE_BACKED);
     }
   }
   if (error_msg.empty() && !service_path_.empty()) {
     // TODO(kuan): differentiate between bad psk and user passphrases.
-    VirtualNetwork* vpn = cros->FindVirtualNetworkByPath(service_path_);
-    if (vpn && vpn->failed()) {
-      if (vpn->error() == ERROR_BAD_PASSPHRASE) {
-        error_msg = l10n_util::GetStringUTF8(
-            IDS_OPTIONS_SETTINGS_INTERNET_OPTIONS_BAD_PASSPHRASE);
-      } else {
-        error_msg = vpn->GetErrorString();
-      }
-    }
+    const NetworkState* vpn = NetworkHandler::Get()->network_state_handler()->
+        GetNetworkState(service_path_);
+    if (vpn && vpn->connection_state() == flimflam::kStateFailure)
+      error_msg = ash::network_connect::ErrorString(vpn->error());
   }
   if (!error_msg.empty()) {
-    error_label_->SetText(UTF8ToUTF16(error_msg));
+    error_label_->SetText(error_msg);
     error_label_->SetVisible(true);
   } else {
     error_label_->SetVisible(false);
   }
 }
 
 void VPNConfigView::UpdateCanLogin() {
   parent_->GetDialogClientView()->UpdateDialogButtons();
 }
 
-bool VPNConfigView::UserCertRequired() const {
-  return provider_type_ == PROVIDER_TYPE_L2TP_IPSEC_USER_CERT;
-}
-
 bool VPNConfigView::HaveUserCerts() const {
   return CertLibrary::Get()->NumCertificates(CertLibrary::CERT_TYPE_USER) > 0;
 }
 
 bool VPNConfigView::IsUserCertValid() const {
   if (!user_cert_combobox_ || !enable_user_cert_)
     return false;
   int index = user_cert_combobox_->selected_index();
   if (index < 0)
     return false;
@@ skipping 17 matching lines @@
   return result;
 }
 
 const std::string VPNConfigView::GetPassphraseFromField(
     PassphraseTextfield* textfield) const {
   if (!textfield)
     return std::string();
   return textfield->GetPassphrase();
 }
 
-void VPNConfigView::ParseVPNUIProperty(NetworkPropertyUIData* property_ui_data,
-                                       Network* network,
-                                       const std::string& dict_key,
-                                       const std::string& key) {
+void VPNConfigView::ParseVPNUIProperty(
+    const NetworkState* network,
+    const std::string& dict_key,
+    const std::string& key,
+    NetworkPropertyUIData* property_ui_data) {
   onc::ONCSource onc_source = onc::ONC_SOURCE_NONE;
   const base::DictionaryValue* onc =
-      NetworkConfigView::FindPolicyForActiveUser(network, &onc_source);
+      network_connect::FindPolicyForActiveUser(network, &onc_source);
 
-  VLOG_IF(1, !onc) << "No ONC found for VPN network " << network->unique_id();
+  VLOG_IF(1, !onc) << "No ONC found for VPN network " << network->guid();
   property_ui_data->ParseOncProperty(
       onc_source,
       onc,
       base::StringPrintf("%s.%s.%s",
                          onc::network_config::kVPN,
                          dict_key.c_str(),
                          key.c_str()));
 }
 
 }  // namespace chromeos