ui/ozone: Update DrmDisplays during conflict resolution

Patch https://crrev.com/c/3032738 introduced logic to detect and resolve
EDID-based display ID collisions in freshly created DisplaySnapshots,
but neglected to update the underlying DrmDisplays in Ozone.

This CL is rectifying this oversight. In addition, it resolves similar
conflicts during initialization, when dummy display representations are
created while the GPU process is coming online.

Of note, this CL separates DisplaySnapshot creation from DrmDisplay
object state updates. This allows us to resolve the collisions before
caching the new display ID within each DrmDisplay representation.

Bug: b:193019614, b:193027414
Test: display_unittests && ozone_unittests
Change-Id: I274f6912a9c8cf1d9b08e30406eed80077a2ed9a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4071916
Commit-Queue: Gil Dekel <gildekel@chromium.org>
Reviewed-by: Daniel Nicoara <dnicoara@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1078439}

ui/ozone/platform/drm/common/display_types.h

@@ -21,6 +21,9 @@ using MovableDisplaySnapshots =
 
 using EventPropertyMap = base::flat_map<std::string, std::string>;
 
+using MapEdidIdToDisplaySnapshot =
+    base::flat_map<int64_t, display::DisplaySnapshot*>;
+
 }  // namespace ui
 
 #endif  // UI_OZONE_PLATFORM_DRM_COMMON_DISPLAY_TYPES_H_

ui/ozone/platform/drm/gpu/drm_display.cc

@@ -149,33 +149,28 @@ uint32_t DrmDisplay::connector() const {
   return connector_->connector_id;
 }
 
