Extend vpnProvider to allow reconnections

Upcoming changes in the Chrome OS networking daemon (shill) will allow
third party VPNs to transition "backwards" from Online->Configuring if
the default physical connection changes.  Add a "reconnect" flag so
that VPN apps can signal their compatibility with this new scheme,
and add the necessary UI changes so that Chrome can identify
reconnections and present them to the user in a sensible way.

Also, change the UI so that users can cancel VPN reconnections
(and connections) using the "Disconnect" button.

BUG=514343

Review URL: https://codereview.chromium.org/1722453002

Cr-Commit-Position: refs/heads/master@{#381566}

11 files changed, 85 insertions, 13 deletions

diff --git a/ash/system/chromeos/network/vpn_list_view.cc b/ash/system/chromeos/network/vpn_list_view.cc
index a80853b..ed130be 100644
--- a/ash/system/chromeos/network/vpn_list_view.cc
+++ b/ash/system/chromeos/network/vpn_list_view.cc
@@ -210,7 +210,7 @@ void VPNListNetworkEntry::UpdateFromNetworkState(
                   ui::network_icon::GetLabelForNetwork(
                       network, ui::network_icon::ICON_TYPE_LIST),
                   IsConnectedOrConnecting(network));
-  if (network->IsConnectedState()) {
+  if (IsConnectedOrConnecting(network)) {
     disconnect_button_ = new DisconnectButton(this);
     AddChildView(disconnect_button_);
     SetBorder(
diff --git a/chrome/test/data/extensions/api_test/vpn_provider/basic.js b/chrome/test/data/extensions/api_test/vpn_provider/basic.js
index 15d41aa..c8615b7 100644
--- a/chrome/test/data/extensions/api_test/vpn_provider/basic.js
+++ b/chrome/test/data/extensions/api_test/vpn_provider/basic.js
@@ -212,7 +212,8 @@ var testRoutines = {
           mtu: "1600",
           broadcastAddress: "10.10.10.255",
           domainSearch: ["foo", "bar"],
-          dnsServers: ["8.8.8.8"]
+          dnsServers: ["8.8.8.8"],
+          reconnect: "false"
         };
         chrome.vpnProvider.setParameters(params, onSetParameterComplete);
       }
diff --git a/chromeos/dbus/shill_third_party_vpn_driver_client.cc b/chromeos/dbus/shill_third_party_vpn_driver_client.cc
index a3f3f7b..63b1851 100644
--- a/chromeos/dbus/shill_third_party_vpn_driver_client.cc
+++ b/chromeos/dbus/shill_third_party_vpn_driver_client.cc
@@ -27,7 +27,8 @@ const char* kSetParametersKeyList[] = {
     shill::kSubnetPrefixParameterThirdPartyVpn,
     shill::kMtuParameterThirdPartyVpn,
     shill::kDomainSearchParameterThirdPartyVpn,
-    shill::kDnsServersParameterThirdPartyVpn};
+    shill::kDnsServersParameterThirdPartyVpn,
+    shill::kReconnectParameterThirdPartyVpn};
 
 // The ShillThirdPartyVpnDriverClient implementation.
 class ShillThirdPartyVpnDriverClientImpl
diff --git a/chromeos/network/network_state.cc b/chromeos/network/network_state.cc
index 6c022bf..df5f825 100644
--- a/chromeos/network/network_state.cc
+++ b/chromeos/network/network_state.cc
@@ -90,7 +90,14 @@ bool NetworkState::PropertyChanged(const std::string& key,
   if (key == shill::kSignalStrengthProperty) {
     return GetIntegerValue(key, value, &signal_strength_);
   } else if (key == shill::kStateProperty) {
-    return GetStringValue(key, value, &connection_state_);
+    std::string saved_state = connection_state_;
+    if (GetStringValue(key, value, &connection_state_)) {
+      if (connection_state_ != saved_state)
+        last_connection_state_ = saved_state;
+      return true;
+    } else {
+      return false;
+    }
   } else if (key == shill::kVisibleProperty) {
     return GetBooleanValue(key, value, &visible_);
   } else if (key == shill::kConnectableProperty) {
@@ -350,6 +357,11 @@ bool NetworkState::IsConnectingState() const {
   return visible() && StateIsConnecting(connection_state_);
 }
 
+bool NetworkState::IsReconnecting() const {
+  return visible() && StateIsConnecting(connection_state_) &&
+         StateIsConnected(last_connection_state_);
+}
+
 bool NetworkState::IsInProfile() const {
   // kTypeEthernetEap is always saved. We need this check because it does
   // not show up in the visible list, but its properties may not be available
diff --git a/chromeos/network/network_state.h b/chromeos/network/network_state.h
index 6128a62..24bb8c0 100644
--- a/chromeos/network/network_state.h
+++ b/chromeos/network/network_state.h
@@ -107,6 +107,10 @@ class CHROMEOS_EXPORT NetworkState : public ManagedState {
   bool IsConnectedState() const;
   bool IsConnectingState() const;
 
+  // Returns true if |last_connection_state_| is connected, and
+  // |connection_state_| is connecting.
+  bool IsReconnecting() const;
+
   // Returns true if this is a network stored in a profile.
   bool IsInProfile() const;
 
@@ -161,6 +165,7 @@ class CHROMEOS_EXPORT NetworkState : public ManagedState {
   std::string device_path_;
   std::string guid_;
   std::string connection_state_;
+  std::string last_connection_state_;
   std::string profile_path_;
   std::vector<uint8_t> raw_ssid_;  // Unknown encoding. Not necessarily UTF-8.
   int priority_ = 0;
diff --git a/extensions/browser/api/vpn_provider/vpn_provider_api.cc b/extensions/browser/api/vpn_provider/vpn_provider_api.cc
index 3529f63..601835d 100644
--- a/extensions/browser/api/vpn_provider/vpn_provider_api.cc
+++ b/extensions/browser/api/vpn_provider/vpn_provider_api.cc
@@ -143,6 +143,11 @@ void ConvertParameters(const api_vpn::Parameters& parameters,
       shill::kDnsServersParameterThirdPartyVpn,
       base::JoinString(parameters.dns_servers, ip_delimiter));
 
+  if (parameters.reconnect) {
+    parameter_value->SetStringWithoutPathExpansion(
+        shill::kReconnectParameterThirdPartyVpn, *parameters.reconnect);
+  }
+
   return;
 }
 
diff --git a/extensions/browser/api/vpn_provider/vpn_service.cc b/extensions/browser/api/vpn_provider/vpn_service.cc
index 7293f70..09452d5 100644
--- a/extensions/browser/api/vpn_provider/vpn_service.cc
+++ b/extensions/browser/api/vpn_provider/vpn_service.cc
@@ -114,8 +114,13 @@ void VpnService::VpnConfiguration::OnPlatformMessage(uint32_t message) {
 
   api_vpn::PlatformMessage platform_message =
       static_cast<api_vpn::PlatformMessage>(message);
-  vpn_service_->SetActiveConfiguration(
-      platform_message == api_vpn::PLATFORM_MESSAGE_CONNECTED ? this : nullptr);
+
+  if (platform_message == api_vpn::PLATFORM_MESSAGE_CONNECTED) {
+    vpn_service_->SetActiveConfiguration(this);
+  } else if (platform_message == api_vpn::PLATFORM_MESSAGE_DISCONNECTED ||
+             platform_message == api_vpn::PLATFORM_MESSAGE_ERROR) {
+    vpn_service_->SetActiveConfiguration(nullptr);
+  }
 
   // TODO(kaliamoorthi): Update the lower layers to get the error message and
   // pass in the error instead of std::string().
diff --git a/extensions/common/api/vpn_provider.idl b/extensions/common/api/vpn_provider.idl
index a826522..75bd873 100644
--- a/extensions/common/api/vpn_provider.idl
+++ b/extensions/common/api/vpn_provider.idl
@@ -40,6 +40,19 @@ namespace vpnProvider {
     DOMString[]? domainSearch;
     // A list of IPs for the DNS servers.
     DOMString[] dnsServers;
+    // Whether or not the VPN extension implements auto-reconnection.
+    //
+    // If true, the <code>linkDown</code>, <code>linkUp</code>,
+    // <code>linkChanged</code>, <code>suspend</code>, and <code>resume</code>
+    // platform messages will be used to signal the respective events.
+    // If false, the system will forcibly disconnect the VPN if the network
+    // topology changes, and the user will need to reconnect manually.
+    // (default: false)
+    //
+    // This property is new in Chrome 51; it will generate an exception in
+    // earlier versions. try/catch can be used to conditionally enable the
+    // feature based on browser support.
+    DOMString? reconnect;
   };
 
   // The enum is used by the platform to notify the client of the VPN session
@@ -52,7 +65,20 @@ namespace vpnProvider {
     // An error occurred in VPN connection, for example a timeout. A description
     // of the error is give as the <ahref="#property-onPlatformMessage-error">
     // error argument to onPlatformMessage</a>.
-    error
+    error,
+    // The default physical network connection is down.
+    linkDown,
+    // The default physical network connection is back up.
+    linkUp,
+    // The default physical network connection changed, e.g. wifi-&gt;mobile.
+    linkChanged,
+    // The OS is preparing to suspend, so the VPN should drop its connection.
+    // The extension is not guaranteed to receive this event prior to
+    // suspending.
+    suspend,
+    // The OS has resumed and the user has logged back in, so the VPN should
+    // try to reconnect.
+    resume
   };
 
   // The enum is used by the VPN client to inform the platform
diff --git a/ui/chromeos/network/network_icon.cc b/ui/chromeos/network/network_icon.cc
index b1785d6..395bd62 100644
--- a/ui/chromeos/network/network_icon.cc
+++ b/ui/chromeos/network/network_icon.cc
@@ -741,7 +741,12 @@ base::string16 GetLabelForNetwork(const chromeos::NetworkState* network,
   DCHECK(network);
   std::string activation_state = network->activation_state();
   if (icon_type == ICON_TYPE_LIST) {
-    // Show "<network>: [Connecting|Activating]..."
+    // Show "<network>: [Connecting|Activating|Reconnecting]..."
+    if (network->IsReconnecting()) {
+      return l10n_util::GetStringFUTF16(
+          IDS_ASH_STATUS_TRAY_NETWORK_LIST_RECONNECTING,
+          base::UTF8ToUTF16(network->name()));
+    }
     if (network->IsConnectingState()) {
       return l10n_util::GetStringFUTF16(
           IDS_ASH_STATUS_TRAY_NETWORK_LIST_CONNECTING,
@@ -760,7 +765,13 @@ base::string16 GetLabelForNetwork(const chromeos::NetworkState* network,
           base::UTF8ToUTF16(network->name()));
     }
   } else {
-    // Show "[Connected to|Connecting to|Activating] <network>" (non-list view).
+    // Show "[Connected to|Connecting to|Activating|Reconnecting to] <network>"
+    // (non-list view).
+    if (network->IsReconnecting()) {
+      return l10n_util::GetStringFUTF16(
+          IDS_ASH_STATUS_TRAY_NETWORK_RECONNECTING,
+          base::UTF8ToUTF16(network->name()));
+    }
     if (network->IsConnectedState()) {
       return l10n_util::GetStringFUTF16(IDS_ASH_STATUS_TRAY_NETWORK_CONNECTED,
                                         base::UTF8ToUTF16(network->name()));
@@ -827,10 +838,10 @@ void GetDefaultNetworkImageAndLabel(IconType icon_type,
 
   const NetworkState* network;
   // If we are connecting to a network, and there is either no connected
-  // network, or the connection was user requested, use the connecting
-  // network.
+  // network, or the connection was user requested, or shill triggered a
+  // reconnection, use the connecting network.
   if (connecting_network &&
-      (!connected_network ||
+      (!connected_network || connecting_network->IsReconnecting() ||
        connect_handler->HasConnectingNetwork(connecting_network->path()))) {
     network = connecting_network;
   } else {
diff --git a/ui/chromeos/network/network_state_notifier.cc b/ui/chromeos/network/network_state_notifier.cc
index af5a2b8..4394cdd 100644
--- a/ui/chromeos/network/network_state_notifier.cc
+++ b/ui/chromeos/network/network_state_notifier.cc
@@ -197,7 +197,7 @@ bool NetworkStateNotifier::UpdateDefaultNetwork(const NetworkState* network) {
 
 void NetworkStateNotifier::UpdateVpnConnectionState(const NetworkState* vpn) {
   if (vpn->path() == connected_vpn_) {
-    if (!vpn->IsConnectedState()) {
+    if (!vpn->IsConnectedState() && !vpn->IsConnectingState()) {
       ShowVpnDisconnectedNotification(vpn);
       connected_vpn_.clear();
     }
diff --git a/ui/chromeos/ui_chromeos_strings.grd b/ui/chromeos/ui_chromeos_strings.grd
index 67f0971..f49bc36 100644
--- a/ui/chromeos/ui_chromeos_strings.grd
+++ b/ui/chromeos/ui_chromeos_strings.grd
@@ -138,6 +138,9 @@
       <message name="IDS_ASH_STATUS_TRAY_NETWORK_CONNECTING" desc="Message for the network tray tooltip and network list when connecting to a network.">
         Connecting to <ph name="NAME">$1<ex>GoogleGuest</ex></ph>
       </message>
+      <message name="IDS_ASH_STATUS_TRAY_NETWORK_RECONNECTING" desc="Message for the network tray tooltip and network list when reconnecting to a network.">
+        Reconnecting to <ph name="NAME">$1<ex>Company VPN</ex></ph>
+      </message>
       <message name="IDS_ASH_STATUS_TRAY_NETWORK_LIST_ACTIVATE" desc="Message for the network list to activate the network.">
         Activate <ph name="NETWORKSERVICE">$1<ex>YBH Cellular</ex></ph>
       </message>
@@ -147,6 +150,9 @@
       <message name="IDS_ASH_STATUS_TRAY_NETWORK_LIST_CONNECTING" desc="Message for the network list when connecting to a network.">
         <ph name="NAME">$1<ex>GoogleGuest</ex></ph>: Connecting...
       </message>
+      <message name="IDS_ASH_STATUS_TRAY_NETWORK_LIST_RECONNECTING" desc="Message for the network list when reconnecting to a network.">
+        <ph name="NAME">$1<ex>Company VPN</ex></ph>: Reconnecting...
+      </message>
       <message name="IDS_ASH_STATUS_TRAY_NETWORK_NOT_CONNECTED" desc="Description in status area or network list when no network is connected.">
         No network
       </message>