Show media router toolbar icon ephemerally for active local routes and issues

chrome/browser/ui/toolbar/media_router_action_unittest.cc

 // 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 <memory>
+#include <string>
+#include <vector>
+
+#include "base/auto_reset.h"
+#include "base/command_line.h"
 #include "base/macros.h"
 #include "chrome/browser/extensions/browser_action_test_util.h"
 #include "chrome/browser/extensions/extension_action_test_util.h"
 #include "chrome/browser/ui/browser_commands.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
+#include "chrome/browser/ui/toolbar/component_toolbar_actions_factory.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_dialog_controller_impl.h"
 #include "chrome/browser/ui/webui/media_router/media_router_test.h"
 #include "chrome/grit/generated_resources.h"
 #include "content/public/browser/site_instance.h"
 #include "content/public/test/test_utils.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/gfx/color_palette.h"
@@ skipping 17 matching lines @@
 
 class TestMediaRouterAction : public MediaRouterAction {
  public:
   TestMediaRouterAction(Browser* browser,
                         ToolbarActionsBar* toolbar_actions_bar)
       : MediaRouterAction(browser, toolbar_actions_bar),
         controller_(nullptr),
         platform_delegate_(nullptr) {}
   ~TestMediaRouterAction() override {}
 
+  void ActiveTabChanged(content::WebContents* old_contents,
+                                          content::WebContents* new_contents,

[imcheng, 2016/09/16 21:47:49]

nit: please fix indent

[takumif, 2016/09/20 14:50:46]

Done.

+                                          int index,
+                                          int reason) override {
+    // This would be null if |controller_| hasn't been set.
+    if (GetMediaRouterDialogController()) {
+      MediaRouterAction::ActiveTabChanged(
+          old_contents, new_contents, index, reason);
+    }
+  }
+
   void SetMediaRouterDialogController(
       MediaRouterDialogControllerImpl* controller) {
     DCHECK(controller);
     controller_ = controller;
+    controller_->SetMediaRouterAction(GetWeakPtr());

[imcheng, 2016/09/16 21:47:49]

Is this needed if you don't call SetDelegate a second time? See comment below.

[takumif, 2016/09/20 14:50:46]

No, this is not necessary. Removing.

   }
 
  private:
   MediaRouterDialogControllerImpl* GetMediaRouterDialogController()
       override {
     return controller_;
   }
 
   MediaRouterActionPlatformDelegate* GetPlatformDelegate() override {
     return platform_delegate_;
@@ skipping 47 matching lines @@
         warning_icon_(
             gfx::CreateVectorIcon(gfx::VectorIconId::MEDIA_ROUTER_WARNING,
                                   gfx::kPlaceholderColor)) {}
 
   ~MediaRouterActionUnitTest() override {}
 
   // MediaRouterTest:
   void SetUp() override {
     MediaRouterTest::SetUp();
     toolbar_model_ = extensions::extension_action_test_util::
-        CreateToolbarModelForProfileWithoutWaitingForReady(profile());
+        CreateToolbarModelForProfile(profile());
 
     // browser() will only be valid once BrowserWithTestWindowTest::SetUp()
     // has run.
     browser_action_test_util_.reset(
         new BrowserActionTestUtil(browser(), false));
     action_.reset(
         new TestMediaRouterAction(
             browser(),
             browser_action_test_util_->GetToolbarActionsBar()));
 
+    action()->SetDelegate(new MockToolbarActionViewDelegate());
+
     local_display_route_list_.push_back(
         media_router::MediaRoute("routeId1", fake_source1_, "sinkId1",
                                  "description", true, std::string(), true));
     non_local_display_route_list_.push_back(
         media_router::MediaRoute("routeId2", fake_source1_, "sinkId2",
                                  "description", false, std::string(), true));
     non_local_display_route_list_.push_back(
         media_router::MediaRoute("routeId3", fake_source2_, "sinkId3",
                                  "description", true, std::string(), false));
   }
 
   void TearDown() override {
+    action_.reset();
     browser_action_test_util_.reset();
-    action_.reset();
     MediaRouterTest::TearDown();
   }
 
   TestMediaRouterAction* action() { return action_.get(); }
   const media_router::Issue* fake_issue_notification() {
     return &fake_issue_notification_;
   }
   const media_router::Issue* fake_issue_warning() {
     return &fake_issue_warning_;
   }
@@ skipping 160 matching lines @@
       active_icon(), action()->GetIcon(nullptr, gfx::Size())));
 
   // Update |current_icon_| when the local route is closed.
   action()->OnRoutesUpdated(non_local_display_route_list(),
                             empty_route_id_list());
   EXPECT_TRUE(gfx::test::AreImagesEqual(
       idle_icon(), action()->GetIcon(nullptr, gfx::Size())));
 }
 
 TEST_F(MediaRouterActionUnitTest, IconPressedState) {
+  base::AutoReset<bool> disable_animations(

[imcheng, 2016/09/16 21:47:49]

Please add comments on why this is needed?

[takumif, 2016/09/20 14:50:46]

This is no longer necessary. Removed.

+      &ToolbarActionsBar::disable_animations_for_testing_, true);
+
   // Start with one window with one tab.
   EXPECT_EQ(0, browser()->tab_strip_model()->count());
   chrome::NewTab(browser());
   EXPECT_EQ(1, browser()->tab_strip_model()->count());
 
-  // Create a reference to initiator contents.
-  WebContents* initiator_ =
-      browser()->tab_strip_model()->GetActiveWebContents();
-
-  MediaRouterDialogControllerImpl::CreateForWebContents(initiator_);
-  MediaRouterDialogControllerImpl* dialog_controller_ =
-      MediaRouterDialogControllerImpl::FromWebContents(initiator_);
-  ASSERT_TRUE(dialog_controller_);
+  WebContents* initiator = browser()->tab_strip_model()->GetActiveWebContents();
+  MediaRouterDialogControllerImpl::CreateForWebContents(initiator);
+  MediaRouterDialogControllerImpl* dialog_controller =
+      MediaRouterDialogControllerImpl::FromWebContents(initiator);
+  ASSERT_TRUE(dialog_controller);
 
   // Sets the controller to use for TestMediaRouterAction.
-  action()->SetMediaRouterDialogController(dialog_controller_);
+  action()->SetMediaRouterDialogController(dialog_controller);
 
   // Create a ToolbarActionViewDelegate to use for MediaRouterAction.
   std::unique_ptr<MockToolbarActionViewDelegate> mock_delegate(
       new MockToolbarActionViewDelegate());
-
-  EXPECT_CALL(*mock_delegate, GetCurrentWebContents()).WillOnce(
-      testing::Return(initiator_));
-  EXPECT_CALL(*mock_delegate, OnPopupClosed()).Times(1);
   action()->SetDelegate(mock_delegate.get());

[imcheng, 2016/09/16 21:47:49]

You changed the test to call SetDelegate() during SetUp(). Is this still needed?

[takumif, 2016/09/20 14:50:46]

Removed the first call.

 
-  EXPECT_CALL(*mock_delegate, OnPopupShown(true)).Times(1);
   action()->ExecuteAction(true);
+  EXPECT_TRUE(dialog_controller->IsShowingMediaRouterDialog());
 
   // Pressing the icon while the popup is shown should close the popup
-  EXPECT_CALL(*mock_delegate, OnPopupClosed()).Times(1);
   action()->ExecuteAction(true);
+  EXPECT_FALSE(dialog_controller->IsShowingMediaRouterDialog());
 
   EXPECT_CALL(*mock_delegate, OnPopupShown(true)).Times(1);
-  dialog_controller_->CreateMediaRouterDialog();
+  dialog_controller->CreateMediaRouterDialog();
 
   EXPECT_CALL(*mock_delegate, OnPopupClosed()).Times(1);
-  dialog_controller_->HideMediaRouterDialog();
+  dialog_controller->HideMediaRouterDialog();
 }