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

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

 // Copyright 2015 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 "ash/common/system/chromeos/network/vpn_list_view.h"
 
 #include <memory>
 #include <utility>
 #include <vector>
 
@@ skipping 41 matching lines @@
 #include "ui/views/view.h"
 
 namespace ash {
 
 namespace {
 
 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) {
   if (network.type() != shill::kTypeVPN)
     return false;
   const bool network_uses_third_party_provider =
       network.vpn_provider_type() == shill::kProviderThirdPartyVpn;
   if (!provider.third_party)
     return !network_uses_third_party_provider;
   return network_uses_third_party_provider &&
@@ skipping 78 matching lines @@
 class VPNListNetworkEntry : public VPNListEntryBase,
                             public network_icon::AnimationObserver {
  public:
   VPNListNetworkEntry(VPNListView* parent,
                       const chromeos::NetworkState* network);
   ~VPNListNetworkEntry() override;
 
   // 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,
                             const gfx::ImageSkia& image);
   void SetupConnectingItemMd(const base::string16& text,
                              const gfx::ImageSkia& image);
 
   const std::string guid_;
 
+  const std::string service_path_;
+
   views::LabelButton* disconnect_button_ = nullptr;
 
   DISALLOW_COPY_AND_ASSIGN(VPNListNetworkEntry);
 };
 
 VPNListEntryBase::VPNListEntryBase(VPNListView* parent)
     : HoverHighlightView(parent) {
   if (!UseMd())
     SetBorder(views::CreateEmptyBorder(0, kTrayPopupPaddingHorizontal, 0, 0));
 }
 
 VPNListNetworkEntry::VPNListNetworkEntry(VPNListView* parent,
                                          const chromeos::NetworkState* network)
-    : VPNListEntryBase(parent), guid_(network->guid()) {
+    : VPNListEntryBase(parent),
+      guid_(network->guid()),
+      service_path_(network->path()) {
   UpdateFromNetworkState(network);
 }
 
 VPNListNetworkEntry::~VPNListNetworkEntry() {
   network_icon::NetworkIconAnimation::GetInstance()->RemoveObserver(this);
 }
 
 void VPNListNetworkEntry::NetworkIconChanged() {
   UpdateFromNetworkState(chromeos::NetworkHandler::Get()
                              ->network_state_handler()
                              ->GetNetworkStateFromGuid(guid_));

[tdanderson, 2017/03/22 23:03:54]

In the breaking CL, this query is instead performed via
GetNetworkState(service_path_). Currently this is the only place |guid_| is
used. Can |guid_| be removed, and can we instead call
GetNetworkState(service_path_) here? It is unclear to me whether that will make
a functional difference.

[varkha, 2017/03/23 00:00:31]

Removing service_path_ was conscious choice - see
https://codereview.chromium.org/2698473007. I have changed the code to not cache
the service_path and fetch it instead from guid. It would probably be better to
plumb through NetworkConnect::DisconnectFromNetworkId or something like that but
it would make possible merge more difficult. Adding stevenjb@ as a reviewer to
take a look if that is OK and maybe consider adding that interface for network
group.

 }
 
+void VPNListNetworkEntry::ButtonPressed(Button* sender,
+                                        const ui::Event& event) {
+  if (sender != disconnect_button_) {
+    VPNListEntryBase::ButtonPressed(sender, event);
+    return;
+  }
+
+  WmShell::Get()->RecordUserMetricsAction(
+      UMA_STATUS_AREA_VPN_DISCONNECT_CLICKED);
+  chromeos::NetworkHandler::Get()
+      ->network_connection_handler()
+      ->DisconnectNetwork(service_path_, base::Bind(&base::DoNothing),
+                          base::Bind(&IgnoreDisconnectError));
+}
+
 void VPNListNetworkEntry::UpdateFromNetworkState(
     const chromeos::NetworkState* network) {
   if (network && network->IsConnectingState())
     network_icon::NetworkIconAnimation::GetInstance()->AddObserver(this);
   else
     network_icon::NetworkIconAnimation::GetInstance()->RemoveObserver(this);
 
   if (!network) {
     // This is a transient state where the network has been removed already but
     // the network list in the UI has not been updated yet.
@@ skipping 227 matching lines @@
     }
   }
 
   // Add providers without any configured networks, in the order that the
   // providers were returned by the extensions system.
   for (const VPNProvider& provider : providers)
     AddProviderAndNetworks(provider, networks);
 }
 
 }  // namespace ash