Delay display configuration after waking up from suspend with multi displays

ui/display/chromeos/display_configurator_unittest.cc

 // Copyright 2014 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 "ui/display/chromeos/display_configurator.h"
 
 #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"
 #include "base/message_loop/message_loop.h"
 #include "base/run_loop.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "ui/display/chromeos/test/action_logger_util.h"
 #include "ui/display/chromeos/test/test_native_display_delegate.h"
 #include "ui/display/fake_display_snapshot.h"
 #include "ui/display/util/display_util.h"
 
 namespace ui {
 namespace test {
 
 namespace {
 
 int64_t kDisplayIds[3] = {123, 456, 789};
 
 std::unique_ptr<ui::DisplayMode> MakeDisplayMode(int width,
                                                  int height,
                                                  bool is_interlaced,
                                                  float refresh_rate) {
   return base::MakeUnique<ui::DisplayMode>(gfx::Size(width, height),
                                            is_interlaced, refresh_rate);
 }
 
+enum CallbackResult {

[Daniel Erat, 2016/10/25 16:05:33]

i don't understand what you're verifying by checking if the callback is run now.
the production code just passes DoNothing to it, and i _think_ it's always run
synchronously by tests. is it to check that SetDisplayPower is called
synchronously? if so, is there a simpler way you can check that, e.g. by just
checking the power state before calling Wait()?

[afakhry, 2016/10/25 20:14:11]

ResumeDisplays() can result in a delayed configuration in multiple tests for
which the callback will be invoked asynchronously. We're not just trying to
verify that SetDisplayPower() is called, but rather the whole operation
completed as expected.

So validate the correct delay AND the correct callback result (true/false).

[Daniel Erat, 2016/10/26 15:50:00]

thanks for the detailed explanation!

i understand why you're passing the callback to SetDisplayPower and verifying
that it's called with the correct value. the thing that confuses me is just why
the callback is also passed to ResumeDisplays. the real code in
PowerEventObserver passes DoNothing to ResumeDisplays, so it's not clear to me
what's being tested by passing a callback to ResumeDisplays in the tests.

sorry if i'm being dense. :-)

+  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 = base::TimeDelta::FromMilliseconds(
+    DisplayConfigurator::kResumeConfigureMultiDisplayDelayMs);
+
 class TestObserver : public DisplayConfigurator::Observer {
  public:
   explicit TestObserver(DisplayConfigurator* configurator)
       : configurator_(configurator) {
     Reset();
     configurator_->AddObserver(this);
   }
   ~TestObserver() override { configurator_->RemoveObserver(this); }
 
   int num_changes() const { return num_changes_; }
@@ skipping 76 matching lines @@
   bool SoftwareMirroringEnabled() const override {
     return software_mirroring_enabled_;
   }
 
  private:
   bool software_mirroring_enabled_;
 
   DISALLOW_COPY_AND_ASSIGN(TestMirroringController);
 };
 
+// Abstracts waiting for the display configuration to be completed and getting
+// the time it took to complete.
+class ConfigurationWaiter {
+ public:
+  ConfigurationWaiter(DisplayConfigurator::TestApi* test_api)
+      : on_configured_callback_(base::Bind(&ConfigurationWaiter::OnConfigured,
+                                           base::Unretained(this))),
+        test_api_(test_api),
+        callback_result_(CALLBACK_NOT_CALLED),
+        pending_configuration_(true) {}
+
+  ~ConfigurationWaiter() = default;
+
+  void Reset() {
+    callback_result_ = CALLBACK_NOT_CALLED;
+    pending_configuration_ = true;
+  }
+
+  // Simulates Waiting for the configuration to complete by manually triggering

[Daniel Erat, 2016/10/25 16:05:32]

nit: s/Waiting/waiting/

[afakhry, 2016/10/25 20:14:11]

Done.

+  // the configuration timer and returns how long it was supposed to take for
+  // timer task to complete.
+  base::TimeDelta Wait() WARN_UNUSED_RESULT {
+    base::RunLoop().RunUntilIdle();
+    if (!pending_configuration_)
+      return base::TimeDelta();
+
+    const base::TimeDelta delay = test_api_->GetConfigureDelay();
+    if (!test_api_->TriggerConfigureTimeout())
+      return base::TimeDelta::Max();
+
+    return delay;
+  }
+
+  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;
+  }
+
+  // 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().
+  const DisplayConfigurator::ConfigurationCallback on_configured_callback_;
+
+  DisplayConfigurator::TestApi* test_api_;  // Not owned.
+
+  // The status of the display configuration.
+  CallbackResult callback_result_;
+
+  // True if we are expecting 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:
-  enum CallbackResult {
-    CALLBACK_FAILURE,
-    CALLBACK_SUCCESS,
-    CALLBACK_NOT_CALLED,
-  };
-
   DisplayConfiguratorTest()
       : small_mode_(gfx::Size(1366, 768), false, 60.0f),
         big_mode_(gfx::Size(2560, 1600), false, 60.0f),
         observer_(&configurator_),
         test_api_(&configurator_),
+        config_waiter_(&test_api_),
         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 {}
 
   void SetUp() override {
     log_.reset(new ActionLogger());
 
     native_display_delegate_ = new TestNativeDisplayDelegate(log_.get());
     configurator_.SetDelegateForTesting(
         std::unique_ptr<NativeDisplayDelegate>(native_display_delegate_));
 
@@ skipping 21 matching lines @@
                       .SetId(kDisplayIds[2])
                       .SetNativeMode(small_mode_.Clone())
                       .SetCurrentMode(small_mode_.Clone())
                       .SetType(DISPLAY_CONNECTION_TYPE_HDMI)
                       .SetIsAspectPerservingScaling(true)
                       .Build();
 
     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);
   }
 
   void EnableContentProtectionCallback(bool status) {
     enable_content_protection_status_ = status;
     enable_content_protection_call_count_++;
   }
 
   void QueryContentProtectionCallback(
@@ skipping 40 matching lines @@
     EXPECT_EQ(
         JoinActions(
             kInitXRandR, kGrab,
             GetFramebufferAction(small_mode_.size(), outputs_[0].get(), nullptr)
                 .c_str(),
             GetCrtcAction(*outputs_[0], &small_mode_, gfx::Point(0, 0)).c_str(),
             kForceDPMS, kUngrab, nullptr),
         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;
     return result;
   }
 
   base::MessageLoop message_loop_;
   TestStateController state_controller_;
   TestMirroringController mirroring_controller_;
   DisplayConfigurator configurator_;
   TestObserver observer_;
   std::unique_ptr<ActionLogger> log_;
   TestNativeDisplayDelegate* native_display_delegate_;  // not owned
   DisplayConfigurator::TestApi test_api_;
-
+  ConfigurationWaiter config_waiter_;
   bool enable_content_protection_status_;
   int enable_content_protection_call_count_;
   DisplayConfigurator::QueryProtectionResponse
       query_content_protection_response_;
   int query_content_protection_call_count_;
 
   std::unique_ptr<DisplaySnapshot> outputs_[3];
 
-  CallbackResult callback_result_;
   CallbackResult display_control_result_;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(DisplayConfiguratorTest);
 };
 
 }  // namespace
 
 TEST_F(DisplayConfiguratorTest, FindDisplayModeMatchingSize) {
   std::unique_ptr<ui::DisplaySnapshot> output =
@@ skipping 356 matching lines @@
           GetCrtcAction(*outputs_[0], &small_mode_, gfx::Point(0, 0)).c_str(),
           GetCrtcAction(*outputs_[1], &small_mode_, gfx::Point(0, 0)).c_str(),
           kUngrab, nullptr),
       log_->GetActionsAndClear());
   EXPECT_FALSE(mirroring_controller_.SoftwareMirroringEnabled());
   EXPECT_EQ(1, observer_.num_changes());
 
   // 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, GetFramebufferAction(big_mode_.size(), outputs_[0].get(),
                                       outputs_[1].get())
                      .c_str(),
           GetCrtcAction(*outputs_[0], nullptr, gfx::Point(0, 0)).c_str(),
           GetCrtcAction(*outputs_[1], &big_mode_, gfx::Point(0, 0)).c_str(),
           kForceDPMS, kUngrab, nullptr),
       log_->GetActionsAndClear());
   EXPECT_EQ(MULTIPLE_DISPLAY_STATE_SINGLE, configurator_.display_state());
   EXPECT_EQ(1, observer_.num_changes());
 
   // When all displays are turned off, the framebuffer should switch back
   // to the mirrored size.
   observer_.Reset();
