Reland "Remove display id in VirtualDisplayUtil::AddDisplay."
This is a reland of commit 557a678e03118da342b0b98c75932901857314ee
Serial numbers need to start at one on mac. A serial number of 0 was causing a crash.
Original change's description:
> Remove display id in VirtualDisplayUtil::AddDisplay.
>
> Removes the ability for callers to specify a display ID on
> VirtualDisplayUtil::AddDisplay. Most platforms (except for Mac) cannot
> influence the resulting display::Screen id. Instead, each virtual
> display controller maintains its own internal ids.
>
>
> Bug: 40271794
> Change-Id: If8707c2ed4ae98e59ea831f43e7fdd28797a1c3f
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5840551
> Reviewed-by: Mitsuru Oshima <oshima@chromium.org>
> Commit-Queue: Brad Triebwasser <btriebw@chromium.org>
> Reviewed-by: Mike Wasserman <msw@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#1356226}
Bug: 40271794
Change-Id: Ia6127882163faf7db7082570a3d77df0d07da71a
Include-Ci-Only-Tests: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5870624
Reviewed-by: Mike Wasserman <msw@chromium.org>
Reviewed-by: Mitsuru Oshima <oshima@chromium.org>
Commit-Queue: Brad Triebwasser <btriebw@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1357809}
diff --git a/ash/display/display_manager_unittest.cc b/ash/display/display_manager_unittest.cc
index 4125b5f..fc336ee 100644
--- a/ash/display/display_manager_unittest.cc
+++ b/ash/display/display_manager_unittest.cc
@@ -5483,33 +5483,6 @@
UpdateDisplay("800x600,800x600");
}
-// Tests adding a display via the VirtualDisplayUtil interface.
-// This test should roughly match other platforms, i.e.
-// //ui/display/linux/test/virtual_display_util_linux_interactive_uitest.cc
-// TODO(crbug.com/40271794): Consolidate testing of VirtualDisplayUtil.
-TEST_F(DisplayManagerTest, VirtualDisplayUtilAddDisplay) {
- const display::Screen* screen = display::Screen::GetScreen();
- std::unique_ptr<display::test::VirtualDisplayUtil> virtual_display_util =
- std::make_unique<display::test::DisplayManagerTestApi>(display_manager());
- int initial_display_count = screen->GetNumDisplays();
- int64_t display_id = virtual_display_util->AddDisplay(
- 2, display::test::VirtualDisplayUtil::k1920x1080);
- EXPECT_NE(display_id, display::kInvalidDisplayId);
- EXPECT_EQ(screen->GetNumDisplays(), initial_display_count + 1);
- display::Display d;
- EXPECT_TRUE(screen->GetDisplayWithDisplayId(display_id, &d));
- EXPECT_EQ(d.size(), gfx::Size(1920, 1080));
-
- // Expect failure when adding a duplicate index.
- EXPECT_EQ(virtual_display_util->AddDisplay(
- 2, display::test::VirtualDisplayUtil::k1920x1080),
- display::kInvalidDisplayId);
-
- virtual_display_util->ResetDisplays();
- EXPECT_FALSE(screen->GetDisplayWithDisplayId(display_id, &d));
- EXPECT_EQ(screen->GetNumDisplays(), initial_display_count);
-}
-
// Tests adding and removing displays via the VirtualDisplayUtil interface.
// This test should roughly match other platforms, i.e.
// TODO(crbug.com/40271794): Consolidate testing of VirtualDisplayUtil.
@@ -5520,22 +5493,27 @@
int64_t display_id[3];
int initial_display_count = screen->GetNumDisplays();
display_id[0] = virtual_display_util->AddDisplay(
- 2, display::test::VirtualDisplayUtil::k1920x1080);
+ display::test::VirtualDisplayUtil::k1920x1080);
EXPECT_NE(display_id[0], display::kInvalidDisplayId);
+ EXPECT_EQ(screen->GetNumDisplays(), initial_display_count + 1);
display::Display d;
EXPECT_TRUE(screen->GetDisplayWithDisplayId(display_id[0], &d));
+ EXPECT_EQ(d.size(), gfx::Size(1920, 1080));
display_id[1] = virtual_display_util->AddDisplay(
- 3, display::test::VirtualDisplayUtil::k1024x768);
+ display::test::VirtualDisplayUtil::k1024x768);
EXPECT_NE(display_id[1], display::kInvalidDisplayId);
+ EXPECT_EQ(screen->GetNumDisplays(), initial_display_count + 2);
EXPECT_TRUE(screen->GetDisplayWithDisplayId(display_id[1], &d));
+ EXPECT_EQ(d.size(), gfx::Size(1024, 768));
display_id[2] = virtual_display_util->AddDisplay(
- 4, display::test::VirtualDisplayUtil::k1920x1080);
+ display::test::VirtualDisplayUtil::k1920x1080);
EXPECT_NE(display_id[2], display::kInvalidDisplayId);
- EXPECT_TRUE(screen->GetDisplayWithDisplayId(display_id[2], &d));
-
EXPECT_EQ(screen->GetNumDisplays(), initial_display_count + 3);
+ EXPECT_TRUE(screen->GetDisplayWithDisplayId(display_id[2], &d));
+ EXPECT_EQ(d.size(), gfx::Size(1920, 1080));
+
virtual_display_util->RemoveDisplay(display_id[1]);
EXPECT_EQ(screen->GetNumDisplays(), initial_display_count + 2);
// Only virtual display 2 should no longer exist.
diff --git a/chrome/browser/ui/exclusive_access/fullscreen_controller_interactive_browsertest.cc b/chrome/browser/ui/exclusive_access/fullscreen_controller_interactive_browsertest.cc
index 576e447..2575b95a 100644
--- a/chrome/browser/ui/exclusive_access/fullscreen_controller_interactive_browsertest.cc
+++ b/chrome/browser/ui/exclusive_access/fullscreen_controller_interactive_browsertest.cc
@@ -1176,7 +1176,7 @@
if ((virtual_display_util_ = display::test::VirtualDisplayUtil::TryCreate(
display::Screen::GetScreen()))) {
secondary_display_id_ = virtual_display_util_->AddDisplay(
- 1, display::test::VirtualDisplayUtil::k1024x768);
+ display::test::VirtualDisplayUtil::k1024x768);
} else {
GTEST_SKIP() << "Skipping test; unavailable multi-screen support.";
}
diff --git a/chrome/browser/ui/test/popup_multiscreen_interactive_uitest.cc b/chrome/browser/ui/test/popup_multiscreen_interactive_uitest.cc
index c056bb8..f1c2d09 100644
--- a/chrome/browser/ui/test/popup_multiscreen_interactive_uitest.cc
+++ b/chrome/browser/ui/test/popup_multiscreen_interactive_uitest.cc
@@ -91,7 +91,7 @@
if ((virtual_display_util_ = display::test::VirtualDisplayUtil::TryCreate(
display::Screen::GetScreen()))) {
virtual_display_util_->AddDisplay(
- 1, display::test::VirtualDisplayUtil::k1024x768);
+ display::test::VirtualDisplayUtil::k1024x768);
return true;
}
return false;
diff --git a/chrome/browser/ui/web_applications/web_app_interactive_uitest.cc b/chrome/browser/ui/web_applications/web_app_interactive_uitest.cc
index 551432c..bba2bfc5 100644
--- a/chrome/browser/ui/web_applications/web_app_interactive_uitest.cc
+++ b/chrome/browser/ui/web_applications/web_app_interactive_uitest.cc
@@ -51,7 +51,7 @@
if ((virtual_display_util = display::test::VirtualDisplayUtil::TryCreate(
display::Screen::GetScreen()))) {
virtual_display_util->AddDisplay(
- 1, display::test::VirtualDisplayUtil::k1024x768);
+ display::test::VirtualDisplayUtil::k1024x768);
} else {
GTEST_SKIP() << "Skipping test; unavailable multi-screen support.";
}
diff --git a/ui/display/linux/test/virtual_display_util_linux.cc b/ui/display/linux/test/virtual_display_util_linux.cc
index 7da85c5..f6b21e8 100644
--- a/ui/display/linux/test/virtual_display_util_linux.cc
+++ b/ui/display/linux/test/virtual_display_util_linux.cc
@@ -100,15 +100,7 @@
}
int64_t VirtualDisplayUtilLinux::AddDisplay(
- uint8_t id,
const DisplayParams& display_params) {
- if (requested_ids_to_display_ids_.contains(id) ||
- std::find(requested_ids_.begin(), requested_ids_.end(), id) !=
- requested_ids_.end()) {
- LOG(ERROR) << "Virtual display with id " << id
- << " already exists or requested.";
- return kInvalidDisplayId;
- }
if (current_layout_.configs.size() - initial_layout_.configs.size() >
kMaxDisplays) {
LOG(ERROR) << "Cannot exceed " << kMaxDisplays << " virtual displays.";
@@ -118,15 +110,13 @@
last_requested_layout_ = current_layout_;
AppendScreen(last_requested_layout_, display_params.resolution,
display_params.dpi);
- requested_ids_.push_back(id);
randr_output_manager_->SetLayout(last_requested_layout_);
- detected_added_display_ids_.clear();
+ size_t initial_detected_displays = detected_added_display_ids_.size();
StartWaiting();
- CHECK_EQ(detected_added_display_ids_.size(), 1u)
+ CHECK_EQ(detected_added_display_ids_.size(), initial_detected_displays + 1u)
<< "Did not detect exactly one new display.";
// Reconcile the added resizer display ID to the detected display::Display id.
- int64_t new_display_id = detected_added_display_ids_.front();
- detected_added_display_ids_.pop_front();
+ int64_t new_display_id = detected_added_display_ids_.back();
x11::RandRMonitorLayout prev_layout = current_layout_;
current_layout_ = randr_output_manager_->GetLayout();
for (const auto& layout : current_layout_.configs) {
@@ -145,7 +135,7 @@
void VirtualDisplayUtilLinux::RemoveDisplay(int64_t display_id) {
if (!display_id_to_randr_id_.contains(display_id)) {
LOG(ERROR) << "Invalid display_id. Missing mapping for " << display_id
- << " to resizer ID.";
+ << " to randr ID.";
return;
}
last_requested_layout_ = current_layout_;
@@ -168,26 +158,13 @@
void VirtualDisplayUtilLinux::OnDisplayAdded(
const display::Display& new_display) {
- // TODO(crbug.com/40257169): Support adding multiple displays at a time, or
- // ignoring external display configuration changes.
- CHECK_EQ(requested_ids_.size(), 1u)
- << "An extra display was detected that was either not requested by this "
- "controller, or multiple displays were requested concurrently. This "
- "is not supported.";
detected_added_display_ids_.push_back(new_display.id());
- uint8_t requested_id = requested_ids_.front();
- requested_ids_.pop_front();
- requested_ids_to_display_ids_[requested_id] = new_display.id();
OnDisplayAddedOrRemoved(new_display.id());
}
void VirtualDisplayUtilLinux::OnDisplaysRemoved(
const display::Displays& removed_displays) {
for (const auto& display : removed_displays) {
- base::EraseIf(requested_ids_to_display_ids_,
- [&](std::pair<uint8_t, int64_t>& pair) {
- return pair.second == display.id();
- });
base::EraseIf(
display_id_to_randr_id_,
[&](std::pair<DisplayId, x11::RandRMonitorConfig::ScreenId>& pair) {
@@ -208,10 +185,10 @@
bool VirtualDisplayUtilLinux::RequestedLayoutIsSet() {
// Checks that the number of virtual displays (delta of last requested layout
- // minus initial layout) is equal to the number of requested displays.
+ // minus initial layout) is equal to the number of detected virtual displays.
return last_requested_layout_.configs.size() -
initial_layout_.configs.size() ==
- requested_ids_to_display_ids_.size();
+ detected_added_display_ids_.size();
}
void VirtualDisplayUtilLinux::StartWaiting() {
diff --git a/ui/display/linux/test/virtual_display_util_linux.h b/ui/display/linux/test/virtual_display_util_linux.h
index ee9df63..fdeec1c1 100644
--- a/ui/display/linux/test/virtual_display_util_linux.h
+++ b/ui/display/linux/test/virtual_display_util_linux.h
@@ -33,7 +33,7 @@
static bool IsAPIAvailable();
// VirtualDisplayUtil overrides:
- int64_t AddDisplay(uint8_t id, const DisplayParams& display_params) override;
+ int64_t AddDisplay(const DisplayParams& display_params) override;
void RemoveDisplay(int64_t display_id) override;
void ResetDisplays() override;
@@ -66,25 +66,13 @@
// Last layout request sent to `randr_output_manager_`.
x11::RandRMonitorLayout last_requested_layout_;
- // There are lots of IDS to track here:
- // 1. The user-requested ID set in AddDisplay().
- // 2. The xrandr display ID
- // 3. The display ID detected by the display::Screen implementation.
- using RequestedId = uint8_t;
- using DisplayId = int64_t;
+ using RandrOutputId = int64_t; // The RandROutputManager output ID
+ using DisplayId = int64_t; // The display ID used by display::Screen.
// Queue of displays added via OnDisplayAdded. Removed as they are reconciled
// and moved to `display_id_to_randr_id_`.
base::circular_deque<DisplayId> detected_added_display_ids_;
- base::flat_map<DisplayId, x11::RandRMonitorConfig::ScreenId>
- display_id_to_randr_id_;
-
- // Tracks display IDs requested in AddDisplay(). The IDs don't do anything in
- // this implementation, but they are tracked to prevent the user from
- // specifying the same ID twice without deleting it first (to match other
- // platform behavior);
- base::circular_deque<RequestedId> requested_ids_;
- base::flat_map<RequestedId, DisplayId> requested_ids_to_display_ids_;
+ base::flat_map<DisplayId, RandrOutputId> display_id_to_randr_id_;
};
} // namespace test
diff --git a/ui/display/linux/test/virtual_display_util_linux_interactive_uitest.cc b/ui/display/linux/test/virtual_display_util_linux_interactive_uitest.cc
index 6efacab..106b8131 100644
--- a/ui/display/linux/test/virtual_display_util_linux_interactive_uitest.cc
+++ b/ui/display/linux/test/virtual_display_util_linux_interactive_uitest.cc
@@ -46,46 +46,31 @@
EXPECT_TRUE(virtual_display_util_->IsAPIAvailable());
}
-TEST_F(VirtualDisplayUtilLinuxInteractiveUitest, AddDisplay) {
- int initial_display_count = screen()->GetNumDisplays();
- int64_t display_id = virtual_display_util_->AddDisplay(
- 1, display::test::VirtualDisplayUtilLinux::k1920x1080);
- EXPECT_NE(display_id, display::kInvalidDisplayId);
- EXPECT_EQ(screen()->GetNumDisplays(), initial_display_count + 1);
- display::Display d;
- EXPECT_TRUE(screen()->GetDisplayWithDisplayId(display_id, &d));
- EXPECT_EQ(d.size(), gfx::Size(1920, 1080));
-
- // Expect failure when adding a duplicate index.
- EXPECT_EQ(virtual_display_util_->AddDisplay(
- 1, display::test::VirtualDisplayUtilLinux::k1920x1080),
- display::kInvalidDisplayId);
-
- virtual_display_util_->ResetDisplays();
- EXPECT_FALSE(screen()->GetDisplayWithDisplayId(display_id, &d));
- EXPECT_EQ(screen()->GetNumDisplays(), initial_display_count);
-}
-
TEST_F(VirtualDisplayUtilLinuxInteractiveUitest, AddRemove) {
int64_t display_id[3];
int initial_display_count = screen()->GetNumDisplays();
display_id[0] = virtual_display_util_->AddDisplay(
- 0, display::test::VirtualDisplayUtilLinux::k1920x1080);
+ display::test::VirtualDisplayUtilLinux::k1920x1080);
EXPECT_NE(display_id[0], display::kInvalidDisplayId);
display::Display d;
+ EXPECT_EQ(screen()->GetNumDisplays(), initial_display_count + 1);
EXPECT_TRUE(screen()->GetDisplayWithDisplayId(display_id[0], &d));
+ EXPECT_EQ(d.size(), gfx::Size(1920, 1080));
display_id[1] = virtual_display_util_->AddDisplay(
- 1, display::test::VirtualDisplayUtilLinux::k1024x768);
+ display::test::VirtualDisplayUtilLinux::k1024x768);
EXPECT_NE(display_id[1], display::kInvalidDisplayId);
+ EXPECT_EQ(screen()->GetNumDisplays(), initial_display_count + 2);
EXPECT_TRUE(screen()->GetDisplayWithDisplayId(display_id[1], &d));
+ EXPECT_EQ(d.size(), gfx::Size(1024, 768));
display_id[2] = virtual_display_util_->AddDisplay(
- 2, display::test::VirtualDisplayUtilLinux::k1920x1080);
+ display::test::VirtualDisplayUtilLinux::k1920x1080);
EXPECT_NE(display_id[2], display::kInvalidDisplayId);
- EXPECT_TRUE(screen()->GetDisplayWithDisplayId(display_id[2], &d));
-
EXPECT_EQ(screen()->GetNumDisplays(), initial_display_count + 3);
+ EXPECT_TRUE(screen()->GetDisplayWithDisplayId(display_id[2], &d));
+ EXPECT_EQ(d.size(), gfx::Size(1920, 1080));
+
virtual_display_util_->RemoveDisplay(display_id[1]);
EXPECT_EQ(screen()->GetNumDisplays(), initial_display_count + 2);
// Only virtual display 2 should no longer exist.
diff --git a/ui/display/mac/test/virtual_display_util_mac.h b/ui/display/mac/test/virtual_display_util_mac.h
index 297907f..866bcea 100644
--- a/ui/display/mac/test/virtual_display_util_mac.h
+++ b/ui/display/mac/test/virtual_display_util_mac.h
@@ -34,8 +34,7 @@
VirtualDisplayUtilMac& operator=(const VirtualDisplayUtilMac&) = delete;
// VirtualDisplayUtil overrides:
- int64_t AddDisplay(uint8_t display_id,
- const DisplayParams& display_params) override;
+ int64_t AddDisplay(const DisplayParams& display_params) override;
void RemoveDisplay(int64_t display_id) override;
void ResetDisplays() override;
@@ -91,6 +90,10 @@
void OnDisplayAddedOrRemoved(int64_t id);
+ // Add a new display with a given serial number.
+ int64_t AddDisplay(int64_t serial_number,
+ const DisplayParams& display_params);
+
// Wait for the display with the given `id` to be added.
// Return immediately if the display is already available.
void WaitForDisplay(int64_t id, bool added);
diff --git a/ui/display/mac/test/virtual_display_util_mac.mm b/ui/display/mac/test/virtual_display_util_mac.mm
index 4a04180..084ca0c 100644
--- a/ui/display/mac/test/virtual_display_util_mac.mm
+++ b/ui/display/mac/test/virtual_display_util_mac.mm
@@ -297,6 +297,14 @@
display_metrics_change_observer.Wait();
}
+// static
+uint64_t SynthesizeSerialNumber() {
+ static uint64_t synthesized_serial_num = 1;
+ CHECK_LT(synthesized_serial_num, std::numeric_limits<uint64_t>::max())
+ << "All synthesized display IDs in use.";
+ return synthesized_serial_num++;
+}
+
} // namespace
namespace display::test {
@@ -311,45 +319,9 @@
screen_->RemoveObserver(this);
}
-int64_t VirtualDisplayUtilMac::AddDisplay(uint8_t display_id,
- const DisplayParams& display_params) {
- CHECK(!display_params.resolution.IsEmpty());
- CHECK(!display_params.dpi.IsZero());
- CHECK_EQ(display_params.dpi.x(), display_params.dpi.y());
-
- NSString* display_name =
- [NSString stringWithFormat:@"Virtual Display #%d", display_id];
- CGVirtualDisplay* display = CreateVirtualDisplay(
- display_params.resolution.width(), display_params.resolution.height(),
- display_params.dpi.x(), /*hiDPI=*/display_params.dpi.x() >= 200,
- display_name, display_id);
- DCHECK(display);
-
- // TODO(crbug.com/40148077): Please remove this log or replace it with
- // [D]CHECK() ASAP when the TEST is stable.
- LOG(INFO) << "VirtualDisplayUtilMac::" << __func__
- << " - display id: " << display_id
- << ". CreateVirtualDisplay success.";
-
- int64_t id = display.displayID;
- DCHECK_NE(id, 0u);
-
- WaitForDisplay(id, /*added=*/true);
-
- EnsureDisplayWithResolution(screen_, id,
- gfx::Size(display_params.resolution.width(),
- display_params.resolution.height()));
-
- // TODO(crbug.com/40148077): Please remove this log or replace it with
- // [D]CHECK() ASAP when the TEST is stable.
- LOG(INFO) << "VirtualDisplayUtilMac::" << __func__
- << " - display id: " << display_id << "(" << id
- << "). WaitForDisplay success.";
-
- DCHECK(!g_display_map[id]);
- g_display_map[id] = display;
-
- return id;
+int64_t VirtualDisplayUtilMac::AddDisplay(const DisplayParams& display_params) {
+ int64_t serial_number = SynthesizeSerialNumber();
+ return AddDisplay(serial_number, display_params);
}
void VirtualDisplayUtilMac::RemoveDisplay(int64_t display_id) {
@@ -419,6 +391,47 @@
StartWaiting();
}
+int64_t VirtualDisplayUtilMac::AddDisplay(int64_t serial_number,
+ const DisplayParams& display_params) {
+ CHECK(!display_params.resolution.IsEmpty());
+ CHECK(!display_params.dpi.IsZero());
+ CHECK_EQ(display_params.dpi.x(), display_params.dpi.y());
+
+ NSString* display_name =
+ [NSString stringWithFormat:@"Virtual Display #%lld", serial_number];
+ CGVirtualDisplay* display = CreateVirtualDisplay(
+ display_params.resolution.width(), display_params.resolution.height(),
+ display_params.dpi.x(), /*hiDPI=*/display_params.dpi.x() >= 200,
+ display_name, serial_number);
+ DCHECK(display);
+
+ // TODO(crbug.com/40148077): Please remove this log or replace it with
+ // [D]CHECK() ASAP when the TEST is stable.
+ LOG(INFO) << "VirtualDisplayUtilMac::" << __func__
+ << " - serial number: " << serial_number
+ << ". CreateVirtualDisplay success.";
+
+ int64_t id = display.displayID;
+ DCHECK_NE(id, 0u);
+
+ WaitForDisplay(id, /*added=*/true);
+
+ EnsureDisplayWithResolution(screen_, id,
+ gfx::Size(display_params.resolution.width(),
+ display_params.resolution.height()));
+
+ // TODO(crbug.com/40148077): Please remove this log or replace it with
+ // [D]CHECK() ASAP when the TEST is stable.
+ LOG(INFO) << "VirtualDisplayUtilMac::" << __func__
+ << " - serial number: " << serial_number << "(" << id
+ << "). WaitForDisplay success.";
+
+ DCHECK(!g_display_map[id]);
+ g_display_map[id] = display;
+
+ return id;
+}
+
// static
bool VirtualDisplayUtilMac::IsAPIAvailable() {
// TODO(crbug.com/40148077): Support headless bots.
diff --git a/ui/display/mac/test/virtual_display_util_mac_interactive_uitest.mm b/ui/display/mac/test/virtual_display_util_mac_interactive_uitest.mm
index a6f2880a..63b9c8eab 100644
--- a/ui/display/mac/test/virtual_display_util_mac_interactive_uitest.mm
+++ b/ui/display/mac/test/virtual_display_util_mac_interactive_uitest.mm
@@ -52,7 +52,7 @@
TEST_F(VirtualDisplayUtilMacInteractiveUitest, AddDisplay) {
int64_t id = virtual_display_util_mac_->AddDisplay(
- 1, display::test::VirtualDisplayUtilMac::k1920x1080);
+ display::test::VirtualDisplayUtilMac::k1920x1080);
EXPECT_NE(id, display::kInvalidDisplayId);
display::Display d;
@@ -62,7 +62,7 @@
TEST_F(VirtualDisplayUtilMacInteractiveUitest, RemoveDisplay) {
int64_t id = virtual_display_util_mac_->AddDisplay(
- 1, display::test::VirtualDisplayUtilMac::k1920x1080);
+ display::test::VirtualDisplayUtilMac::k1920x1080);
int display_count = display::Screen::GetScreen()->GetNumDisplays();
EXPECT_GT(display_count, 1);
@@ -82,11 +82,11 @@
int display_count = display::Screen::GetScreen()->GetNumDisplays();
virtual_display_util_mac_->AddDisplay(
- 1, display::test::VirtualDisplayUtilMac::k1920x1080);
+ display::test::VirtualDisplayUtilMac::k1920x1080);
EXPECT_EQ(display::Screen::GetScreen()->GetNumDisplays(), display_count + 1);
virtual_display_util_mac_->AddDisplay(
- 2, display::test::VirtualDisplayUtilMac::k1920x1080);
+ display::test::VirtualDisplayUtilMac::k1920x1080);
EXPECT_EQ(display::Screen::GetScreen()->GetNumDisplays(), display_count + 2);
virtual_display_util_mac_->ResetDisplays();
@@ -95,7 +95,7 @@
TEST_F(VirtualDisplayUtilMacInteractiveUitest, EnsureDisplayWithResolutionHD) {
int64_t id = virtual_display_util_mac_->AddDisplay(
- 1, display::test::VirtualDisplayUtil::k1920x1080);
+ display::test::VirtualDisplayUtil::k1920x1080);
display::Display d;
display::Screen::GetScreen()->GetDisplayWithDisplayId(id, &d);
@@ -104,7 +104,7 @@
TEST_F(VirtualDisplayUtilMacInteractiveUitest, EnsureDisplayWithResolutionXGA) {
int64_t id = virtual_display_util_mac_->AddDisplay(
- 1, display::test::VirtualDisplayUtil::k1024x768);
+ display::test::VirtualDisplayUtil::k1024x768);
display::Display d;
display::Screen::GetScreen()->GetDisplayWithDisplayId(id, &d);
diff --git a/ui/display/test/display_manager_test_api.cc b/ui/display/test/display_manager_test_api.cc
index 7d242ee..14642d5 100644
--- a/ui/display/test/display_manager_test_api.cc
+++ b/ui/display/test/display_manager_test_api.cc
@@ -76,8 +76,7 @@
maximum_support_display_ = kDefaultMaxSupportDisplayTest;
}
-int64_t DisplayManagerTestApi::AddDisplay(uint8_t id,
- const DisplayParams& display_params) {
+int64_t DisplayManagerTestApi::AddDisplay(const DisplayParams& display_params) {
const Displays& current_displays = display_manager_->active_display_list();
if (current_displays.size() >= maximum_support_display_) {
LOG(ERROR) << "Display limit exceeded.";
@@ -86,10 +85,6 @@
int64_t new_display_id = GetASynthesizedDisplayId();
std::vector<ManagedDisplayInfo> current_display_infos;
for (const Display& display : current_displays) {
- if (display_id_to_add_display_id_[display.id()] == id) {
- LOG(ERROR) << "Display with ID " << id << " already exists.";
- return kInvalidDisplayId;
- }
ManagedDisplayInfo display_info =
GetInternalManagedDisplayInfo(display.id());
gfx::Rect bounds = display_info.bounds_in_native();
@@ -110,7 +105,6 @@
current_display_infos.push_back(new_display);
UpdateDisplayWithDisplayInfoList(current_display_infos,
/*from_native_platform=*/false);
- display_id_to_add_display_id_[new_display.id()] = id;
return new_display.id();
}
@@ -119,7 +113,6 @@
std::vector<ManagedDisplayInfo> desired_display_infos;
for (const Display& display : active_displays) {
if (display.id() == display_id) {
- display_id_to_add_display_id_.erase(display_id);
continue;
}
desired_display_infos.push_back(
diff --git a/ui/display/test/display_manager_test_api.h b/ui/display/test/display_manager_test_api.h
index 0196673..53a1000b 100644
--- a/ui/display/test/display_manager_test_api.h
+++ b/ui/display/test/display_manager_test_api.h
@@ -39,7 +39,7 @@
~DisplayManagerTestApi() override;
// VirtualDisplayUtil:
- int64_t AddDisplay(uint8_t id, const DisplayParams& display_params) override;
+ int64_t AddDisplay(const DisplayParams& display_params) override;
void RemoveDisplay(int64_t display_id) override;
void ResetDisplays() override;
@@ -93,9 +93,6 @@
static size_t maximum_support_display_;
raw_ptr<DisplayManager> display_manager_; // not owned
-
- // TODO(crbug.com/40271794): Remove this once AddDisplay id is removed.
- base::flat_map<uint64_t, uint8_t> display_id_to_add_display_id_;
};
class DISPLAY_EXPORT ScopedSetInternalDisplayId {
diff --git a/ui/display/test/virtual_display_util.h b/ui/display/test/virtual_display_util.h
index 197c9a4..350c8c6 100644
--- a/ui/display/test/virtual_display_util.h
+++ b/ui/display/test/virtual_display_util.h
@@ -49,11 +49,9 @@
// requirements).
static std::unique_ptr<VirtualDisplayUtil> TryCreate(Screen* screen);
- // `id` is used to uniquely identify the virtual display. This function
- // returns the generated display::Display id, which can be used with the
- // Screen instance or passed to `RemoveDisplay`.
- virtual int64_t AddDisplay(uint8_t id,
- const DisplayParams& display_params) = 0;
+ // Adds a virtual display and returns the generated display::Display id, which
+ // can be used with the Screen instance or passed to `RemoveDisplay`.
+ virtual int64_t AddDisplay(const DisplayParams& display_params) = 0;
// Remove a virtual display corresponding to the specified display ID.
virtual void RemoveDisplay(int64_t display_id) = 0;
// Remove all added virtual displays.
diff --git a/ui/display/win/test/virtual_display_util_win.cc b/ui/display/win/test/virtual_display_util_win.cc
index d4c715c..817f693 100644
--- a/ui/display/win/test/virtual_display_util_win.cc
+++ b/ui/display/win/test/virtual_display_util_win.cc
@@ -7,6 +7,7 @@
#include <algorithm>
#include <functional>
#include <iterator>
+#include <limits>
#include "base/containers/fixed_flat_map.h"
#include "base/containers/flat_tree.h"
@@ -143,8 +144,8 @@
return DisplayDriverController::IsDriverInstalled();
}
-int64_t VirtualDisplayUtilWin::AddDisplay(uint8_t id,
- const DisplayParams& display_params) {
+int64_t VirtualDisplayUtilWin::AddDisplay(const DisplayParams& display_params) {
+ uint8_t id = SynthesizeInternalDisplayId();
if (virtual_displays_.find(id) != virtual_displays_.end()) {
LOG(ERROR) << "Duplicate virtual display ID added: " << id;
return kInvalidDisplayId;
@@ -272,6 +273,14 @@
}
// static
+uint8_t VirtualDisplayUtilWin::SynthesizeInternalDisplayId() {
+ static uint8_t synthesized_display_id = 0;
+ CHECK_LT(synthesized_display_id, std::numeric_limits<uint8_t>::max())
+ << "All synthesized display IDs in use.";
+ return synthesized_display_id++;
+}
+
+// static
std::unique_ptr<VirtualDisplayUtil> VirtualDisplayUtil::TryCreate(
Screen* screen) {
if (!VirtualDisplayUtilWin::IsAPIAvailable()) {
diff --git a/ui/display/win/test/virtual_display_util_win.h b/ui/display/win/test/virtual_display_util_win.h
index 2f6a0a60..7bd6c7f0 100644
--- a/ui/display/win/test/virtual_display_util_win.h
+++ b/ui/display/win/test/virtual_display_util_win.h
@@ -8,6 +8,7 @@
#include <vector>
#include "base/containers/flat_map.h"
+#include "base/containers/flat_set.h"
#include "base/memory/weak_ptr.h"
#include "base/run_loop.h"
#include "third_party/win_virtual_display/controller/display_driver_controller.h"
@@ -39,7 +40,7 @@
static bool IsAPIAvailable();
// VirtualDisplayUtil overrides:
- int64_t AddDisplay(uint8_t id, const DisplayParams& display_params) override;
+ int64_t AddDisplay(const DisplayParams& display_params) override;
void RemoveDisplay(int64_t display_id) override;
void ResetDisplays() override;
@@ -54,6 +55,9 @@
void StartWaiting();
void StopWaiting();
+ // Creates a new internal display ID to identify the display to the driver.
+ static uint8_t SynthesizeInternalDisplayId();
+
raw_ptr<Screen> screen_;
// True if the environment was considered headless during initialization.
const bool is_headless_;
@@ -61,7 +65,7 @@
DisplayDriverController driver_controller_;
// Contains the last configuration that was set.
DriverProperties current_config_;
- // Map of virtual display ID (product code) to corresponding display ID.
+ // Map of internal display ID (product code) to corresponding display ID.
base::flat_map<unsigned short, int64_t> virtual_displays_;
// Copy of the display list when this utility was constructed.
std::vector<display::Display> initial_displays_;
diff --git a/ui/display/win/test/virtual_display_util_win_interactive_uitest.cc b/ui/display/win/test/virtual_display_util_win_interactive_uitest.cc
index a8226a3..95aa3e15 100644
--- a/ui/display/win/test/virtual_display_util_win_interactive_uitest.cc
+++ b/ui/display/win/test/virtual_display_util_win_interactive_uitest.cc
@@ -42,45 +42,31 @@
EXPECT_TRUE(virtual_display_util_win_.IsAPIAvailable());
}
-TEST_F(VirtualDisplayUtilWinInteractiveUitest, AddDisplay) {
- int initial_display_count = screen()->GetNumDisplays();
- int64_t display_id = virtual_display_util_win_.AddDisplay(
- 1, display::test::VirtualDisplayUtilWin::k1920x1080);
- EXPECT_NE(display_id, display::kInvalidDisplayId);
- EXPECT_EQ(screen()->GetNumDisplays(), initial_display_count + 1);
- display::Display d;
- EXPECT_TRUE(screen()->GetDisplayWithDisplayId(display_id, &d));
- EXPECT_EQ(d.size(), gfx::Size(1920, 1080));
-
- // Expect failure when adding a duplicate index.
- EXPECT_EQ(virtual_display_util_win_.AddDisplay(
- 1, display::test::VirtualDisplayUtilWin::k1920x1080),
- display::kInvalidDisplayId);
-
- virtual_display_util_win_.ResetDisplays();
- EXPECT_FALSE(screen()->GetDisplayWithDisplayId(display_id, &d));
-}
-
TEST_F(VirtualDisplayUtilWinInteractiveUitest, AddRemove) {
int64_t display_id[3];
int initial_display_count = screen()->GetNumDisplays();
display_id[0] = virtual_display_util_win_.AddDisplay(
- 0, display::test::VirtualDisplayUtilWin::k1920x1080);
+ display::test::VirtualDisplayUtilWin::k1920x1080);
EXPECT_NE(display_id[0], display::kInvalidDisplayId);
display::Display d;
+ EXPECT_EQ(screen()->GetNumDisplays(), initial_display_count + 1);
EXPECT_TRUE(screen()->GetDisplayWithDisplayId(display_id[0], &d));
+ EXPECT_EQ(d.size(), gfx::Size(1920, 1080));
display_id[1] = virtual_display_util_win_.AddDisplay(
- 1, display::test::VirtualDisplayUtilWin::k1024x768);
+ display::test::VirtualDisplayUtilWin::k1024x768);
EXPECT_NE(display_id[1], display::kInvalidDisplayId);
+ EXPECT_EQ(screen()->GetNumDisplays(), initial_display_count + 2);
EXPECT_TRUE(screen()->GetDisplayWithDisplayId(display_id[1], &d));
+ EXPECT_EQ(d.size(), gfx::Size(1024, 768));
display_id[2] = virtual_display_util_win_.AddDisplay(
- 2, display::test::VirtualDisplayUtilWin::k1920x1080);
+ display::test::VirtualDisplayUtilWin::k1920x1080);
EXPECT_NE(display_id[2], display::kInvalidDisplayId);
+ EXPECT_EQ(screen()->GetNumDisplays(), initial_display_count + 3);
EXPECT_TRUE(screen()->GetDisplayWithDisplayId(display_id[2], &d));
+ EXPECT_EQ(d.size(), gfx::Size(1920, 1080));
- EXPECT_EQ(screen()->GetNumDisplays(), initial_display_count+3);
virtual_display_util_win_.RemoveDisplay(display_id[1]);
EXPECT_EQ(screen()->GetNumDisplays(), initial_display_count+2);
// Only virtual display 2 should no longer exist.