Notify Files App when ARC++ app is installed/removed

components/arc/intent_helper/arc_intent_helper_bridge_unittest.cc

Index: components/arc/intent_helper/arc_intent_helper_bridge_unittest.cc
diff --git a/components/arc/intent_helper/arc_intent_helper_bridge_unittest.cc b/components/arc/intent_helper/arc_intent_helper_bridge_unittest.cc
index 07ed13cbe76adc195375a16a6c65db3c5492aa26..e407bea7da84a761747d6e78479ff76e84185029 100644
--- a/components/arc/intent_helper/arc_intent_helper_bridge_unittest.cc
+++ b/components/arc/intent_helper/arc_intent_helper_bridge_unittest.cc
@@ -5,15 +5,45 @@
 #include "components/arc/intent_helper/arc_intent_helper_bridge.h"
 
 #include <utility>
+#include <vector>

[Yusuke Sato, 2016/11/28 19:24:54]

This is already in arc_intent_helper_bridge.h. Remove?

[oka, 2016/11/29 06:56:49]

Done.

 
+#include "base/memory/ptr_util.h"

[Yusuke Sato, 2016/11/28 19:24:54]

What is this for?

[oka, 2016/11/29 06:56:49]

Removed.

 #include "components/arc/common/intent_helper.mojom.h"
+#include "components/arc/intent_helper/activity_icon_loader.h"
+#include "components/arc/intent_helper/local_activity_resolver.h"
+#include "components/arc/test/fake_arc_bridge_service.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace arc {
 
+class ArcIntentHelperTest : public testing::Test {

[Yusuke Sato, 2016/11/28 19:24:54]

Please put this in an anonymous namespace.

 namespace arc {
 
 namespace {
 
 class ... {
 
 };
 
 }  // namespace
 
 }  // namespace arc

[oka, 2016/11/29 06:56:49]

Done.

+ public:
+  ArcIntentHelperTest()
+      : icon_loader_(new ActivityIconLoader),

[Yusuke Sato, 2016/11/28 19:24:54]

nit: could you add ()? IIUC, Chromium folks are trying to always prefer 'new
T()' style.

[oka, 2016/11/29 06:56:49]

Done.

+        activity_resolver_(new LocalActivityResolver) {}

[Yusuke Sato, 2016/11/28 19:24:54]

same

[oka, 2016/11/29 06:56:49]

Done.

+
+ protected:
+  std::unique_ptr<FakeArcBridgeService> fake_arc_bridge_service_;
+  scoped_refptr<ActivityIconLoader> icon_loader_;
+  scoped_refptr<LocalActivityResolver> activity_resolver_;
+  std::unique_ptr<ArcIntentHelperBridge> instance_;
+
+ private:
+  void SetUp() override {
+    fake_arc_bridge_service_.reset(new FakeArcBridgeService);

[Yusuke Sato, 2016/11/28 19:24:54]

same

[oka, 2016/11/29 06:56:49]

Done.

+    instance_.reset(new ArcIntentHelperBridge(
+        fake_arc_bridge_service_.get(), icon_loader_, activity_resolver_));
+  }
+
+  void TearDown() override {
+    instance_.reset();
+    fake_arc_bridge_service_.reset();
+  }
+};
+
 // Tests if IsIntentHelperPackage works as expected. Probably too trivial
 // to test but just in case.
-TEST(ArcIntentHelperTest, TestIsIntentHelperPackage) {
+TEST_F(ArcIntentHelperTest, TestIsIntentHelperPackage) {
   EXPECT_FALSE(ArcIntentHelperBridge::IsIntentHelperPackage(""));
   EXPECT_FALSE(ArcIntentHelperBridge::IsIntentHelperPackage(
       ArcIntentHelperBridge::kArcIntentHelperPackageName + std::string("a")));
@@ -25,7 +55,7 @@ TEST(ArcIntentHelperTest, TestIsIntentHelperPackage) {
 }
 
 // Tests if FilterOutIntentHelper removes handlers as expected.
-TEST(ArcIntentHelperTest, TestFilterOutIntentHelper) {
+TEST_F(ArcIntentHelperTest, TestFilterOutIntentHelper) {
   {
     std::vector<mojom::IntentHandlerInfoPtr> orig;
     std::vector<mojom::IntentHandlerInfoPtr> filtered =
@@ -101,4 +131,21 @@ TEST(ArcIntentHelperTest, TestFilterOutIntentHelper) {
   }
 }
 
+// Tests if observer is called when intent filter is updated.
+TEST_F(ArcIntentHelperTest, TestObserverIsCalled) {
+  struct FakeObserver : public ArcIntentHelperObserver {
+   public:
+    FakeObserver() : updated_(false) {}
+    void OnAppsUpdated() override { updated_ = true; }
+    bool updated_;
+  };
+
+  std::unique_ptr<FakeObserver> observer(new FakeObserver);

[Yusuke Sato, 2016/11/28 19:24:54]

same

[oka, 2016/11/29 06:56:49]

Done.

+  instance_->AddObserver(observer.get());
+  std::vector<mojom::IntentFilterPtr> v;
+  instance_->OnIntentFiltersUpdated(std::move(v));

[Yusuke Sato, 2016/11/28 19:24:54]

nit: I'd add EXPECT_FALSE() call between L145&146.

[oka, 2016/11/29 06:56:49]

Done.

+
+  EXPECT_TRUE(observer->updated_);
+}

[Yusuke Sato, 2016/11/28 19:24:54]

Can you also do

 observer->updated_ = false;  // or observer.Reset();
 instance_->RemoveObserver(observer.get());
 instance_->OnIntentFiltersUpdated(std::move(v2));
 EXPECT_FALSE(observer->updated_);

to test RemoveObserver?

[oka, 2016/11/29 06:56:49]

Done.

+
 }  // namespace arc