Reland: chromeos: Convert ash VPNDelegate interface to mojo

chrome/browser/ui/ash/vpn_list_client.cc

Index: chrome/browser/ui/ash/vpn_list_client.cc
diff --git a/chrome/browser/ui/ash/vpn_delegate_chromeos.cc b/chrome/browser/ui/ash/vpn_list_client.cc
similarity index 57%
rename from chrome/browser/ui/ash/vpn_delegate_chromeos.cc
rename to chrome/browser/ui/ash/vpn_list_client.cc
index 53936d1249bb9386f3b531e3b1ac8dc4a090de01..08505e0c5d8f31829cd05b1a3cb9d91f6e50cad5 100644
--- a/chrome/browser/ui/ash/vpn_delegate_chromeos.cc
+++ b/chrome/browser/ui/ash/vpn_list_client.cc
@@ -2,8 +2,9 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "chrome/browser/ui/ash/vpn_delegate_chromeos.h"
+#include "chrome/browser/ui/ash/vpn_list_client.h"
 
+#include "ash/public/interfaces/vpn_list.mojom.h"
 #include "base/bind.h"
 #include "base/location.h"
 #include "base/logging.h"
@@ -11,11 +12,13 @@
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/chromeos/profiles/profile_helper.h"
 #include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/ui/ash/ash_util.h"
 #include "chrome/browser/ui/ash/system_tray_client.h"
 #include "components/user_manager/user.h"
 #include "components/user_manager/user_manager.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/notification_source.h"
+#include "content/public/common/service_manager_connection.h"
 #include "extensions/browser/api/vpn_provider/vpn_service.h"
 #include "extensions/browser/api/vpn_provider/vpn_service_factory.h"
 #include "extensions/browser/extension_registry.h"
@@ -23,6 +26,7 @@
 #include "extensions/common/extension_set.h"
 #include "extensions/common/permissions/api_permission.h"
 #include "extensions/common/permissions/permissions_data.h"
+#include "services/service_manager/public/cpp/connector.h"
 #include "third_party/cros_system_api/dbus/shill/dbus-constants.h"
 #include "ui/base/l10n/l10n_util.h"
 
@@ -44,7 +48,11 @@ Profile* GetProfileForPrimaryUser() {
 
 }  // namespace
 
