Skip to content

GitLab

T
tangled
Commit 34052950 authored by Bailey Berro's avatar Bailey Berro Committed by Commit Bot
Browse files
Options

Don't show display notifications if kReduceDisplayNotifications is enabled 

This change gates display notifications on the kReduceDisplayNotifications
flag. Notifications warning the user that too
many displays are connected and that resolution has changed will still
be shown regardless of flag state.

Bug: 1008640
Change-Id: I3fbf69fca2476f888f825d7e0d738bee54c6fab5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1827747
Commit-Queue: Bailey Berro <baileyberro@chromium.org>
Reviewed-by: default avatarJames Cook <jamescook@chromium.org>
Reviewed-by: default avatarZentaro Kavanagh <zentaro@chromium.org>
Cr-Commit-Position: refs/heads/master@{#701216}
parent f1b884e7
Hide whitespace changes
Inline Side-by-side
Showing with 110 additions and 18 deletions
ash/display/screen_orientation_controller_unittest.cc
View file @ 34052950
......@@ -12,6 +12,7 @@
#include "ash/display/screen_orientation_controller.h"
#include "ash/display/screen_orientation_controller_test_api.h"
#include "ash/public/cpp/app_types.h"
#include "ash/public/cpp/ash_features.h"
#include "ash/public/cpp/ash_switches.h"
#include "ash/shell.h"
#include "ash/system/screen_layout_observer.h"
......@@ -24,6 +25,7 @@
#include "ash/wm/window_state.h"
#include "base/command_line.h"
#include "base/macros.h"
#include "base/test/scoped_feature_list.h"
#include "ui/aura/client/aura_constants.h"
#include "ui/aura/window.h"
#include "ui/compositor/layer_type.h"
......@@ -441,6 +443,10 @@ TEST_F(ScreenOrientationControllerTest, RotationLockPreventsRotation) {
// Tests that the screen rotation notifications are suppressed when
// triggered by the accelerometer.
TEST_F(ScreenOrientationControllerTest, BlockRotationNotifications) {
base::test::ScopedFeatureList scoped_feature_list;
scoped_feature_list.InitAndDisableFeature(
features::kReduceDisplayNotifications);
EnableTabletMode(true);
Shell::Get()->screen_layout_observer()->set_show_notifications_for_testing(
true);
......
ash/system/screen_layout_observer.cc
View file @ 34052950
......@@ -11,6 +11,7 @@
#include "ash/display/screen_orientation_controller.h"
#include "ash/metrics/user_metrics_action.h"
#include "ash/metrics/user_metrics_recorder.h"
#include "ash/public/cpp/ash_features.h"
#include "ash/public/cpp/notification_utils.h"
#include "ash/public/cpp/system_tray_client.h"
#include "ash/resources/vector_icons/vector_icons.h"
......@@ -442,10 +443,19 @@ void ScreenLayoutObserver::OnDisplayConfigurationChanged() {
base::string16 message;
base::string16 additional_message;
if (GetDisplayMessageForNotification(old_info,
should_notify_has_unassociated_display,
&message, &additional_message))
CreateOrUpdateNotification(message, additional_message);
if (!GetDisplayMessageForNotification(old_info,
should_notify_has_unassociated_display,
&message, &additional_message))
return;
if (features::IsReduceDisplayNotificationsEnabled() &&
!should_notify_has_unassociated_display) {
// If display notifications should be suppressed and the notification is not
// to alert the user of an unassociated display, do not show a notification.
return;
}
CreateOrUpdateNotification(message, additional_message);
}
bool ScreenLayoutObserver::GetExitMirrorModeMessage(
......
ash/system/screen_layout_observer_unittest.cc
View file @ 34052950
......@@ -4,6 +4,7 @@
#include "ash/system/screen_layout_observer.h"
#include "ash/public/cpp/ash_features.h"
#include "ash/shell.h"
#include "ash/strings/grit/ash_strings.h"
#include "ash/test/ash_test_base.h"
......@@ -53,13 +54,20 @@ class ScreenLayoutObserverTest : public AshTestBase {
base::string16 GetUnifiedDisplayName();
bool IsNotificationShown() const;
base::test::ScopedFeatureList scoped_feature_list_;
private:
const message_center::Notification* GetDisplayNotification() const;
DISALLOW_COPY_AND_ASSIGN(ScreenLayoutObserverTest);
};
ScreenLayoutObserverTest::ScreenLayoutObserverTest() = default;
ScreenLayoutObserverTest::ScreenLayoutObserverTest() {
scoped_feature_list_.InitAndDisableFeature(
features::kReduceDisplayNotifications);
}
ScreenLayoutObserverTest::~ScreenLayoutObserverTest() = default;
......@@ -116,6 +124,11 @@ base::string16 ScreenLayoutObserverTest::GetUnifiedDisplayName() {
display_manager()->GetDisplayNameForId(display::kUnifiedDisplayId));
}
bool ScreenLayoutObserverTest::IsNotificationShown() const {
return !(GetDisplayNotificationText().empty() &&
GetDisplayNotificationAdditionalText().empty());
}
const message_center::Notification*
ScreenLayoutObserverTest::GetDisplayNotification() const {
const message_center::NotificationList::Notifications notifications =
......@@ -157,8 +170,7 @@ TEST_F(ScreenLayoutObserverTest, DisplayNotifications) {
// No-update
CloseNotification();
UpdateDisplay("400x400");
EXPECT_TRUE(GetDisplayNotificationText().empty());
EXPECT_TRUE(GetDisplayNotificationAdditionalText().empty());
EXPECT_FALSE(IsNotificationShown());
// Extended.
CloseNotification();
......@@ -176,8 +188,8 @@ TEST_F(ScreenLayoutObserverTest, DisplayNotifications) {
display::ManagedDisplayInfo second_display_info =
display::CreateDisplayInfo(second_display_id, gfx::Rect(2, 2, 500, 500));
std::vector<display::ManagedDisplayInfo> display_info_list;
display_info_list.emplace_back(first_display_info);
display_info_list.emplace_back(second_display_info);
display_info_list.push_back(first_display_info);
display_info_list.push_back(second_display_info);
display_manager()->OnNativeDisplaysChanged(display_info_list);
// Simulate that device can support at most two displays and user
......@@ -242,7 +254,7 @@ TEST_F(ScreenLayoutObserverTest, DisplayNotifications) {
// Restore mirror mode.
CloseNotification();
display_info_list.emplace_back(second_display_info);
display_info_list.push_back(second_display_info);
display_manager()->OnNativeDisplaysChanged(display_info_list);
EXPECT_EQ(l10n_util::GetStringFUTF16(IDS_ASH_STATUS_TRAY_DISPLAY_MIRRORING,
GetMirroringDisplayNames()),
......@@ -273,6 +285,74 @@ TEST_F(ScreenLayoutObserverTest, DisplayNotifications) {
EXPECT_TRUE(GetDisplayNotificationText().empty());
}
TEST_F(ScreenLayoutObserverTest, DisplayNotificationsDisabled) {
scoped_feature_list_.Reset();
scoped_feature_list_.InitAndEnableFeature(
features::kReduceDisplayNotifications);
Shell::Get()->screen_layout_observer()->set_show_notifications_for_testing(
true);
UpdateDisplay("400x400");
display::Display::SetInternalDisplayId(display_manager()->first_display_id());
EXPECT_TRUE(GetDisplayNotificationText().empty());
// Rotation.
UpdateDisplay("400x400/r");
EXPECT_FALSE(IsNotificationShown());
// Extended.
UpdateDisplay("400x400,200x200");
EXPECT_FALSE(IsNotificationShown());
const int64_t first_display_id =
display::Screen::GetScreen()->GetPrimaryDisplay().id();
const int64_t second_display_id = first_display_id + 1;
display::ManagedDisplayInfo first_display_info =
display::CreateDisplayInfo(first_display_id, gfx::Rect(1, 1, 500, 500));
display::ManagedDisplayInfo second_display_info =
display::CreateDisplayInfo(second_display_id, gfx::Rect(2, 2, 500, 500));
std::vector<display::ManagedDisplayInfo> display_info_list;
display_info_list.push_back(first_display_info);
display_info_list.push_back(second_display_info);
display_manager()->OnNativeDisplaysChanged(display_info_list);
// Simulate that device can support at most two displays and user
// connects it with three displays. Notification should still be created to
// warn user of it. See issue 827406 (https://crbug.com/827406).
display::test::DisplayManagerTestApi(Shell::Get()->display_manager())
.set_maximum_display(2u);
UpdateDisplay("400x400,200x200,100x100");
EXPECT_TRUE(GetDisplayNotificationText().empty());
EXPECT_EQ(l10n_util::GetStringUTF16(
IDS_ASH_STATUS_TRAY_DISPLAY_REMOVED_EXCEEDED_MAXIMUM),
GetDisplayNotificationAdditionalText());
EXPECT_TRUE(GetDisplayNotificationText().empty());
UpdateDisplay("400x400,200x200");
CloseNotification();
// Start tablet mode and wait until display mode is updated.
Shell::Get()->tablet_mode_controller()->SetEnabledForTest(true);
base::RunLoop().RunUntilIdle();
// Exit mirror mode manually. Now display mode should be extending mode.
display_manager()->SetMirrorMode(display::MirrorMode::kOff, base::nullopt);
EXPECT_FALSE(IsNotificationShown());
// Simulate that device can support at most two displays and user connects
// it with three displays. Because device is in tablet mode, display mode
// becomes mirror mode from extending mode. Under this circumstance, user is
// still notified of connecting more displays than maximum. See issue 827406
// (https://crbug.com/827406). Notification should still be shown.
UpdateDisplay("400x400,200x200,100x100");
EXPECT_EQ(l10n_util::GetStringUTF16(
IDS_ASH_STATUS_TRAY_DISPLAY_REMOVED_EXCEEDED_MAXIMUM),
GetDisplayNotificationAdditionalText());
EXPECT_EQ(l10n_util::GetStringFUTF16(IDS_ASH_STATUS_TRAY_DISPLAY_MIRRORING,
GetMirroringDisplayNames()),
GetDisplayNotificationText());
CloseNotification();
}
// Zooming in Unified Mode results in display size changes rather than changes
// in the UI scales, in which case, we still want to show a notification when
// the source of change is not the settings ui.
......@@ -371,14 +451,12 @@ TEST_F(ScreenLayoutObserverTest, OverscanDisplay) {
// /o creates the default overscan.
UpdateDisplay("400x400, 300x300/o");
EXPECT_TRUE(GetDisplayNotificationText().empty());
EXPECT_TRUE(GetDisplayNotificationAdditionalText().empty());
EXPECT_FALSE(IsNotificationShown());
// Reset the overscan.
Shell::Get()->display_manager()->SetOverscanInsets(
display_manager()->GetSecondaryDisplay().id(), gfx::Insets());
EXPECT_TRUE(GetDisplayNotificationText().empty());
EXPECT_TRUE(GetDisplayNotificationAdditionalText().empty());
EXPECT_FALSE(IsNotificationShown());
}
// Tests that exiting mirror mode by closing the lid shows the correct "exiting
......@@ -515,8 +593,7 @@ TEST_F(ScreenLayoutObserverTest, DockedModeWithExternalPrimaryDisplayMessage) {
// Close the lid. We go to docked mode, but we show no notifications.
UpdateDisplay("400x400");
EXPECT_TRUE(GetDisplayNotificationText().empty());
EXPECT_TRUE(GetDisplayNotificationAdditionalText().empty());
EXPECT_FALSE(IsNotificationShown());
}
// Tests that rotation notifications are only shown when the rotation source is
......@@ -532,8 +609,7 @@ TEST_F(ScreenLayoutObserverTest, RotationNotification) {
display_manager()->SetDisplayRotation(
primary_id, display::Display::ROTATE_90,
display::Display::RotationSource::ACCELEROMETER);
EXPECT_TRUE(GetDisplayNotificationText().empty());
EXPECT_TRUE(GetDisplayNotificationAdditionalText().empty());
EXPECT_FALSE(IsNotificationShown());
// The user source.
display_manager()->SetDisplayRotation(primary_id,
......
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