-  configurator_.SetDisplayPower(
-      chromeos::DISPLAY_POWER_ALL_OFF,
-      DisplayConfigurator::kSetDisplayPowerNoFlags,
-      base::Bind(&DisplayConfiguratorTest::OnConfiguredCallback,
-                 base::Unretained(this)));
-  EXPECT_EQ(CALLBACK_SUCCESS, PopCallbackResult());
+  config_waiter_.Reset();
+  configurator_.SetDisplayPower(chromeos::DISPLAY_POWER_ALL_OFF,
+                                DisplayConfigurator::kSetDisplayPowerNoFlags,
+                                config_waiter_.on_configuration_callback());
+  EXPECT_LT(config_waiter_.Wait(), kMaxShortDelayMs);
+  EXPECT_EQ(CALLBACK_SUCCESS, config_waiter_.callback_result());
   EXPECT_EQ(
       JoinActions(
           kGrab, GetFramebufferAction(small_mode_.size(), outputs_[0].get(),
                                       outputs_[1].get())
                      .c_str(),
           GetCrtcAction(*outputs_[0], nullptr, gfx::Point(0, 0)).c_str(),
           GetCrtcAction(*outputs_[1], nullptr, gfx::Point(0, 0)).c_str(),
           kUngrab, nullptr),
       log_->GetActionsAndClear());
   EXPECT_EQ(MULTIPLE_DISPLAY_STATE_DUAL_MIRROR, configurator_.display_state());
   EXPECT_FALSE(mirroring_controller_.SoftwareMirroringEnabled());
   EXPECT_EQ(1, observer_.num_changes());
 
   // Turn all displays on and check that mirroring is still used.
   observer_.Reset();
-  configurator_.SetDisplayPower(
-      chromeos::DISPLAY_POWER_ALL_ON,
-      DisplayConfigurator::kSetDisplayPowerNoFlags,
-      base::Bind(&DisplayConfiguratorTest::OnConfiguredCallback,
-                 base::Unretained(this)));
-  EXPECT_EQ(CALLBACK_SUCCESS, PopCallbackResult());
+  config_waiter_.Reset();
+  configurator_.SetDisplayPower(chromeos::DISPLAY_POWER_ALL_ON,
+                                DisplayConfigurator::kSetDisplayPowerNoFlags,
+                                config_waiter_.on_configuration_callback());
+  EXPECT_LT(config_waiter_.Wait(), kMaxShortDelayMs);
+  EXPECT_EQ(CALLBACK_SUCCESS, config_waiter_.callback_result());
   EXPECT_EQ(
       JoinActions(
           kGrab, GetFramebufferAction(small_mode_.size(), outputs_[0].get(),
                                       outputs_[1].get())
                      .c_str(),
           GetCrtcAction(*outputs_[0], &small_mode_, gfx::Point(0, 0)).c_str(),
           GetCrtcAction(*outputs_[1], &small_mode_, gfx::Point(0, 0)).c_str(),
           kForceDPMS, kUngrab, nullptr),
       log_->GetActionsAndClear());
   EXPECT_EQ(MULTIPLE_DISPLAY_STATE_DUAL_MIRROR, configurator_.display_state());
