Make SetDisplayPower() take a callback to signal completion

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 "base/memory/scoped_vector.h"
 #include "base/message_loop/message_loop.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "ui/display/chromeos/test/action_logger_util.h"
@@ skipping 99 matching lines @@
 
   DISALLOW_COPY_AND_ASSIGN(TestMirroringController);
 };
 
 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),
         observer_(&configurator_),
-        test_api_(&configurator_) {}
+        test_api_(&configurator_),
+        callback_status_(false),
+        callback_called_(false) {}
   ~DisplayConfiguratorTest() override {}
 
   void SetUp() override {
     log_.reset(new ActionLogger());
 
     native_display_delegate_ = new TestNativeDisplayDelegate(log_.get());
     configurator_.SetDelegateForTesting(
         scoped_ptr<NativeDisplayDelegate>(native_display_delegate_));
 
     configurator_.set_state_controller(&state_controller_);
@@ skipping 15 matching lines @@
     o->set_native_mode(&big_mode_);
     modes.push_back(&big_mode_);
     o->set_modes(modes);
     o->set_type(DISPLAY_CONNECTION_TYPE_HDMI);
     o->set_is_aspect_preserving_scaling(true);
     o->set_display_id(456);
 
     UpdateOutputs(2, false);
   }
 
+  void OnConfiguredCallback(bool status) {
+    callback_called_ = true;
+    callback_status_ = status;
+  }
+
   // Predefined modes that can be used by outputs.
   const DisplayMode small_mode_;
   const DisplayMode big_mode_;
 
  protected:
   // Configures |native_display_delegate_| to return the first |num_outputs|
   // entries from
   // |outputs_|. If |send_events| is true, also sends screen-change and
   // output-change events to |configurator_| and triggers the configure
   // timeout if one was scheduled.
@@ skipping 19 matching lines @@
     configurator_.ForceInitialConfigure(0);
     EXPECT_EQ(JoinActions(kInitXRandR, kGrab,
                           GetFramebufferAction(small_mode_.size(), &outputs_[0],
                                                NULL).c_str(),
                           GetCrtcAction(outputs_[0], &small_mode_,
                                         gfx::Point(0, 0)).c_str(),
                           kForceDPMS, kUngrab, NULL),
               log_->GetActionsAndClear());
   }
 
+  void CheckAndResetCallback(bool expected_status) {
+    EXPECT_TRUE(callback_called_);
+    EXPECT_EQ(expected_status, callback_status_);
+
+    callback_status_ = false;
+    callback_called_ = false;
+  }
+
   base::MessageLoop message_loop_;
   TestStateController state_controller_;
   TestMirroringController mirroring_controller_;
   DisplayConfigurator configurator_;
   TestObserver observer_;
   scoped_ptr<ActionLogger> log_;
   TestNativeDisplayDelegate* native_display_delegate_;  // not owned
   DisplayConfigurator::TestApi test_api_;
 
   TestDisplaySnapshot outputs_[2];
 
+  bool callback_status_;
+  bool callback_called_;
+
  private:
   DISALLOW_COPY_AND_ASSIGN(DisplayConfiguratorTest);
 };
 
 }  // namespace
 
 TEST_F(DisplayConfiguratorTest, FindDisplayModeMatchingSize) {
   ScopedVector<const DisplayMode> modes;
 
   // Fields are width, height, interlaced, refresh rate.
@@ skipping 206 matching lines @@
           NULL),
       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();
   configurator_.SetDisplayPower(
       chromeos::DISPLAY_POWER_INTERNAL_OFF_EXTERNAL_ON,
-      DisplayConfigurator::kSetDisplayPowerNoFlags);
+      DisplayConfigurator::kSetDisplayPowerNoFlags,
+      base::Bind(&DisplayConfiguratorTest::OnConfiguredCallback,
+                 base::Unretained(this)));
+  CheckAndResetCallback(true);

[Daniel Erat, 2015/01/30 17:14:07]

CheckAndResetCallback() failures won't produce meaningful line numbers or stack
traces, will they? how about using an enum instead?

  enum CallbackResult {
    CALLBACK_SUCCESS,
    CALLBACK_FAILURE,
    CALLBACK_NOT_CALLED,
  }

  CallbackResult PopCallbackResult() {
    // choose enum value, clear the "called" field, and return
  }

