Don't reconfigure displays when they are suspended

When a Chrome OS device enters suspend, Chrome suspends the displays and
then probes and reconfigures them on resume.  However, an increasing
number of Chrome OS devices make use of the dark resume state to perform
some processing without alerting the user that the Chrome OS device has
woken up.  Attempting to configure the displays in this state causes the
screen to flash on and off, which can alert the user that their Chrome
OS device is doing something and is generally a bad user experience.
Instead, don't configure the displays in dark resume.  They are going to
be probed and reconfigured after a full resume anyway so there should be
no harm in skipping the reconfiguration during a dark resume.

BUG=chrome-os-partner:35253

Review URL: https://codereview.chromium.org/871653002

Cr-Commit-Position: refs/heads/master@{#314494}

3 files changed, 64 insertions, 0 deletions

diff --git a/ui/display/chromeos/display_configurator.cc b/ui/display/chromeos/display_configurator.cc
index e6a379c..bcfc46a 100644
--- a/ui/display/chromeos/display_configurator.cc
+++ b/ui/display/chromeos/display_configurator.cc
@@ -456,6 +456,7 @@ DisplayConfigurator::DisplayConfigurator()
       force_configure_(false),
       next_display_protection_client_id_(1),
       display_externally_controlled_(false),
+      displays_suspended_(false),
       layout_manager_(new DisplayLayoutManagerImpl(this)),
       weak_ptr_factory_(this) {
 }
@@ -856,9 +857,13 @@ void DisplayConfigurator::SuspendDisplays() {
     // racing with the HandleSuspendReadiness message.
     native_display_delegate_->SyncWithServer();
   }
+
+  displays_suspended_ = true;
 }
 
 void DisplayConfigurator::ResumeDisplays() {
+  displays_suspended_ = false;
+
   configure_timer_.Start(
       FROM_HERE,
       base::TimeDelta::FromMilliseconds(kResumeDelayMs),
@@ -875,6 +880,14 @@ void DisplayConfigurator::ConfigureDisplays() {
 }
 
 void DisplayConfigurator::RunPendingConfiguration() {
+  // Don't do anything if the displays are currently suspended.  Instead we will
+  // probe and reconfigure the displays if necessary in ResumeDisplays().
+  if (displays_suspended_) {
+    VLOG(1) << "Displays are currently suspended.  Not attempting to "
+            << "reconfigure them.";
+    return;
+  }
+
   // Configuration task is currently running. Do not start a second
   // configuration.
   if (configuration_task_)
diff --git a/ui/display/chromeos/display_configurator.h b/ui/display/chromeos/display_configurator.h
index 62afdee..c77b96a 100644
--- a/ui/display/chromeos/display_configurator.h
+++ b/ui/display/chromeos/display_configurator.h
@@ -414,6 +414,9 @@ class DISPLAY_EXPORT DisplayConfigurator : public NativeDisplayObserver {
   // Display controlled by an external entity.
   bool display_externally_controlled_;
 
+  // Whether the displays are currently suspended.
+  bool displays_suspended_;
+
   scoped_ptr<DisplayLayoutManager> layout_manager_;
 
   scoped_ptr<UpdateDisplayConfigurationTask> configuration_task_;
diff --git a/ui/display/chromeos/display_configurator_unittest.cc b/ui/display/chromeos/display_configurator_unittest.cc
index 96257b6..86b1473 100644
--- a/ui/display/chromeos/display_configurator_unittest.cc
+++ b/ui/display/chromeos/display_configurator_unittest.cc
@@ -965,6 +965,54 @@ TEST_F(DisplayConfiguratorTest, ContentProtection) {
             log_->GetActionsAndClear());
 }
 
+TEST_F(DisplayConfiguratorTest, DoNotConfigureWithSuspendedDisplays) {
+  InitWithSingleOutput();
+
+  // The DisplayConfigurator may occasionally receive OnConfigurationChanged()
+  // after the displays have been suspended.  This event should be ignored since
+  // the DisplayConfigurator will force a probe and reconfiguration of displays
+  // at resume time.
+  configurator_.SuspendDisplays();
+  EXPECT_EQ(kNoActions, log_->GetActionsAndClear());
+
+  configurator_.OnConfigurationChanged();
+  EXPECT_TRUE(test_api_.TriggerConfigureTimeout());
+  EXPECT_EQ(kNoActions, log_->GetActionsAndClear());
+
+  configurator_.ResumeDisplays();
+  EXPECT_TRUE(test_api_.TriggerConfigureTimeout());
+  EXPECT_EQ(
+      JoinActions(
+          kGrab,
+          GetFramebufferAction(small_mode_.size(), &outputs_[0], NULL).c_str(),
+          GetCrtcAction(outputs_[0], &small_mode_, gfx::Point(0, 0)).c_str(),
+          kForceDPMS,
+          kUngrab,
+          NULL),
+      log_->GetActionsAndClear());
+
+  // If a configuration task is pending when the displays are suspended, that
+  // task should not run either.
+  configurator_.OnConfigurationChanged();
+  configurator_.SuspendDisplays();
+  EXPECT_EQ(kNoActions, log_->GetActionsAndClear());
+
+  EXPECT_TRUE(test_api_.TriggerConfigureTimeout());
+  EXPECT_EQ(kNoActions, log_->GetActionsAndClear());
+
+  configurator_.ResumeDisplays();
+  EXPECT_TRUE(test_api_.TriggerConfigureTimeout());
+  EXPECT_EQ(
+      JoinActions(
+          kGrab,
+          GetFramebufferAction(small_mode_.size(), &outputs_[0], NULL).c_str(),
+          GetCrtcAction(outputs_[0], &small_mode_, gfx::Point(0, 0)).c_str(),
+          kForceDPMS,
+          kUngrab,
+          NULL),
+      log_->GetActionsAndClear());
+}
+
 TEST_F(DisplayConfiguratorTest, ContentProtectionTwoClients) {
   DisplayConfigurator::ContentProtectionClientId client1 =
       configurator_.RegisterContentProtectionClient();