-VPNDelegateChromeOS::VPNDelegateChromeOS() : weak_factory_(this) {
+VpnListClient::VpnListClient() : binding_(this), weak_factory_(this) {
+  // Set this object as the VpnListClient service for ash.
+  ash::mojom::VpnListPtr vpn_list = ConnectToVpnList();
+  vpn_list->SetClient(binding_.CreateInterfacePtrAndBind());
+
   if (user_manager::UserManager::Get()->GetPrimaryUser()) {
     // If a user is logged in, start observing the primary user's extension
     // registry immediately.
@@ -57,36 +65,33 @@ VPNDelegateChromeOS::VPNDelegateChromeOS() : weak_factory_(this) {
   }
 }
 
-VPNDelegateChromeOS::~VPNDelegateChromeOS() {
+VpnListClient::~VpnListClient() {
   if (extension_registry_)
     extension_registry_->RemoveObserver(this);
 }
 
-void VPNDelegateChromeOS::ShowAddPage(const ash::VPNProvider::Key& key) {
-  if (!key.third_party) {
-    // Show the "add network" dialog for the built-in OpenVPN/L2TP provider.
-    SystemTrayClient::Get()->ShowNetworkCreate(shill::kTypeVPN);
-    return;
-  }
+void VpnListClient::ShowBuiltInAddNetworkPage() {
+  SystemTrayClient::Get()->ShowNetworkCreate(shill::kTypeVPN);
+}
 
+void VpnListClient::ShowThirdPartyAddNetworkPage(
+    const std::string& extension_id) {
   Profile* const profile = GetProfileForPrimaryUser();
   if (!profile)
     return;
 
-  // Request that the third-party VPN provider identified by |key.extension_id|
-  // show its "add network" dialog.
+  // Request that the third-party VPN provider show its "add network" dialog.
   chromeos::VpnServiceFactory::GetForBrowserContext(profile)
-      ->SendShowAddDialogToExtension(key.extension_id);
+      ->SendShowAddDialogToExtension(extension_id);
 }
 
-void VPNDelegateChromeOS::OnExtensionLoaded(
-    content::BrowserContext* browser_context,
-    const extensions::Extension* extension) {
+void VpnListClient::OnExtensionLoaded(content::BrowserContext* browser_context,
+                                      const extensions::Extension* extension) {
   if (IsVPNProvider(extension))
     UpdateVPNProviders();
 }
 
-void VPNDelegateChromeOS::OnExtensionUnloaded(
+void VpnListClient::OnExtensionUnloaded(
     content::BrowserContext* browser_context,
     const extensions::Extension* extension,
     extensions::UnloadedExtensionInfo::Reason reason) {
@@ -94,15 +99,15 @@ void VPNDelegateChromeOS::OnExtensionUnloaded(
     UpdateVPNProviders();
 }
 
-void VPNDelegateChromeOS::OnShutdown(extensions::ExtensionRegistry* registry) {
+void VpnListClient::OnShutdown(extensions::ExtensionRegistry* registry) {
   DCHECK(extension_registry_);
   extension_registry_->RemoveObserver(this);
   extension_registry_ = nullptr;
 }
 
-void VPNDelegateChromeOS::Observe(int type,
-                                  const content::NotificationSource& source,
-                                  const content::NotificationDetails& details) {
+void VpnListClient::Observe(int type,
+                            const content::NotificationSource& source,
+                            const content::NotificationDetails& details) {
   DCHECK_EQ(chrome::NOTIFICATION_PROFILE_CREATED, type);
   const Profile* const profile = content::Source<Profile>(source).ptr();
   if (!chromeos::ProfileHelper::Get()->IsPrimaryProfile(profile)) {
@@ -119,26 +124,53 @@ void VPNDelegateChromeOS::Observe(int type,
   registrar_.RemoveAll();
   base::ThreadTaskRunnerHandle::Get()->PostTask(
       FROM_HERE,
-      base::Bind(&VPNDelegateChromeOS::AttachToPrimaryUserExtensionRegistry,
+      base::Bind(&VpnListClient::AttachToPrimaryUserExtensionRegistry,
                  weak_factory_.GetWeakPtr()));
 }
 
-void VPNDelegateChromeOS::UpdateVPNProviders() {
+ash::mojom::VpnListPtr VpnListClient::ConnectToVpnList() {
+  ash::mojom::VpnListPtr vpn_list;
+  service_manager::Connector* connector =
+      content::ServiceManagerConnection::GetForProcess()->GetConnector();
+  // Under mash the VpnList interface is in the ash process. In classic ash
+  // we provide it to ourself.
+  if (chrome::IsRunningInMash())
+    connector->ConnectToInterface("ash", &vpn_list);
+  else
+    connector->ConnectToInterface("content_browser", &vpn_list);
+  return vpn_list;
+}
+
+void VpnListClient::UpdateVPNProviders() {
   DCHECK(extension_registry_);
 
-  std::vector<ash::VPNProvider> third_party_providers;
+  std::vector<ash::mojom::ThirdPartyVpnProviderPtr> third_party_providers;
   for (const auto& extension : extension_registry_->enabled_extensions()) {
-    if (IsVPNProvider(extension.get())) {
-      third_party_providers.push_back(ash::VPNProvider(
-          ash::VPNProvider::Key(extension->id()), extension->name()));
-    }
+    if (!IsVPNProvider(extension.get()))
+      continue;
+
+    ash::mojom::ThirdPartyVpnProviderPtr provider =
+        ash::mojom::ThirdPartyVpnProvider::New();
+    provider->name = extension->name();
+    provider->extension_id = extension->id();
+    third_party_providers.push_back(std::move(provider));
   }
 
-  // Ash adds the built-in OpenVPN/L2TP provider.
-  SetThirdPartyVpnProviders(third_party_providers);
+  // Ash starts without any third-party providers. If we've never sent one then
+  // there's no need to send an empty list. This case commonly occurs on startup
+  // when the user has no third-party VPN extensions installed.
+  if (!sent_providers_ && third_party_providers.empty())
+    return;
+
+  // It's rare to install or remove VPN provider extensions, so don't bother
+  // caching the interface pointer between calls to this function.
+  ash::mojom::VpnListPtr vpn_list = ConnectToVpnList();
+  vpn_list->SetThirdPartyVpnProviders(std::move(third_party_providers));

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

Also, if we eliminate the UI calls, we wouldn't need to invert the relationship,
i.e. this class could be the server and the UI can request the list and listen
to updates. If we do it this way then aren't we assuming that the client already
exists?

[James Cook, 2016/11/19 00:49:18]

Dunno if this was clear in my other CL, but there's one other reason to invert
the relationship: Ash needs to be able to decide, synchronously when the system
tray menu opens, whether or not to show the row for VPNs row. So it needs to
have the list of VPNs before that, otherwise we have to wait for an IPC
round-trip to chrome (which might have a busy UI thread) before we can spawn the
menu. Regardless of how we think about client and server, ash still needs a
master list, and still needs updates from chrome.

I think it's reasonable to assume that the client (ash) always exists when
chrome starts up. That's consistent with how Chrome runs on other platforms --
the "OS" provides system UI, and chrome pokes into it to do stuff like show dock
icons.

+
+  sent_providers_ = true;
 }
 
-void VPNDelegateChromeOS::AttachToPrimaryUserExtensionRegistry() {
+void VpnListClient::AttachToPrimaryUserExtensionRegistry() {
   DCHECK(!extension_registry_);
   extension_registry_ =
       extensions::ExtensionRegistry::Get(GetProfileForPrimaryUser());