arc: Provide a per-service getter for ArcServiceManager

This change adds getters for individual ArcServices from
ArcServiceManager.  This prevents ArcServiceManager::Observer from
becoming a place to put random stuff to observe that would be better
suited for individual services to expose.

BUG=672840
TEST=git cl try

Review-Url: https://codereview.chromium.org/2622843002
Cr-Commit-Position: refs/heads/master@{#442934}

components/arc/BUILD.gn

@@ -193,6 +193,7 @@ static_library("arc_test_support") {
 source_set("unit_tests") {
   testonly = true
   sources = [
+    "arc_service_manager_unittest.cc",
     "arc_session_runner_unittest.cc",
     "bluetooth/arc_bluetooth_bridge_unittest.cc",
     "bluetooth/bluetooth_struct_traits_unittest.cc",

components/arc/arc_service_manager.cc

@@ -4,8 +4,6 @@
 
 #include "components/arc/arc_service_manager.h"
 
-#include <utility>
-
 #include "base/logging.h"
 #include "base/memory/ptr_util.h"
 #include "base/task_runner.h"
@@ -83,9 +81,32 @@ ArcBridgeService* ArcServiceManager::arc_bridge_service() {
   return arc_bridge_service_.get();
 }
 
-void ArcServiceManager::AddService(std::unique_ptr<ArcService> service) {
+bool ArcServiceManager::AddServiceInternal(
+    const std::string& name,
+    std::unique_ptr<ArcService> service) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  if (!name.empty() && services_.count(name) != 0) {
+    LOG(ERROR) << "Ignoring registration of service with duplicate name: "
+               << name;
+    return false;
+  }
+  services_.insert(std::make_pair(name, std::move(service)));
+  return true;
+}
+
+ArcService* ArcServiceManager::GetNamedServiceInternal(
+    const std::string& name) {
   DCHECK(thread_checker_.CalledOnValidThread());
-  services_.emplace_back(std::move(service));
+  if (name.empty()) {
+    LOG(ERROR) << "kArcServiceName[] should be a fully-qualified class name.";
+    return nullptr;
+  }
+  auto service = services_.find(name);
+  if (service == services_.end()) {
+    LOG(ERROR) << "Named service " << name << " not found";
+    return nullptr;
+  }
+  return service->second.get();
 }
 
 void ArcServiceManager::AddObserver(Observer* observer) {

components/arc/arc_service_manager.h

@@ -6,6 +6,10 @@
 #define COMPONENTS_ARC_ARC_SERVICE_MANAGER_H_
 
 #include <memory>
+#include <string>
+#include <type_traits>
+#include <unordered_map>
+#include <utility>
 #include <vector>
 
 #include "base/macros.h"
@@ -22,6 +26,40 @@ class ArcBridgeService;
 class ArcIntentHelperObserver;
 class ArcService;
 
+namespace internal {
+
+// If an ArcService is declared with a name, e.g.:
+//
+// class MyArcService : public ArcService {
+//  public:
+//   static const char kArcServiceName[];
+//   ...
+// };
+//
+// it can then be retrieved from ArcServiceManager in a type-safe way using
+// GetService<T>(). This two functions allow AddService() to get the name only
+// if it was provided, or use an empty string otherwise.
+//
+// Although the typename is always specified explicitly by the caller, the
+// parameter is required in order for SFINAE to work correctly.  It is not used
+// and can be nullptr, though.
+//
+// In order to avoid collisions, kArcServiceName should be the fully-qualified
+// name of the class.
+template <typename T>
+decltype(T::kArcServiceName, std::string()) GetArcServiceName(T* unused) {
+  if (strlen(T::kArcServiceName) == 0)
+    LOG(ERROR) << "kArcServiceName[] should be a fully-qualified class name.";
+  return T::kArcServiceName;
+}
+
+template <typename T>
+std::string GetArcServiceName(...) {
+  return std::string();
+}
+
+}  // namespace internal
+
 // Manages creation and destruction of services that communicate with the ARC
 // instance via the ArcBridgeService.
 class ArcServiceManager {
@@ -43,8 +81,22 @@ class ArcServiceManager {
   // class was created on.
   ArcBridgeService* arc_bridge_service();
 
-  // Adds a service to the managed services list.
-  void AddService(std::unique_ptr<ArcService> service);
+  // Adds a service to the managed services list. Returns false if another
+  // named service with that name had already been added.
+  template <typename T>
+  bool AddService(std::unique_ptr<T> service) {
+    return AddServiceInternal(internal::GetArcServiceName<T>(nullptr),
+                              std::move(service));
+  }
+
+  // Gets the named service from the managed services list. This uses SFINAE, so
+  // you can only call this function if the service specified by T provides a
+  // static member variable called kArcServiceName[] (otherwise this will not
+  // compile).
+  template <typename T>
+  T* GetService() {
+    return static_cast<T*>(GetNamedServiceInternal(T::kArcServiceName));
+  }
 
   // Gets the global instance of the ARC Service Manager. This can only be
   // called on the thread that this class was created on.
@@ -82,6 +134,11 @@ class ArcServiceManager {
  private:
   class IntentHelperObserverImpl;  // implemented in arc_service_manager.cc.
 
+  // Helper methods for AddService and GetService.
+  bool AddServiceInternal(const std::string& name,
+                          std::unique_ptr<ArcService> service);
+  ArcService* GetNamedServiceInternal(const std::string& name);
+
   base::ThreadChecker thread_checker_;
   scoped_refptr<base::TaskRunner> blocking_task_runner_;
 
@@ -89,7 +146,7 @@ class ArcServiceManager {
   std::unique_ptr<ArcIntentHelperObserver> intent_helper_observer_;
 
   std::unique_ptr<ArcBridgeService> arc_bridge_service_;
-  std::vector<std::unique_ptr<ArcService>> services_;
+  std::unordered_multimap<std::string, std::unique_ptr<ArcService>> services_;
   scoped_refptr<ActivityIconLoader> icon_loader_;
   scoped_refptr<LocalActivityResolver> activity_resolver_;
 

components/arc/arc_service_manager_unittest.cc

+// Copyright 2017 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 <memory>
+#include <utility>
+
+#include "base/auto_reset.h"
+#include "base/memory/ptr_util.h"
+#include "components/arc/arc_bridge_service.h"
+#include "components/arc/arc_service_manager.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace arc {
+
+namespace {
+
+class AnonymousService : public ArcService {
+ public:
+  AnonymousService(ArcBridgeService* arc_bridge_service, bool* alive)
+      : ArcService(arc_bridge_service), alive_(alive, true) {}
+  ~AnonymousService() override = default;
+
+ private:
+  base::AutoReset<bool> alive_;
+};
+
+class NamedService : public ArcService {
+ public:
+  static const char kArcServiceName[];
+  NamedService(ArcBridgeService* arc_bridge_service, bool* alive)
+      : ArcService(arc_bridge_service), alive_(alive, true) {}
+  ~NamedService() override = default;
+
+ private:
+  base::AutoReset<bool> alive_;
+};
+
+class DifferentNamedService : public ArcService {
+ public:
+  static const char kArcServiceName[];
+  DifferentNamedService(ArcBridgeService* arc_bridge_service, bool* alive)
+      : ArcService(arc_bridge_service), alive_(alive, true) {}
+  ~DifferentNamedService() override = default;
+
+ private:
+  base::AutoReset<bool> alive_;
+};
+
+class EmptyNamedService : public ArcService {
+ public:
+  static const char kArcServiceName[];
+  EmptyNamedService(ArcBridgeService* arc_bridge_service, bool* alive)
+      : ArcService(arc_bridge_service), alive_(alive, true) {}
+  ~EmptyNamedService() override = default;
+
+ private:
+  base::AutoReset<bool> alive_;
+};
+
+const char NamedService::kArcServiceName[] =
+    "arc::(anonymous namespace)::NamedService";
+
+const char DifferentNamedService::kArcServiceName[] =
+    "arc::(anonymous namespace)::DifferentNamedService";
+
+const char EmptyNamedService::kArcServiceName[] = "";
+
+}  // namespace
+
+class ArcServiceManagerTest : public testing::Test {
+ public:
+  ArcServiceManagerTest() = default;
+
+  void SetUp() override {
+    arc_bridge_service_ = base::MakeUnique<ArcBridgeService>();
+  }
+
+  void TearDown() override { arc_bridge_service_.reset(); }
+
+  ArcBridgeService* arc_bridge_service() { return arc_bridge_service_.get(); }
+
+ private:
+  std::unique_ptr<ArcBridgeService> arc_bridge_service_;
+
+  DISALLOW_COPY_AND_ASSIGN(ArcServiceManagerTest);
+};
+
+// Exercises the basic getter functionality of ArcServiceManager.
+TEST_F(ArcServiceManagerTest, BasicGetter) {
+  bool named_service_alive = false;
+
+  // ArcServiceManager is empty, GetService() should return nullptr.
+  auto manager = base::MakeUnique<ArcServiceManager>(nullptr);
+  EXPECT_EQ(nullptr, manager->GetService<NamedService>());
+
+  EXPECT_TRUE(manager->AddService(base::MakeUnique<NamedService>(
+      arc_bridge_service(), &named_service_alive)));
+  EXPECT_TRUE(named_service_alive);
+  EXPECT_NE(nullptr, manager->GetService<NamedService>());
+
+  // Finally, the service should not be alive anymore.
+  manager.reset();
+  EXPECT_FALSE(named_service_alive);
+}
+
+// There is no way to distinguish between anonymous services, so it should be
+// possible to add them twice (not that it's recommended).
+TEST_F(ArcServiceManagerTest, MultipleAnonymousServices) {
+  bool anonymous_service_alive = false;
+  bool second_anonymous_service_alive = false;
+
+  auto manager = base::MakeUnique<ArcServiceManager>(nullptr);
+
+  EXPECT_TRUE(manager->AddService(base::MakeUnique<AnonymousService>(
+      arc_bridge_service(), &anonymous_service_alive)));
+  EXPECT_TRUE(anonymous_service_alive);
+  EXPECT_TRUE(manager->AddService(base::MakeUnique<AnonymousService>(
+      arc_bridge_service(), &second_anonymous_service_alive)));
+  EXPECT_TRUE(second_anonymous_service_alive);
+
+  // Finally, the individual services should not be alive anymore.
+  manager.reset();
+  EXPECT_FALSE(anonymous_service_alive);
+  EXPECT_FALSE(second_anonymous_service_alive);
+}
+
+// Named services can only be added once, but can still be retrieved.
+TEST_F(ArcServiceManagerTest, MultipleNamedServices) {
+  bool named_service_alive = false;
+  bool second_named_service_alive = false;
+  bool different_named_service_alive = false;
+
+  auto manager = base::MakeUnique<ArcServiceManager>(nullptr);
+
+  auto named_service = base::MakeUnique<NamedService>(arc_bridge_service(),
+                                                      &named_service_alive);
+  NamedService* raw_named_service = named_service.get();
+  EXPECT_TRUE(named_service_alive);
+  EXPECT_TRUE(manager->AddService(std::move(named_service)));
+  auto second_named_service = base::MakeUnique<NamedService>(
+      arc_bridge_service(), &second_named_service_alive);
+  EXPECT_TRUE(second_named_service_alive);
+  // This will fail and immediately destroy the service.
+  EXPECT_FALSE(manager->AddService(std::move(second_named_service)));
+  EXPECT_FALSE(second_named_service_alive);
+
+  // We should still be able to add a different-named service.
+  auto different_named_service = base::MakeUnique<DifferentNamedService>(
+      arc_bridge_service(), &different_named_service_alive);
+  DifferentNamedService* raw_different_named_service =
+      different_named_service.get();
+  EXPECT_TRUE(different_named_service_alive);
+  EXPECT_TRUE(manager->AddService(std::move(different_named_service)));
+
+  // And find both.
+  EXPECT_EQ(raw_named_service, manager->GetService<NamedService>());
+  EXPECT_EQ(raw_different_named_service,
+            manager->GetService<DifferentNamedService>());
+
+  manager.reset();
+  EXPECT_FALSE(named_service_alive);
+  EXPECT_FALSE(different_named_service_alive);
+}
+
+// Named services with an empty name are treated as anonymous services.
+// Developers shouldn't do that, though, and will trigger an error log.
+TEST_F(ArcServiceManagerTest, EmptyNamedServices) {
+  bool empty_named_service_alive = false;
+
+  auto manager = base::MakeUnique<ArcServiceManager>(nullptr);
+
+  EXPECT_TRUE(manager->AddService(base::MakeUnique<EmptyNamedService>(
+      arc_bridge_service(), &empty_named_service_alive)));
+  EXPECT_TRUE(empty_named_service_alive);
+  EXPECT_EQ(nullptr, manager->GetService<EmptyNamedService>());
+
+  manager.reset();
+  EXPECT_FALSE(empty_named_service_alive);
+}
+
+}  // namespace arc