Commit f817f3e5 authored by Muyao Xu's avatar Muyao Xu Committed by Commit Bot

Add logs in CastToolbarButton

This CL adds logs every time |has_local_display_route_|'s value changes.
When its value is true, the cast toolbar icon should be blue, unless
there are FATL or WARNING issues.

Bug: b/165366974, 1117673
Change-Id: I082fadb913cdad4527020f2f10f06051d610deee
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2472531
Commit-Queue: Muyao Xu <muyaoxu@google.com>
Reviewed-by: default avatarTommy Steimel <steimel@chromium.org>
Reviewed-by: default avatarTakumi Fujimoto <takumif@chromium.org>
Cr-Commit-Position: refs/heads/master@{#820503}
parent 59fb3d9e
...@@ -7,6 +7,7 @@ ...@@ -7,6 +7,7 @@
#include "base/bind_helpers.h" #include "base/bind_helpers.h"
#include "base/run_loop.h" #include "base/run_loop.h"
#include "base/test/scoped_feature_list.h" #include "base/test/scoped_feature_list.h"
#include "build/build_config.h"
#include "chrome/browser/media/router/chrome_media_router_factory.h" #include "chrome/browser/media/router/chrome_media_router_factory.h"
#include "chrome/browser/ui/browser.h" #include "chrome/browser/ui/browser.h"
#include "chrome/browser/ui/global_media_controls/media_toolbar_button_observer.h" #include "chrome/browser/ui/global_media_controls/media_toolbar_button_observer.h"
...@@ -273,6 +274,12 @@ class TestMediaRouter : public media_router::MockMediaRouter { ...@@ -273,6 +274,12 @@ class TestMediaRouter : public media_router::MockMediaRouter {
return std::make_unique<TestMediaRouter>(); return std::make_unique<TestMediaRouter>();
} }
media_router::LoggerImpl* GetLogger() override {
if (!logger_)
logger_ = std::make_unique<media_router::LoggerImpl>();
return logger_.get();
}
void RegisterMediaRoutesObserver( void RegisterMediaRoutesObserver(
media_router::MediaRoutesObserver* observer) override { media_router::MediaRoutesObserver* observer) override {
routes_observers_.push_back(observer); routes_observers_.push_back(observer);
...@@ -291,6 +298,7 @@ class TestMediaRouter : public media_router::MockMediaRouter { ...@@ -291,6 +298,7 @@ class TestMediaRouter : public media_router::MockMediaRouter {
private: private:
std::vector<media_router::MediaRoutesObserver*> routes_observers_; std::vector<media_router::MediaRoutesObserver*> routes_observers_;
std::unique_ptr<media_router::LoggerImpl> logger_;
}; };
} // anonymous namespace } // anonymous namespace
......
...@@ -20,11 +20,16 @@ ...@@ -20,11 +20,16 @@
#include "ui/base/theme_provider.h" #include "ui/base/theme_provider.h"
#include "ui/gfx/color_palette.h" #include "ui/gfx/color_palette.h"
#include "ui/gfx/paint_vector_icon.h" #include "ui/gfx/paint_vector_icon.h"
#include "ui/gfx/vector_icon_types.h"
#include "ui/native_theme/native_theme.h" #include "ui/native_theme/native_theme.h"
#include "ui/views/controls/button/button_controller.h" #include "ui/views/controls/button/button_controller.h"
namespace media_router { namespace media_router {
namespace {
constexpr char kLoggerComponent[] = "CastToolbarButton";
}
// static // static
std::unique_ptr<CastToolbarButton> CastToolbarButton::Create(Browser* browser) { std::unique_ptr<CastToolbarButton> CastToolbarButton::Create(Browser* browser) {
// These objects may be null in tests. // These objects may be null in tests.
...@@ -55,7 +60,8 @@ CastToolbarButton::CastToolbarButton( ...@@ -55,7 +60,8 @@ CastToolbarButton::CastToolbarButton(
MediaRoutesObserver(media_router), MediaRoutesObserver(media_router),
browser_(browser), browser_(browser),
profile_(browser_->profile()), profile_(browser_->profile()),
context_menu_(std::move(context_menu)) { context_menu_(std::move(context_menu)),
logger_(media_router->GetLogger()) {
button_controller()->set_notify_action( button_controller()->set_notify_action(
views::ButtonController::NotifyAction::kOnPress); views::ButtonController::NotifyAction::kOnPress);
...@@ -145,27 +151,36 @@ void CastToolbarButton::UpdateIcon() { ...@@ -145,27 +151,36 @@ void CastToolbarButton::UpdateIcon() {
using Severity = media_router::IssueInfo::Severity; using Severity = media_router::IssueInfo::Severity;
const auto severity = const auto severity =
current_issue_ ? current_issue_->severity : Severity::NOTIFICATION; current_issue_ ? current_issue_->severity : Severity::NOTIFICATION;
if (severity == Severity::NOTIFICATION && !has_local_display_route_) { const gfx::VectorIcon* new_icon = nullptr;
UpdateIconsWithStandardColors(vector_icons::kMediaRouterIdleIcon); SkColor icon_color;
UpdateLayoutInsetDelta();
return;
}
const gfx::VectorIcon* icon = &vector_icons::kMediaRouterActiveIcon; if (severity == Severity::NOTIFICATION && !has_local_display_route_) {
SkColor icon_color = gfx::kGoogleBlue500; new_icon = &vector_icons::kMediaRouterIdleIcon;
// Highest priority is to indicate whether there's an issue. icon_color = gfx::kPlaceholderColor;
if (severity == Severity::FATAL) { } else if (severity == Severity::FATAL) {
icon = &vector_icons::kMediaRouterErrorIcon; new_icon = &vector_icons::kMediaRouterErrorIcon;
icon_color = GetNativeTheme()->GetSystemColor( icon_color = GetNativeTheme()->GetSystemColor(
ui::NativeTheme::kColorId_AlertSeverityHigh); ui::NativeTheme::kColorId_AlertSeverityHigh);
} else if (severity == Severity::WARNING) { } else if (severity == Severity::WARNING) {
icon = &vector_icons::kMediaRouterWarningIcon; new_icon = &vector_icons::kMediaRouterWarningIcon;
icon_color = GetNativeTheme()->GetSystemColor( icon_color = GetNativeTheme()->GetSystemColor(
ui::NativeTheme::kColorId_AlertSeverityMedium); ui::NativeTheme::kColorId_AlertSeverityMedium);
} else {
new_icon = &vector_icons::kMediaRouterActiveIcon;
icon_color = gfx::kGoogleBlue500;
} }
for (auto state : kButtonStates) if (icon_ == new_icon)
SetImageModel(state, ui::ImageModel::FromVectorIcon(*icon, icon_color)); return;
icon_ = new_icon;
LogIconChange(icon_);
if (icon_color == gfx::kPlaceholderColor) {
UpdateIconsWithStandardColors(*icon_);
} else {
for (auto state : kButtonStates)
SetImageModel(state, ui::ImageModel::FromVectorIcon(*icon_, icon_color));
}
UpdateLayoutInsetDelta(); UpdateLayoutInsetDelta();
} }
...@@ -194,4 +209,25 @@ void CastToolbarButton::ButtonPressed() { ...@@ -194,4 +209,25 @@ void CastToolbarButton::ButtonPressed() {
} }
} }
void CastToolbarButton::LogIconChange(const gfx::VectorIcon* icon) {
if (icon_ == &vector_icons::kMediaRouterIdleIcon) {
logger_->LogInfo(
mojom::LogCategory::kUi, kLoggerComponent,
"Cast toolbar icon indicates no active session nor issues.", "", "",
"");
} else if (icon_ == &vector_icons::kMediaRouterErrorIcon) {
logger_->LogInfo(mojom::LogCategory::kUi, kLoggerComponent,
"Cast toolbar icon shows a fatal issue.", "", "", "");
} else if (icon_ == &vector_icons::kMediaRouterWarningIcon) {
logger_->LogInfo(mojom::LogCategory::kUi, kLoggerComponent,
"Cast toolbar icon shows a warning issue.", "", "", "");
} else if (icon_ == &vector_icons::kMediaRouterActiveIcon) {
logger_->LogInfo(mojom::LogCategory::kUi, kLoggerComponent,
"Cast toolbar icon is blue, indicating an active session.",
"", "", "");
} else {
NOTREACHED();
}
}
} // namespace media_router } // namespace media_router
...@@ -17,6 +17,7 @@ class Browser; ...@@ -17,6 +17,7 @@ class Browser;
namespace media_router { namespace media_router {
class MediaRouter; class MediaRouter;
class LoggerImpl;
// Cast icon shown in the trusted area of toolbar. Its lifetime is tied to that // Cast icon shown in the trusted area of toolbar. Its lifetime is tied to that
// of its parent ToolbarView. The icon is made visible in following situations: // of its parent ToolbarView. The icon is made visible in following situations:
...@@ -69,6 +70,8 @@ class CastToolbarButton : public ToolbarButton, ...@@ -69,6 +70,8 @@ class CastToolbarButton : public ToolbarButton,
void ButtonPressed(); void ButtonPressed();
void LogIconChange(const gfx::VectorIcon* icon);
Browser* const browser_; Browser* const browser_;
Profile* const profile_; Profile* const profile_;
...@@ -79,6 +82,10 @@ class CastToolbarButton : public ToolbarButton, ...@@ -79,6 +82,10 @@ class CastToolbarButton : public ToolbarButton,
bool has_local_display_route_ = false; bool has_local_display_route_ = false;
const gfx::VectorIcon* icon_ = nullptr;
LoggerImpl* const logger_;
DISALLOW_COPY_AND_ASSIGN(CastToolbarButton); DISALLOW_COPY_AND_ASSIGN(CastToolbarButton);
}; };
......
...@@ -85,8 +85,12 @@ class CastToolbarButtonTest : public ChromeViewsTestBase { ...@@ -85,8 +85,12 @@ class CastToolbarButtonTest : public ChromeViewsTestBase {
Browser::CreateParams browser_params(profile_.get(), true); Browser::CreateParams browser_params(profile_.get(), true);
browser_params.window = window_.get(); browser_params.window = window_.get();
browser_ = std::make_unique<Browser>(browser_params); browser_ = std::make_unique<Browser>(browser_params);
MediaRouter* media_router = MockMediaRouter* media_router = static_cast<MockMediaRouter*>(
MediaRouterFactory::GetApiForBrowserContext(profile_.get()); MediaRouterFactory::GetApiForBrowserContext(profile_.get()));
logger_ = std::make_unique<LoggerImpl>();
ON_CALL(*media_router, GetLogger())
.WillByDefault(testing::Return(logger_.get()));
auto context_menu = std::make_unique<MediaRouterContextualMenu>( auto context_menu = std::make_unique<MediaRouterContextualMenu>(
browser_.get(), false, &context_menu_observer_); browser_.get(), false, &context_menu_observer_);
...@@ -131,6 +135,7 @@ class CastToolbarButtonTest : public ChromeViewsTestBase { ...@@ -131,6 +135,7 @@ class CastToolbarButtonTest : public ChromeViewsTestBase {
CastToolbarButton* button_ = nullptr; // owned by |widget_|. CastToolbarButton* button_ = nullptr; // owned by |widget_|.
MockContextMenuObserver context_menu_observer_; MockContextMenuObserver context_menu_observer_;
std::unique_ptr<TestingProfile> profile_; std::unique_ptr<TestingProfile> profile_;
std::unique_ptr<LoggerImpl> logger_;
gfx::Image idle_icon_; gfx::Image idle_icon_;
gfx::Image warning_icon_; gfx::Image warning_icon_;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment