[ash-md] Restores handler for VPN Disconnect button in system menu

ash/common/system/chromeos/network/vpn_list_view.cc

Index: ash/common/system/chromeos/network/vpn_list_view.cc
diff --git a/ash/common/system/chromeos/network/vpn_list_view.cc b/ash/common/system/chromeos/network/vpn_list_view.cc
index 942f233e686c2c62b4e03dc399483bb9ae68c57f..bb171a83b0a4b88247259ea34343415eae91c4ff 100644
--- a/ash/common/system/chromeos/network/vpn_list_view.cc
+++ b/ash/common/system/chromeos/network/vpn_list_view.cc
@@ -59,6 +59,9 @@ bool UseMd() {
   return MaterialDesignController::IsSystemTrayMenuMaterial();
 }
 
+void IgnoreDisconnectError(const std::string& error_name,
+                           std::unique_ptr<base::DictionaryValue> error_data) {}
+
 // Indicates whether |network| belongs to this VPN provider.
 bool VpnProviderMatchesNetwork(const VPNProvider& provider,
                                const chromeos::NetworkState& network) {
@@ -157,6 +160,9 @@ class VPNListNetworkEntry : public VPNListEntryBase,
   // network_icon::AnimationObserver:
   void NetworkIconChanged() override;
 
+  // views::ButtonListener:
+  void ButtonPressed(Button* sender, const ui::Event& event) override;
+
  private:
   void UpdateFromNetworkState(const chromeos::NetworkState* network);
   void SetupConnectedItemMd(const base::string16& text,
@@ -193,6 +199,26 @@ void VPNListNetworkEntry::NetworkIconChanged() {
                              ->GetNetworkStateFromGuid(guid_));
 }
 
+void VPNListNetworkEntry::ButtonPressed(Button* sender,
+                                        const ui::Event& event) {
+  if (sender != disconnect_button_) {
+    VPNListEntryBase::ButtonPressed(sender, event);
+    return;
+  }
+
+  const chromeos::NetworkState* network = chromeos::NetworkHandler::Get()
+                                              ->network_state_handler()
+                                              ->GetNetworkStateFromGuid(guid_);
+  if (!network)
+    return;
+  WmShell::Get()->RecordUserMetricsAction(
+      UMA_STATUS_AREA_VPN_DISCONNECT_CLICKED);
+  chromeos::NetworkHandler::Get()
+      ->network_connection_handler()
+      ->DisconnectNetwork(network->path(), base::Bind(&base::DoNothing),
+                          base::Bind(&IgnoreDisconnectError));

[stevenjb, 2017/03/23 18:33:13]

Yeah, so, what we really want is:

chromeos::NetworkConnect::Get()->DisconnectFromNetworkId(network->guid())

That will require adding NetworkConnect::DisconectFromNetworkId(guid), but I
think it is worth it for the extra clarity and to avoid the |network| lookup and
direct NetworkConnectionHandler dependency here.

NetworkConnect::DisconectFromNetworkId would include all of the code above,
including IgnoreDisconnectError.

Thanks!

[varkha, 2017/03/23 19:03:26]

Agreed. It seems like NetworkConnect::ConnectToNetworkId uses a string ID rather
than guid and then the impl takes that and looks up a path, then uses the path
to connect. Is that how we want the DisconnectFromNetworkId to work or did you
really mean guid and not a string ID there?

[stevenjb, 2017/03/23 19:14:05]

"string ID" == guid.

[varkha, 2017/03/23 19:20:50]

Oh I see. I think I misread the code in
NetworkStateListDetailedView::HandleViewClicked() because it uses both *FromGuid
and *Id suffixes and because it uses |network->guid()| where it could just use
|guid|.

+}
+
 void VPNListNetworkEntry::UpdateFromNetworkState(
     const chromeos::NetworkState* network) {
   if (network && network->IsConnectingState())