chromeos: Save display power state on config failure.

ui/display/chromeos/display_configurator.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/bind.h"
 #include "base/command_line.h"
 #include "base/logging.h"
 #include "base/strings/string_number_conversions.h"
@@ skipping 142 matching lines @@
 
   return best_mode;
 }
 
 DisplayConfigurator::DisplayConfigurator()
     : state_controller_(NULL),
       mirroring_controller_(NULL),
       is_panel_fitting_enabled_(false),
       configure_display_(base::SysInfo::IsRunningOnChromeOS()),
       display_state_(MULTIPLE_DISPLAY_STATE_INVALID),
-      power_state_(chromeos::DISPLAY_POWER_ALL_ON),
+      requested_power_state_(chromeos::DISPLAY_POWER_ALL_ON),
+      current_power_state_(chromeos::DISPLAY_POWER_ALL_ON),
       next_display_protection_client_id_(1) {}
 
 DisplayConfigurator::~DisplayConfigurator() {
   if (native_display_delegate_)
     native_display_delegate_->RemoveObserver(this);
 }
 
 void DisplayConfigurator::SetDelegateForTesting(
     scoped_ptr<NativeDisplayDelegate> display_delegate) {
   DCHECK(!native_display_delegate_);
 
   native_display_delegate_ = display_delegate.Pass();
   configure_display_ = true;
 }
 
 void DisplayConfigurator::SetInitialDisplayPower(
     chromeos::DisplayPowerState power_state) {
   DCHECK_EQ(display_state_, MULTIPLE_DISPLAY_STATE_INVALID);
-  power_state_ = power_state;
+  requested_power_state_ = current_power_state_ = power_state;
 }
 
 void DisplayConfigurator::Init(bool is_panel_fitting_enabled) {
   is_panel_fitting_enabled_ = is_panel_fitting_enabled;
   if (!configure_display_)
     return;
 
   // If the delegate is already initialized don't update it (For example, tests
   // set their own delegates).
   if (!native_display_delegate_) {
     native_display_delegate_ = CreatePlatformNativeDisplayDelegate();
     native_display_delegate_->AddObserver(this);
   }
 }
 
 void DisplayConfigurator::ForceInitialConfigure(
     uint32_t background_color_argb) {
   if (!configure_display_)
     return;
 
   native_display_delegate_->GrabServer();
   native_display_delegate_->Initialize();
 
   UpdateCachedDisplays();
   if (cached_displays_.size() > 1 && background_color_argb)
     native_display_delegate_->SetBackgroundColor(background_color_argb);
-  const MultipleDisplayState new_state = ChooseDisplayState(power_state_);
-  const bool success =
-      EnterStateOrFallBackToSoftwareMirroring(new_state, power_state_);
+  const MultipleDisplayState new_state = ChooseDisplayState(
+      requested_power_state_);
+  const bool success = EnterStateOrFallBackToSoftwareMirroring(
+      new_state, requested_power_state_);
 
   // Force the DPMS on chrome startup as the driver doesn't always detect
   // that all displays are on when signing out.
   native_display_delegate_->ForceDPMSOn();
   native_display_delegate_->UngrabServer();
   NotifyObservers(success, new_state);
 }
 
 bool DisplayConfigurator::IsMirroring() const {
   return display_state_ == MULTIPLE_DISPLAY_STATE_DUAL_MIRROR ||
@@ skipping 213 matching lines @@
 bool DisplayConfigurator::SetDisplayPower(
     chromeos::DisplayPowerState power_state,
     int flags) {
   if (!configure_display_)
     return false;
 
   VLOG(1) << "SetDisplayPower: power_state="
           << DisplayPowerStateToString(power_state) << " flags=" << flags
           << ", configure timer="
           << (configure_timer_.IsRunning() ? "Running" : "Stopped");
-  if (power_state == power_state_ && !(flags & kSetDisplayPowerForceProbe))
+  if (power_state == current_power_state_ &&
+      !(flags & kSetDisplayPowerForceProbe))
     return true;
 
   native_display_delegate_->GrabServer();
   UpdateCachedDisplays();
 
   const MultipleDisplayState new_state = ChooseDisplayState(power_state);
   bool attempted_change = false;
   bool success = false;
 
   bool only_if_single_internal_display =
       flags & kSetDisplayPowerOnlyIfSingleInternalDisplay;
   bool single_internal_display =
       cached_displays_.size() == 1 &&
       cached_displays_[0].display->type() == DISPLAY_CONNECTION_TYPE_INTERNAL;
   if (single_internal_display || !only_if_single_internal_display) {
     success = EnterStateOrFallBackToSoftwareMirroring(new_state, power_state);
     attempted_change = true;
 
     // Force the DPMS on since the driver doesn't always detect that it
     // should turn on. This is needed when coming back from idle suspend.
     if (success && power_state != chromeos::DISPLAY_POWER_ALL_OFF)
       native_display_delegate_->ForceDPMSOn();
   }
 
   native_display_delegate_->UngrabServer();
   if (attempted_change)
     NotifyObservers(success, new_state);
   return true;

[dnicoara, 2014/09/25 13:59:54]

This is a bit unrelated to this CL, but should we return |success| here?

[Daniel Erat, 2014/09/25 17:31:06]

i'm fine with changing it. it's unclear what the return value is supposed to
mean (on top of nobody checking it); false is returned if |configure_display_|
is set, for instance. but saying something like, "Returns true if the system
successfully enters (or was already in) |power_state|." seems reasonable to me.

[dnicoara, 2014/09/25 17:50:35]

Thanks! This makes it more consistent with other similar calls.

 }
 
 bool DisplayConfigurator::SetDisplayMode(MultipleDisplayState new_state) {
   if (!configure_display_)
     return false;
 
   VLOG(1) << "SetDisplayMode: state=" << DisplayStateToString(new_state);
   if (display_state_ == new_state) {
     // Cancel software mirroring if the state is moving from
     // MULTIPLE_DISPLAY_STATE_DUAL_EXTENDED to
     // MULTIPLE_DISPLAY_STATE_DUAL_EXTENDED.
     if (mirroring_controller_ &&
         new_state == MULTIPLE_DISPLAY_STATE_DUAL_EXTENDED)
       mirroring_controller_->SetSoftwareMirroring(false);
     NotifyObservers(true, new_state);
     return true;
   }
 
   native_display_delegate_->GrabServer();
   UpdateCachedDisplays();
-  const bool success =
-      EnterStateOrFallBackToSoftwareMirroring(new_state, power_state_);
+  const bool success = EnterStateOrFallBackToSoftwareMirroring(
+      new_state, requested_power_state_);
   native_display_delegate_->UngrabServer();
 
   NotifyObservers(success, new_state);
   return success;
 }
 
 void DisplayConfigurator::OnConfigurationChanged() {
   // Configure displays with |kConfigureDelayMs| delay,
   // so that time-consuming ConfigureDisplays() won't be called multiple times.
   if (configure_timer_.IsRunning()) {
@@ skipping 19 matching lines @@
 void DisplayConfigurator::RemoveObserver(Observer* observer) {
   observers_.RemoveObserver(observer);
 }
 
 void DisplayConfigurator::SuspendDisplays() {
   // If the display is off due to user inactivity and there's only a single
   // internal display connected, switch to the all-on state before
   // suspending.  This shouldn't be very noticeable to the user since the
   // backlight is off at this point, and doing this lets us resume directly
   // into the "on" state, which greatly reduces resume times.
-  if (power_state_ == chromeos::DISPLAY_POWER_ALL_OFF) {
+  if (requested_power_state_ == chromeos::DISPLAY_POWER_ALL_OFF) {
     SetDisplayPower(chromeos::DISPLAY_POWER_ALL_ON,
                     kSetDisplayPowerOnlyIfSingleInternalDisplay);
 
     // We need to make sure that the monitor configuration we just did actually
     // completes before we return, because otherwise the X message could be
     // racing with the HandleSuspendReadiness message.
     native_display_delegate_->SyncWithServer();
   }
 }
 
 void DisplayConfigurator::ResumeDisplays() {
   // Force probing to ensure that we pick up any changes that were made
   // while the system was suspended.
   configure_timer_.Start(
       FROM_HERE,
       base::TimeDelta::FromMilliseconds(kResumeDelayMs),
       base::Bind(base::IgnoreResult(&DisplayConfigurator::SetDisplayPower),
                  base::Unretained(this),
-                 power_state_,
+                 requested_power_state_,
                  kSetDisplayPowerForceProbe));
 }
 
 void DisplayConfigurator::UpdateCachedDisplays() {
   std::vector<DisplaySnapshot*> snapshots =
       native_display_delegate_->GetDisplays();
 
   cached_displays_.clear();
   for (size_t i = 0; i < snapshots.size(); ++i) {
     DisplayState display_state;
@@ skipping 131 matching lines @@
 
   return false;
 }
 
 void DisplayConfigurator::ConfigureDisplays() {
   if (!configure_display_)
     return;
 
   native_display_delegate_->GrabServer();
   UpdateCachedDisplays();
-  const MultipleDisplayState new_state = ChooseDisplayState(power_state_);
-  const bool success =
-      EnterStateOrFallBackToSoftwareMirroring(new_state, power_state_);
+  const MultipleDisplayState new_state = ChooseDisplayState(
+      requested_power_state_);
+  const bool success = EnterStateOrFallBackToSoftwareMirroring(
+      new_state, requested_power_state_);
   native_display_delegate_->UngrabServer();
 
   NotifyObservers(success, new_state);
 }
 
 void DisplayConfigurator::NotifyObservers(
     bool success,
     MultipleDisplayState attempted_state) {
   if (success) {
     FOR_EACH_OBSERVER(
         Observer, observers_, OnDisplayModeChanged(cached_displays_));
   } else {
     FOR_EACH_OBSERVER(
         Observer, observers_, OnDisplayModeChangeFailed(attempted_state));
   }
 }
 
 bool DisplayConfigurator::EnterStateOrFallBackToSoftwareMirroring(
     MultipleDisplayState display_state,
     chromeos::DisplayPowerState power_state) {
   bool success = EnterState(display_state, power_state);
   if (mirroring_controller_) {
     bool enable_software_mirroring = false;
     if (!success && display_state == MULTIPLE_DISPLAY_STATE_DUAL_MIRROR) {
       if (display_state_ != MULTIPLE_DISPLAY_STATE_DUAL_EXTENDED ||
-          power_state_ != power_state)
+          current_power_state_ != power_state)
         EnterState(MULTIPLE_DISPLAY_STATE_DUAL_EXTENDED, power_state);
       enable_software_mirroring = success =
           display_state_ == MULTIPLE_DISPLAY_STATE_DUAL_EXTENDED;
     }
     mirroring_controller_->SetSoftwareMirroring(enable_software_mirroring);
   }
   return success;
 }
 
 bool DisplayConfigurator::EnterState(MultipleDisplayState display_state,
                                      chromeos::DisplayPowerState power_state) {
   std::vector<bool> display_power;
   int num_on_displays =
       GetDisplayPower(cached_displays_, power_state, &display_power);
   VLOG(1) << "EnterState: display=" << DisplayStateToString(display_state)
           << " power=" << DisplayPowerStateToString(power_state);
 
+  // Save the requested state so we'll try to use it next time even if we fail.
+  requested_power_state_ = power_state;
+
   // Framebuffer dimensions.
   gfx::Size size;
 
   std::vector<gfx::Point> new_origins(cached_displays_.size(), gfx::Point());
   std::vector<const DisplayMode*> new_mode;
   for (size_t i = 0; i < cached_displays_.size(); ++i)
     new_mode.push_back(cached_displays_[i].display->current_mode());
 
   switch (display_state) {
     case MULTIPLE_DISPLAY_STATE_INVALID:
@@ skipping 146 matching lines @@
       // false to let the observers be aware.
       if (display_state == MULTIPLE_DISPLAY_STATE_DUAL_MIRROR &&
           display_power[i] &&
           state.display->current_mode() != state.mirror_mode)
         all_succeeded = false;
     }
   }
 
   if (all_succeeded) {
     display_state_ = display_state;
-    power_state_ = power_state;
+    current_power_state_ = power_state;
     framebuffer_size_ = size;
   }
   return all_succeeded;
 }
 
 MultipleDisplayState DisplayConfigurator::ChooseDisplayState(
     chromeos::DisplayPowerState power_state) const {
   int num_on_displays = GetDisplayPower(cached_displays_, power_state, NULL);
   switch (cached_displays_.size()) {
     case 0:
@@ skipping 20 matching lines @@
         return state_controller_->GetStateForDisplayIds(display_ids);
       }
     }
     default:
       NOTREACHED();
   }
   return MULTIPLE_DISPLAY_STATE_INVALID;
 }
 
 }  // namespace ui