@@ skipping 29 matching lines @@
           kUngrab, nullptr),
       log_->GetActionsAndClear());
   EXPECT_EQ(MULTIPLE_DISPLAY_STATE_DUAL_EXTENDED,
             configurator_.display_state());
   EXPECT_TRUE(mirroring_controller_.SoftwareMirroringEnabled());
   EXPECT_EQ(1, observer_.num_changes());
 
   // 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, GetFramebufferAction(big_mode_.size(), outputs_[0].get(),
                                       outputs_[1].get())
                      .c_str(),
           GetCrtcAction(*outputs_[0], nullptr, gfx::Point(0, 0)).c_str(),
           GetCrtcAction(*outputs_[1], &big_mode_, gfx::Point(0, 0)).c_str(),
           kForceDPMS, kUngrab, nullptr),
       log_->GetActionsAndClear());
   EXPECT_EQ(MULTIPLE_DISPLAY_STATE_SINGLE, configurator_.display_state());
   EXPECT_FALSE(mirroring_controller_.SoftwareMirroringEnabled());
   EXPECT_EQ(1, observer_.num_changes());
 
   // When all displays are turned off, the framebuffer should switch back
   // to the extended + software mirroring.
   observer_.Reset();
-  configurator_.SetDisplayPower(
-      chromeos::DISPLAY_POWER_ALL_OFF,
-      DisplayConfigurator::kSetDisplayPowerNoFlags,
-      base::Bind(&DisplayConfiguratorTest::OnConfiguredCallback,
-                 base::Unretained(this)));
-  EXPECT_EQ(CALLBACK_SUCCESS, PopCallbackResult());
+  config_waiter_.Reset();
+  configurator_.SetDisplayPower(chromeos::DISPLAY_POWER_ALL_OFF,
+                                DisplayConfigurator::kSetDisplayPowerNoFlags,
+                                config_waiter_.on_configuration_callback());
+  EXPECT_LT(config_waiter_.Wait(), kMaxShortDelayMs);
+  EXPECT_EQ(CALLBACK_SUCCESS, config_waiter_.callback_result());
   EXPECT_EQ(
       JoinActions(
           kGrab,
           GetFramebufferAction(gfx::Size(big_mode_.size().width(), kDualHeight),
                                outputs_[0].get(), outputs_[1].get())
               .c_str(),
           GetCrtcAction(*outputs_[0], nullptr, gfx::Point(0, 0)).c_str(),
           GetCrtcAction(*outputs_[1], nullptr,
                         gfx::Point(0, small_mode_.size().height() +
                                           DisplayConfigurator::kVerticalGap))
               .c_str(),
           kUngrab, nullptr),
       log_->GetActionsAndClear());
   EXPECT_EQ(MULTIPLE_DISPLAY_STATE_DUAL_EXTENDED,
             configurator_.display_state());
   EXPECT_TRUE(mirroring_controller_.SoftwareMirroringEnabled());
   EXPECT_EQ(1, observer_.num_changes());
 
   // Turn all displays on and check that mirroring is still used.
   observer_.Reset();
-  configurator_.SetDisplayPower(
-      chromeos::DISPLAY_POWER_ALL_ON,
-      DisplayConfigurator::kSetDisplayPowerNoFlags,
-      base::Bind(&DisplayConfiguratorTest::OnConfiguredCallback,
-                 base::Unretained(this)));
-  EXPECT_EQ(CALLBACK_SUCCESS, PopCallbackResult());
+  config_waiter_.Reset();
+  configurator_.SetDisplayPower(chromeos::DISPLAY_POWER_ALL_ON,
+                                DisplayConfigurator::kSetDisplayPowerNoFlags,
+                                config_waiter_.on_configuration_callback());
+  EXPECT_LT(config_waiter_.Wait(), kMaxShortDelayMs);
+  EXPECT_EQ(CALLBACK_SUCCESS, config_waiter_.callback_result());
   EXPECT_EQ(
       JoinActions(
           kGrab,
           GetFramebufferAction(gfx::Size(big_mode_.size().width(), kDualHeight),
                                outputs_[0].get(), outputs_[1].get())
               .c_str(),
           GetCrtcAction(*outputs_[0], &small_mode_, gfx::Point(0, 0)).c_str(),
           GetCrtcAction(*outputs_[1], &big_mode_,
                         gfx::Point(0, small_mode_.size().height() +
                                           DisplayConfigurator::kVerticalGap))
               .c_str(),
           kForceDPMS, kUngrab, nullptr),
       log_->GetActionsAndClear());
   EXPECT_EQ(MULTIPLE_DISPLAY_STATE_DUAL_EXTENDED,
             configurator_.display_state());
   EXPECT_TRUE(mirroring_controller_.SoftwareMirroringEnabled());
   EXPECT_EQ(1, observer_.num_changes());
 }
 
 TEST_F(DisplayConfiguratorTest, SuspendAndResume) {
   InitWithSingleOutput();
 
   // No preparation is needed before suspending when the display is already
   // on.  The configurator should still reprobe on resume in case a display
   // 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());
+  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(framebuffer_size.ToString(),
             configurator_.framebuffer_size().ToString());
   EXPECT_EQ(JoinActions(
                 kGrab, GetFramebufferAction(small_mode_.size(),
                                             outputs_[0].get(), nullptr)
                            .c_str(),
                 GetCrtcAction(*outputs_[0], nullptr, gfx::Point(0, 0)).c_str(),
                 kUngrab, kSync, nullptr),
             log_->GetActionsAndClear());
-  configurator_.ResumeDisplays();
+
+  // No resume delay in single display mode.
+  config_waiter_.Reset();
+  configurator_.ResumeDisplays(config_waiter_.on_configuration_callback());
+  EXPECT_LT(config_waiter_.Wait(), kMaxShortDelayMs);
+  EXPECT_EQ(CALLBACK_SUCCESS, config_waiter_.callback_result());
   EXPECT_EQ(
       JoinActions(
           kGrab,
           GetFramebufferAction(small_mode_.size(), outputs_[0].get(), nullptr)
               .c_str(),
           GetCrtcAction(*outputs_[0], &small_mode_, gfx::Point(0, 0)).c_str(),
           kForceDPMS, kUngrab, nullptr),
       log_->GetActionsAndClear());
 
   // Now turn the display off before suspending and check that the
   // configurator turns it back on and syncs with the server.
-  configurator_.SetDisplayPower(
-      chromeos::DISPLAY_POWER_ALL_OFF,
-      DisplayConfigurator::kSetDisplayPowerNoFlags,
-      base::Bind(&DisplayConfiguratorTest::OnConfiguredCallback,
-                 base::Unretained(this)));
-  EXPECT_EQ(CALLBACK_SUCCESS, PopCallbackResult());
+  config_waiter_.Reset();
+  configurator_.SetDisplayPower(chromeos::DISPLAY_POWER_ALL_OFF,
+                                DisplayConfigurator::kSetDisplayPowerNoFlags,
+                                config_waiter_.on_configuration_callback());
+  EXPECT_LT(config_waiter_.Wait(), kMaxShortDelayMs);
+  EXPECT_EQ(CALLBACK_SUCCESS, config_waiter_.callback_result());
   EXPECT_EQ(JoinActions(
                 kGrab, GetFramebufferAction(small_mode_.size(),
                                             outputs_[0].get(), nullptr)
                            .c_str(),
                 GetCrtcAction(*outputs_[0], nullptr, gfx::Point(0, 0)).c_str(),
                 kUngrab, nullptr),
             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_.ResumeDisplays(config_waiter_.on_configuration_callback());
+  EXPECT_LT(config_waiter_.Wait(), kMaxShortDelayMs);
+  EXPECT_EQ(CALLBACK_SUCCESS, config_waiter_.callback_result());
   EXPECT_EQ(kNoActions, log_->GetActionsAndClear());
 
-  configurator_.SetDisplayPower(
-      chromeos::DISPLAY_POWER_ALL_ON,
-      DisplayConfigurator::kSetDisplayPowerNoFlags,
-      base::Bind(&DisplayConfiguratorTest::OnConfiguredCallback,
-                 base::Unretained(this)));
+  config_waiter_.Reset();
+  configurator_.SetDisplayPower(chromeos::DISPLAY_POWER_ALL_ON,
+                                DisplayConfigurator::kSetDisplayPowerNoFlags,
+                                config_waiter_.on_configuration_callback());
+  EXPECT_LT(config_waiter_.Wait(), kMaxShortDelayMs);
+  EXPECT_EQ(CALLBACK_SUCCESS, config_waiter_.callback_result());
   EXPECT_EQ(
       JoinActions(
           kGrab,
           GetFramebufferAction(small_mode_.size(), outputs_[0].get(), nullptr)
               .c_str(),
           GetCrtcAction(*outputs_[0], &small_mode_, gfx::Point(0, 0)).c_str(),
           kForceDPMS, kUngrab, nullptr),
       log_->GetActionsAndClear());
 
   state_controller_.set_state(MULTIPLE_DISPLAY_STATE_DUAL_MIRROR);
   UpdateOutputs(2, true);
   EXPECT_EQ(
       JoinActions(
           kGrab, GetFramebufferAction(small_mode_.size(), outputs_[0].get(),
                                       outputs_[1].get())
                      .c_str(),
           GetCrtcAction(*outputs_[0], &small_mode_, gfx::Point(0, 0)).c_str(),
           GetCrtcAction(*outputs_[1], &small_mode_, gfx::Point(0, 0)).c_str(),
           kUngrab, nullptr),
       log_->GetActionsAndClear());
 
-  configurator_.SetDisplayPower(
-      chromeos::DISPLAY_POWER_ALL_OFF,
-      DisplayConfigurator::kSetDisplayPowerNoFlags,
-      base::Bind(&DisplayConfiguratorTest::OnConfiguredCallback,
-                 base::Unretained(this)));
-  EXPECT_EQ(CALLBACK_SUCCESS, PopCallbackResult());
+  config_waiter_.Reset();
+  configurator_.SetDisplayPower(chromeos::DISPLAY_POWER_ALL_OFF,
+                                DisplayConfigurator::kSetDisplayPowerNoFlags,
+                                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, GetFramebufferAction(small_mode_.size(), outputs_[0].get(),
                                       outputs_[1].get())
                      .c_str(),
           GetCrtcAction(*outputs_[0], nullptr, gfx::Point(0, 0)).c_str(),
           GetCrtcAction(*outputs_[1], nullptr, gfx::Point(0, 0)).c_str(),
           kUngrab, nullptr),
       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_.ResumeDisplays(config_waiter_.on_configuration_callback());
+  EXPECT_LT(config_waiter_.Wait(), kMaxShortDelayMs);
+  EXPECT_EQ(CALLBACK_SUCCESS, config_waiter_.callback_result());
   EXPECT_EQ(kNoActions, log_->GetActionsAndClear());
 
-  configurator_.SetDisplayPower(
-      chromeos::DISPLAY_POWER_ALL_ON,
-      DisplayConfigurator::kSetDisplayPowerNoFlags,
-      base::Bind(&DisplayConfiguratorTest::OnConfiguredCallback,
-                 base::Unretained(this)));
+  config_waiter_.Reset();
+  configurator_.SetDisplayPower(chromeos::DISPLAY_POWER_ALL_ON,
+                                DisplayConfigurator::kSetDisplayPowerNoFlags,
+                                config_waiter_.on_configuration_callback());
+  EXPECT_GE(config_waiter_.Wait(), kMinLongDelayMs);
+  EXPECT_EQ(CALLBACK_SUCCESS, config_waiter_.callback_result());
   EXPECT_EQ(
       JoinActions(
           kGrab,
           GetFramebufferAction(small_mode_.size(), outputs_[0].get(), nullptr)
               .c_str(),
           GetCrtcAction(*outputs_[0], &small_mode_, gfx::Point(0, 0)).c_str(),
           kForceDPMS, kUngrab, nullptr),
       log_->GetActionsAndClear());
 }
 
 TEST_F(DisplayConfiguratorTest, Headless) {
   UpdateOutputs(0, false);
   EXPECT_EQ(kNoActions, log_->GetActionsAndClear());
   Init(false);
   EXPECT_EQ(kNoActions, log_->GetActionsAndClear());
   configurator_.ForceInitialConfigure(0);
   EXPECT_EQ(JoinActions(kInitXRandR, kGrab, kForceDPMS, kUngrab, nullptr),
             log_->GetActionsAndClear());
 
   // Not much should happen when the display power state is changed while
   // no displays are connected.
-  configurator_.SetDisplayPower(
-      chromeos::DISPLAY_POWER_ALL_OFF,
-      DisplayConfigurator::kSetDisplayPowerNoFlags,
-      base::Bind(&DisplayConfiguratorTest::OnConfiguredCallback,
-                 base::Unretained(this)));
-  EXPECT_EQ(CALLBACK_SUCCESS, PopCallbackResult());
+  config_waiter_.Reset();
+  configurator_.SetDisplayPower(chromeos::DISPLAY_POWER_ALL_OFF,
+                                DisplayConfigurator::kSetDisplayPowerNoFlags,
+                                config_waiter_.on_configuration_callback());
+  EXPECT_LT(config_waiter_.Wait(), kMaxShortDelayMs);
+  EXPECT_EQ(CALLBACK_SUCCESS, config_waiter_.callback_result());
   EXPECT_EQ(JoinActions(kGrab, kUngrab, nullptr), log_->GetActionsAndClear());
-  configurator_.SetDisplayPower(
-      chromeos::DISPLAY_POWER_ALL_ON,
-      DisplayConfigurator::kSetDisplayPowerNoFlags,
-      base::Bind(&DisplayConfiguratorTest::OnConfiguredCallback,
-                 base::Unretained(this)));
-  EXPECT_EQ(CALLBACK_SUCCESS, PopCallbackResult());
+  config_waiter_.Reset();
+  configurator_.SetDisplayPower(chromeos::DISPLAY_POWER_ALL_ON,
+                                DisplayConfigurator::kSetDisplayPowerNoFlags,
+                                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, nullptr),
             log_->GetActionsAndClear());
 
   // Connect an external display and check that it's configured correctly.
   outputs_[0] = display::FakeDisplaySnapshot::Builder()
                     .SetId(kDisplayIds[0])
                     .SetNativeMode(big_mode_.Clone())
                     .SetCurrentMode(big_mode_.Clone())
                     .AddMode(small_mode_.Clone())
                     .SetType(DISPLAY_CONNECTION_TYPE_INTERNAL)
@@ skipping 223 matching lines @@
             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(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, GetFramebufferAction(small_mode_.size(),
                                             outputs_[0].get(), nullptr)
                            .c_str(),
                 GetCrtcAction(*outputs_[0], nullptr, gfx::Point(0, 0)).c_str(),
                 kUngrab, kSync, nullptr),
             log_->GetActionsAndClear());
 
   // The configuration timer should not be started when the displays
   // are suspended.
   configurator_.OnConfigurationChanged();
   EXPECT_FALSE(test_api_.TriggerConfigureTimeout());
   EXPECT_EQ(kNoActions, log_->GetActionsAndClear());
 
   // Calls to SetDisplayPower should do nothing if the power state doesn't
   // change.
