Delay display configuration after waking up from suspend with multi displays

ui/display/chromeos/display_configurator_unittest.cc

Index: ui/display/chromeos/display_configurator_unittest.cc
diff --git a/ui/display/chromeos/display_configurator_unittest.cc b/ui/display/chromeos/display_configurator_unittest.cc
index d04850e6806e125db8540233cff9b72c1c2658c0..b34ff8e036d871b3ed2b1fc829ba87ba454d1e75 100644
--- a/ui/display/chromeos/display_configurator_unittest.cc
+++ b/ui/display/chromeos/display_configurator_unittest.cc
@@ -7,6 +7,7 @@
 #include <stddef.h>
 #include <stdint.h>
 
+#include "base/callback_helpers.h"
 #include "base/macros.h"
 #include "base/memory/ptr_util.h"
 #include "base/memory/scoped_vector.h"
@@ -23,6 +24,23 @@ namespace test {
 
 namespace {
 
+enum CallbackResult {
+  CALLBACK_FAILURE,
+  CALLBACK_SUCCESS,
+  CALLBACK_NOT_CALLED,
+};
+
+// The delay less than which we expect the configuration to be finished.
+constexpr base::TimeDelta kMaxShortDelayMs =
+    base::TimeDelta::FromMilliseconds(
+        DisplayConfigurator::kConfigureDelayMs / 3);
+
+// The minimum configuration delay we expect when resuming while in 2+ display
+// mode.
+constexpr base::TimeDelta kMinLongDelayMs =

[Daniel Erat, 2016/10/24 16:31:04]

do these tests actually wait this long for the code to run? it looks like they
might. if so, please find a way to avoid doing this -- tests need to run quickly
and shouldn't sleep. see the existing
DisplayConfigurator::TestApi::TriggerConfigureTimeout() method used by other
tests. to test the delay before you make the timer fire, add a new method to
TestApi like:

  base::TimeDelta GetConfigureDelay() const {
    return configurator_->configure_timer_.IsRunning() ?
        configurator_->configure_timer_.GetCurrentDelay() :
        base::TimeDelta();
  }

[afakhry, 2016/10/25 00:37:10]

That's a great suggestion! The tests indeed used to wait for long durations,
which wasn't pleasant and could introduce future flakiness.

This simplifies the waiters logic a lot.
Done.

+    base::TimeDelta::FromMilliseconds(
+        DisplayConfigurator::kResumeConfigureMultiDisplayDelayMs);
+
 class TestObserver : public DisplayConfigurator::Observer {
  public:
   explicit TestObserver(DisplayConfigurator* configurator)
@@ -119,14 +137,84 @@ class TestMirroringController
   DISALLOW_COPY_AND_ASSIGN(TestMirroringController);
 };
 
-class DisplayConfiguratorTest : public testing::Test {
+// Abstracts waiting for the display configuration to be completed and getting
+// the time it took to complete.
+class ConfigurationWaiter {
  public:
-  enum CallbackResult {
-    CALLBACK_FAILURE,
-    CALLBACK_SUCCESS,
-    CALLBACK_NOT_CALLED,
-  };
+  ConfigurationWaiter()
+      : on_configured_callback_(base::Bind(&ConfigurationWaiter::OnConfigured,
+                                           base::Unretained(this))),
+        start_(base::TimeTicks::Now()),
+        end_(base::TimeTicks::Max()),
+        callback_result_(CALLBACK_NOT_CALLED),
+        pending_configuration_(true) {
+  }
+
+  ~ConfigurationWaiter() = default;
+
+  void Reset() {
+    start_ = base::TimeTicks::Now();
+    end_ = base::TimeTicks::Max();
+    callback_result_ = CALLBACK_NOT_CALLED;
+    pending_configuration_ = true;
+  }
+
+  // Waits for the configuration to complete and returns how long it took since
+  // we created or Reset()ed the timer.
+  base::TimeDelta Wait() WARN_UNUSED_RESULT {
+    if (pending_configuration_) {
+      base::RunLoop run_loop;
+      quit_closure_ = run_loop.QuitClosure();
+      run_loop.Run();
+    }
+
+    return end_ - start_;
+  }
+
+  const DisplayConfigurator::ConfigurationCallback&
+  on_configuration_callback() const {
+    return on_configured_callback_;
+  }
+
+  CallbackResult callback_result() const { return callback_result_; }
+
+ private:
+  void OnConfigured(bool status) {
+    callback_result_ = status ? CALLBACK_SUCCESS : CALLBACK_FAILURE;
+
+    pending_configuration_ = false;
+    end_ = base::TimeTicks::Now();
+
+    if (!quit_closure_.is_null())
+      base::ResetAndReturn(&quit_closure_).Run();
+  }
+
+  // The callback that should be used with the DisplayConfigurator calls to be
+  // invoked with the display configuration is done. It will always execute this
+  // waiter's OnConfigured() to end the wait (if any) and update the timers.
+  const DisplayConfigurator::ConfigurationCallback on_configured_callback_;
+
+  // Time at which we start measuring how long display configuration took.
+  base::TimeTicks start_;
+
+  // Time at which the display configuration was complete.
+  base::TimeTicks end_;
 
+  // The quit closure of the waiter runloop.
+  base::Closure quit_closure_;
+
+  // The status of the display configuration.
+  CallbackResult callback_result_;
+
+  // True if we are expecting and waiting for a display configuration
+  // completion, false if the configuration has already been completed.
+  bool pending_configuration_;
+
+  DISALLOW_COPY_AND_ASSIGN(ConfigurationWaiter);
+};
+
+class DisplayConfiguratorTest : public testing::Test {
+ public:
   DisplayConfiguratorTest()
       : small_mode_(gfx::Size(1366, 768), false, 60.0f),
         big_mode_(gfx::Size(2560, 1600), false, 60.0f),
@@ -135,7 +223,6 @@ class DisplayConfiguratorTest : public testing::Test {
         enable_content_protection_status_(0),
         enable_content_protection_call_count_(0),
         query_content_protection_call_count_(0),
-        callback_result_(CALLBACK_NOT_CALLED),
         display_control_result_(CALLBACK_NOT_CALLED) {}
   ~DisplayConfiguratorTest() override {}
 
@@ -184,10 +271,6 @@ class DisplayConfiguratorTest : public testing::Test {
     UpdateOutputs(2, false);
   }
 
-  void OnConfiguredCallback(bool status) {
-    callback_result_ = (status ? CALLBACK_SUCCESS : CALLBACK_FAILURE);
-  }
-
   void OnDisplayControlUpdated(bool status) {
     display_control_result_ = (status ? CALLBACK_SUCCESS : CALLBACK_FAILURE);
   }
@@ -247,12 +330,6 @@ class DisplayConfiguratorTest : public testing::Test {
               log_->GetActionsAndClear());
   }
 
-  CallbackResult PopCallbackResult() {
-    CallbackResult result = callback_result_;
-    callback_result_ = CALLBACK_NOT_CALLED;
-    return result;
-  }
-
   CallbackResult PopDisplayControlResult() {
     CallbackResult result = display_control_result_;
     display_control_result_ = CALLBACK_NOT_CALLED;
@@ -276,7 +353,6 @@ class DisplayConfiguratorTest : public testing::Test {
 
   TestDisplaySnapshot outputs_[3];
 
-  CallbackResult callback_result_;
   CallbackResult display_control_result_;
 
  private:
@@ -669,12 +745,13 @@ TEST_F(DisplayConfiguratorTest, SetDisplayPower) {
   // Turning off the internal display should switch the external display to
   // its native mode.
   observer_.Reset();
+  ConfigurationWaiter config_waiter;
   configurator_.SetDisplayPower(
       chromeos::DISPLAY_POWER_INTERNAL_OFF_EXTERNAL_ON,
       DisplayConfigurator::kSetDisplayPowerNoFlags,
-      base::Bind(&DisplayConfiguratorTest::OnConfiguredCallback,
-                 base::Unretained(this)));
-  EXPECT_EQ(CALLBACK_SUCCESS, PopCallbackResult());
+      config_waiter.on_configuration_callback());
+  EXPECT_LT(config_waiter.Wait(), kMaxShortDelayMs);
+  EXPECT_EQ(CALLBACK_SUCCESS, config_waiter.callback_result());
   EXPECT_EQ(
       JoinActions(
           kGrab,
@@ -692,12 +769,13 @@ TEST_F(DisplayConfiguratorTest, SetDisplayPower) {
   // When all displays are turned off, the framebuffer should switch back
   // to the mirrored size.
   observer_.Reset();
+  config_waiter.Reset();
   configurator_.SetDisplayPower(
       chromeos::DISPLAY_POWER_ALL_OFF,
       DisplayConfigurator::kSetDisplayPowerNoFlags,
-      base::Bind(&DisplayConfiguratorTest::OnConfiguredCallback,
-                 base::Unretained(this)));
-  EXPECT_EQ(CALLBACK_SUCCESS, PopCallbackResult());
+      config_waiter.on_configuration_callback());
+  EXPECT_LT(config_waiter.Wait(), kMaxShortDelayMs);
+  EXPECT_EQ(CALLBACK_SUCCESS, config_waiter.callback_result());
   EXPECT_EQ(
       JoinActions(kGrab,
                   GetFramebufferAction(
@@ -713,12 +791,13 @@ TEST_F(DisplayConfiguratorTest, SetDisplayPower) {
 
   // Turn all displays on and check that mirroring is still used.
   observer_.Reset();
+  config_waiter.Reset();
   configurator_.SetDisplayPower(
       chromeos::DISPLAY_POWER_ALL_ON,
       DisplayConfigurator::kSetDisplayPowerNoFlags,
-      base::Bind(&DisplayConfiguratorTest::OnConfiguredCallback,
-                 base::Unretained(this)));
-  EXPECT_EQ(CALLBACK_SUCCESS, PopCallbackResult());
+      config_waiter.on_configuration_callback());
+  EXPECT_LT(config_waiter.Wait(), kMaxShortDelayMs);
+  EXPECT_EQ(CALLBACK_SUCCESS, config_waiter.callback_result());
   EXPECT_EQ(
       JoinActions(
           kGrab,
@@ -770,12 +849,13 @@ TEST_F(DisplayConfiguratorTest, SetDisplayPower) {
   // Turning off the internal display should switch the external display to
   // its native mode.
   observer_.Reset();
+  config_waiter.Reset();
   configurator_.SetDisplayPower(
       chromeos::DISPLAY_POWER_INTERNAL_OFF_EXTERNAL_ON,
       DisplayConfigurator::kSetDisplayPowerNoFlags,
-      base::Bind(&DisplayConfiguratorTest::OnConfiguredCallback,
-                 base::Unretained(this)));
-  EXPECT_EQ(CALLBACK_SUCCESS, PopCallbackResult());
+      config_waiter.on_configuration_callback());
+  EXPECT_LT(config_waiter.Wait(), kMaxShortDelayMs);
+  EXPECT_EQ(CALLBACK_SUCCESS, config_waiter.callback_result());
   EXPECT_EQ(
       JoinActions(
           kGrab,
@@ -794,12 +874,13 @@ TEST_F(DisplayConfiguratorTest, SetDisplayPower) {
   // When all displays are turned off, the framebuffer should switch back
   // to the extended + software mirroring.
   observer_.Reset();
+  config_waiter.Reset();
   configurator_.SetDisplayPower(
       chromeos::DISPLAY_POWER_ALL_OFF,
       DisplayConfigurator::kSetDisplayPowerNoFlags,
-      base::Bind(&DisplayConfiguratorTest::OnConfiguredCallback,
-                 base::Unretained(this)));
-  EXPECT_EQ(CALLBACK_SUCCESS, PopCallbackResult());
+      config_waiter.on_configuration_callback());
+  EXPECT_LT(config_waiter.Wait(), kMaxShortDelayMs);
+  EXPECT_EQ(CALLBACK_SUCCESS, config_waiter.callback_result());
   EXPECT_EQ(
       JoinActions(
           kGrab,
@@ -823,12 +904,13 @@ TEST_F(DisplayConfiguratorTest, SetDisplayPower) {
 
   // Turn all displays on and check that mirroring is still used.
   observer_.Reset();
+  config_waiter.Reset();
   configurator_.SetDisplayPower(
       chromeos::DISPLAY_POWER_ALL_ON,
       DisplayConfigurator::kSetDisplayPowerNoFlags,
-      base::Bind(&DisplayConfiguratorTest::OnConfiguredCallback,
-                 base::Unretained(this)));
-  EXPECT_EQ(CALLBACK_SUCCESS, PopCallbackResult());
+      config_waiter.on_configuration_callback());
+  EXPECT_LT(config_waiter.Wait(), kMaxShortDelayMs);
+  EXPECT_EQ(CALLBACK_SUCCESS, config_waiter.callback_result());
   EXPECT_EQ(
       JoinActions(
           kGrab,
@@ -860,9 +942,10 @@ TEST_F(DisplayConfiguratorTest, SuspendAndResume) {
   // was connected while suspended.
   const gfx::Size framebuffer_size = configurator_.framebuffer_size();
   DCHECK(!framebuffer_size.IsEmpty());
-  configurator_.SuspendDisplays(base::Bind(
-      &DisplayConfiguratorTest::OnConfiguredCallback, base::Unretained(this)));
-  EXPECT_EQ(CALLBACK_SUCCESS, PopCallbackResult());
+  ConfigurationWaiter config_waiter;
+  configurator_.SuspendDisplays(config_waiter.on_configuration_callback());
+  EXPECT_LT(config_waiter.Wait(), kMaxShortDelayMs);
+  EXPECT_EQ(CALLBACK_SUCCESS, config_waiter.callback_result());
   EXPECT_EQ(framebuffer_size.ToString(),
             configurator_.framebuffer_size().ToString());
   EXPECT_EQ(
@@ -874,7 +957,13 @@ TEST_F(DisplayConfiguratorTest, SuspendAndResume) {
           kSync,
           NULL),
       log_->GetActionsAndClear());
-  configurator_.ResumeDisplays();
+
+  // No resume delay in single display mode.
+  config_waiter.Reset();
+  configurator_.ResumeDisplaysWithCallback(
+      config_waiter.on_configuration_callback());
+  EXPECT_LT(config_waiter.Wait(), kMaxShortDelayMs);
+  EXPECT_EQ(CALLBACK_SUCCESS, config_waiter.callback_result());
   EXPECT_EQ(
       JoinActions(
           kGrab,
@@ -887,12 +976,13 @@ TEST_F(DisplayConfiguratorTest, SuspendAndResume) {
 
   // Now turn the display off before suspending and check that the
   // configurator turns it back on and syncs with the server.
+  config_waiter.Reset();
   configurator_.SetDisplayPower(
       chromeos::DISPLAY_POWER_ALL_OFF,
       DisplayConfigurator::kSetDisplayPowerNoFlags,
-      base::Bind(&DisplayConfiguratorTest::OnConfiguredCallback,
-                 base::Unretained(this)));
-  EXPECT_EQ(CALLBACK_SUCCESS, PopCallbackResult());
+      config_waiter.on_configuration_callback());
+  EXPECT_LT(config_waiter.Wait(), kMaxShortDelayMs);
+  EXPECT_EQ(CALLBACK_SUCCESS, config_waiter.callback_result());
   EXPECT_EQ(
       JoinActions(
           kGrab,
@@ -902,19 +992,26 @@ TEST_F(DisplayConfiguratorTest, SuspendAndResume) {
           NULL),
       log_->GetActionsAndClear());
 
-  configurator_.SuspendDisplays(base::Bind(
-      &DisplayConfiguratorTest::OnConfiguredCallback, base::Unretained(this)));
-  EXPECT_EQ(CALLBACK_SUCCESS, PopCallbackResult());
+  config_waiter.Reset();
+  configurator_.SuspendDisplays(config_waiter.on_configuration_callback());
+  EXPECT_LT(config_waiter.Wait(), kMaxShortDelayMs);
+  EXPECT_EQ(CALLBACK_SUCCESS, config_waiter.callback_result());
   EXPECT_EQ(kSync, log_->GetActionsAndClear());
 
-  configurator_.ResumeDisplays();
+  config_waiter.Reset();
+  configurator_.ResumeDisplaysWithCallback(
+      config_waiter.on_configuration_callback());
+  EXPECT_LT(config_waiter.Wait(), kMaxShortDelayMs);
+  EXPECT_EQ(CALLBACK_SUCCESS, config_waiter.callback_result());
   EXPECT_EQ(kNoActions, log_->GetActionsAndClear());
 
+  config_waiter.Reset();
   configurator_.SetDisplayPower(
       chromeos::DISPLAY_POWER_ALL_ON,
       DisplayConfigurator::kSetDisplayPowerNoFlags,
-      base::Bind(&DisplayConfiguratorTest::OnConfiguredCallback,
-                 base::Unretained(this)));
+      config_waiter.on_configuration_callback());
+  EXPECT_LT(config_waiter.Wait(), kMaxShortDelayMs);
+  EXPECT_EQ(CALLBACK_SUCCESS, config_waiter.callback_result());
   EXPECT_EQ(
       JoinActions(
           kGrab,
@@ -938,12 +1035,15 @@ TEST_F(DisplayConfiguratorTest, SuspendAndResume) {
           NULL),
       log_->GetActionsAndClear());
 
+  config_waiter.Reset();
   configurator_.SetDisplayPower(
       chromeos::DISPLAY_POWER_ALL_OFF,
       DisplayConfigurator::kSetDisplayPowerNoFlags,
-      base::Bind(&DisplayConfiguratorTest::OnConfiguredCallback,
-                 base::Unretained(this)));
-  EXPECT_EQ(CALLBACK_SUCCESS, PopCallbackResult());
+      config_waiter.on_configuration_callback());
+  EXPECT_LT(config_waiter.Wait(), kMaxShortDelayMs);
+  EXPECT_EQ(CALLBACK_SUCCESS, config_waiter.callback_result());
+  EXPECT_EQ(MULTIPLE_DISPLAY_STATE_DUAL_MIRROR,
+            configurator_.display_state());
   EXPECT_EQ(
       JoinActions(
           kGrab,
@@ -955,22 +1055,35 @@ TEST_F(DisplayConfiguratorTest, SuspendAndResume) {
           NULL),
       log_->GetActionsAndClear());
 
-  configurator_.SuspendDisplays(base::Bind(
-      &DisplayConfiguratorTest::OnConfiguredCallback, base::Unretained(this)));
-  EXPECT_EQ(CALLBACK_SUCCESS, PopCallbackResult());
+  // No delay in suspend.
+  config_waiter.Reset();
+  configurator_.SuspendDisplays(config_waiter.on_configuration_callback());
+  EXPECT_LT(config_waiter.Wait(), kMaxShortDelayMs);
+  EXPECT_EQ(CALLBACK_SUCCESS, config_waiter.callback_result());
+  EXPECT_EQ(chromeos::DISPLAY_POWER_ALL_OFF,
+            configurator_.current_power_state());
+  EXPECT_EQ(MULTIPLE_DISPLAY_STATE_DUAL_MIRROR,
+            configurator_.display_state());
   EXPECT_EQ(kSync, log_->GetActionsAndClear());
 
   // If a display is disconnected while suspended, the configurator should
-  // pick up the change and only turn on the internal display.
+  // pick up the change and only turn on the internal display. The should be
+  // a longer configuration delay when we set the displays back to on.
   UpdateOutputs(1, false);
-  configurator_.ResumeDisplays();
+  config_waiter.Reset();
+  configurator_.ResumeDisplaysWithCallback(
+      config_waiter.on_configuration_callback());
+  EXPECT_LT(config_waiter.Wait(), kMaxShortDelayMs);
+  EXPECT_EQ(CALLBACK_SUCCESS, config_waiter.callback_result());
   EXPECT_EQ(kNoActions, log_->GetActionsAndClear());
 
+  config_waiter.Reset();
   configurator_.SetDisplayPower(
       chromeos::DISPLAY_POWER_ALL_ON,
       DisplayConfigurator::kSetDisplayPowerNoFlags,
-      base::Bind(&DisplayConfiguratorTest::OnConfiguredCallback,
-                 base::Unretained(this)));
+      config_waiter.on_configuration_callback());
+  EXPECT_GE(config_waiter.Wait(), kMinLongDelayMs);
+  EXPECT_EQ(CALLBACK_SUCCESS, config_waiter.callback_result());
   EXPECT_EQ(
       JoinActions(
           kGrab,
@@ -993,19 +1106,21 @@ TEST_F(DisplayConfiguratorTest, Headless) {
 
   // Not much should happen when the display power state is changed while
   // no displays are connected.
+  ConfigurationWaiter config_waiter;
   configurator_.SetDisplayPower(
       chromeos::DISPLAY_POWER_ALL_OFF,
       DisplayConfigurator::kSetDisplayPowerNoFlags,
-      base::Bind(&DisplayConfiguratorTest::OnConfiguredCallback,
-                 base::Unretained(this)));
-  EXPECT_EQ(CALLBACK_SUCCESS, PopCallbackResult());
+      config_waiter.on_configuration_callback());
+  EXPECT_LT(config_waiter.Wait(), kMaxShortDelayMs);
+  EXPECT_EQ(CALLBACK_SUCCESS, config_waiter.callback_result());
   EXPECT_EQ(JoinActions(kGrab, kUngrab, NULL), log_->GetActionsAndClear());
+  config_waiter.Reset();
   configurator_.SetDisplayPower(
       chromeos::DISPLAY_POWER_ALL_ON,
       DisplayConfigurator::kSetDisplayPowerNoFlags,
-      base::Bind(&DisplayConfiguratorTest::OnConfiguredCallback,
-                 base::Unretained(this)));
-  EXPECT_EQ(CALLBACK_SUCCESS, PopCallbackResult());
+      config_waiter.on_configuration_callback());
+  EXPECT_LT(config_waiter.Wait(), kMaxShortDelayMs);
+  EXPECT_EQ(CALLBACK_SUCCESS, config_waiter.callback_result());
   EXPECT_EQ(JoinActions(kGrab, kForceDPMS, kUngrab, NULL),
             log_->GetActionsAndClear());
 
@@ -1249,9 +1364,10 @@ TEST_F(DisplayConfiguratorTest, DoNotConfigureWithSuspendedDisplays) {
   // 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(base::Bind(
-      &DisplayConfiguratorTest::OnConfiguredCallback, base::Unretained(this)));
-  EXPECT_EQ(CALLBACK_SUCCESS, PopCallbackResult());
+  ConfigurationWaiter config_waiter;
+  configurator_.SuspendDisplays(config_waiter.on_configuration_callback());
+  EXPECT_LT(config_waiter.Wait(), kMaxShortDelayMs);
+  EXPECT_EQ(CALLBACK_SUCCESS, config_waiter.callback_result());
   EXPECT_EQ(
       JoinActions(
           kGrab,
@@ -1270,19 +1386,21 @@ TEST_F(DisplayConfiguratorTest, DoNotConfigureWithSuspendedDisplays) {
 
   // Calls to SetDisplayPower should do nothing if the power state doesn't
   // change.
+  config_waiter.Reset();
   configurator_.SetDisplayPower(
       chromeos::DISPLAY_POWER_ALL_OFF,
       DisplayConfigurator::kSetDisplayPowerNoFlags,
-      base::Bind(&DisplayConfiguratorTest::OnConfiguredCallback,
-                 base::Unretained(this)));
-  EXPECT_EQ(CALLBACK_SUCCESS, PopCallbackResult());
+      config_waiter.on_configuration_callback());
+  EXPECT_LT(config_waiter.Wait(), kMaxShortDelayMs);
+  EXPECT_EQ(CALLBACK_SUCCESS, config_waiter.callback_result());
   EXPECT_EQ(kNoActions, log_->GetActionsAndClear());
+  config_waiter.Reset();
   configurator_.SetDisplayPower(
       chromeos::DISPLAY_POWER_ALL_ON,
       DisplayConfigurator::kSetDisplayPowerNoFlags,
-      base::Bind(&DisplayConfiguratorTest::OnConfiguredCallback,
-                 base::Unretained(this)));
-  EXPECT_EQ(CALLBACK_SUCCESS, PopCallbackResult());
+      config_waiter.on_configuration_callback());
+  EXPECT_LT(config_waiter.Wait(), kMaxShortDelayMs);
+  EXPECT_EQ(CALLBACK_SUCCESS, config_waiter.callback_result());
   EXPECT_EQ(
       JoinActions(
           kGrab,
@@ -1316,9 +1434,10 @@ TEST_F(DisplayConfiguratorTest, DoNotConfigureWithSuspendedDisplays) {
   // task should not run either and the timer should be stopped. The displays
   // should be turned off by suspend.
   configurator_.OnConfigurationChanged();
-  configurator_.SuspendDisplays(base::Bind(
-      &DisplayConfiguratorTest::OnConfiguredCallback, base::Unretained(this)));
-  EXPECT_EQ(CALLBACK_SUCCESS, PopCallbackResult());
+  config_waiter.Reset();
+  configurator_.SuspendDisplays(config_waiter.on_configuration_callback());
+  EXPECT_LT(config_waiter.Wait(), kMaxShortDelayMs);
+  EXPECT_EQ(CALLBACK_SUCCESS, config_waiter.callback_result());
   EXPECT_EQ(
       JoinActions(
           kGrab,
@@ -1547,12 +1666,13 @@ TEST_F(DisplayConfiguratorTest, SaveDisplayPowerStateOnConfigFailure) {
   observer_.Reset();
 
   // Turn off the internal display, simulating docked mode.
+  ConfigurationWaiter config_waiter;
   configurator_.SetDisplayPower(
       chromeos::DISPLAY_POWER_INTERNAL_OFF_EXTERNAL_ON,
       DisplayConfigurator::kSetDisplayPowerNoFlags,
-      base::Bind(&DisplayConfiguratorTest::OnConfiguredCallback,
-                 base::Unretained(this)));
-  EXPECT_EQ(CALLBACK_SUCCESS, PopCallbackResult());
+      config_waiter.on_configuration_callback());
+  EXPECT_LT(config_waiter.Wait(), kMaxShortDelayMs);
+  EXPECT_EQ(CALLBACK_SUCCESS, config_waiter.callback_result());
   EXPECT_EQ(1, observer_.num_changes());
   EXPECT_EQ(0, observer_.num_failures());
   log_->GetActionsAndClear();
@@ -1563,9 +1683,9 @@ TEST_F(DisplayConfiguratorTest, SaveDisplayPowerStateOnConfigFailure) {
   configurator_.SetDisplayPower(
       chromeos::DISPLAY_POWER_ALL_ON,
       DisplayConfigurator::kSetDisplayPowerNoFlags,
-      base::Bind(&DisplayConfiguratorTest::OnConfiguredCallback,
-                 base::Unretained(this)));
-  EXPECT_EQ(CALLBACK_FAILURE, PopCallbackResult());
+      config_waiter.on_configuration_callback());
+  EXPECT_LT(config_waiter.Wait(), kMaxShortDelayMs);
+  EXPECT_EQ(CALLBACK_FAILURE, config_waiter.callback_result());
   EXPECT_EQ(1, observer_.num_changes());
   EXPECT_EQ(1, observer_.num_failures());
   log_->GetActionsAndClear();
@@ -1595,12 +1715,13 @@ TEST_F(DisplayConfiguratorTest, DontRestoreStalePowerStateAfterResume) {
   observer_.Reset();
 
   // Turn off the internal display, simulating docked mode.
+  ConfigurationWaiter config_waiter;
   configurator_.SetDisplayPower(
       chromeos::DISPLAY_POWER_INTERNAL_OFF_EXTERNAL_ON,
       DisplayConfigurator::kSetDisplayPowerNoFlags,
-      base::Bind(&DisplayConfiguratorTest::OnConfiguredCallback,
-                 base::Unretained(this)));
-  EXPECT_EQ(CALLBACK_SUCCESS, PopCallbackResult());
+      config_waiter.on_configuration_callback());
+  EXPECT_LT(config_waiter.Wait(), kMaxShortDelayMs);
+  EXPECT_EQ(CALLBACK_SUCCESS, config_waiter.callback_result());
   EXPECT_EQ(1, observer_.num_changes());
   EXPECT_EQ(0, observer_.num_failures());
   EXPECT_EQ(
@@ -1618,9 +1739,10 @@ TEST_F(DisplayConfiguratorTest, DontRestoreStalePowerStateAfterResume) {
   // Suspend and resume the system. Resuming should restore the previous power
   // state and force a probe. Suspend should turn off the displays since an
   // external monitor is connected.
-  configurator_.SuspendDisplays(base::Bind(
-      &DisplayConfiguratorTest::OnConfiguredCallback, base::Unretained(this)));
-  EXPECT_EQ(CALLBACK_SUCCESS, PopCallbackResult());
+  config_waiter.Reset();
+  configurator_.SuspendDisplays(config_waiter.on_configuration_callback());
+  EXPECT_LT(config_waiter.Wait(), kMaxShortDelayMs);
+  EXPECT_EQ(CALLBACK_SUCCESS, config_waiter.callback_result());
   EXPECT_EQ(2, observer_.num_changes());
   EXPECT_EQ(
       JoinActions(
@@ -1635,12 +1757,13 @@ TEST_F(DisplayConfiguratorTest, DontRestoreStalePowerStateAfterResume) {
       log_->GetActionsAndClear());
 
   // Before the task runs, exit docked mode.
+  config_waiter.Reset();
   configurator_.SetDisplayPower(
       chromeos::DISPLAY_POWER_ALL_ON,
       DisplayConfigurator::kSetDisplayPowerNoFlags,
-      base::Bind(&DisplayConfiguratorTest::OnConfiguredCallback,
-                 base::Unretained(this)));
-  EXPECT_EQ(CALLBACK_SUCCESS, PopCallbackResult());
+      config_waiter.on_configuration_callback());
+  EXPECT_LT(config_waiter.Wait(), kMaxShortDelayMs);
+  EXPECT_EQ(CALLBACK_SUCCESS, config_waiter.callback_result());
   EXPECT_EQ(3, observer_.num_changes());
   EXPECT_EQ(0, observer_.num_failures());
   EXPECT_EQ(
@@ -1656,7 +1779,13 @@ TEST_F(DisplayConfiguratorTest, DontRestoreStalePowerStateAfterResume) {
       log_->GetActionsAndClear());
 
   // Check that the display states are not changed after resuming.
-  configurator_.ResumeDisplays();
+  config_waiter.Reset();
+  configurator_.ResumeDisplaysWithCallback(
+      config_waiter.on_configuration_callback());
+  EXPECT_LT(config_waiter.Wait(), kMaxShortDelayMs);
+  EXPECT_EQ(CALLBACK_SUCCESS, config_waiter.callback_result());
+  EXPECT_EQ(chromeos::DISPLAY_POWER_ALL_ON,
+            configurator_.current_power_state());
   EXPECT_EQ(kNoActions, log_->GetActionsAndClear());
 }
 
@@ -1696,21 +1825,20 @@ TEST_F(DisplayConfiguratorTest,
 
   native_display_delegate_->set_run_async(true);
 
+  ConfigurationWaiter config_waiter;
   configurator_.SetDisplayPower(
       chromeos::DISPLAY_POWER_ALL_OFF,
       DisplayConfigurator::kSetDisplayPowerNoFlags,
-      base::Bind(&DisplayConfiguratorTest::OnConfiguredCallback,
-                 base::Unretained(this)));
+      config_waiter.on_configuration_callback());
 
   configurator_.SetDisplayPower(
       chromeos::DISPLAY_POWER_ALL_ON,
       DisplayConfigurator::kSetDisplayPowerNoFlags,
-      base::Bind(&DisplayConfiguratorTest::OnConfiguredCallback,
-                 base::Unretained(this)));
+      config_waiter.on_configuration_callback());
 
-  EXPECT_EQ(CALLBACK_NOT_CALLED, PopCallbackResult());
-  base::RunLoop().RunUntilIdle();
-  EXPECT_EQ(CALLBACK_SUCCESS, PopCallbackResult());
+  EXPECT_EQ(CALLBACK_NOT_CALLED, config_waiter.callback_result());
+  EXPECT_LT(config_waiter.Wait(), kMaxShortDelayMs);
+  EXPECT_EQ(CALLBACK_SUCCESS, config_waiter.callback_result());
   EXPECT_EQ(1, observer_.num_changes());
   EXPECT_EQ(0, observer_.num_failures());
 
@@ -1731,9 +1859,10 @@ TEST_F(DisplayConfiguratorTest,
           kUngrab, NULL),
       log_->GetActionsAndClear());
 
+  config_waiter.Reset();
   EXPECT_TRUE(test_api_.TriggerConfigureTimeout());
-  base::RunLoop().RunUntilIdle();
-  EXPECT_EQ(CALLBACK_SUCCESS, PopCallbackResult());
+  EXPECT_LT(config_waiter.Wait(), kMaxShortDelayMs);
+  EXPECT_EQ(CALLBACK_SUCCESS, config_waiter.callback_result());
   EXPECT_EQ(2, observer_.num_changes());
   EXPECT_EQ(0, observer_.num_failures());
 
@@ -1764,13 +1893,13 @@ TEST_F(DisplayConfiguratorTest,
   // Fail display configuration.
   native_display_delegate_->set_max_configurable_pixels(-1);
 
+  ConfigurationWaiter config_waiter;
   configurator_.SetDisplayPower(
       chromeos::DISPLAY_POWER_ALL_OFF,
       DisplayConfigurator::kSetDisplayPowerNoFlags,
-      base::Bind(&DisplayConfiguratorTest::OnConfiguredCallback,
-                 base::Unretained(this)));
-
-  EXPECT_EQ(CALLBACK_FAILURE, PopCallbackResult());
+      config_waiter.on_configuration_callback());
+  EXPECT_LT(config_waiter.Wait(), kMaxShortDelayMs);
+    EXPECT_EQ(CALLBACK_FAILURE, config_waiter.callback_result());
   EXPECT_EQ(0, observer_.num_changes());
   EXPECT_EQ(1, observer_.num_failures());
 
@@ -1794,11 +1923,12 @@ TEST_F(DisplayConfiguratorTest,
 
   // This configuration should trigger a display configuration since the
   // previous configuration failed.
+  config_waiter.Reset();
   configurator_.SetDisplayPower(
       chromeos::DISPLAY_POWER_ALL_ON,
       DisplayConfigurator::kSetDisplayPowerNoFlags,
-      base::Bind(&DisplayConfiguratorTest::OnConfiguredCallback,
-                 base::Unretained(this)));
+      config_waiter.on_configuration_callback());
+  EXPECT_LT(config_waiter.Wait(), kMaxShortDelayMs);
 
   EXPECT_EQ(0, observer_.num_changes());
   EXPECT_EQ(2, observer_.num_failures());
@@ -1884,13 +2014,13 @@ TEST_F(DisplayConfiguratorTest, TestWithThreeDisplays) {
       log_->GetActionsAndClear());
 
   // Verify that turning the power off works.
+  ConfigurationWaiter config_waiter;
   configurator_.SetDisplayPower(
       chromeos::DISPLAY_POWER_ALL_OFF,
       DisplayConfigurator::kSetDisplayPowerNoFlags,
-      base::Bind(&DisplayConfiguratorTest::OnConfiguredCallback,
-                 base::Unretained(this)));
-
-  EXPECT_EQ(CALLBACK_SUCCESS, PopCallbackResult());
+      config_waiter.on_configuration_callback());
+  EXPECT_LT(config_waiter.Wait(), kMaxShortDelayMs);
+  EXPECT_EQ(CALLBACK_SUCCESS, config_waiter.callback_result());
   EXPECT_EQ(
       JoinActions(
           kGrab, GetFramebufferAction(
@@ -1911,13 +2041,13 @@ TEST_F(DisplayConfiguratorTest, TestWithThreeDisplays) {
           kUngrab, NULL),
       log_->GetActionsAndClear());
 
+  config_waiter.Reset();
   configurator_.SetDisplayPower(
       chromeos::DISPLAY_POWER_ALL_ON,
       DisplayConfigurator::kSetDisplayPowerNoFlags,
-      base::Bind(&DisplayConfiguratorTest::OnConfiguredCallback,
-                 base::Unretained(this)));
-
-  EXPECT_EQ(CALLBACK_SUCCESS, PopCallbackResult());
+      config_waiter.on_configuration_callback());
+  EXPECT_LT(config_waiter.Wait(), kMaxShortDelayMs);
+  EXPECT_EQ(CALLBACK_SUCCESS, config_waiter.callback_result());
   EXPECT_EQ(
       JoinActions(
           kGrab, GetFramebufferAction(
@@ -1957,5 +2087,91 @@ TEST_F(DisplayConfiguratorTest, TestWithThreeDisplays) {
       log_->GetActionsAndClear());
 }
 
+// Tests the suspend and resume behavior when in dual or multi display modes.
+TEST_F(DisplayConfiguratorTest, SuspendResumeWithMultipleDisplays) {
+  InitWithSingleOutput();
+  state_controller_.set_state(MULTIPLE_DISPLAY_STATE_DUAL_EXTENDED);
+  observer_.Reset();
+  UpdateOutputs(2, true);
+  EXPECT_EQ(MULTIPLE_DISPLAY_STATE_DUAL_EXTENDED,
+            configurator_.display_state());
+  EXPECT_FALSE(mirroring_controller_.SoftwareMirroringEnabled());
+  EXPECT_EQ(1, observer_.num_changes());
+  EXPECT_EQ(chromeos::DISPLAY_POWER_ALL_ON,
+            configurator_.current_power_state());
+
+  // Suspending displays should result in an immediate configuration without
+  // delays, even in dual display mode.
+  ConfigurationWaiter config_waiter;
+  configurator_.SuspendDisplays(config_waiter.on_configuration_callback());
+  EXPECT_LT(config_waiter.Wait(), kMaxShortDelayMs);
+  EXPECT_EQ(CALLBACK_SUCCESS, config_waiter.callback_result());
+  EXPECT_EQ(chromeos::DISPLAY_POWER_ALL_OFF,
+            configurator_.current_power_state());
+  EXPECT_EQ(MULTIPLE_DISPLAY_STATE_DUAL_EXTENDED,
+            configurator_.display_state());
+
+  // Resuming from suspend with dual displays, should be done after a delay, and
+  // after the configuration we should still expect to be in a dual display
+  // mode.
+  config_waiter.Reset();
+  configurator_.ResumeDisplaysWithCallback(
+      config_waiter.on_configuration_callback());
+  EXPECT_GE(config_waiter.Wait(), kMinLongDelayMs);
+  EXPECT_EQ(CALLBACK_SUCCESS, config_waiter.callback_result());
+  EXPECT_EQ(chromeos::DISPLAY_POWER_ALL_ON,
+            configurator_.current_power_state());
+  EXPECT_EQ(MULTIPLE_DISPLAY_STATE_DUAL_EXTENDED,
+            configurator_.display_state());
+
+  // Suspend displays and disconnect one of them while in suspend.
+  config_waiter.Reset();
+  configurator_.SuspendDisplays(config_waiter.on_configuration_callback());
+  EXPECT_LT(config_waiter.Wait(), kMaxShortDelayMs);
+  EXPECT_EQ(CALLBACK_SUCCESS, config_waiter.callback_result());
+  EXPECT_EQ(MULTIPLE_DISPLAY_STATE_DUAL_EXTENDED,
+            configurator_.display_state());
+  EXPECT_EQ(chromeos::DISPLAY_POWER_ALL_OFF,
+            configurator_.current_power_state());
+  UpdateOutputs(1, false);
+
+  // Now resume, and expect that we'll still have a long delay since we were in
+  // dual mode before suspend. The configurator should pick up the change and
+  // detect that we are in single display mode now.
+  config_waiter.Reset();
+  configurator_.ResumeDisplaysWithCallback(
+      config_waiter.on_configuration_callback());
+  EXPECT_GE(config_waiter.Wait(), kMinLongDelayMs);
+  EXPECT_EQ(CALLBACK_SUCCESS, config_waiter.callback_result());
+  EXPECT_EQ(chromeos::DISPLAY_POWER_ALL_ON,
+            configurator_.current_power_state());
+  EXPECT_EQ(MULTIPLE_DISPLAY_STATE_SINGLE, configurator_.display_state());
+
+  // Verify that the above is the exact same behavior for 3+ displays.
+  UpdateOutputs(3, true);
+  EXPECT_EQ(MULTIPLE_DISPLAY_STATE_MULTI_EXTENDED,
+            configurator_.display_state());
+  // Suspend.
+  config_waiter.Reset();
+  configurator_.SuspendDisplays(config_waiter.on_configuration_callback());
+  EXPECT_LT(config_waiter.Wait(), kMaxShortDelayMs);
+  EXPECT_EQ(CALLBACK_SUCCESS, config_waiter.callback_result());
+  EXPECT_EQ(MULTIPLE_DISPLAY_STATE_MULTI_EXTENDED,
+            configurator_.display_state());
+  EXPECT_EQ(chromeos::DISPLAY_POWER_ALL_OFF,
+            configurator_.current_power_state());
+
+  // Resume and expect the correct delay.
+  config_waiter.Reset();
+  configurator_.ResumeDisplaysWithCallback(
+      config_waiter.on_configuration_callback());
+  EXPECT_GE(config_waiter.Wait(), kMinLongDelayMs);
+  EXPECT_EQ(CALLBACK_SUCCESS, config_waiter.callback_result());
+  EXPECT_EQ(chromeos::DISPLAY_POWER_ALL_ON,
+            configurator_.current_power_state());
+  EXPECT_EQ(MULTIPLE_DISPLAY_STATE_MULTI_EXTENDED,
+            configurator_.display_state());
+}
+
 }  // namespace test
 }  // namespace ui