Reflect the current state in Media Router Action icon.

chrome/browser/ui/toolbar/media_router_action_unittest.cc

Index: chrome/browser/ui/toolbar/media_router_action_unittest.cc
diff --git a/chrome/browser/ui/toolbar/media_router_action_unittest.cc b/chrome/browser/ui/toolbar/media_router_action_unittest.cc
new file mode 100644
index 0000000000000000000000000000000000000000..e10eb6ceaab04a56a8b3152e0be7dcd4d429c535
--- /dev/null
+++ b/chrome/browser/ui/toolbar/media_router_action_unittest.cc
@@ -0,0 +1,323 @@
+// Copyright 2015 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 "base/message_loop/message_loop.h"
+#include "chrome/browser/media/router/issues_observer.h"
+#include "chrome/browser/media/router/media_router_factory.h"
+#include "chrome/browser/media/router/media_router_mojo_impl.h"
+#include "chrome/browser/media/router/media_router_type_converters.h"
+#include "chrome/browser/media/router/media_source_helper.h"
+#include "chrome/browser/media/router/test_helper.h"
+#include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/toolbar/media_router_action.h"
+#include "chrome/browser/ui/toolbar/toolbar_action_view_delegate.h"
+#include "chrome/browser/ui/webui/media_router/media_router_test.h"
+#include "chrome/grit/generated_resources.h"
+#include "chrome/test/base/test_browser_window.h"
+#include "chrome/test/base/testing_profile.h"
+#include "content/public/test/browser_test_utils.h"
+#include "content/public/test/test_utils.h"
+#include "extensions/browser/process_manager.h"
+#include "testing/gtest/include/gtest/gtest.h"
+#include "ui/base/l10n/l10n_util.h"
+#include "ui/base/resource/resource_bundle.h"
+
+class TestActionIssuesObserver : public ActionIssuesObserver {
+ public:
+  explicit TestActionIssuesObserver(media_router::MediaRouter* router,
+                                    MediaRouterAction* action)
+      : ActionIssuesObserver(router, action) {}
+  ~TestActionIssuesObserver() override {};
+
+  // IssuesObserver:
+  void OnIssueUpdated(const media_router::Issue* issue) override {
+    ActionIssuesObserver::OnIssueUpdated(issue);

[Kevin M, 2015/07/31 18:18:40]

If this method just delegates to the superclass, then you can just omit it
entirely.

I'm not quite sure what the purpose of these test observer classes are?

[apacible, 2015/08/01 02:05:29]

Removed.

+  }
+};
+
+class TestActionMediaRoutesObserver : public ActionMediaRoutesObserver {
+ public:
+  explicit TestActionMediaRoutesObserver(media_router::MediaRouter* router,
+                                         MediaRouterAction* action)
+      : ActionMediaRoutesObserver(router, action) {}
+  ~TestActionMediaRoutesObserver() override {};
+
+  // MediaRoutesObserver:
+  void OnRoutesUpdated(const std::vector<media_router::MediaRoute>& routes)
+      override {
+    ActionMediaRoutesObserver::OnRoutesUpdated(routes);
+  }
+};
+
+class TestMediaRouterAction : public MediaRouterAction {

[Kevin M, 2015/07/31 18:18:40]

It would be better to test this class directly instead of subclassing it. Hooks
introduced by test subclasses can obfuscate real bugs.

[apacible, 2015/08/01 02:05:29]

I need a valid MediaRouter* so I subclassed it to set MediaRouterMojoImpl* as a
mock Media Router. I'm open to alternate ways to do this.

+ public:
+  TestMediaRouterAction(Browser* browser,
+      extensions::ProcessManager* process_manager)
+      : MediaRouterAction(browser),
+        mock_media_router_(
+            new media_router::MediaRouterMojoImpl(process_manager)) {
+  }
+  ~TestMediaRouterAction() override {};
+
+  // ToolbarActionViewController:
+  void SetDelegate(ToolbarActionViewDelegate* delegate) override {
+    delegate_ = delegate;
+
+    if (delegate_) {
+      issues_observer_.reset(new TestActionIssuesObserver(
+          GetMediaRouter(), this));
+      routes_observer_.reset(new TestActionMediaRoutesObserver(
+          GetMediaRouter(), this));
+    } else {
+      issues_observer_.reset();
+      routes_observer_.reset();
+    }
+  }
+
+  // MediaRouterAction:
+  media_router::MediaRouter* GetMediaRouter() override {
+    return mock_media_router_;
+  }
+
+ private:
+  media_router::MediaRouterMojoImpl* mock_media_router_;
+
+  friend class MediaRouterActionUnitTest;
+  FRIEND_TEST_ALL_PREFIXES(MediaRouterActionUnitTest, Initialization);
+  FRIEND_TEST_ALL_PREFIXES(MediaRouterActionUnitTest, UpdateIssues);
+  FRIEND_TEST_ALL_PREFIXES(MediaRouterActionUnitTest, UpdateRoutes);
+  FRIEND_TEST_ALL_PREFIXES(MediaRouterActionUnitTest, UpdateIssuesAndRoutes);
+};
+
+class TestToolbarActionViewDelegate : public ToolbarActionViewDelegate {
+ public:
+  TestToolbarActionViewDelegate() {}
+
+  // ToolbarActionViewDelegate:
+  ToolbarActionViewController* GetPreferredPopupViewController() {
+    return nullptr;
+  }
+  content::WebContents* GetCurrentWebContents() const override {
+    return nullptr;
+  }
+  void UpdateState() override {};
+  bool IsMenuRunning() const override { return false; };
+  void OnPopupShown(bool by_user) override {};
+  void OnPopupClosed() override {};
+
+ protected:
+  ~TestToolbarActionViewDelegate() override {};
+};
+
+class MediaRouterActionUnitTest : public MediaRouterTest {
+ public:
+  MediaRouterActionUnitTest()
+      : fake_issue_notification_(media_router::Issue("title notification",
+            "message notification",
+            media_router::IssueAction(media_router::IssueAction::TYPE_OK),
+            std::vector<media_router::IssueAction>(),
+            "route_id",
+            media_router::Issue::NOTIFICATION,
+            false, "")),
+        fake_issue_warning_(media_router::Issue("title warning",
+            "message warning",
+            media_router::IssueAction(
+                media_router::IssueAction::TYPE_LEARN_MORE),
+            std::vector<media_router::IssueAction>(),
+            "route_id",
+            media_router::Issue::WARNING,
+            false,
+            "www.google.com")),
+        fake_issue_fatal_(media_router::Issue("title fatal",
+            "message fatal",
+            media_router::IssueAction(media_router::IssueAction::TYPE_OK),
+            std::vector<media_router::IssueAction>(),
+            "route_id",
+            media_router::Issue::FATAL,
+            true,
+            "")),
+        fake_sink1_("fakeSink1", "Fake Sink 1"),
+        fake_sink2_("fakeSink2", "Fake Sink 2"),
+        fake_source1_("fakeSource1"),
+        fake_source2_("fakeSource2"),
+        fake_route_local_("route1", fake_source1_, fake_sink1_, "desc1",
+            true, "path.html"),
+        fake_route_remote_("route2", fake_source2_, fake_sink2_, "desc2",
+            false, "path.html") {
+  }
+
+  ~MediaRouterActionUnitTest() override {}
+
+  // BrowserWithTestWindowTest:
+  void SetUp() override {
+    BrowserWithTestWindowTest::SetUp();
+
+    extensions::ProcessManager* process_manager =
+        extensions::ProcessManager::Get(profile());
+    action_.reset(new TestMediaRouterAction(browser(), process_manager));
+  }
+
+ protected:
+  scoped_ptr<TestMediaRouterAction> action_;
+
+  // Fake Issues.
+  media_router::Issue fake_issue_notification_;
+  media_router::Issue fake_issue_warning_;
+  media_router::Issue fake_issue_fatal_;
+
+  // Fake Sinks.
+  media_router::MediaSink fake_sink1_;
+  media_router::MediaSink fake_sink2_;
+
+  // Fake Sources.
+  media_router::MediaSource fake_source1_;
+  media_router::MediaSource fake_source2_;
+
+  // Fake Routes.
+  media_router::MediaRoute fake_route_local_;
+  media_router::MediaRoute fake_route_remote_;
+
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(MediaRouterActionUnitTest);
+};
+
+// Tests the initial state of MediaRouterAction after construction.
+TEST_F(MediaRouterActionUnitTest, Initialization) {
+  EXPECT_EQ("media_router_action", action_->id_);
+  EXPECT_EQ(l10n_util::GetStringUTF16(IDS_MEDIA_ROUTER_TITLE), action_->name_);
+  EXPECT_EQ(MediaRouterActionState::MEDIA_ROUTER_ACTION_IDLE, action_->state_);
+  EXPECT_FALSE(action_->issue_);
+  EXPECT_FALSE(action_->local_active_route_exists_);
+  EXPECT_FALSE(action_->delegate_);
+  EXPECT_FALSE(action_->issues_observer_);
+  EXPECT_FALSE(action_->routes_observer_);
+}
+
+// Tests the state of the MediaRouterAction based on updates to issues.
+TEST_F(MediaRouterActionUnitTest, UpdateIssues) {
+  // Set the delegate to register the observers.
+  action_->SetDelegate(new TestToolbarActionViewDelegate());
+  media_router::IssuesObserver* issues_observer =
+      action_->issues_observer_.get();
+  EXPECT_TRUE(issues_observer);
+
+  // Initially, there are no issues.
+  EXPECT_EQ(MediaRouterActionState::MEDIA_ROUTER_ACTION_IDLE, action_->state_);
+
+  // Don't |state_| since the issue is only a notification.
+  issues_observer->OnIssueUpdated(&fake_issue_notification_);
+  EXPECT_EQ(MediaRouterActionState::MEDIA_ROUTER_ACTION_IDLE, action_->state_);
+
+  // Update |state_| since the issue is a warning.
+  issues_observer->OnIssueUpdated(&fake_issue_warning_);
+  EXPECT_EQ(MediaRouterActionState::MEDIA_ROUTER_ACTION_WARNING,
+            action_->state_);
+
+  // Update |state_| since the issue is fatal.
+  issues_observer->OnIssueUpdated(&fake_issue_fatal_);
+  EXPECT_EQ(MediaRouterActionState::MEDIA_ROUTER_ACTION_ERROR,
+            action_->state_);
+
+  // Clear the issue.
+  issues_observer->OnIssueUpdated(nullptr);
+  EXPECT_EQ(MediaRouterActionState::MEDIA_ROUTER_ACTION_IDLE, action_->state_);
+}
+
+// Tests the state of the MediaRouterAction based on updates to routes.
+TEST_F(MediaRouterActionUnitTest, UpdateRoutes) {
+  // Set the delegate to register the observers.
+  action_->SetDelegate(new TestToolbarActionViewDelegate());
+  media_router::MediaRoutesObserver* routes_observer =
+      action_->routes_observer_.get();
+  EXPECT_TRUE(routes_observer);
+
+  // Initially, there are no routes.
+  EXPECT_EQ(MediaRouterActionState::MEDIA_ROUTER_ACTION_IDLE, action_->state_);
+
+  std::vector<media_router::MediaRoute> routes =
+      { fake_route_local_, fake_route_remote_ };
+
+  // Update |state_| since there is a local route.
+  routes_observer->OnRoutesUpdated(routes);
+  EXPECT_EQ(MediaRouterActionState::MEDIA_ROUTER_ACTION_ACTIVE,
+            action_->state_);
+
+  // Update |state_| since there are no more local routes.
+  routes = { fake_route_remote_ };
+  routes_observer->OnRoutesUpdated(routes);
+  EXPECT_EQ(MediaRouterActionState::MEDIA_ROUTER_ACTION_IDLE, action_->state_);
+
+  // |state_| stays the same if there are no local routes or no routes.
+  routes = {};
+  routes_observer->OnRoutesUpdated(routes);
+  EXPECT_EQ(MediaRouterActionState::MEDIA_ROUTER_ACTION_IDLE, action_->state_);
+}
+
+// Tests the state of the MediaRouterAction based on updates to both issues
+// and routes.
+TEST_F(MediaRouterActionUnitTest, UpdateIssuesAndRoutes) {
+  // Set the delegate to register the observers.
+  action_->SetDelegate(new TestToolbarActionViewDelegate());
+  media_router::MediaRoutesObserver* routes_observer =
+      action_->routes_observer_.get();
+  EXPECT_TRUE(routes_observer);
+  media_router::IssuesObserver* issues_observer =
+      action_->issues_observer_.get();
+  EXPECT_TRUE(issues_observer);
+
+  // Initially, there are no issues or routes.
+  EXPECT_EQ(MediaRouterActionState::MEDIA_ROUTER_ACTION_IDLE, action_->state_);
+
+  // There is no change in |state_| since notification issues do not update
+  // the state.
+  issues_observer->OnIssueUpdated(&fake_issue_notification_);
+  EXPECT_EQ(MediaRouterActionState::MEDIA_ROUTER_ACTION_IDLE, action_->state_);
+
+  // Non-local routes also do not have an effect on |state_|.
+  std::vector<media_router::MediaRoute> routes = { fake_route_remote_ };
+  routes_observer->OnRoutesUpdated(routes);
+  EXPECT_EQ(MediaRouterActionState::MEDIA_ROUTER_ACTION_IDLE, action_->state_);
+
+  // Update |state_| since there is a local route.
+  routes = { fake_route_local_ };
+  routes_observer->OnRoutesUpdated(routes);
+  EXPECT_EQ(MediaRouterActionState::MEDIA_ROUTER_ACTION_ACTIVE,
+            action_->state_);
+
+  // Update |state_|, with a priority to reflect the warning issue rather than
+  // local route.
+  issues_observer->OnIssueUpdated(&fake_issue_warning_);
+  EXPECT_EQ(MediaRouterActionState::MEDIA_ROUTER_ACTION_WARNING,
+            action_->state_);
+
+  // Swapping the local route for a non-local one makes no difference to the
+  // |state_|.
+  routes = { fake_route_remote_ };
+  routes_observer->OnRoutesUpdated(routes);
+  EXPECT_EQ(MediaRouterActionState::MEDIA_ROUTER_ACTION_WARNING,
+            action_->state_);
+
+  // Update |state_| since the issue has been updated to fatal.
+  issues_observer->OnIssueUpdated(&fake_issue_fatal_);
+  EXPECT_EQ(MediaRouterActionState::MEDIA_ROUTER_ACTION_ERROR,
+            action_->state_);
+
+  // Fatal issues still take precedent over local routes.
+  routes = { fake_route_local_ };
+  routes_observer->OnRoutesUpdated(routes);
+  EXPECT_EQ(MediaRouterActionState::MEDIA_ROUTER_ACTION_ERROR,
+            action_->state_);
+
+  // When the fatal issue is dismissed, |state_| reflects the existing local
+  // route.
+  issues_observer->OnIssueUpdated(nullptr);
+  EXPECT_EQ(MediaRouterActionState::MEDIA_ROUTER_ACTION_ACTIVE,
+            action_->state_);
+
+  // Update |state_| when the local route is swapped out for a non-local route.
+  routes = { fake_route_remote_ };
+  routes_observer->OnRoutesUpdated(routes);
+  EXPECT_EQ(MediaRouterActionState::MEDIA_ROUTER_ACTION_IDLE, action_->state_);
+}