Get rid of RefCounted for ActivityIconLoader.

components/arc/intent_helper/arc_intent_helper_bridge.h

 // Copyright 2016 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 COMPONENTS_ARC_INTENT_HELPER_ARC_INTENT_HELPER_BRIDGE_H_
 #define COMPONENTS_ARC_INTENT_HELPER_ARC_INTENT_HELPER_BRIDGE_H_
 
 #include <memory>
 #include <string>
 #include <vector>
 
 #include "ash/link_handler_model_factory.h"
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
 #include "base/observer_list.h"
 #include "base/threading/thread_checker.h"
 #include "components/arc/arc_service.h"
 #include "components/arc/common/intent_helper.mojom.h"
 #include "components/arc/instance_holder.h"
+#include "components/arc/intent_helper/activity_icon_loader.h"
 #include "components/arc/intent_helper/arc_intent_helper_observer.h"
 #include "mojo/public/cpp/bindings/binding.h"
 
 namespace ash {
 
 class LinkHandlerModel;
 
 }  // namespace ash
 
 namespace arc {
 
-class ActivityIconLoader;
 class ArcBridgeService;
 class IntentFilter;
 class LocalActivityResolver;
 
 // Receives intents from ARC.
 class ArcIntentHelperBridge
     : public ArcService,
       public InstanceHolder<mojom::IntentHelperInstance>::Observer,
       public mojom::IntentHelperHost,
       public ash::LinkHandlerModelFactory {
  public:
   enum class GetResult {
     // Failed. The intent_helper instance is not yet ready. This is a temporary
     // error.
     FAILED_ARC_NOT_READY,
     // Failed. Either ARC is not supported at all or intent_helper instance
     // version is too old.
     FAILED_ARC_NOT_SUPPORTED,
   };
 
   ArcIntentHelperBridge(
       ArcBridgeService* bridge_service,
-      const scoped_refptr<ActivityIconLoader>& icon_loader,
       const scoped_refptr<LocalActivityResolver>& activity_resolver);
   ~ArcIntentHelperBridge() override;
 
   void AddObserver(ArcIntentHelperObserver* observer);
   void RemoveObserver(ArcIntentHelperObserver* observer);
 
+  ActivityIconLoader* icon_loader() { return &icon_loader_; }

[hidehiko, 2017/01/27 16:04:06]

Note: or, alternatively we may want to provide GetActivityIcon() in this class,
which just proxies to ActivityIconLoader.
Then, external classes just can call ArcIntentHelperBridge::GetActivityIcon
directly, and ActivityIconLoader can be encapsulated.

The concern is, then, we would probably need many "using" to alias signatures in
ActivityIconLoader.

Yusuke, WDYT?

[Yusuke Sato, 2017/01/27 18:21:17]

Just in case, can you mention that the returned pointer shouldn't be held in the
caller's class? (another idea is to explicitly return an weakptr but it's
probably overkill.)

[hidehiko, 2017/01/30 17:04:03]

Instead, I removed this method, in the new PS.

+
   // InstanceHolder<mojom::IntentHelperInstance>::Observer
   void OnInstanceReady() override;
   void OnInstanceClosed() override;
 
   // mojom::IntentHelperHost
   void OnIconInvalidated(const std::string& package_name) override;
   void OnIntentFiltersUpdated(
       std::vector<IntentFilter> intent_filters) override;
   void OnOpenDownloads() override;
   void OnOpenUrl(const std::string& url) override;
@@ skipping 15 matching lines @@
   // false and updates |out_error_code| if it's not nullptr.
   static bool IsIntentHelperAvailable(GetResult* out_error_code);
 
   // For supporting ArcServiceManager::GetService<T>().
   static const char kArcServiceName[];
 
   static const char kArcIntentHelperPackageName[];
 
  private:
   mojo::Binding<mojom::IntentHelperHost> binding_;
-  scoped_refptr<ActivityIconLoader> icon_loader_;
+  ActivityIconLoader icon_loader_;
   scoped_refptr<LocalActivityResolver> activity_resolver_;
 
   base::ThreadChecker thread_checker_;
 
   base::ObserverList<ArcIntentHelperObserver> observer_list_;
 
   DISALLOW_COPY_AND_ASSIGN(ArcIntentHelperBridge);
 };
 
 }  // namespace arc
 
 #endif  // COMPONENTS_ARC_INTENT_HELPER_ARC_INTENT_HELPER_BRIDGE_H_