Reland: chromeos: Convert ash VPNDelegate interface to mojo

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

Index: ash/common/system/chromeos/network/vpn_list.cc
diff --git a/ash/common/system/chromeos/network/vpn_delegate.cc b/ash/common/system/chromeos/network/vpn_list.cc
similarity index 60%
rename from ash/common/system/chromeos/network/vpn_delegate.cc
rename to ash/common/system/chromeos/network/vpn_list.cc
index 44fa49c969884a365c90168c7a4e830e77f7a35b..2d934df162ed42654a1f6e0fd90ad13bcd9717d6 100644
--- a/ash/common/system/chromeos/network/vpn_delegate.cc
+++ b/ash/common/system/chromeos/network/vpn_list.cc
@@ -2,10 +2,13 @@
 // 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_delegate.h"
+#include "ash/common/system/chromeos/network/vpn_list.h"
+
+#include <utility>
 
 #include "ash/common/strings/grit/ash_strings.h"
 #include "chromeos/network/network_state.h"
+#include "services/service_manager/public/cpp/connector.h"
 #include "third_party/cros_system_api/dbus/service_constants.h"
 #include "ui/base/l10n/l10n_util.h"
 
@@ -39,15 +42,15 @@ bool VPNProvider::operator==(const VPNProvider& other) const {
   return key == other.key && name == other.name;
 }
 
-VPNDelegate::Observer::~Observer() {}
+VpnList::Observer::~Observer() {}
 
-VPNDelegate::VPNDelegate() {
+VpnList::VpnList() {
   AddBuiltInProvider();
 }
 
-VPNDelegate::~VPNDelegate() {}
+VpnList::~VpnList() {}
 
-bool VPNDelegate::HaveThirdPartyVPNProviders() const {
+bool VpnList::HaveThirdPartyVPNProviders() const {
   for (const VPNProvider& provider : vpn_providers_) {
     if (provider.key.third_party)
       return true;
@@ -55,28 +58,53 @@ bool VPNDelegate::HaveThirdPartyVPNProviders() const {
   return false;
 }
 
-void VPNDelegate::AddObserver(Observer* observer) {
+void VpnList::ShowAddPage(const VPNProvider::Key& key) {

[stevenjb, 2016/11/19 00:04:58]

This is kind of an odd method to have in a class named simply "VpnList".

I'd actually rather see the mojo call on the same interface that we use to show
Settings. (Ultimately all networking UI is going to be attached to Settings
anyway). Does that seem reasonable?

+  // May not be bound in tests or when running without Chrome.
+  if (!vpn_list_client_.is_bound())
+    return;
+
+  if (key.third_party)
+    vpn_list_client_->ShowThirdPartyAddNetworkPage(key.extension_id);
+  else
+    vpn_list_client_->ShowBuiltInAddNetworkPage();
+}
+
+void VpnList::AddObserver(Observer* observer) {
   observer_list_.AddObserver(observer);
 }
 
-void VPNDelegate::RemoveObserver(Observer* observer) {
+void VpnList::RemoveObserver(Observer* observer) {
   observer_list_.RemoveObserver(observer);
 }
 
-void VPNDelegate::SetThirdPartyVpnProviders(
-    const std::vector<VPNProvider>& providers) {
+void VpnList::BindRequest(mojom::VpnListRequest request) {
+  bindings_.AddBinding(this, std::move(request));
+}
+
+void VpnList::SetClient(mojom::VpnListClientPtr client) {
+  vpn_list_client_ = std::move(client);
+}
+
+void VpnList::SetThirdPartyVpnProviders(
+    std::vector<mojom::ThirdPartyVpnProviderPtr> providers) {
   // Reset the list with the extension-backed providers.
-  vpn_providers_ = providers;
+  vpn_providers_.clear();
+  vpn_providers_.reserve(providers.size() + 1);
+  for (const auto& provider : providers) {
+    VPNProvider::Key key(provider->extension_id);
+    vpn_providers_.push_back(VPNProvider(key, provider->name));
+  }
+  // Add the OpenVPN provider.
   AddBuiltInProvider();
   NotifyObservers();
 }
 
-void VPNDelegate::NotifyObservers() {
+void VpnList::NotifyObservers() {
   for (auto& observer : observer_list_)
     observer.OnVPNProvidersChanged();
 }
 
-void VPNDelegate::AddBuiltInProvider() {
+void VpnList::AddBuiltInProvider() {
   // The VPNProvider::Key() constructor generates a key that identifies that
   // built-in provider and has no extension ID.
   vpn_providers_.push_back(VPNProvider(