DeviceDisplayResolution: Don't show popup for policy-forced resolution
Whenever resolution is changed for an external display, popup was shown
suggesting user to rollback changes. This CL hides the popup for
policy-forced changes.
Bug: 938859
Change-Id: Iadb11ccac9c2cb5c2cc90bc4f71a127042ae9800
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1505933
Reviewed-by: Ahmed Fakhry <afakhry@chromium.org>
Reviewed-by: Dan Erat <derat@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Reviewed-by: Dominick Ng <dominickn@chromium.org>
Reviewed-by: Sergey Poromov <poromov@chromium.org>
Commit-Queue: Zakhar Voit <voit@google.com>
Cr-Commit-Position: refs/heads/master@{#642448}diff --git a/ash/display/cros_display_config.cc b/ash/display/cros_display_config.cc
index 54d47ea18..88962686 100644
--- a/ash/display/cros_display_config.cc
+++ b/ash/display/cros_display_config.cc
@@ -409,7 +409,8 @@
// Attempts to set the display mode for display |id|.
mojom::DisplayConfigResult SetDisplayMode(
int64_t id,
- const mojom::DisplayMode& display_mode) {
+ const mojom::DisplayMode& display_mode,
+ mojom::DisplayConfigSource source) {
display::DisplayManager* display_manager = GetDisplayManager();
display::ManagedDisplayMode current_mode;
@@ -430,7 +431,7 @@
if (!Shell::Get()
->resolution_notification_controller()
->PrepareNotificationAndSetDisplayMode(
- id, current_mode, new_mode, base::BindOnce([]() {
+ id, current_mode, new_mode, source, base::BindOnce([]() {
Shell::Get()->display_prefs()->MaybeStoreDisplayPrefs();
}))) {
return mojom::DisplayConfigResult::kSetDisplayModeError;
@@ -634,6 +635,7 @@
void CrosDisplayConfig::SetDisplayProperties(
const std::string& id,
mojom::DisplayConfigPropertiesPtr properties,
+ mojom::DisplayConfigSource source,
SetDisplayPropertiesCallback callback) {
const display::Display display = GetDisplay(id);
mojom::DisplayConfigResult result =
@@ -691,7 +693,7 @@
// will have already been applied. TODO(stevenjb): Validate the display mode
// before applying any properties.
if (properties->display_mode) {
- result = SetDisplayMode(display.id(), *properties->display_mode);
+ result = SetDisplayMode(display.id(), *properties->display_mode, source);
if (result != mojom::DisplayConfigResult::kSuccess) {
std::move(callback).Run(result);
return;
diff --git a/ash/display/cros_display_config.h b/ash/display/cros_display_config.h
index b2904bb9..bcc19cf 100644
--- a/ash/display/cros_display_config.h
+++ b/ash/display/cros_display_config.h
@@ -38,6 +38,7 @@
GetDisplayUnitInfoListCallback callback) override;
void SetDisplayProperties(const std::string& id,
mojom::DisplayConfigPropertiesPtr properties,
+ mojom::DisplayConfigSource source,
SetDisplayPropertiesCallback callback) override;
void SetUnifiedDesktopEnabled(bool enabled) override;
void OverscanCalibration(const std::string& display_id,
diff --git a/ash/display/cros_display_config_unittest.cc b/ash/display/cros_display_config_unittest.cc
index 91ecf39..5cd27e0f 100644
--- a/ash/display/cros_display_config_unittest.cc
+++ b/ash/display/cros_display_config_unittest.cc
@@ -130,7 +130,7 @@
mojom::DisplayConfigResult result;
base::RunLoop run_loop;
cros_display_config_->SetDisplayProperties(
- id, std::move(properties),
+ id, std::move(properties), mojom::DisplayConfigSource::kUser,
base::BindOnce(&SetResult, &result, run_loop.QuitClosure()));
run_loop.Run();
return result;
diff --git a/ash/display/display_prefs_unittest.cc b/ash/display/display_prefs_unittest.cc
index ba09ee0b..fcb4eb6 100644
--- a/ash/display/display_prefs_unittest.cc
+++ b/ash/display/display_prefs_unittest.cc
@@ -632,8 +632,10 @@
display::ManagedDisplayMode old_mode(gfx::Size(400, 300));
display::ManagedDisplayMode new_mode(gfx::Size(500, 400));
EXPECT_TRUE(shell->resolution_notification_controller()
- ->PrepareNotificationAndSetDisplayMode(id, old_mode, new_mode,
- base::OnceClosure()));
+ ->PrepareNotificationAndSetDisplayMode(
+ id, old_mode, new_mode,
+ ash::mojom::DisplayConfigSource::kUser,
+ base::OnceClosure()));
UpdateDisplay("500x400#500x400|400x300|300x200");
const base::DictionaryValue* properties =
diff --git a/ash/display/resolution_notification_controller.cc b/ash/display/resolution_notification_controller.cc
index b5a0355..1b9c08d 100644
--- a/ash/display/resolution_notification_controller.cc
+++ b/ash/display/resolution_notification_controller.cc
@@ -115,14 +115,16 @@
int64_t display_id,
const display::ManagedDisplayMode& old_resolution,
const display::ManagedDisplayMode& new_resolution,
+ ash::mojom::DisplayConfigSource source,
base::OnceClosure accept_callback) {
Shell::Get()->screen_layout_observer()->SetDisplayChangedFromSettingsUI(
display_id);
display::DisplayManager* const display_manager =
Shell::Get()->display_manager();
- if (display::Display::IsInternalDisplayId(display_id)) {
+ if (source == ash::mojom::DisplayConfigSource::kPolicy ||
+ display::Display::IsInternalDisplayId(display_id)) {
// We don't show notifications to confirm/revert the resolution change in
- // the case of an internal display.
+ // the case of an internal display or policy-forced changes.
return display_manager->SetDisplayMode(display_id, new_resolution);
}
diff --git a/ash/display/resolution_notification_controller.h b/ash/display/resolution_notification_controller.h
index 6763090..7484613 100644
--- a/ash/display/resolution_notification_controller.h
+++ b/ash/display/resolution_notification_controller.h
@@ -9,6 +9,7 @@
#include "ash/ash_export.h"
#include "ash/display/window_tree_host_manager.h"
+#include "ash/public/interfaces/cros_display_config.mojom.h"
#include "base/callback.h"
#include "base/gtest_prod_util.h"
#include "base/macros.h"
@@ -32,7 +33,8 @@
ResolutionNotificationController();
~ResolutionNotificationController() override;
- // If |display_id| is not the internal display, Prepare a resolution change
+ // If |display_id| is not the internal display and |source| is |kSourceUser|
+ // (which means user initiated the change), Prepare a resolution change
// notification for |display_id| from |old_resolution| to |new_resolution|,
// which offers a button to revert the change in case something goes wrong.
// The notification times out if there's only one display connected and the
@@ -44,9 +46,10 @@
// In case SetDisplayMode() fails, the prepared notification will be
// discarded.
//
- // If |display_id| is the internal display, the resolution change is applied
- // directly without preparing the confirm/revert notification (this kind of
- // notification is only useful for external displays).
+ // If |display_id| is the internal display or |source| is |kSourcePolicy|, the
+ // resolution change is applied directly without preparing the confirm/revert
+ // notification (this kind of notification is only useful for external
+ // displays).
//
// This method does not create a notification itself. The notification will be
// created the next OnDisplayConfigurationChanged(), which will be called
@@ -59,6 +62,7 @@
int64_t display_id,
const display::ManagedDisplayMode& old_resolution,
const display::ManagedDisplayMode& new_resolution,
+ ash::mojom::DisplayConfigSource source,
base::OnceClosure accept_callback) WARN_UNUSED_RESULT;
// Returns true if the notification is visible or scheduled to be visible and
diff --git a/ash/display/resolution_notification_controller_unittest.cc b/ash/display/resolution_notification_controller_unittest.cc
index 7146f0c7..c8dad741 100644
--- a/ash/display/resolution_notification_controller_unittest.cc
+++ b/ash/display/resolution_notification_controller_unittest.cc
@@ -57,7 +57,8 @@
void SetDisplayResolutionAndNotifyWithResolution(
const display::Display& display,
const gfx::Size& new_resolution,
- const gfx::Size& actual_new_resolution) {
+ const gfx::Size& actual_new_resolution,
+ mojom::DisplayConfigSource source = mojom::DisplayConfigSource::kUser) {
const display::ManagedDisplayInfo& info =
display_manager()->GetDisplayInfo(display.id());
display::ManagedDisplayMode old_mode(
@@ -68,7 +69,7 @@
old_mode.native(), old_mode.device_scale_factor());
EXPECT_TRUE(controller()->PrepareNotificationAndSetDisplayMode(
- display.id(), old_mode, new_mode,
+ display.id(), old_mode, new_mode, source,
base::BindOnce(&ResolutionNotificationControllerTest::OnAccepted,
base::Unretained(this))));
@@ -89,10 +90,12 @@
base::RunLoop().RunUntilIdle();
}
- void SetDisplayResolutionAndNotify(const display::Display& display,
- const gfx::Size& new_resolution) {
+ void SetDisplayResolutionAndNotify(
+ const display::Display& display,
+ const gfx::Size& new_resolution,
+ mojom::DisplayConfigSource source = mojom::DisplayConfigSource::kUser) {
SetDisplayResolutionAndNotifyWithResolution(display, new_resolution,
- new_resolution);
+ new_resolution, source);
}
static base::string16 GetNotificationMessage() {
@@ -183,6 +186,25 @@
EXPECT_EQ(59.0, mode.refresh_rate());
}
+// Check that notification is not shown when changes are forced by policy.
+TEST_F(ResolutionNotificationControllerTest, ForcedByPolicy) {
+ UpdateDisplay("300x300#300x300%57|200x200%58,250x250#250x250%59|200x200%60");
+ int64_t id2 = display_manager()->GetSecondaryDisplay().id();
+ ASSERT_EQ(0, accept_count());
+ EXPECT_FALSE(IsNotificationVisible());
+
+ // Changes the resolution and apply the result.
+ SetDisplayResolutionAndNotify(display_manager()->GetSecondaryDisplay(),
+ gfx::Size(200, 200),
+ mojom::DisplayConfigSource::kPolicy);
+ EXPECT_FALSE(IsNotificationVisible());
+ EXPECT_FALSE(IsScreenLayoutObserverNotificationVisible());
+ display::ManagedDisplayMode mode;
+ EXPECT_TRUE(display_manager()->GetSelectedModeForDisplayId(id2, &mode));
+ EXPECT_EQ("200x200", mode.size().ToString());
+ EXPECT_EQ(60.0, mode.refresh_rate());
+}
+
TEST_F(ResolutionNotificationControllerTest, ClickMeansAccept) {
UpdateDisplay("300x300#300x300%57|200x200%58,250x250#250x250%59|200x200%60");
int64_t id2 = display_manager()->GetSecondaryDisplay().id();
diff --git a/ash/public/cpp/default_scale_factor_retriever_unittest.cc b/ash/public/cpp/default_scale_factor_retriever_unittest.cc
index 4173905..df1edad 100644
--- a/ash/public/cpp/default_scale_factor_retriever_unittest.cc
+++ b/ash/public/cpp/default_scale_factor_retriever_unittest.cc
@@ -49,6 +49,7 @@
}
void SetDisplayProperties(const std::string& id,
ash::mojom::DisplayConfigPropertiesPtr properties,
+ ash::mojom::DisplayConfigSource source,
SetDisplayPropertiesCallback callback) override {}
void SetUnifiedDesktopEnabled(bool enabled) override {}
void OverscanCalibration(const std::string& display_id,
diff --git a/ash/public/interfaces/cros_display_config.mojom b/ash/public/interfaces/cros_display_config.mojom
index 7039c8b7..ac2899a1 100644
--- a/ash/public/interfaces/cros_display_config.mojom
+++ b/ash/public/interfaces/cros_display_config.mojom
@@ -63,6 +63,12 @@
kShowNative,
};
+// Describes who initiated configuration change.
+enum DisplayConfigSource {
+ kUser = 0,
+ kPolicy
+};
+
// Defines a pair of display + touch points used for touch calibration.
struct TouchCalibrationPair {
// The coordinates of the display point.
@@ -239,9 +245,12 @@
GetDisplayUnitInfoList(bool single_unified) =>
(array<DisplayUnitInfo> info_list);
- // Sets |properties| for individual display with identifier |id|. Returns
- // Success if the properties are valid or an error value otherwise.
- SetDisplayProperties(string id, DisplayConfigProperties properties) =>
+ // Sets |properties| for individual display with identifier |id|. |source|
+ // should describe who initiated the change. Returns Success if the properties
+ // are valid or an error value otherwise.
+ SetDisplayProperties(string id,
+ DisplayConfigProperties properties,
+ DisplayConfigSource source) =>
(DisplayConfigResult result);
// Enables or disables unified desktop mode. If the current display mode is
diff --git a/chrome/browser/chromeos/policy/display_resolution_handler.cc b/chrome/browser/chromeos/policy/display_resolution_handler.cc
index feb7d5e..b52a2d6 100644
--- a/chrome/browser/chromeos/policy/display_resolution_handler.cc
+++ b/chrome/browser/chromeos/policy/display_resolution_handler.cc
@@ -233,6 +233,7 @@
resized_display_ids_.insert(display_id);
cros_display_config->SetDisplayProperties(
display_unit_info->id, std::move(new_config),
+ ash::mojom::DisplayConfigSource::kPolicy,
base::BindOnce([](ash::mojom::DisplayConfigResult result) {
if (result == ash::mojom::DisplayConfigResult::kSuccess) {
VLOG(1) << "Successfully changed display mode.";
diff --git a/chrome/browser/chromeos/policy/display_rotation_default_handler.cc b/chrome/browser/chromeos/policy/display_rotation_default_handler.cc
index 86cc004d..f261dc8 100644
--- a/chrome/browser/chromeos/policy/display_rotation_default_handler.cc
+++ b/chrome/browser/chromeos/policy/display_rotation_default_handler.cc
@@ -68,7 +68,8 @@
config_properties->rotation =
ash::mojom::DisplayRotation::New(display_rotation_default_);
cros_display_config->SetDisplayProperties(
- display_unit_info->id, std::move(config_properties), base::DoNothing());
+ display_unit_info->id, std::move(config_properties),
+ ash::mojom::DisplayConfigSource::kPolicy, base::DoNothing());
}
}
diff --git a/chrome/browser/extensions/system_display/display_info_provider_chromeos.cc b/chrome/browser/extensions/system_display/display_info_provider_chromeos.cc
index 5a329246..0445bcf 100644
--- a/chrome/browser/extensions/system_display/display_info_provider_chromeos.cc
+++ b/chrome/browser/extensions/system_display/display_info_provider_chromeos.cc
@@ -375,6 +375,7 @@
cros_display_config_->SetDisplayProperties(
display_id_str, std::move(config_properties),
+ ash::mojom::DisplayConfigSource::kUser,
base::BindOnce(
[](ErrorCallback callback, ash::mojom::DisplayConfigResult result) {
std::move(callback).Run(GetStringResult(result));
diff --git a/chrome/browser/ui/views/frame/top_controls_slide_controller_chromeos_browsertest.cc b/chrome/browser/ui/views/frame/top_controls_slide_controller_chromeos_browsertest.cc
index 05d1cf39..e96ccd6 100644
--- a/chrome/browser/ui/views/frame/top_controls_slide_controller_chromeos_browsertest.cc
+++ b/chrome/browser/ui/views/frame/top_controls_slide_controller_chromeos_browsertest.cc
@@ -809,6 +809,7 @@
config_properties->rotation = ash::mojom::DisplayRotation::New(rotation);
ash::mojom::DisplayConfigResult result;
waiter_for.SetDisplayProperties(display_id, std::move(config_properties),
+ ash::mojom::DisplayConfigSource::kUser,
&result);
EXPECT_EQ(result, ash::mojom::DisplayConfigResult::kSuccess);
diff --git a/chrome/browser/ui/webui/chromeos/login/oobe_display_chooser.cc b/chrome/browser/ui/webui/chromeos/login/oobe_display_chooser.cc
index def6426..a0e563f 100644
--- a/chrome/browser/ui/webui/chromeos/login/oobe_display_chooser.cc
+++ b/chrome/browser/ui/webui/chromeos/login/oobe_display_chooser.cc
@@ -99,7 +99,8 @@
config_properties->set_primary = true;
cros_display_config_ptr_->SetDisplayProperties(
base::NumberToString(device.target_display_id),
- std::move(config_properties), base::DoNothing());
+ std::move(config_properties), ash::mojom::DisplayConfigSource::kUser,
+ base::DoNothing());
break;
}
}
diff --git a/chrome/browser/ui/webui/chromeos/login/oobe_display_chooser_unittest.cc b/chrome/browser/ui/webui/chromeos/login/oobe_display_chooser_unittest.cc
index 083f591..3a5adc9c 100644
--- a/chrome/browser/ui/webui/chromeos/login/oobe_display_chooser_unittest.cc
+++ b/chrome/browser/ui/webui/chromeos/login/oobe_display_chooser_unittest.cc
@@ -48,6 +48,7 @@
GetDisplayUnitInfoListCallback callback) override {}
void SetDisplayProperties(const std::string& id,
ash::mojom::DisplayConfigPropertiesPtr properties,
+ ash::mojom::DisplayConfigSource source,
SetDisplayPropertiesCallback callback) override {
if (properties->set_primary) {
int64_t display_id;
