Simplify ArcServiceManager by stop deriving it from ArcIntentHelperObserver

It is slightly weird for ArcServiceManager to handle (only) one of the
ArcService instances it has in a special way with inheritance, and this
CL fixes that. ArcServiceManager looks more neutral now and no longer
has to #include arc_intent_helper_observer.h on the header side. This
removes all virtual member functions from ArcServiceManager too.

Instead, do the same thing with a pair of member variable and its getter
function, which is more consistent with the existing ArcServiceManager
code. All the implementation details go to the .cc side.

This is a follow-up CL for
https://codereview.chromium.org/2487623002/.

BUG=672840
TEST=try

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

chrome/browser/chromeos/arc/arc_service_launcher.cc

@@ -101,9 +101,10 @@ void ArcServiceLauncher::Initialize() {
   auto intent_helper = base::MakeUnique<ArcIntentHelperBridge>(
       arc_bridge_service, arc_service_manager_->icon_loader(),
       arc_service_manager_->activity_resolver());
-  // We don't have to remove observer since
-  // ArcServiceManager always outlives ArcIntentHelperBridge.
-  intent_helper->AddObserver(arc_service_manager_.get());
+  // We don't have to call ArcIntentHelperBridge::RemoveObserver() in
+  // ~ArcServiceManager() since the observer in ArcServiceManager always
+  // outlives the ArcIntentHelperBridge object.
+  intent_helper->AddObserver(arc_service_manager_->intent_helper_observer());
   arc_service_manager_->AddService(std::move(intent_helper));
   arc_service_manager_->AddService(
       base::MakeUnique<ArcImeService>(arc_bridge_service));

components/arc/arc_service_manager.cc

@@ -6,11 +6,13 @@
 
 #include <utility>
 
+#include "base/logging.h"
 #include "base/memory/ptr_util.h"
 #include "base/sequenced_task_runner.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "components/arc/arc_bridge_service.h"
 #include "components/arc/arc_bridge_service_impl.h"
+#include "components/arc/intent_helper/arc_intent_helper_observer.h"
 
 namespace arc {
 
@@ -24,9 +26,33 @@ ArcBridgeService* g_arc_bridge_service_for_testing = nullptr;
 
 }  // namespace
 
+class ArcServiceManager::IntentHelperObserverImpl
+    : public ArcIntentHelperObserver {
+ public:
+  explicit IntentHelperObserverImpl(ArcServiceManager* manager);
+  ~IntentHelperObserverImpl() override = default;
+
+ private:
+  void OnAppsUpdated() override;
+  ArcServiceManager* const manager_;
+
+  DISALLOW_COPY_AND_ASSIGN(IntentHelperObserverImpl);
+};
+
+ArcServiceManager::IntentHelperObserverImpl::IntentHelperObserverImpl(
+    ArcServiceManager* manager)
+    : manager_(manager) {}
+
+void ArcServiceManager::IntentHelperObserverImpl::OnAppsUpdated() {
+  DCHECK(manager_->thread_checker_.CalledOnValidThread());
+  for (auto& observer : manager_->observer_list_)
+    observer.OnAppsUpdated();
+}
+
 ArcServiceManager::ArcServiceManager(
     scoped_refptr<base::TaskRunner> blocking_task_runner)
     : blocking_task_runner_(blocking_task_runner),
+      intent_helper_observer_(base::MakeUnique<IntentHelperObserverImpl>(this)),
       icon_loader_(new ActivityIconLoader()),
       activity_resolver_(new LocalActivityResolver()) {
   DCHECK(!g_arc_service_manager);
@@ -85,12 +111,6 @@ void ArcServiceManager::RemoveObserver(Observer* observer) {
   observer_list_.RemoveObserver(observer);
 }
 
-void ArcServiceManager::OnAppsUpdated() {
-  DCHECK(thread_checker_.CalledOnValidThread());
-  for (auto& observer : observer_list_)
-    observer.OnAppsUpdated();
-}
-
 void ArcServiceManager::Shutdown() {
   DCHECK(thread_checker_.CalledOnValidThread());
   icon_loader_ = nullptr;

components/arc/arc_service_manager.h

@@ -14,17 +14,17 @@
 #include "base/task_runner.h"
 #include "base/threading/thread_checker.h"
 #include "components/arc/intent_helper/activity_icon_loader.h"
-#include "components/arc/intent_helper/arc_intent_helper_observer.h"
 #include "components/arc/intent_helper/local_activity_resolver.h"
 
 namespace arc {
 
 class ArcBridgeService;
+class ArcIntentHelperObserver;
 class ArcService;
 
 // Manages creation and destruction of services that communicate with the ARC
 // instance via the ArcBridgeService.
-class ArcServiceManager : public arc::ArcIntentHelperObserver {
+class ArcServiceManager {
  public:
   class Observer {
    public:
@@ -37,7 +37,7 @@ class ArcServiceManager : public arc::ArcIntentHelperObserver {
 
   explicit ArcServiceManager(
       scoped_refptr<base::TaskRunner> blocking_task_runner);
-  ~ArcServiceManager() override;
+  ~ArcServiceManager();
 
   // |arc_bridge_service| can only be accessed on the thread that this
   // class was created on.
@@ -56,9 +56,6 @@ class ArcServiceManager : public arc::ArcIntentHelperObserver {
   void AddObserver(Observer* observer);
   void RemoveObserver(Observer* observer);
 
-  // arc::ArcIntentHelperObserver overrides.
-  void OnAppsUpdated() override;
-
   // Called to shut down all ARC services.
   void Shutdown();
 
@@ -79,10 +76,20 @@ class ArcServiceManager : public arc::ArcIntentHelperObserver {
     return activity_resolver_;
   }
 
+  // Returns the IntentHelperObserver instance owned by ArcServiceManager.
+  ArcIntentHelperObserver* intent_helper_observer() {
+    return intent_helper_observer_.get();
+  }
+
  private:
+  class IntentHelperObserverImpl;  // implemented in arc_service_manager.cc.
+
   base::ThreadChecker thread_checker_;
   scoped_refptr<base::TaskRunner> blocking_task_runner_;
 
+  // An object for observing the ArcIntentHelper instance in |services_|.
+  std::unique_ptr<ArcIntentHelperObserver> intent_helper_observer_;
+
   std::unique_ptr<ArcBridgeService> arc_bridge_service_;
   std::vector<std::unique_ptr<ArcService>> services_;
   scoped_refptr<ActivityIconLoader> icon_loader_;

components/arc/intent_helper/arc_intent_helper_observer.h

@@ -11,8 +11,6 @@ class ArcIntentHelperObserver {
  public:
   // Called when app's intent filter is updated.
   virtual void OnAppsUpdated() = 0;
-
- protected:
   virtual ~ArcIntentHelperObserver() = default;
 };