ui/display: don't clear ColorSpace after CTM/de/gamma configuration

Many moons ago, DisplayConfigurator::SetColorMatrix() /
SetGammaCorrection() were only called once after parsing a hypothetical
ICC file -- those days CrOS didn't manipulate the display's color
characteristics, so we configured either / both, and cleared the
ColorSpace to avoid overwriting those two DRM tables unnecessarily. Note
that the matrix/de/gamma tables were loaded after every wake up.

Time after that came Night Light and other color mgmt projects, and this
code path started being used when coming out of sleep/hibernate etc,
with the caveat that after applying their matrix/de/gamma tables, the
cached colorspaces was/were still cleared and lost.

The fix is then simple: since we need the |color_space| to stick around,
this CL avoids clearing it.

RunColorCorrectionClosureSync() is then superfluous and can be removed,
making evident that searching for |display_id| in |cached_displays_| is
only for verification purposes:

Bug: b:159224397, 1091552
Change-Id: I553646217c1012221844242c96148b8526657217
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2342583Reviewed-by: Ahmed Fakhry <afakhry@chromium.org>
Reviewed-by: Daniel Nicoara <dnicoara@chromium.org>
Commit-Queue: Miguel Casas <mcasas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#796442}

ui/display/manager/display_configurator.cc

@@ -44,31 +44,14 @@ struct DisplayState {
   const DisplayMode* mirror_mode = nullptr;
 };
 
-// This is used for calling either SetColorMatrix() or SetGammaCorrection()
-// depending on the given |color_correction_closure| which is run synchronously.
-// If |reset_color_space_on_success| is true and running
-// |color_correction_closure| returns true, then the color space of the display
-// with |display_id| will be reset.
-bool RunColorCorrectionClosureSync(
+// Returns whether |display_id| can be found in |display_list|,
+bool IsDisplayIdInDisplayStateList(
     int64_t display_id,
-    const DisplayConfigurator::DisplayStateList& cached_displays,
-    bool reset_color_space_on_success,
-    base::OnceCallback<bool(void)> color_correction_closure) {
-  for (DisplaySnapshot* display : cached_displays) {
-    if (display->display_id() != display_id)
-      continue;
-
-    const bool success = std::move(color_correction_closure).Run();
-
-    // Nullify the |display|s ColorSpace to avoid correcting colors twice, if
-    // we have successfully configured something.
-    if (success && reset_color_space_on_success)
-      display->reset_color_space();
-
-    return success;
-  }
-
-  return false;
+    const DisplayConfigurator::DisplayStateList& display_list) {
+  return std::find_if(display_list.begin(), display_list.end(),
+                      [display_id](DisplaySnapshot* display) {
+                        return display->display_id() == display_id;
+                      }) != display_list.end();
 }
 
 // Returns true if a platform native |mode| is equal to a |managed_mode|.
@@ -761,25 +744,19 @@ void DisplayConfigurator::ForceInitialConfigure() {
 bool DisplayConfigurator::SetColorMatrix(
     int64_t display_id,
     const std::vector<float>& color_matrix) {
-  return RunColorCorrectionClosureSync(
-      display_id, cached_displays_,
-      !color_matrix.empty() /* reset_color_space_on_success */,
-      base::BindOnce(&NativeDisplayDelegate::SetColorMatrix,
-                     base::Unretained(native_display_delegate_.get()),
-                     display_id, color_matrix));
+  if (!IsDisplayIdInDisplayStateList(display_id, cached_displays_))
+    return false;
+  return native_display_delegate_->SetColorMatrix(display_id, color_matrix);
 }
 
 bool DisplayConfigurator::SetGammaCorrection(
     int64_t display_id,
     const std::vector<GammaRampRGBEntry>& degamma_lut,
     const std::vector<GammaRampRGBEntry>& gamma_lut) {
-  const bool reset_color_space_on_success =
-      !degamma_lut.empty() || !gamma_lut.empty();
-  return RunColorCorrectionClosureSync(
-      display_id, cached_displays_, reset_color_space_on_success,
-      base::BindOnce(&NativeDisplayDelegate::SetGammaCorrection,
-                     base::Unretained(native_display_delegate_.get()),
-                     display_id, degamma_lut, gamma_lut));
+  if (!IsDisplayIdInDisplayStateList(display_id, cached_displays_))
+    return false;
+  return native_display_delegate_->SetGammaCorrection(display_id, degamma_lut,
+                                                      gamma_lut);
 }
 
 void DisplayConfigurator::SetPrivacyScreen(int64_t display_id, bool enabled) {

ui/display/types/display_snapshot.h

@@ -71,7 +71,6 @@ class DISPLAY_TYPES_EXPORT DisplaySnapshot {
     return color_correction_in_linear_space_;
   }
   const gfx::ColorSpace& color_space() const { return color_space_; }
-  void reset_color_space() { color_space_ = gfx::ColorSpace(); }
   uint32_t bits_per_channel() const { return bits_per_channel_; }
   const std::string& display_name() const { return display_name_; }
   const base::FilePath& sys_path() const { return sys_path_; }
@@ -122,7 +121,7 @@ class DISPLAY_TYPES_EXPORT DisplaySnapshot {
   // instead of gamma compressed one.
   const bool color_correction_in_linear_space_;
 
-  gfx::ColorSpace color_space_;
+  const gfx::ColorSpace color_space_;
 
   uint32_t bits_per_channel_;