-std::unique_ptr<display::DisplaySnapshot> DrmDisplay::Update(
-    HardwareDisplayControllerInfo* info,
-    uint8_t device_index) {
-  std::unique_ptr<display::DisplaySnapshot> params = CreateDisplaySnapshot(
-      info, drm_->get_fd(), drm_->device_path(), device_index, origin_);
-  base_connector_id_ = params->base_connector_id();
-  crtc_ = info->crtc()->crtc_id;
+void DrmDisplay::Update(HardwareDisplayControllerInfo* info,
+                        const display::DisplaySnapshot* display_snapshot) {
   // We take ownership of |info|'s connector because it will not be used again
   // beyond this point. It is safe to assume that |connector_| is populated
   // since it was obtained from GetDisplayInfosAndInvalidCrtcs(), which discards
   // invalid (nullptr) connectors.
   connector_ = info->ReleaseConnector();
   DCHECK(connector_);
 
-  display_id_ = params->display_id();
+  crtc_ = info->crtc()->crtc_id;
+  display_id_ = display_snapshot->display_id();
+  base_connector_id_ = display_snapshot->base_connector_id();
   modes_ = GetDrmModeVector(connector_.get());
-  is_hdr_capable_ =
-      params->bits_per_channel() > 8 && params->color_space().IsHDR();
+  is_hdr_capable_ = display_snapshot->bits_per_channel() > 8 &&
+                    display_snapshot->color_space().IsHDR();
   privacy_screen_property_ =
       std::make_unique<PrivacyScreenProperty>(drm(), connector_.get());
 #if BUILDFLAG(IS_CHROMEOS_ASH)
   is_hdr_capable_ =
       is_hdr_capable_ &&
       base::FeatureList::IsEnabled(display::features::kUseHDRTransferFunction);
 #endif
-
-  return params;
 }
 
 // When reading DRM state always check that it's still valid. Any sort of events

ui/ozone/platform/drm/gpu/drm_display.h

@@ -69,11 +69,12 @@ class DrmDisplay {
   uint32_t crtc() const { return crtc_; }
   uint32_t connector() const;
   const std::vector<drmModeModeInfo>& modes() const { return modes_; }
+  const gfx::Point& origin() { return origin_; }
 
-  std::unique_ptr<display::DisplaySnapshot> Update(
-      HardwareDisplayControllerInfo* info,
-      uint8_t device_index);
-
+  // Updates the internal state of this display in accordance to |info| and
+  // |display_snapshot|.
+  void Update(HardwareDisplayControllerInfo* info,
+              const display::DisplaySnapshot* display_snapshot);
   void SetOrigin(const gfx::Point origin) { origin_ = origin; }
   bool GetHDCPState(display::HDCPState* state,
                     display::ContentProtectionMethod* protection_method);

ui/ozone/platform/drm/gpu/drm_gpu_display_manager.cc

@@ -31,8 +31,6 @@ namespace ui {
 namespace {
 constexpr char kMultipleDisplayIdsCollisionDetected[] =
     "Display.MultipleDisplays.GenerateId.CollisionDetection";
-using MapDisplayIdToIndexAndSnapshotPair =
-    base::flat_map<int64_t, display::DisplaySnapshot*>;
 
 // A list of property names that are blocked from issuing a full display
 // configuration (modeset) via a udev display CHANGE event.
@@ -137,7 +135,7 @@ MovableDisplaySnapshots DrmGpuDisplayManager::GetDisplays() {
 
   const DrmDeviceVector& devices = drm_device_manager_->GetDrmDevices();
   size_t device_index = 0;
-  MapDisplayIdToIndexAndSnapshotPair id_collision_map;
+  MapEdidIdToDisplaySnapshot edid_id_collision_map;
   bool collision_detected = false;
   for (const auto& drm : devices) {
     if (device_index >= kMaxDrmCount) {
@@ -156,32 +154,48 @@ MovableDisplaySnapshots DrmGpuDisplayManager::GetDisplays() {
           old_displays,
           DisplayComparator(drm, display_info->crtc()->crtc_id,
                             display_info->connector()->connector_id));
+      std::unique_ptr<DrmDisplay> current_drm_display;
       if (it != old_displays.end()) {
-        displays_.push_back(std::move(*it));
+        current_drm_display = std::move(*it);
         old_displays.erase(it);
       } else {
-        displays_.push_back(std::make_unique<DrmDisplay>(drm));
+        current_drm_display = std::make_unique<DrmDisplay>(drm);
       }
 
-      // Do not use |display_info| beyond this point, since some of its internal
-      // references will be surrendered.
-      auto display_snapshot = displays_.back()->Update(
-          display_info.get(), static_cast<uint8_t>(device_index));
-
-      const auto colliding_display_snapshot_iter =
-          id_collision_map.find(display_snapshot->edid_display_id());
-      if (colliding_display_snapshot_iter != id_collision_map.end()) {
-        // There is a collision between |display_snapshot| and a previous
-        // display. Resolve it by adding their connector indices to their
-        // display IDs, respectively.
+      // Create the new DisplaySnapshot and resolve display ID collisions.
+      std::unique_ptr<display::DisplaySnapshot> current_display_snapshot =
+          CreateDisplaySnapshot(display_info.get(),
+                                current_drm_display->drm()->get_fd(),
+                                current_drm_display->drm()->device_path(),
+                                static_cast<uint8_t>(device_index),
+                                current_drm_display->origin());
+
+      const auto colliding_display_snapshot_iter = edid_id_collision_map.find(
+          current_display_snapshot->edid_display_id());
+      if (colliding_display_snapshot_iter != edid_id_collision_map.end()) {
         collision_detected = true;
-        display_snapshot->AddIndexToDisplayId();
-        colliding_display_snapshot_iter->second->AddIndexToDisplayId();
-      } else {
-        id_collision_map[display_snapshot->edid_display_id()] =
-            display_snapshot.get();
+
+        // Resolve collisions by adding each colliding display's connector index
+        // to its display ID.
+        current_display_snapshot->AddIndexToDisplayId();
+
+        display::DisplaySnapshot* colliding_display_snapshot =
+            colliding_display_snapshot_iter->second;
+        colliding_display_snapshot->AddIndexToDisplayId();
+        edid_id_collision_map[colliding_display_snapshot->edid_display_id()] =
+            colliding_display_snapshot;
       }
-      params_list.push_back(std::move(display_snapshot));
+
+      // Do not use |display_info| beyond this point, since some of its internal
+      // references will be surrendered.
+      current_drm_display->Update(display_info.get(),
+                                  current_display_snapshot.get());
+
+      // Update the map with the new (or potentially resolved) display snapshot.
+      edid_id_collision_map[current_display_snapshot->edid_display_id()] =
+          current_display_snapshot.get();
+      params_list.push_back(std::move(current_display_snapshot));
+      displays_.push_back(std::move(current_drm_display));
     }
     device_index++;
   }

ui/ozone/platform/drm/host/drm_display_host_manager.cc

@@ -208,13 +208,34 @@ DrmDisplayHostManager::DrmDisplayHostManager(
   auto display_infos =
       GetAvailableDisplayControllerInfos(primary_drm_device_handle_->fd());
   has_dummy_display_ = !display_infos.empty();
-  for (const auto& display_info : display_infos) {
-    displays_.push_back(std::make_unique<DrmDisplayHost>(
-        proxy_,
+  MapEdidIdToDisplaySnapshot edid_id_collision_map;
+  for (auto& display_info : display_infos) {
+    // Create a dummy DisplaySnapshot and resolve display ID collisions.
+    std::unique_ptr<display::DisplaySnapshot> current_display_snapshot =
         CreateDisplaySnapshot(
             display_info.get(), primary_drm_device_handle_->fd(),
-            primary_drm_device_handle_->sys_path(), 0, gfx::Point()),
-        true /* is_dummy */));
+            primary_drm_device_handle_->sys_path(), 0, gfx::Point());
+
+    const auto colliding_display_snapshot_iter =
+        edid_id_collision_map.find(current_display_snapshot->edid_display_id());
+    if (colliding_display_snapshot_iter != edid_id_collision_map.end()) {
+      // Resolve collisions by adding each colliding display's connector index
+      // to its display ID.
+      current_display_snapshot->AddIndexToDisplayId();
+
+      display::DisplaySnapshot* colliding_display_snapshot =
+          colliding_display_snapshot_iter->second;
+      colliding_display_snapshot->AddIndexToDisplayId();
+      edid_id_collision_map[colliding_display_snapshot->edid_display_id()] =
+          colliding_display_snapshot;
+    }
+
+    // Update the map with the new (or potentially resolved) display snapshot.
+    edid_id_collision_map[current_display_snapshot->edid_display_id()] =
+        current_display_snapshot.get();
+
+    displays_.push_back(std::make_unique<DrmDisplayHost>(
+        proxy_, std::move(current_display_snapshot), true /* is_dummy */));
   }
 }