Show Media Router toolbar icon ephemerally for MR dialogs

chrome/browser/ui/toolbar/media_router_action_controller.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 CHROME_BROWSER_UI_TOOLBAR_MEDIA_ROUTER_ACTION_CONTROLLER_H_
 #define CHROME_BROWSER_UI_TOOLBAR_MEDIA_ROUTER_ACTION_CONTROLLER_H_
 
 #include <vector>
 
 #include "chrome/browser/extensions/component_migration_helper.h"
 #include "chrome/browser/media/router/issues_observer.h"
 #include "chrome/browser/media/router/media_routes_observer.h"
 #include "chrome/browser/profiles/profile.h"
 #include "components/prefs/pref_change_registrar.h"
 
 using extensions::ComponentMigrationHelper;
 
+namespace media_router {
+class MockMediaRouterActionController;
+}  // namespace media_router
+
 // Controller for MediaRouterAction that determines when to show and hide the
 // action icon on the toolbar. There should be one instance of this class per
 // profile, and it should only be used on the UI thread.
 class MediaRouterActionController : public media_router::IssuesObserver,
                                     public media_router::MediaRoutesObserver {
  public:
   explicit MediaRouterActionController(Profile* profile);
   ~MediaRouterActionController() override;
 
   // media_router::IssuesObserver:
   void OnIssueUpdated(const media_router::Issue* issue) override;
 
   // media_router::MediaRoutesObserver:
   void OnRoutesUpdated(const std::vector<media_router::MediaRoute>& routes,
                        const std::vector<media_router::MediaRoute::Id>&
                            joinable_route_ids) override;
 
+  // Overridden in tests.

[mark a. foltz, 2016/10/17 18:57:40]

While that's true, can you also document what these methods are for in the real
implementation?

[takumif, 2016/10/27 23:06:38]

Done.

+  virtual void OnDialogShown();
+  virtual void OnDialogHidden();
+
  private:
   friend class MediaRouterActionControllerUnitTest;
-  FRIEND_TEST_ALL_PREFIXES(MediaRouterActionControllerUnitTest, EphemeralIcon);
+  friend class media_router::MockMediaRouterActionController;
+  FRIEND_TEST_ALL_PREFIXES(MediaRouterActionControllerUnitTest,
+                           EphemeralIconForRoutesAndIssues);
+  FRIEND_TEST_ALL_PREFIXES(MediaRouterActionControllerUnitTest,
+                           EphemeralIconForDialog);
 
   // Constructor for injecting dependencies in tests.

[mark a. foltz, 2016/10/17 18:57:40]

Would prefer making this public versus having to friend and forward declare
mocks in the  implementation to use it.  I think it's good to have a constructor
in the API that explicitly declares dependencies.

[takumif, 2016/10/27 23:06:38]

Done.

   MediaRouterActionController(
       Profile* profile,
       media_router::MediaRouter* router,
       ComponentMigrationHelper::ComponentActionDelegate*
           component_action_delegate,
       ComponentMigrationHelper* component_migration_helper);
 
   // Adds or removes the Media Router action icon to/from
   // |component_action_delegate_| if necessary, depending on whether or not
-  // we have issues or local routes.
-  void MaybeAddOrRemoveAction();
+  // we have issues, local routes or a dialog.
+  virtual void MaybeAddOrRemoveAction();
 
   // Returns |true| if the Media Router action should be present on the toolbar
   // or the overflow menu.
   bool ShouldEnableAction() const;
 
   // The profile |this| is associated with. There should be one instance of this
   // class per profile.
   Profile* const profile_;
 
   // The delegate that is responsible for showing and hiding the icon on the
   // toolbar. It outlives |this|.
   ComponentMigrationHelper::ComponentActionDelegate* const
       component_action_delegate_;
 
   // Responsible for changing the pref to always show or hide component actions.
   // It is owned by ToolbarActionsModel and outlives |this|.
   ComponentMigrationHelper* const component_migration_helper_;
 
   bool has_issue_ = false;
   bool has_local_display_route_ = false;
 
+  // The number of dialogs that are currently open.
+  size_t dialog_count_ = 0;
+
   PrefChangeRegistrar pref_change_registrar_;
 
   DISALLOW_COPY_AND_ASSIGN(MediaRouterActionController);
 };
 
 #endif  // CHROME_BROWSER_UI_TOOLBAR_MEDIA_ROUTER_ACTION_CONTROLLER_H_