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..f7a22d19a6de183f6e9c0c5dcdd8447dcb192369 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"
@@ -186,6 +187,11 @@ class DisplayConfiguratorTest : public testing::Test {
 
   void OnConfiguredCallback(bool status) {
     callback_result_ = (status ? CALLBACK_SUCCESS : CALLBACK_FAILURE);
+
+    if (!quit_closure_.is_null())
+      base::ResetAndReturn(&quit_closure_).Run();
+
+    configuration_done_ = true;
   }
 
   void OnDisplayControlUpdated(bool status) {
@@ -250,16 +256,28 @@ class DisplayConfiguratorTest : public testing::Test {
   CallbackResult PopCallbackResult() {
     CallbackResult result = callback_result_;
     callback_result_ = CALLBACK_NOT_CALLED;
+    configuration_done_ = false;
     return result;
   }
 
   CallbackResult PopDisplayControlResult() {
     CallbackResult result = display_control_result_;
     display_control_result_ = CALLBACK_NOT_CALLED;
+    configuration_done_ = false;
     return result;
   }
 
+  void WaitForConfiguration() {

[Daniel Erat, 2016/10/19 16:57:56]

how about making this take an expected_duration parameter and check that the
timer has the expected delay? it could e.g. return false if the delay is wrong,
and the callers could check that.

[afakhry, 2016/10/21 20:03:03]

I think the best approach is to encapsulate the waiting logic into its own
object. Please take a look at the ConfigurationWaiter.

+    if (!configuration_done_) {
+      base::RunLoop run_loop;
+      quit_closure_ = run_loop.QuitClosure();
+      run_loop.Run();
+    }
+    configuration_done_ = false;
+  }
+
   base::MessageLoop message_loop_;
+  base::Closure quit_closure_;
   TestStateController state_controller_;
   TestMirroringController mirroring_controller_;
   DisplayConfigurator configurator_;
@@ -279,6 +297,8 @@ class DisplayConfiguratorTest : public testing::Test {
   CallbackResult callback_result_;
   CallbackResult display_control_result_;
 
+  bool configuration_done_ = false;

[Daniel Erat, 2016/10/18 22:17:41]

please add a comment describing when this is true and when it's false.

+
  private:
   DISALLOW_COPY_AND_ASSIGN(DisplayConfiguratorTest);
 };
@@ -718,6 +738,7 @@ TEST_F(DisplayConfiguratorTest, SetDisplayPower) {
       DisplayConfigurator::kSetDisplayPowerNoFlags,
       base::Bind(&DisplayConfiguratorTest::OnConfiguredCallback,
                  base::Unretained(this)));
+  WaitForConfiguration();
   EXPECT_EQ(CALLBACK_SUCCESS, PopCallbackResult());
   EXPECT_EQ(
       JoinActions(
@@ -828,6 +849,7 @@ TEST_F(DisplayConfiguratorTest, SetDisplayPower) {
       DisplayConfigurator::kSetDisplayPowerNoFlags,
       base::Bind(&DisplayConfiguratorTest::OnConfiguredCallback,
                  base::Unretained(this)));
+  WaitForConfiguration();
   EXPECT_EQ(CALLBACK_SUCCESS, PopCallbackResult());
   EXPECT_EQ(
       JoinActions(
@@ -971,6 +993,7 @@ TEST_F(DisplayConfiguratorTest, SuspendAndResume) {
       DisplayConfigurator::kSetDisplayPowerNoFlags,
       base::Bind(&DisplayConfiguratorTest::OnConfiguredCallback,
                  base::Unretained(this)));
+  WaitForConfiguration();
   EXPECT_EQ(
       JoinActions(
           kGrab,
@@ -1640,6 +1663,7 @@ TEST_F(DisplayConfiguratorTest, DontRestoreStalePowerStateAfterResume) {
       DisplayConfigurator::kSetDisplayPowerNoFlags,
       base::Bind(&DisplayConfiguratorTest::OnConfiguredCallback,
                  base::Unretained(this)));
+  WaitForConfiguration();
   EXPECT_EQ(CALLBACK_SUCCESS, PopCallbackResult());
   EXPECT_EQ(3, observer_.num_changes());
   EXPECT_EQ(0, observer_.num_failures());
@@ -1916,6 +1940,7 @@ TEST_F(DisplayConfiguratorTest, TestWithThreeDisplays) {
       DisplayConfigurator::kSetDisplayPowerNoFlags,
       base::Bind(&DisplayConfiguratorTest::OnConfiguredCallback,
                  base::Unretained(this)));
+  WaitForConfiguration();
 
   EXPECT_EQ(CALLBACK_SUCCESS, PopCallbackResult());
   EXPECT_EQ(