Add ash::VPNDelegate and Chrome OS implementation

ash/system/chromeos/network/vpn_delegate.h

+// 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.
+
+#ifndef ASH_SYSTEM_CHROMEOS_NETWORK_VPN_DELEGATE_H
+#define ASH_SYSTEM_CHROMEOS_NETWORK_VPN_DELEGATE_H
+
+#include <string>
+#include <vector>
+
+#include "ash/ash_export.h"
+#include "base/macros.h"
+#include "base/observer_list.h"
+
+namespace ash {
+
+// Describes a VPN provider.
+struct ASH_EXPORT VPNProvider {
+  VPNProvider(const std::string& name, const std::string& id);
+  ~VPNProvider();
+
+  // Human-readable name.
+  const std::string name;
+  // Opaque ID. The empty string is also a valid ID.

[stevenjb, 2015/03/11 16:52:52]

The empty string is used to indicate a built-in VPN, right? We should indicate
that, 'valid ID' is a bit confusing.

[bartfab (slow), 2015/03/11 17:27:04]

I wanted to make it as opaque as possible. I documented that the ID may be empty
but refrained from explaining what it stands for. In theory, the delegate's
consumers should not need to know or care. In practice, there is one spot in ash
where I explicitly look for an empty ID, but that is just for UMA logging. If we
moved the logging into the delegate, we could make the ID truly opaque. I would
still want to document that the ID may be empty though, to make sure it does not
trip up anyone's code.

[stevenjb, 2015/03/11 18:23:02]

I don't think "as opaque as possible" is a good objective :)

Seriously though, while I understand your point, I think we should define the
built-in type kOpenVpnOrL2TP here to make this as clear as possible and solve
the problem of where to define the shared constant. I don't see any problem with
defining that in the Delegate. That both makes this more clear and allows for an
easy transition to separating kOpenVpnOrL2TP into two types later.

[bartfab (slow), 2015/03/11 18:27:14]

The NetworkState annotates each VPN network with its provider ID. So we would
need to put a constant in a place where this code, ash and NetworkState can
access it. Since NetworkState lives in chromeos/network, I cannot think of any
such place.

[stevenjb, 2015/03/11 18:36:23]

I don't understand. id is populated in vpn_delegate_chromeos.cc:

 vpn_providers_.push_back(ash::VPNProvider(
        l10n_util::GetStringUTF8(IDS_NETWORK_VPN_BUILT_IN_PROVIDER),
        std::string()));

You would just need to replace std::string with kOpenVpnOrL2TP, it doesn't need
to have anything to do with NetworkState.

[bartfab (slow), 2015/03/11 22:36:26]

For networks using a third-party VPN provider, I need to correlate the network
with the corresponding provider, based on the ID. So VPNDelegate and
NetworkState need to be using the same ID schema. In my current implementation,
I do the same thing for networks using the built-in VPN support. I simply set
the ID to "" in VPNDelegate and NetworkState, creating a correlation between the
two.

I can change it so that for the built-in VPN support, I look at the network type
instead. This will complicate the UI code somewhat as it will no longer be able
to just compare IDs. But it is doable if you feel strongly.

+  const std::string id;
+};
+
+// This delegate provides UI code in ash, e.g. |VPNList|, with access to the
+// list of VPN providers enabled in the primary user's profile. The delegate
+// furthermore allows the UI code to request that a VPN provider show its "add
+// network" dialog.
+class ASH_EXPORT VPNDelegate {
+ public:
+  // An observer that is notified whenever the list of VPN providers enabled in
+  // the primary user's profile changes.
+  class Observer {
+   public:
+    virtual void OnVPNProvidersChanged() = 0;
+
+   protected:
+    virtual ~Observer();
+
+   private:
+    DISALLOW_ASSIGN(Observer);
+  };
+
+  VPNDelegate();
+  virtual ~VPNDelegate();
+
+  // Returns |true| if at least one third-party VPN provider is enabled in the
+  // primary user's profile, in addition to the built-in OpenVPN/L2TP provider.
+  virtual bool HaveThirdPartyVPNProviders() const = 0;
+
+  // Returns the list of VPN providers enabled in the primary user's profile.
+  virtual const std::vector<VPNProvider>& GetVPNProviders() const = 0;
+
+  // Requests that the VPN provider identified by |id| show its "add network"
+  // dialog.
+  virtual void ShowAddPage(const std::string& id) = 0;
+
+  void AddObserver(Observer* observer);
+  void RemoveObserver(Observer* observer);
+
+ protected:
+  // Notify observers that the list of VPN providers enabled in the primary
+  // user's profile has changed.
+  void NotifyObservers();
+
+ private:
+  ObserverList<Observer, true> observer_list_;
+
+  DISALLOW_COPY_AND_ASSIGN(VPNDelegate);
+};
+
+}  // namespace ash
+
+#endif  // ASH_SYSTEM_CHROMEOS_NETWORK_VPN_DELEGATE_H