-  configurator_.SetDisplayPower(
-      chromeos::DISPLAY_POWER_ALL_OFF,
-      DisplayConfigurator::kSetDisplayPowerNoFlags,
-      base::Bind(&DisplayConfiguratorTest::OnConfiguredCallback,
-                 base::Unretained(this)));
-  EXPECT_EQ(CALLBACK_SUCCESS, PopCallbackResult());
+  config_waiter_.Reset();
+  configurator_.SetDisplayPower(chromeos::DISPLAY_POWER_ALL_OFF,
+                                DisplayConfigurator::kSetDisplayPowerNoFlags,
+                                config_waiter_.on_configuration_callback());
+  EXPECT_LT(config_waiter_.Wait(), kMaxShortDelayMs);
+  EXPECT_EQ(CALLBACK_SUCCESS, config_waiter_.callback_result());
   EXPECT_EQ(kNoActions, log_->GetActionsAndClear());
-  configurator_.SetDisplayPower(
-      chromeos::DISPLAY_POWER_ALL_ON,
-      DisplayConfigurator::kSetDisplayPowerNoFlags,
-      base::Bind(&DisplayConfiguratorTest::OnConfiguredCallback,
-                 base::Unretained(this)));
-  EXPECT_EQ(CALLBACK_SUCCESS, PopCallbackResult());
+  config_waiter_.Reset();
+  configurator_.SetDisplayPower(chromeos::DISPLAY_POWER_ALL_ON,
+                                DisplayConfigurator::kSetDisplayPowerNoFlags,
+                                config_waiter_.on_configuration_callback());
+  EXPECT_LT(config_waiter_.Wait(), kMaxShortDelayMs);
+  EXPECT_EQ(CALLBACK_SUCCESS, config_waiter_.callback_result());
   EXPECT_EQ(
       JoinActions(
           kGrab,
           GetFramebufferAction(small_mode_.size(), outputs_[0].get(), nullptr)
               .c_str(),
           GetCrtcAction(*outputs_[0], &small_mode_, gfx::Point(0, 0)).c_str(),
           kForceDPMS, kUngrab, nullptr),
       log_->GetActionsAndClear());
 
   UpdateOutputs(2, false);
   configurator_.SetDisplayMode(MULTIPLE_DISPLAY_STATE_DUAL_MIRROR);
   EXPECT_EQ(
       JoinActions(
           kGrab, GetFramebufferAction(small_mode_.size(), outputs_[0].get(),
                                       outputs_[1].get())
                      .c_str(),
           GetCrtcAction(*outputs_[0], &small_mode_, gfx::Point(0, 0)).c_str(),
           GetCrtcAction(*outputs_[1], &small_mode_, gfx::Point(0, 0)).c_str(),
           kUngrab, nullptr),
       log_->GetActionsAndClear());
 
   // The DisplayConfigurator should do nothing at resume time if there is no
   // state change.
