Create MediaRouterActionController and MediaRouterUIService

chrome/browser/ui/toolbar/media_router_action_controller.cc

+// 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"
+
+MediaRouterActionController::MediaRouterActionController(
+    Profile* profile,
+    media_router::MediaRouter* router)
+    : MediaRouterActionController(profile,
+                                  router,

[mark a. foltz, 2016/09/09 17:48:28]

MediaRouterFactory::GetApiForBrowserContext(profile)

[takumif, 2016/09/12 19:01:31]

Done.

+                                  ToolbarActionsModel::Get(profile)) {}
+
+MediaRouterActionController::MediaRouterActionController(
+    Profile* profile,
+    media_router::MediaRouter* router,
+    extensions::ComponentMigrationHelper::ComponentActionDelegate*
+        component_action_delegate)
+    : media_router::IssuesObserver(router),
+      media_router::MediaRoutesObserver(router),
+      profile_(profile),
+      component_action_delegate_(component_action_delegate),
+      has_local_display_route_(false) {
+  DCHECK(profile_);
+  DCHECK(component_action_delegate_);
+  pref_change_registrar_.Init(profile->GetPrefs());
+  pref_change_registrar_.Add(
+      prefs::kMediaRouterAlwaysShowActionIcon,
+      base::Bind(&MediaRouterActionController::MaybeAddOrRemoveAction,
+                 base::Unretained(this)));
+}
+
+MediaRouterActionController::~MediaRouterActionController() {}

[mark a. foltz, 2016/09/09 17:48:28]

Will the pref_change_registar_ dtor unregister our callback?

[takumif, 2016/09/12 19:01:31]

Yes, it does.

+
+void MediaRouterActionController::OnIssueUpdated(
+    const media_router::Issue* issue) {
+  issue_.reset(issue ? new media_router::Issue(*issue) : nullptr);
+  MaybeAddOrRemoveAction();
+  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();
+
+  MaybeAddOrRemoveAction();
+  FOR_EACH_OBSERVER(Observer, observers_,
+                    OnLocalRouteUpdated(has_local_display_route_));
+}
+
+void MediaRouterActionController::SetAlwaysShowActionPref(bool always_show) {
+  profile_->GetPrefs()->SetBoolean(prefs::kMediaRouterAlwaysShowActionIcon,

[mark a. foltz, 2016/09/09 17:48:28]

Check the implementation of SetBoolean() to see if setting the same value is a
no-op.  Otherwise, check the current value then set if different.

[takumif, 2016/09/12 19:01:31]

Yes they do check for equality before setting it.

+                                   always_show);
+  MaybeAddOrRemoveAction();
+}
+
+bool MediaRouterActionController::HasLocalRoute() const {
+  return has_local_display_route_;
+}
+
+media_router::Issue* MediaRouterActionController::GetIssue() const {
+  return issue_.get();
+}
+
+void MediaRouterActionController::AddObserver(
+    MediaRouterActionController::Observer* observer) {
+  observers_.AddObserver(observer);
+}
+
+void MediaRouterActionController::RemoveObserver(
+    MediaRouterActionController::Observer* observer) {
+  observers_.RemoveObserver(observer);
+}
+
+void MediaRouterActionController::MaybeAddOrRemoveAction() {
+  const std::string& action_id =

[mark a. foltz, 2016/09/09 17:48:28]

Super nit: Use a using declaration or type alias instead of a variable
allocation.

http://en.cppreference.com/w/cpp/language/type_alias

[takumif, 2016/09/12 19:01:31]

Sorry, I'm not sure what you mean.
ComponentToolbarActionsFactory::kMediaRouterActionId is not a type, so it
wouldn't apply?

+      ComponentToolbarActionsFactory::kMediaRouterActionId;
+
+  if (IsActionEnabled()) {
+    if (!component_action_delegate_->HasComponentAction(action_id))
+      component_action_delegate_->AddComponentAction(action_id);
+  } else {
+    if (component_action_delegate_->HasComponentAction(action_id))
+      component_action_delegate_->RemoveComponentAction(action_id);
+  }
+}
+
+bool MediaRouterActionController::GetAlwaysShowActionPref() {
+  return profile_->GetPrefs()->GetBoolean(
+      prefs::kMediaRouterAlwaysShowActionIcon);
+}
+
+bool MediaRouterActionController::IsActionEnabled() {
+  return GetAlwaysShowActionPref() || has_local_display_route_ || issue_;

[mark a. foltz, 2016/09/09 17:48:28]

You're not actually calling anything on |issue_|; could it be a boolean
has_issue_ ?

[takumif, 2016/09/12 19:01:31]

Changed to has_issue_.

+}