and then in the tests:

  EXPECT_EQ(CALLBACK_SUCCESS, PopCallbackResult());

[dnicoara, 2015/01/30 18:25:28]

Done.

   EXPECT_EQ(
       JoinActions(
           kGrab,
           GetFramebufferAction(big_mode_.size(), &outputs_[0], &outputs_[1])
               .c_str(),
           GetCrtcAction(outputs_[0], NULL, gfx::Point(0, 0)).c_str(),
           GetCrtcAction(outputs_[1], &big_mode_, gfx::Point(0, 0)).c_str(),
           kForceDPMS,
           kUngrab,
           NULL),
       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);
+  configurator_.SetDisplayPower(
+      chromeos::DISPLAY_POWER_ALL_OFF,
+      DisplayConfigurator::kSetDisplayPowerNoFlags,
+      base::Bind(&DisplayConfiguratorTest::OnConfiguredCallback,
+                 base::Unretained(this)));
+  CheckAndResetCallback(true);
   EXPECT_EQ(
       JoinActions(kGrab,
                   GetFramebufferAction(
                       small_mode_.size(), &outputs_[0], &outputs_[1]).c_str(),
                   GetCrtcAction(outputs_[0], NULL, gfx::Point(0, 0)).c_str(),
                   GetCrtcAction(outputs_[1], NULL, gfx::Point(0, 0)).c_str(),
                   kUngrab,
                   NULL),
       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);
+  configurator_.SetDisplayPower(
+      chromeos::DISPLAY_POWER_ALL_ON,
+      DisplayConfigurator::kSetDisplayPowerNoFlags,
+      base::Bind(&DisplayConfiguratorTest::OnConfiguredCallback,
+                 base::Unretained(this)));
+  CheckAndResetCallback(true);
   EXPECT_EQ(
       JoinActions(
           kGrab,
           GetFramebufferAction(small_mode_.size(), &outputs_[0], &outputs_[1])
               .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,
           NULL),
@@ skipping 29 matching lines @@
   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();
   configurator_.SetDisplayPower(
       chromeos::DISPLAY_POWER_INTERNAL_OFF_EXTERNAL_ON,
-      DisplayConfigurator::kSetDisplayPowerNoFlags);
+      DisplayConfigurator::kSetDisplayPowerNoFlags,
+      base::Bind(&DisplayConfiguratorTest::OnConfiguredCallback,
+                 base::Unretained(this)));
+  CheckAndResetCallback(true);
   EXPECT_EQ(
       JoinActions(
           kGrab,
           GetFramebufferAction(big_mode_.size(), &outputs_[0], &outputs_[1])
               .c_str(),
           GetCrtcAction(outputs_[0], NULL, gfx::Point(0, 0)).c_str(),
           GetCrtcAction(outputs_[1], &big_mode_, gfx::Point(0, 0)).c_str(),
           kForceDPMS,
           kUngrab,
           NULL),
       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);
+  configurator_.SetDisplayPower(
+      chromeos::DISPLAY_POWER_ALL_OFF,
+      DisplayConfigurator::kSetDisplayPowerNoFlags,
+      base::Bind(&DisplayConfiguratorTest::OnConfiguredCallback,
+                 base::Unretained(this)));
+  CheckAndResetCallback(true);
   EXPECT_EQ(
       JoinActions(
           kGrab,
           GetFramebufferAction(gfx::Size(big_mode_.size().width(), kDualHeight),
                                &outputs_[0],
                                &outputs_[1]).c_str(),
           GetCrtcAction(outputs_[0], NULL, gfx::Point(0, 0)).c_str(),
           GetCrtcAction(outputs_[1],
                         NULL,
                         gfx::Point(0,
                                    small_mode_.size().height() +
                                        DisplayConfigurator::kVerticalGap))
               .c_str(),
           kUngrab,
           NULL),
       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);