+  config_waiter_.Reset();
   UpdateOutputs(1, false);
-  configurator_.ResumeDisplays();
+  configurator_.ResumeDisplays(config_waiter_.on_configuration_callback());
+  EXPECT_LT(config_waiter_.Wait(), kMaxShortDelayMs);
+  EXPECT_EQ(CALLBACK_SUCCESS, config_waiter_.callback_result());
   EXPECT_EQ(kNoActions, log_->GetActionsAndClear());
 
   // If a configuration task is pending when the displays are suspended, that
   // 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, GetFramebufferAction(small_mode_.size(),
                                             outputs_[0].get(), nullptr)
                            .c_str(),
                 GetCrtcAction(*outputs_[0], nullptr, gfx::Point(0, 0)).c_str(),
                 kUngrab, kSync, nullptr),
             log_->GetActionsAndClear());
-
   EXPECT_FALSE(test_api_.TriggerConfigureTimeout());
   EXPECT_EQ(kNoActions, log_->GetActionsAndClear());
 
-  configurator_.ResumeDisplays();
+  config_waiter_.Reset();
+  configurator_.ResumeDisplays(config_waiter_.on_configuration_callback());
+  EXPECT_LT(config_waiter_.Wait(), kMaxShortDelayMs);
+  EXPECT_EQ(CALLBACK_SUCCESS, config_waiter_.callback_result());
   EXPECT_EQ(
       JoinActions(
           kGrab,
           GetFramebufferAction(small_mode_.size(), outputs_[0].get(), nullptr)
               .c_str(),
           GetCrtcAction(*outputs_[0], &small_mode_, gfx::Point(0, 0)).c_str(),
           kForceDPMS, kUngrab, nullptr),
       log_->GetActionsAndClear());
 }
 
@@ skipping 213 matching lines @@
   state_controller_.set_state(MULTIPLE_DISPLAY_STATE_DUAL_EXTENDED);
   Init(false);
   configurator_.ForceInitialConfigure(0);
   log_->GetActionsAndClear();
   observer_.Reset();
 
   // Turn off the internal display, simulating docked mode.
   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();
 
   // Make all subsequent configuration requests fail and try to turn the
   // internal display back on.
   native_display_delegate_->set_max_configurable_pixels(1);
-  configurator_.SetDisplayPower(
-      chromeos::DISPLAY_POWER_ALL_ON,
-      DisplayConfigurator::kSetDisplayPowerNoFlags,
-      base::Bind(&DisplayConfiguratorTest::OnConfiguredCallback,
-                 base::Unretained(this)));
-  EXPECT_EQ(CALLBACK_FAILURE, PopCallbackResult());
+  configurator_.SetDisplayPower(chromeos::DISPLAY_POWER_ALL_ON,
+                                DisplayConfigurator::kSetDisplayPowerNoFlags,
+                                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();
 
   // Simulate the external display getting disconnected and check that the
   // internal display is turned on (i.e. DISPLAY_POWER_ALL_ON is used) rather
   // than the earlier DISPLAY_POWER_INTERNAL_OFF_EXTERNAL_ON state.
   native_display_delegate_->set_max_configurable_pixels(0);
   UpdateOutputs(1, true);
   EXPECT_EQ(
@@ skipping 14 matching lines @@
   state_controller_.set_state(MULTIPLE_DISPLAY_STATE_DUAL_MIRROR);
   Init(false);
   configurator_.ForceInitialConfigure(0);
   log_->GetActionsAndClear();
   observer_.Reset();
 
   // Turn off the internal display, simulating docked mode.
   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(
       JoinActions(
           kGrab, GetFramebufferAction(big_mode_.size(), outputs_[0].get(),
                                       outputs_[1].get())
                      .c_str(),
           GetCrtcAction(*outputs_[0], nullptr, gfx::Point(0, 0)).c_str(),
           GetCrtcAction(*outputs_[1], &big_mode_, gfx::Point(0, 0)).c_str(),
           kForceDPMS, kUngrab, nullptr),
       log_->GetActionsAndClear());
 
   // 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(
           kGrab, GetFramebufferAction(small_mode_.size(), outputs_[0].get(),
                                       outputs_[1].get())
                      .c_str(),
           GetCrtcAction(*outputs_[0], nullptr, gfx::Point(0, 0)).c_str(),
           GetCrtcAction(*outputs_[1], nullptr, gfx::Point(0, 0)).c_str(),
           kUngrab, kSync, nullptr),
       log_->GetActionsAndClear());
 
   // Before the task runs, exit docked mode.
