ui: Fix a race in Privacy Screen Controller

This CL fixes a potential race/bug in PrivacyScreenController where
the cached display snapshots in the display configurator (from which
we derive the internal display's id) may become invalidated while
SetPrivacyScreen() tasks are awaiting execution. For example, this may
happen when rapid display configuration events are triggered when a
device wakes up and is connected to one or more external displays.

In addition, this CL instrument drm_util.cc to help investigating
https://crbug.com/1105919.
Also see:
http://crash/browse?q=product_name%3D%27Chrome_ChromeOS%27+AND+expanded_custom_data.ChromeCrashProto.magic_signature_1.name%3D%27ui%3A%3ADrmNativeDisplayDelegate%3A%3ASetPrivacyScreen%27#productname:1000,productversion:30,magicsignature:50,magicsignature2:50,stablesignature:50,operatingsystemfamily:30,+devicemodel:40,magicsignaturesorted:50

Bug: 1105919
Change-Id: Ic3b6b86bc9a6f50a115e5ec563b4595ab5c24975
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2330891
Commit-Queue: Gil Dekel <gildekel@chromium.org>
Reviewed-by: Ahmed Fakhry <afakhry@chromium.org>
Reviewed-by: Daniel Nicoara <dnicoara@chromium.org>
Cr-Commit-Position: refs/heads/master@{#797897}

ash/display/privacy_screen_controller.cc

@@ -17,8 +17,7 @@
 
 namespace ash {
 
-PrivacyScreenController::PrivacyScreenController()
-    : display_id_(display::kInvalidDisplayId) {
+PrivacyScreenController::PrivacyScreenController() {
   Shell::Get()->session_controller()->AddObserver(this);
   Shell::Get()->display_configurator()->AddObserver(this);
 }
@@ -35,7 +34,7 @@ void PrivacyScreenController::RegisterProfilePrefs(
 }
 
 bool PrivacyScreenController::IsSupported() const {
-  return display_id_ != display::kInvalidDisplayId;
+  return GetSupportedDisplayId() != display::kInvalidDisplayId;
 }
 
 bool PrivacyScreenController::IsManaged() const {
@@ -125,8 +124,6 @@ void PrivacyScreenController::OnSigninScreenPrefServiceInitialized(
 
 void PrivacyScreenController::OnDisplayModeChanged(
     const std::vector<display::DisplaySnapshot*>& displays) {
-  UpdateSupport();
-
   // OnDisplayModeChanged() may fire many times during Chrome's lifetime. We
   // limit automatic user pref initialization to login screen only.
   if (applying_login_screen_prefs_) {
@@ -136,17 +133,19 @@ void PrivacyScreenController::OnDisplayModeChanged(
 }
 
 void PrivacyScreenController::OnStateChanged(bool notify_observers) {
-  if (IsSupported()) {
-    const bool is_enabled = GetEnabled();
-    Shell::Get()->display_configurator()->SetPrivacyScreen(display_id_,
-                                                           is_enabled);
+  const int64_t display_id = GetSupportedDisplayId();
+  if (display_id == display::kInvalidDisplayId)
+    return;
 
-    if (!notify_observers)
-      return;
+  const bool is_enabled = GetEnabled();
+  Shell::Get()->display_configurator()->SetPrivacyScreen(display_id,
+                                                         is_enabled);
 
-    for (Observer& observer : observers_)
-      observer.OnPrivacyScreenSettingChanged(is_enabled);
-  }
+  if (!notify_observers)
+    return;
+
+  for (Observer& observer : observers_)
+    observer.OnPrivacyScreenSettingChanged(is_enabled);
 }
 
 void PrivacyScreenController::InitFromUserPrefs() {
@@ -165,7 +164,7 @@ void PrivacyScreenController::InitFromUserPrefs() {
   OnStateChanged(/*notify_observers=*/false);
 }
 
-void PrivacyScreenController::UpdateSupport() {
+int64_t PrivacyScreenController::GetSupportedDisplayId() const {
   const auto& cached_displays =
       Shell::Get()->display_configurator()->cached_displays();
 
@@ -173,12 +172,11 @@ void PrivacyScreenController::UpdateSupport() {
     if (display->type() == display::DISPLAY_CONNECTION_TYPE_INTERNAL &&
         display->privacy_screen_state() != display::kNotSupported &&
         display->current_mode()) {
-      display_id_ = display->display_id();
-      return;
+      return display->display_id();
     }
   }
 
-  display_id_ = display::kInvalidDisplayId;
+  return display::kInvalidDisplayId;
 }
 
 }  // namespace ash

ash/display/privacy_screen_controller.h

@@ -86,9 +86,9 @@ class ASH_EXPORT PrivacyScreenController
   // OnActiveUserPrefServiceChanged() is called.
   void InitFromUserPrefs();
 
-  // Updates the internal state of the controller to whether or not the
-  // privacy screen feature is currently supported by the device.
-  void UpdateSupport();
+  // Get the ID of the internal display that supports privacy screen. Return
+  // display::kInvalidDisplayId if none is found.
+  int64_t GetSupportedDisplayId() const;
 
   // The pref service of the currently active user. Can be null in
   // ash_unittests.
@@ -98,12 +98,6 @@ class ASH_EXPORT PrivacyScreenController
   // Chrome restart.
   bool applying_login_screen_prefs_ = false;
 
-  // Cache the display id of the internal display if it supports the privacy
-  // screen feature. If |display_id_| is kInvalidDisplayId then it's not
-  // supported, otherwise, it is. Updates every time there's a reconfiguration
-  // of displays.
-  int64_t display_id_;
-
   // Indicates whether PrivacyScreen is enforced by Data Leak Protection
   // feature.
   bool dlp_enforced_ = false;

ui/ozone/platform/drm/common/drm_util.cc

@@ -497,6 +497,14 @@ std::unique_ptr<display::DisplaySnapshot> CreateDisplaySnapshot(
   display::DisplaySnapshot::DisplayModeList modes =
       ExtractDisplayModes(info, active_pixel_size, &current_mode, &native_mode);
 
+  // TODO(https://crbug.com/1105919): Needed for investigating an issue where
+  // non-supporting devices broadcast privacy screen support on certain
+  // displays.
+  VLOG(1) << "DisplaySnapshot created: display_id=" << display_id
+          << " type=" << type << " current_mode="
+          << (current_mode ? current_mode->ToString() : "nullptr")
+          << " privacy_screen_state=" << privacy_screen_state;
+
   return std::make_unique<display::DisplaySnapshot>(
       display_id, origin, physical_size, type, is_aspect_preserving_scaling,
       has_overscan, privacy_screen_state, has_color_correction_matrix,