[Media Router] Check MediaRouterDialogControllerImpl only sets toolbar MediaRouterAction.

chrome/browser/ui/toolbar/media_router_action.cc

Index: chrome/browser/ui/toolbar/media_router_action.cc
diff --git a/chrome/browser/ui/toolbar/media_router_action.cc b/chrome/browser/ui/toolbar/media_router_action.cc
index e397c6e95d786ffef427d45df563935610218389..dbcfb6c81b08c06c2ad0783a3ae0bc8a6412600d 100644
--- a/chrome/browser/ui/toolbar/media_router_action.cc
+++ b/chrome/browser/ui/toolbar/media_router_action.cc
@@ -36,7 +36,8 @@ media_router::MediaRouter* GetMediaRouter(Browser* browser) {
 
 }  // namespace
 
-MediaRouterAction::MediaRouterAction(Browser* browser)
+MediaRouterAction::MediaRouterAction(Browser* browser,
+                                     ToolbarActionsBar* toolbar_actions_bar)
     : media_router::IssuesObserver(GetMediaRouter(browser)),
       media_router::LocalMediaRoutesObserver(GetMediaRouter(browser)),
       media_router_active_icon_(
@@ -53,11 +54,13 @@ MediaRouterAction::MediaRouterAction(Browser* browser)
       has_local_display_route_(false),
       delegate_(nullptr),
       browser_(browser),
+      toolbar_actions_bar_(toolbar_actions_bar),
       platform_delegate_(MediaRouterActionPlatformDelegate::Create(browser)),
       contextual_menu_(browser),
       tab_strip_model_observer_(this),
       weak_ptr_factory_(this) {
   DCHECK(browser_);
+  DCHECK(toolbar_actions_bar_);
   tab_strip_model_observer_.Add(browser_->tab_strip_model());
 
   RegisterObserver();
@@ -192,9 +195,13 @@ void MediaRouterAction::UpdatePopupState() {
   if (!controller)
     return;
 
-  // Immediately keep track of MediaRouterAction in the controller. If it was
-  // already set, this should be a no-op.
-  controller->SetMediaRouterAction(weak_ptr_factory_.GetWeakPtr());
+  // When each browser window is created, its toolbar creates a
+  // MediaRouterAction that is only destroyed when the browser window is torn
+  // down. |controller| will keep track of that instance. If |this| was created
+  // in overflow mode, it will be destroyed when the overflow menu is closed.

[Peter Kasting, 2015/12/03 22:00:19]

Now the problem is that there's lots of useful background info here but I'm not
sure precisely how it relates to what this code is doing.  This is primarily
because I don't really understand what UpdatePopupState() is doing and when it's
run relative to when the other events you describe here (e.g. the app menu
dropdown with the overflow area in it being closed) are happening.  I note that
the header file here includes no comments on what this function does...

[apacible, 2015/12/07 21:52:43]

Added comments in the header file for UpdatePopupState().

+  // If SetMediaRouterAction() was previously called, this is a no-op.
+  if (!toolbar_actions_bar_->in_overflow_mode())
+    controller->SetMediaRouterAction(weak_ptr_factory_.GetWeakPtr());
 
   // Update the button in case the pressed state is out of sync with dialog
   // visibility.