Add display change observer.

Centralize the logic to force a repaint post display changes in an
observer. This way the complexity of refreshing the display is
handled in the observer rather than in every piece of code that
needs to update the display state.

Bug: b/180040068
Test: Ran cast_shell on the desktop to verify there are no crashes
Change-Id: I022850322c8b9177463bdc0526121016ebf0f330
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2727577
Reviewed-by: Daniel Nicoara <dnicoara@chromium.org>
Commit-Queue: Shiv Sakhuja <shivsak@google.com>
Cr-Commit-Position: refs/heads/master@{#873512}

chromecast/browser/BUILD.gn

@@ -309,6 +309,8 @@ cast_source_set("browser_base") {
       "cast_content_window_aura.cc",
       "cast_content_window_aura.h",
       "cast_web_service_aura.cc",
+      "display_configurator_observer.cc",
+      "display_configurator_observer.h",
     ]
 
     deps += [

chromecast/browser/cast_browser_main_parts.cc

@@ -616,6 +616,9 @@ int CastBrowserMainParts::PreMainMessageLoopRun() {
         std::make_unique<RoundedWindowCornersManager>(window_manager_.get());
   }
 
+  display_change_observer_ = std::make_unique<DisplayConfiguratorObserver>(
+      cast_browser_process_->display_configurator(), window_manager_.get());
+
 #if BUILDFLAG(ENABLE_CHROMECAST_EXTENSIONS)
   cast_browser_process_->SetAccessibilityManager(
       std::make_unique<AccessibilityManagerImpl>(window_manager_.get()));

chromecast/browser/cast_browser_main_parts.h

@@ -12,6 +12,7 @@
 #include "base/util/memory_pressure/multi_source_memory_pressure_monitor.h"
 #include "build/build_config.h"
 #include "build/buildflag.h"
+#include "chromecast/browser/display_configurator_observer.h"
 #include "chromecast/chromecast_buildflags.h"
 #include "content/public/browser/browser_context.h"
 #include "content/public/browser/browser_main_parts.h"
@@ -105,6 +106,7 @@ class CastBrowserMainParts : public content::BrowserMainParts {
   std::unique_ptr<CastScreen> cast_screen_;
   std::unique_ptr<CastWindowManagerAura> window_manager_;
   std::unique_ptr<RoundedWindowCornersManager> rounded_window_corners_manager_;
+  std::unique_ptr<DisplayConfiguratorObserver> display_change_observer_;
 #else
   std::unique_ptr<CastWindowManager> window_manager_;
 #endif  //  defined(USE_AURA)

chromecast/browser/cast_display_configurator.cc

@@ -141,6 +141,7 @@ void CastDisplayConfigurator::EnableDisplay(
   config_request.push_back(std::move(display_config_params));
 
   delegate_->Configure(config_request, std::move(callback));
+  NotifyObservers();
 }
 
 void CastDisplayConfigurator::DisableDisplay(
@@ -167,6 +168,7 @@ void CastDisplayConfigurator::SetColorMatrix(
   if (!delegate_ || !display_)
     return;
   delegate_->SetColorMatrix(display_->display_id(), color_matrix);
+  NotifyObservers();
 }
 
 void CastDisplayConfigurator::SetGammaCorrection(
@@ -176,6 +178,12 @@ void CastDisplayConfigurator::SetGammaCorrection(
     return;
 
   delegate_->SetGammaCorrection(display_->display_id(), degamma_lut, gamma_lut);
+  NotifyObservers();
+}
+
+void CastDisplayConfigurator::NotifyObservers() {
+  for (Observer& observer : observers_)
+    observer.OnDisplayStateChanged();
 }
 
 void CastDisplayConfigurator::ForceInitialConfigure() {
@@ -265,5 +273,13 @@ void CastDisplayConfigurator::UpdateScreen(
   touch_device_manager_->OnDisplayConfigured(display_id, rotation, bounds);
 }
 
+void CastDisplayConfigurator::AddObserver(Observer* observer) {
+  observers_.AddObserver(observer);
+}
+
+void CastDisplayConfigurator::RemoveObserver(Observer* observer) {
+  observers_.RemoveObserver(observer);
+}
+
 }  // namespace shell
 }  // namespace chromecast

chromecast/browser/cast_display_configurator.h

@@ -11,6 +11,7 @@
 #include "base/containers/flat_map.h"
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
+#include "base/observer_list.h"
 #include "ui/display/display.h"
 #include "ui/display/types/display_configuration_params.h"
 #include "ui/display/types/native_display_delegate.h"
@@ -40,6 +41,12 @@ class CastTouchDeviceManager;
 // doesn't really do anything when using OzonePlatformCast.
 class CastDisplayConfigurator : public display::NativeDisplayObserver {
  public:
+  class Observer {
+   public:
+    virtual ~Observer() = default;
+    virtual void OnDisplayStateChanged() = 0;
+  };
+
   explicit CastDisplayConfigurator(CastScreen* screen);
   ~CastDisplayConfigurator() override;
 
@@ -50,6 +57,9 @@ class CastDisplayConfigurator : public display::NativeDisplayObserver {
   void EnableDisplay(display::ConfigureCallback callback);
   void DisableDisplay(display::ConfigureCallback callback);
 
+  void AddObserver(Observer* observer);
+  void RemoveObserver(Observer* observer);
+
   void ConfigureDisplayFromCommandLine();
   void SetColorMatrix(const std::vector<float>& color_matrix);
   void SetGammaCorrection(
@@ -58,6 +68,7 @@ class CastDisplayConfigurator : public display::NativeDisplayObserver {
 
  private:
   void ForceInitialConfigure();
+  void NotifyObservers();
   void OnDisplaysAcquired(
       bool force_initial_configure,
       const std::vector<display::DisplaySnapshot*>& displays);
@@ -70,6 +81,8 @@ class CastDisplayConfigurator : public display::NativeDisplayObserver {
                     float device_scale_factor,
                     display::Display::Rotation rotation);
 
+  base::ObserverList<Observer>::Unchecked observers_;
+
   std::unique_ptr<display::NativeDisplayDelegate> delegate_;
   std::unique_ptr<CastTouchDeviceManager> touch_device_manager_;
   display::DisplaySnapshot* display_;

chromecast/browser/display_configurator_observer.cc

@@ -0,0 +1,27 @@
+// Copyright 2021 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "display_configurator_observer.h"
+
+namespace chromecast {
+
+DisplayConfiguratorObserver::DisplayConfiguratorObserver(
+    chromecast::shell::CastDisplayConfigurator* display_configurator,
+    chromecast::CastWindowManagerAura* manager)
+    : display_configurator_(display_configurator), window_manager_(manager) {
+  display_configurator_->AddObserver(this);
+}
+
+DisplayConfiguratorObserver::~DisplayConfiguratorObserver() {
+  display_configurator_->RemoveObserver(this);
+}
+
+void DisplayConfiguratorObserver::OnDisplayStateChanged() {
+  window_manager_->GetRootWindow()
+      ->GetHost()
+      ->compositor()
+      ->ScheduleFullRedraw();
+}
+
+}  // namespace chromecast

chromecast/browser/display_configurator_observer.h

@@ -0,0 +1,42 @@
+// Copyright 2021 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROMECAST_BROWSER_DISPLAY_CONFIGURATOR_OBSERVER_H_
+#define CHROMECAST_BROWSER_DISPLAY_CONFIGURATOR_OBSERVER_H_
+
+#include <memory>
+
+#include "chromecast/browser/cast_display_configurator.h"
+#include "chromecast/graphics/cast_window_manager_aura.h"
+
+namespace chromecast {
+
+// Observer class that can respond to Display Configurator state changes.
+// Forces a repaint to ensure content is refreshed post display configuration
+// change.
+class DisplayConfiguratorObserver
+    : public shell::CastDisplayConfigurator::Observer {
+ public:
+  DisplayConfiguratorObserver(
+      shell::CastDisplayConfigurator* display_configurator,
+      CastWindowManagerAura* manager);
+
+  ~DisplayConfiguratorObserver() override;
+
+  DisplayConfiguratorObserver(const DisplayConfiguratorObserver&) = delete;
+
+  DisplayConfiguratorObserver& operator=(const DisplayConfiguratorObserver&) =
+      delete;
+
+  // CastDisplayConfigurator::Observer
+  void OnDisplayStateChanged() override;
+
+ private:
+  shell::CastDisplayConfigurator* display_configurator_;
+  CastWindowManagerAura* window_manager_;
+};
+
+}  // namespace chromecast
+
+#endif  // CHROMECAST_BROWSER_DISPLAY_CONFIGURATOR_OBSERVER_H_

chromecast/ui/display_settings/color_temperature_animation.cc

@@ -9,10 +9,6 @@
 
 #include "base/numerics/ranges.h"
 #include "base/time/time.h"
-#include "chromecast/graphics/cast_window_manager.h"
-#include "ui/aura/window.h"
-#include "ui/aura/window_tree_host.h"
-#include "ui/compositor/compositor.h"
 #include "ui/compositor/scoped_animation_duration_scale_mode.h"
 
 #if defined(USE_AURA)
@@ -39,19 +35,16 @@ float Interpolate(const std::vector<float>& vec, float idx) {
 }  // namespace
 
 ColorTemperatureAnimation::ColorTemperatureAnimation(
-    CastWindowManager* window_manager,
     shell::CastDisplayConfigurator* display_configurator,
     const DisplaySettingsManager::ColorTemperatureConfig& config)
     : gfx::LinearAnimation(kManualAnimationDuration,
                            kAnimationFrameRate,
                            nullptr),
-      window_manager_(window_manager),
       display_configurator_(display_configurator),
       config_(config),
       start_temperature_(config.neutral_temperature),
       current_temperature_(config.neutral_temperature),
       target_temperature_(config_.neutral_temperature) {
-  DCHECK(window_manager_);
 #if defined(USE_AURA)
   DCHECK(display_configurator_);
 #endif  // defined(USE_AURA)
@@ -123,12 +116,6 @@ void ColorTemperatureAnimation::ApplyValuesToDisplay() {
     std::vector<float> color_matrix = {red_scale, 0, 0, 0,         green_scale,
                                        0,         0, 0, blue_scale};
     display_configurator_->SetColorMatrix(color_matrix);
-    // The CTM is applied on the next swap buffers, so we need to make sure the
-    // root window triggers a swap buffer otherwise the content will not update.
-    window_manager_->GetRootWindow()
-        ->GetHost()
-        ->compositor()
-        ->ScheduleFullRedraw();
 #endif  // defined(USE_AURA)
   }
 }

chromecast/ui/display_settings/color_temperature_animation.h

@@ -13,8 +13,6 @@
 
 namespace chromecast {
 
-class CastWindowManager;
-
 namespace shell {
 class CastDisplayConfigurator;
 }
@@ -24,7 +22,6 @@ class CastDisplayConfigurator;
 class ColorTemperatureAnimation : public gfx::LinearAnimation {
  public:
   ColorTemperatureAnimation(
-      CastWindowManager* window_manager,
       shell::CastDisplayConfigurator* display_configurator,
       const DisplaySettingsManager::ColorTemperatureConfig& config);
   ColorTemperatureAnimation(const ColorTemperatureAnimation&) = delete;
@@ -47,7 +44,6 @@ class ColorTemperatureAnimation : public gfx::LinearAnimation {
 
   void ApplyValuesToDisplay();
 
-  CastWindowManager* const window_manager_;
   shell::CastDisplayConfigurator* const display_configurator_;
 
   const DisplaySettingsManager::ColorTemperatureConfig config_;

chromecast/ui/display_settings/gamma_configurator.cc

@@ -7,10 +7,6 @@
 #include <limits>
 
 #include "chromecast/browser/cast_display_configurator.h"
-#include "chromecast/graphics/cast_window_manager.h"
-#include "ui/aura/window.h"
-#include "ui/aura/window_tree_host.h"
-#include "ui/compositor/compositor.h"
 
 namespace chromecast {
 namespace {
@@ -45,11 +41,8 @@ std::vector<display::GammaRampRGBEntry> InvertGammaLut(
 }  // namespace
 
 GammaConfigurator::GammaConfigurator(
-    CastWindowManager* window_manager,
     shell::CastDisplayConfigurator* display_configurator)
-    : window_manager_(window_manager),
-      display_configurator_(display_configurator) {
-  DCHECK(window_manager_);
+    : display_configurator_(display_configurator) {
   DCHECK(display_configurator_);
 }
 
@@ -69,13 +62,6 @@ void GammaConfigurator::ApplyGammaLut() {
     display_configurator_->SetGammaCorrection({}, InvertGammaLut(gamma_lut_));
   else
     display_configurator_->SetGammaCorrection({}, gamma_lut_);
-
-  // The LUT is applied on the next swap buffers, so we need to make sure the
-  // root window triggers a swap buffer otherwise the content will not update.
-  window_manager_->GetRootWindow()
-      ->GetHost()
-      ->compositor()
-      ->ScheduleFullRedraw();
 }
 
 void GammaConfigurator::SetColorInversion(bool invert) {

chromecast/ui/display_settings/gamma_configurator.h

@@ -11,16 +11,14 @@
 
 namespace chromecast {
 
-class CastWindowManager;
-
 namespace shell {
 class CastDisplayConfigurator;
 }  // namespace shell
 
 class GammaConfigurator {
  public:
-  GammaConfigurator(CastWindowManager* window_manager,
-                    shell::CastDisplayConfigurator* display_configurator);
+  explicit GammaConfigurator(
+      shell::CastDisplayConfigurator* display_configurator);
   GammaConfigurator(const GammaConfigurator&) = delete;
   GammaConfigurator& operator=(const GammaConfigurator&) = delete;
   ~GammaConfigurator();
@@ -33,7 +31,6 @@ class GammaConfigurator {
  private:
   void ApplyGammaLut();
 
-  CastWindowManager* const window_manager_;
   shell::CastDisplayConfigurator* display_configurator_;
 
   bool is_initialized_ = false;

chromecast/ui/display_settings_manager_impl.cc

@@ -17,8 +17,6 @@
 #include "chromecast/browser/cast_browser_process.h"
 #include "chromecast/browser/cast_display_configurator.h"
 #include "chromecast/ui/display_settings/gamma_configurator.h"
-#include "ui/aura/window.h"
-#include "ui/aura/window_tree_host.h"
 #endif  // defined(USE_AURA)
 
 namespace chromecast {
@@ -42,15 +40,13 @@ DisplaySettingsManagerImpl::DisplaySettingsManagerImpl(
       display_configurator_(
           shell::CastBrowserProcess::GetInstance()->display_configurator()),
       gamma_configurator_(
-          std::make_unique<GammaConfigurator>(window_manager_,
-                                              display_configurator_)),
+          std::make_unique<GammaConfigurator>(display_configurator_)),
 #else
       display_configurator_(nullptr),
 #endif  // defined(USE_AURA)
       brightness_(-1.0f),
       screen_power_controller_(ScreenPowerController::Create(this)),
       color_temperature_animation_(std::make_unique<ColorTemperatureAnimation>(
-          window_manager_,
           display_configurator_,
           color_temperature_config)),
       weak_factory_(this) {
@@ -100,9 +96,7 @@ void DisplaySettingsManagerImpl::AddReceiver(
 
 void DisplaySettingsManagerImpl::SetScreenPowerOn(PowerToggleCallback callback) {
 #if defined(USE_AURA)
-  display_configurator_->EnableDisplay(
-      base::BindOnce(&DisplaySettingsManagerImpl::OnScreenEnabled,
-                     weak_factory_.GetWeakPtr(), std::move(callback)));
+  display_configurator_->EnableDisplay(std::move(callback));
 #endif  // defined(USE_AURA)
 }
 
@@ -196,17 +190,4 @@ void DisplaySettingsManagerImpl::UpdateBrightness(float brightness,
     brightness_animation_->AnimateToNewValue(brightness, duration);
 }
 
-void DisplaySettingsManagerImpl::OnScreenEnabled(PowerToggleCallback callback,
-                                                 bool status) {
-#if defined(USE_AURA)
-  // Force a swap buffers otherwise we might be stuck showing the modeset
-  // buffer.
-  window_manager_->GetRootWindow()
-      ->GetHost()
-      ->compositor()
-      ->ScheduleFullRedraw();
-#endif  // defined(USE_AURA)
-  std::move(callback).Run(status);
-}
-
 }  // namespace chromecast

chromecast/ui/display_settings_manager_impl.h

@@ -80,8 +80,6 @@ class DisplaySettingsManagerImpl : public DisplaySettingsManager,
 
   void UpdateBrightness(float brightness, base::TimeDelta duration);
 
-  void OnScreenEnabled(PowerToggleCallback callback, bool status);
-
   CastWindowManager* const window_manager_;
   shell::CastDisplayConfigurator* const display_configurator_;