-  configurator_.SetDisplayPower(
-      chromeos::DISPLAY_POWER_ALL_ON,
-      DisplayConfigurator::kSetDisplayPowerNoFlags,
-      base::Bind(&DisplayConfiguratorTest::OnConfiguredCallback,
-                 base::Unretained(this)));
-  EXPECT_EQ(CALLBACK_SUCCESS, PopCallbackResult());
+  config_waiter_.Reset();
+  configurator_.SetDisplayPower(chromeos::DISPLAY_POWER_ALL_ON,
+                                DisplayConfigurator::kSetDisplayPowerNoFlags,
+                                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(
       JoinActions(
           kGrab, GetFramebufferAction(small_mode_.size(), outputs_[0].get(),
                                       outputs_[1].get())
                      .c_str(),
           GetCrtcAction(*outputs_[0], &small_mode_, gfx::Point(0, 0)).c_str(),
           GetCrtcAction(*outputs_[1], &small_mode_, gfx::Point(0, 0)).c_str(),
           kForceDPMS, kUngrab, nullptr),
       log_->GetActionsAndClear());
 
   // Check that the display states are not changed after resuming.
-  configurator_.ResumeDisplays();
+  config_waiter_.Reset();
+  configurator_.ResumeDisplays(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());
 }
 
 TEST_F(DisplayConfiguratorTest, ExternalControl) {
   InitWithSingleOutput();
   state_controller_.set_state(MULTIPLE_DISPLAY_STATE_SINGLE);
   configurator_.RelinquishControl(
       base::Bind(&DisplayConfiguratorTest::OnDisplayControlUpdated,
                  base::Unretained(this)));
   EXPECT_EQ(CALLBACK_SUCCESS, PopDisplayControlResult());
@@ skipping 17 matching lines @@
        SetDisplayPowerWhilePendingConfigurationTaskRunning) {
   // Start out with two displays in extended mode.
   state_controller_.set_state(MULTIPLE_DISPLAY_STATE_DUAL_EXTENDED);
   Init(false);
   configurator_.ForceInitialConfigure(0);
   log_->GetActionsAndClear();
   observer_.Reset();
 
   native_display_delegate_->set_run_async(true);
 
-  configurator_.SetDisplayPower(
-      chromeos::DISPLAY_POWER_ALL_OFF,
-      DisplayConfigurator::kSetDisplayPowerNoFlags,
-      base::Bind(&DisplayConfiguratorTest::OnConfiguredCallback,
-                 base::Unretained(this)));
+  config_waiter_.Reset();
+  configurator_.SetDisplayPower(chromeos::DISPLAY_POWER_ALL_OFF,
+                                DisplayConfigurator::kSetDisplayPowerNoFlags,
+                                config_waiter_.on_configuration_callback());
 
-  configurator_.SetDisplayPower(
-      chromeos::DISPLAY_POWER_ALL_ON,
-      DisplayConfigurator::kSetDisplayPowerNoFlags,
-      base::Bind(&DisplayConfiguratorTest::OnConfiguredCallback,
-                 base::Unretained(this)));
+  configurator_.SetDisplayPower(chromeos::DISPLAY_POWER_ALL_ON,
+                                DisplayConfigurator::kSetDisplayPowerNoFlags,
+                                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());
 
   const int kDualHeight = small_mode_.size().height() +
                           DisplayConfigurator::kVerticalGap +
                           big_mode_.size().height();
   EXPECT_EQ(
       JoinActions(
           kGrab,
           GetFramebufferAction(gfx::Size(big_mode_.size().width(), kDualHeight),
                                outputs_[0].get(), outputs_[1].get())
               .c_str(),
           GetCrtcAction(*outputs_[0], nullptr, gfx::Point(0, 0)).c_str(),
           GetCrtcAction(*outputs_[1], nullptr,
                         gfx::Point(0, small_mode_.size().height() +
                                           DisplayConfigurator::kVerticalGap))
               .c_str(),
           kUngrab, nullptr),
       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());
 
   EXPECT_EQ(
       JoinActions(
           kGrab,
           GetFramebufferAction(gfx::Size(big_mode_.size().width(), kDualHeight),
                                outputs_[0].get(), outputs_[1].get())
               .c_str(),
           GetCrtcAction(*outputs_[0], &small_mode_, gfx::Point(0, 0)).c_str(),
@@ skipping 10 matching lines @@
   // Start out with two displays in extended mode.
   state_controller_.set_state(MULTIPLE_DISPLAY_STATE_DUAL_EXTENDED);
   Init(false);
   configurator_.ForceInitialConfigure(0);
   log_->GetActionsAndClear();
   observer_.Reset();
 
   // Fail display configuration.
   native_display_delegate_->set_max_configurable_pixels(-1);
 
-  configurator_.SetDisplayPower(
-      chromeos::DISPLAY_POWER_ALL_OFF,
-      DisplayConfigurator::kSetDisplayPowerNoFlags,
-      base::Bind(&DisplayConfiguratorTest::OnConfiguredCallback,
-                 base::Unretained(this)));
-
-  EXPECT_EQ(CALLBACK_FAILURE, PopCallbackResult());
+  config_waiter_.Reset();
+  configurator_.SetDisplayPower(chromeos::DISPLAY_POWER_ALL_OFF,
+                                DisplayConfigurator::kSetDisplayPowerNoFlags,
+                                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());
 
   const int kDualHeight = small_mode_.size().height() +
                           DisplayConfigurator::kVerticalGap +
                           big_mode_.size().height();
 
   EXPECT_EQ(
       JoinActions(
           kGrab,
           GetFramebufferAction(gfx::Size(big_mode_.size().width(), kDualHeight),
                                outputs_[0].get(), outputs_[1].get())
               .c_str(),
           GetCrtcAction(*outputs_[0], nullptr, gfx::Point(0, 0)).c_str(),
           GetCrtcAction(*outputs_[1], nullptr,
                         gfx::Point(0, small_mode_.size().height() +
                                           DisplayConfigurator::kVerticalGap))
               .c_str(),
           kUngrab, nullptr),
       log_->GetActionsAndClear());
 
   // This configuration should trigger a display configuration since the
   // previous configuration failed.
-  configurator_.SetDisplayPower(
-      chromeos::DISPLAY_POWER_ALL_ON,
-      DisplayConfigurator::kSetDisplayPowerNoFlags,
-      base::Bind(&DisplayConfiguratorTest::OnConfiguredCallback,
-                 base::Unretained(this)));
+  config_waiter_.Reset();
+  configurator_.SetDisplayPower(chromeos::DISPLAY_POWER_ALL_ON,
+                                DisplayConfigurator::kSetDisplayPowerNoFlags,
+                                config_waiter_.on_configuration_callback());
+  EXPECT_LT(config_waiter_.Wait(), kMaxShortDelayMs);
 
   EXPECT_EQ(0, observer_.num_changes());
   EXPECT_EQ(2, observer_.num_failures());
   EXPECT_EQ(
       JoinActions(
           kGrab,
           GetFramebufferAction(gfx::Size(big_mode_.size().width(), kDualHeight),
                                outputs_[0].get(), outputs_[1].get())
               .c_str(),
           GetCrtcAction(*outputs_[0], &small_mode_, gfx::Point(0, 0)).c_str(),
@@ skipping 65 matching lines @@
           GetCrtcAction(
               *outputs_[2], &small_mode_,
               gfx::Point(0, small_mode_.size().height() +
                                 big_mode_.size().height() +
                                 2 * DisplayConfigurator::kVerticalGap))
               .c_str(),
           kUngrab, nullptr),
       log_->GetActionsAndClear());
 
   // Verify that turning the power off works.
-  configurator_.SetDisplayPower(
-      chromeos::DISPLAY_POWER_ALL_OFF,
-      DisplayConfigurator::kSetDisplayPowerNoFlags,
-      base::Bind(&DisplayConfiguratorTest::OnConfiguredCallback,
-                 base::Unretained(this)));
-
-  EXPECT_EQ(CALLBACK_SUCCESS, PopCallbackResult());
+  config_waiter_.Reset();
+  configurator_.SetDisplayPower(chromeos::DISPLAY_POWER_ALL_OFF,
+                                DisplayConfigurator::kSetDisplayPowerNoFlags,
+                                config_waiter_.on_configuration_callback());
+  EXPECT_LT(config_waiter_.Wait(), kMaxShortDelayMs);
+  EXPECT_EQ(CALLBACK_SUCCESS, config_waiter_.callback_result());
   EXPECT_EQ(
       JoinActions(
           kGrab, GetFramebufferAction(
                      gfx::Size(big_mode_.size().width(), kTripleHeight),
                      outputs_[0].get(), outputs_[1].get())
                      .c_str(),
           GetCrtcAction(*outputs_[0], nullptr, gfx::Point(0, 0)).c_str(),
           GetCrtcAction(*outputs_[1], nullptr,
                         gfx::Point(0, small_mode_.size().height() +
                                           DisplayConfigurator::kVerticalGap))
               .c_str(),
           GetCrtcAction(
               *outputs_[2], nullptr,
               gfx::Point(0, small_mode_.size().height() +
                                 big_mode_.size().height() +
                                 2 * DisplayConfigurator::kVerticalGap))
               .c_str(),
           kUngrab, nullptr),
       log_->GetActionsAndClear());
 
-  configurator_.SetDisplayPower(
-      chromeos::DISPLAY_POWER_ALL_ON,
-      DisplayConfigurator::kSetDisplayPowerNoFlags,
-      base::Bind(&DisplayConfiguratorTest::OnConfiguredCallback,
-                 base::Unretained(this)));
-
-  EXPECT_EQ(CALLBACK_SUCCESS, PopCallbackResult());
+  config_waiter_.Reset();
+  configurator_.SetDisplayPower(chromeos::DISPLAY_POWER_ALL_ON,
+                                DisplayConfigurator::kSetDisplayPowerNoFlags,
+                                config_waiter_.on_configuration_callback());
+  EXPECT_LT(config_waiter_.Wait(), kMaxShortDelayMs);
+  EXPECT_EQ(CALLBACK_SUCCESS, config_waiter_.callback_result());
   EXPECT_EQ(
       JoinActions(
           kGrab, GetFramebufferAction(
                      gfx::Size(big_mode_.size().width(), kTripleHeight),
                      outputs_[0].get(), outputs_[1].get())
                      .c_str(),
           GetCrtcAction(*outputs_[0], &small_mode_, gfx::Point(0, 0)).c_str(),
           GetCrtcAction(*outputs_[1], &big_mode_,
                         gfx::Point(0, small_mode_.size().height() +
                                           DisplayConfigurator::kVerticalGap))
@@ skipping 19 matching lines @@
               .c_str(),
           GetCrtcAction(*outputs_[0], &small_mode_, gfx::Point(0, 0)).c_str(),
           GetCrtcAction(*outputs_[1], &big_mode_,
                         gfx::Point(0, small_mode_.size().height() +
                                           DisplayConfigurator::kVerticalGap))
               .c_str(),
           kUngrab, nullptr),
       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.
+  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_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_.ResumeDisplays(config_waiter_.on_configuration_callback());
+  EXPECT_GE(test_api_.GetConfigureDelay(), kMinLongDelayMs);

[Daniel Erat, 2016/10/25 16:05:32]

you should use EXPECT_EQ rather than EXPECT_GE for all of these now, right? you
should get back the configured delay instead of some arbitrary delta, i think.

[afakhry, 2016/10/25 20:14:11]

Done.

+  EXPECT_TRUE(test_api_.TriggerConfigureTimeout());
+  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_.ResumeDisplays(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_.ResumeDisplays(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