+  configurator_.SetDisplayPower(
+      chromeos::DISPLAY_POWER_ALL_ON,
+      DisplayConfigurator::kSetDisplayPowerNoFlags,
+      base::Bind(&DisplayConfiguratorTest::OnConfiguredCallback,
+                 base::Unretained(this)));
+  CheckAndResetCallback(true);
   EXPECT_EQ(
       JoinActions(
           kGrab,
           GetFramebufferAction(gfx::Size(big_mode_.size().width(), kDualHeight),
                                &outputs_[0],
                                &outputs_[1]).c_str(),
           GetCrtcAction(outputs_[0], &small_mode_, gfx::Point(0, 0)).c_str(),
           GetCrtcAction(outputs_[1],
                         &big_mode_,
                         gfx::Point(0,
@@ skipping 25 matching lines @@
           kGrab,
           GetFramebufferAction(small_mode_.size(), &outputs_[0], NULL).c_str(),
           GetCrtcAction(outputs_[0], &small_mode_, gfx::Point(0, 0)).c_str(),
           kForceDPMS,
           kUngrab,
           NULL),
       log_->GetActionsAndClear());
 
   // 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);
+  configurator_.SetDisplayPower(
+      chromeos::DISPLAY_POWER_ALL_OFF,
+      DisplayConfigurator::kSetDisplayPowerNoFlags,
+      base::Bind(&DisplayConfiguratorTest::OnConfiguredCallback,
+                 base::Unretained(this)));
+  CheckAndResetCallback(true);
   EXPECT_EQ(
       JoinActions(
           kGrab,
           GetFramebufferAction(small_mode_.size(), &outputs_[0], NULL).c_str(),
           GetCrtcAction(outputs_[0], NULL, gfx::Point(0, 0)).c_str(),
           kUngrab,
           NULL),
       log_->GetActionsAndClear());
 
   configurator_.SuspendDisplays();
@@ skipping 28 matching lines @@
       JoinActions(
           kGrab,
           GetFramebufferAction(small_mode_.size(), &outputs_[0], &outputs_[1])
               .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,
           NULL),
       log_->GetActionsAndClear());
 
-  configurator_.SetDisplayPower(chromeos::DISPLAY_POWER_ALL_OFF,
-                                DisplayConfigurator::kSetDisplayPowerNoFlags);
+  configurator_.SetDisplayPower(
+      chromeos::DISPLAY_POWER_ALL_OFF,
+      DisplayConfigurator::kSetDisplayPowerNoFlags,
+      base::Bind(&DisplayConfiguratorTest::OnConfiguredCallback,
+                 base::Unretained(this)));
+  CheckAndResetCallback(true);
   EXPECT_EQ(
       JoinActions(kGrab,
                   GetFramebufferAction(
                       small_mode_.size(), &outputs_[0], &outputs_[1]).c_str(),
                   GetCrtcAction(outputs_[0], NULL, gfx::Point(0, 0)).c_str(),
                   GetCrtcAction(outputs_[1], NULL, gfx::Point(0, 0)).c_str(),
                   kUngrab,
                   NULL),
       log_->GetActionsAndClear());
 
@@ skipping 20 matching lines @@
   UpdateOutputs(0, false);
   EXPECT_EQ(kNoActions, log_->GetActionsAndClear());
   configurator_.Init(false);
   EXPECT_EQ(kNoActions, log_->GetActionsAndClear());
   configurator_.ForceInitialConfigure(0);
   EXPECT_EQ(JoinActions(kInitXRandR, kGrab, kForceDPMS, kUngrab, NULL),
             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);
+  configurator_.SetDisplayPower(
+      chromeos::DISPLAY_POWER_ALL_OFF,
+      DisplayConfigurator::kSetDisplayPowerNoFlags,
+      base::Bind(&DisplayConfiguratorTest::OnConfiguredCallback,
+                 base::Unretained(this)));
+  CheckAndResetCallback(true);
   EXPECT_EQ(JoinActions(kGrab, kUngrab, NULL), log_->GetActionsAndClear());
-  configurator_.SetDisplayPower(chromeos::DISPLAY_POWER_ALL_ON,
-                                DisplayConfigurator::kSetDisplayPowerNoFlags);
+  configurator_.SetDisplayPower(
+      chromeos::DISPLAY_POWER_ALL_ON,
+      DisplayConfigurator::kSetDisplayPowerNoFlags,
+      base::Bind(&DisplayConfiguratorTest::OnConfiguredCallback,
+                 base::Unretained(this)));
+  CheckAndResetCallback(true);
   EXPECT_EQ(JoinActions(kGrab, kForceDPMS, kUngrab, NULL),
             log_->GetActionsAndClear());
 
   // Connect an external display and check that it's configured correctly.
   outputs_[0].set_current_mode(outputs_[1].current_mode());
   outputs_[0].set_native_mode(outputs_[1].native_mode());
   outputs_[0].set_modes(outputs_[1].modes());
   outputs_[0].set_type(outputs_[1].type());
 
   UpdateOutputs(1, true);
@@ skipping 348 matching lines @@
   // Start out with two displays in extended mode.
   state_controller_.set_state(MULTIPLE_DISPLAY_STATE_DUAL_EXTENDED);
   configurator_.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);
