Create MediaRouterActionController and MediaRouterUIService

chrome/browser/ui/toolbar/media_router_action_controller.cc

Index: chrome/browser/ui/toolbar/media_router_action_controller.cc
diff --git a/chrome/browser/ui/toolbar/media_router_action_controller.cc b/chrome/browser/ui/toolbar/media_router_action_controller.cc
new file mode 100644
index 0000000000000000000000000000000000000000..7a52884275eac463f78c0b32deabb3c4aaf1a21b
--- /dev/null
+++ b/chrome/browser/ui/toolbar/media_router_action_controller.cc
@@ -0,0 +1,94 @@
+// 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.
+
+#include "chrome/browser/ui/toolbar/media_router_action_controller.h"
+
+#include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/toolbar/component_toolbar_actions_factory.h"
+#include "chrome/browser/ui/toolbar/toolbar_actions_model.h"
+#include "chrome/common/pref_names.h"
+
+void MediaRouterActionController::Observer::OnIssueUpdated(
+    const media_router::Issue* issue) {}
+
+void MediaRouterActionController::Observer::OnLocalRouteUpdated(

[mark a. foltz, 2016/09/02 22:19:09]

Should Observer be pure virtual?

[takumif, 2016/09/07 21:48:32]

Done.

+    bool has_local_route) {}
+
+MediaRouterActionController::MediaRouterActionController(
+    Profile* profile,
+    media_router::MediaRouter* router)
+    : media_router::IssuesObserver(router),
+      media_router::MediaRoutesObserver(router),
+      profile_(profile),
+      delegate_(ToolbarActionsModel::Get(profile)),
+      has_local_display_route_(false) {}
+
+MediaRouterActionController::~MediaRouterActionController() {}
+
+void MediaRouterActionController::OnIssueUpdated(
+    const media_router::Issue* issue) {
+  issue_.reset(issue ? new media_router::Issue(*issue) : nullptr);

[imcheng, 2016/08/31 05:39:48]

Unless something changed, we don't actually need to store the entire Issue
object in this class; we only need to know whether there is an Issue currently.
In crrev.com/2176613003 I changed |issue_| to a boolean to avoid copy
construction. It would be good to do it here. 

[takumif, 2016/09/07 21:48:32]

I can make MRActionController store a boolean and issue severity and pass those
on to the MRAction, but those changes would largely overlap with your changes in
the CL you mentioned. Perhaps it'd be better to make those changes after your CL
has landed?

+  UpdateActionVisibility();
+  FOR_EACH_OBSERVER(Observer, observers_, OnIssueUpdated(issue_.get()));
+}
+
+void MediaRouterActionController::OnRoutesUpdated(
+    const std::vector<media_router::MediaRoute>& routes,
+    const std::vector<media_router::MediaRoute::Id>& joinable_route_ids) {
+  has_local_display_route_ =
+      std::find_if(routes.begin(), routes.end(),
+                   [](const media_router::MediaRoute& route) {
+                     return route.is_local() && route.for_display();
+                   }) != routes.end();
+
+  UpdateActionVisibility();
+  FOR_EACH_OBSERVER(Observer, observers_,
+                    OnLocalRouteUpdated(has_local_display_route_));
+}
+
+void MediaRouterActionController::ToggleActionVisibilityPreference() {
+  DCHECK(profile_);
+  profile_->GetPrefs()->SetBoolean(prefs::kMediaRouterAlwaysShowActionIcon,
+                                   !ShouldAlwaysShowIcon());
+  UpdateActionVisibility();
+}
+
+bool MediaRouterActionController::HasLocalRoute() const {

[mark a. foltz, 2016/09/02 22:19:09]

What calls this?

[takumif, 2016/09/07 21:48:32]

This is called by MRAction when it is initialized, to show the correct icon
image.

+  return has_local_display_route_;
+}
+
+media_router::Issue* MediaRouterActionController::GetIssue() const {

[mark a. foltz, 2016/09/02 22:19:09]

Ditto

[takumif, 2016/09/07 21:48:32]

Also MRAction.

+  return issue_.get();
+}
+
+void MediaRouterActionController::AddObserver(
+    MediaRouterActionController::Observer* observer) {
+  observers_.AddObserver(observer);
+}
+
+void MediaRouterActionController::RemoveObserver(
+    MediaRouterActionController::Observer* observer) {
+  observers_.RemoveObserver(observer);
+}
+
+void MediaRouterActionController::UpdateActionVisibility() {
+  DCHECK(delegate_);

[imcheng, 2016/08/31 05:39:48]

Move this DCHECK to ctor.

[takumif, 2016/09/07 21:48:33]

Done.

+  const std::string& action_id =
+      ComponentToolbarActionsFactory::kMediaRouterActionId;
+
+  if (ShouldAlwaysShowIcon() || has_local_display_route_ || issue_) {

[mark a. foltz, 2016/09/02 22:19:09]

1) Consider having a private IsActionEnabled() method to compute this.

2) You need to ask the component migration helper if the user had the Cast
extension installed, or set the initial value of the pref appropriately.

[takumif, 2016/09/07 21:48:32]

1) Done.

2) Added the change to ComponentMigrationHelper to modify the pref to this
patch. MRActionController can observe the change through PrefChangeRegistrar.

+    if (!delegate_->HasComponentAction(action_id))
+      delegate_->AddComponentAction(action_id);
+  } else {
+    if (delegate_->HasComponentAction(action_id))
+      delegate_->RemoveComponentAction(action_id);
+  }
+}
+
+bool MediaRouterActionController::ShouldAlwaysShowIcon() {
+  DCHECK(profile_);

[imcheng, 2016/08/31 05:39:48]

Move this DCHECK to ctor.

[takumif, 2016/09/07 21:48:32]

Done.

+  return profile_->GetPrefs()->GetBoolean(

[mark a. foltz, 2016/09/02 22:19:09]

If the pref value changes through sync, how does this class get notified?

[takumif, 2016/09/07 21:48:32]

I've added a PrefChangeRegistrar to observe changes to the pref, and a unit test
for it.

+      prefs::kMediaRouterAlwaysShowActionIcon);
+}