+      DisplayConfigurator::kSetDisplayPowerNoFlags,
+      base::Bind(&DisplayConfiguratorTest::OnConfiguredCallback,
+                 base::Unretained(this)));
+  CheckAndResetCallback(true);
   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);
+  configurator_.SetDisplayPower(
+      chromeos::DISPLAY_POWER_ALL_ON,
+      DisplayConfigurator::kSetDisplayPowerNoFlags,
+      base::Bind(&DisplayConfiguratorTest::OnConfiguredCallback,
+                 base::Unretained(this)));
+  CheckAndResetCallback(false);
   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(JoinActions(kGrab, GetFramebufferAction(small_mode_.size(),
@@ skipping 11 matching lines @@
   // Start out with two displays in mirrored mode.
   state_controller_.set_state(MULTIPLE_DISPLAY_STATE_DUAL_MIRROR);
   configurator_.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);
+      DisplayConfigurator::kSetDisplayPowerNoFlags,
+      base::Bind(&DisplayConfiguratorTest::OnConfiguredCallback,
+                 base::Unretained(this)));
+  CheckAndResetCallback(true);
   EXPECT_EQ(1, observer_.num_changes());
   EXPECT_EQ(0, observer_.num_failures());
   EXPECT_EQ(
       JoinActions(
           kGrab,
           GetFramebufferAction(big_mode_.size(), &outputs_[0], &outputs_[1])
               .c_str(),
           GetCrtcAction(outputs_[0], NULL, gfx::Point(0, 0)).c_str(),
           GetCrtcAction(outputs_[1], &big_mode_, gfx::Point(0, 0)).c_str(),
           kForceDPMS,
           kUngrab,
           NULL),
       log_->GetActionsAndClear());
 
   // Suspend and resume the system. Resuming should post a task to restore the
   // previous power state, additionally forcing a probe.
   configurator_.SuspendDisplays();
   configurator_.ResumeDisplays();
 
   // Before the task runs, exit docked mode.
-  configurator_.SetDisplayPower(chromeos::DISPLAY_POWER_ALL_ON,
-                                DisplayConfigurator::kSetDisplayPowerNoFlags);
+  configurator_.SetDisplayPower(
+      chromeos::DISPLAY_POWER_ALL_ON,
+      DisplayConfigurator::kSetDisplayPowerNoFlags,
+      base::Bind(&DisplayConfiguratorTest::OnConfiguredCallback,
+                 base::Unretained(this)));
+  CheckAndResetCallback(true);
   EXPECT_EQ(2, observer_.num_changes());
   EXPECT_EQ(0, observer_.num_failures());
   EXPECT_EQ(
       JoinActions(
           kGrab,
           GetFramebufferAction(small_mode_.size(), &outputs_[0], &outputs_[1])
               .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,
@@ skipping 31 matching lines @@
                         GetFramebufferAction(small_mode_.size(), &outputs_[0],
                                              nullptr).c_str(),
                         GetCrtcAction(outputs_[0], &small_mode_,
                                       gfx::Point(0, 0)).c_str(),
                         kUngrab, NULL),
             log_->GetActionsAndClear());
 }
 
 }  // namespace test
 }  // namespace ui