chromeos: Add DisplayPowerServiceProvider.

This makes Chrome export a SetDisplayPower D-Bus method call
for the power manager.

It also reworks OutputConfigurator to cache the
most-recently-requested power state and use it for future
display mode requests.

Finally, it works around a related bug where multiple mouse
events may be generated when the displays are reconfigured,
which would result in a report of user activity that could
abort suspending.

BUG=chromium-os:39289,180348,chrome-os-partner:12662
TBR=sky@chromium.org

Review URL: https://chromiumcodereview.appspot.com/12391004

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@186625 0039d316-1c4b-4281-b951-d872f2087c98

ash/system/chromeos/tray_display.cc

@@ -183,10 +183,12 @@ void TrayDisplay::OnDisplayRemoved(const gfx::Display& old_display) {
     default_->Update();
 }
 
+#if defined(OS_CHROMEOS)
 void TrayDisplay::OnDisplayModeChanged() {
   if (default_)
     default_->Update();
 }
+#endif
 
 }  // namespace internal
 }  // namespace ash

ash/system/chromeos/tray_display.h

@@ -9,7 +9,9 @@
 #include "base/memory/scoped_ptr.h"
 #include "ui/gfx/display_observer.h"
 
+#if defined(OS_CHROMEOS)
 #include "chromeos/display/output_configurator.h"
+#endif
 
 namespace views {
 class View;
@@ -20,7 +22,9 @@ namespace internal {
 class DisplayView;
 
 class TrayDisplay : public SystemTrayItem,
+#if defined(OS_CHROMEOS)
                     public chromeos::OutputConfigurator::Observer,
+#endif
                     public gfx::DisplayObserver {
  public:
   explicit TrayDisplay(SystemTray* system_tray);
@@ -36,8 +40,10 @@ class TrayDisplay : public SystemTrayItem,
   virtual void OnDisplayAdded(const gfx::Display& new_display) OVERRIDE;
   virtual void OnDisplayRemoved(const gfx::Display& old_display) OVERRIDE;
 
+#if defined(OS_CHROMEOS)
   // Overridden from chromeos::OutputConfigurator::Observer
   virtual void OnDisplayModeChanged() OVERRIDE;
+#endif
 
   DisplayView* default_;
 

ash/wm/user_activity_detector.cc

@@ -10,9 +10,14 @@
 
 namespace ash {
 
-const double UserActivityDetector::kNotifyIntervalMs = 200.0;
+const int UserActivityDetector::kNotifyIntervalMs = 200;
 
-UserActivityDetector::UserActivityDetector() : ignore_next_mouse_event_(false) {
+// Too low and mouse events generated at the tail end of reconfiguration
+// will be reported as user activity and turn the screen back on; too high
+// and we'll ignore legitimate activity.
+const int UserActivityDetector::kDisplayPowerChangeIgnoreMouseMs = 1000;
+
+UserActivityDetector::UserActivityDetector() {
 }
 
 UserActivityDetector::~UserActivityDetector() {
@@ -30,8 +35,9 @@ void UserActivityDetector::RemoveObserver(UserActivityObserver* observer) {
   observers_.RemoveObserver(observer);
 }
 
-void UserActivityDetector::OnAllOutputsTurnedOff() {
-  ignore_next_mouse_event_ = true;
+void UserActivityDetector::OnDisplayPowerChanging() {
+  honor_mouse_events_time_ = GetCurrentTime() +
+      base::TimeDelta::FromMilliseconds(kDisplayPowerChangeIgnoreMouseMs);
 }
 
 void UserActivityDetector::OnKeyEvent(ui::KeyEvent* event) {
@@ -39,11 +45,13 @@ void UserActivityDetector::OnKeyEvent(ui::KeyEvent* event) {
 }
 
 void UserActivityDetector::OnMouseEvent(ui::MouseEvent* event) {
-  VLOG_IF(1, ignore_next_mouse_event_) << "ignoring mouse event";
-  if (!(event->flags() & ui::EF_IS_SYNTHESIZED) &&
-      !ignore_next_mouse_event_)
-    MaybeNotify();
-  ignore_next_mouse_event_ = false;
+  if (event->flags() & ui::EF_IS_SYNTHESIZED)
+    return;
+  if (!honor_mouse_events_time_.is_null() &&
+      GetCurrentTime() < honor_mouse_events_time_)
+    return;
+
+  MaybeNotify();
 }
 
 void UserActivityDetector::OnScrollEvent(ui::ScrollEvent* event) {
@@ -58,9 +66,12 @@ void UserActivityDetector::OnGestureEvent(ui::GestureEvent* event) {
   MaybeNotify();
 }
 
+base::TimeTicks UserActivityDetector::GetCurrentTime() const {
+  return !now_for_test_.is_null() ? now_for_test_ : base::TimeTicks::Now();
+}
+
 void UserActivityDetector::MaybeNotify() {
-  base::TimeTicks now =
-      !now_for_test_.is_null() ? now_for_test_ : base::TimeTicks::Now();
+  base::TimeTicks now = GetCurrentTime();
   if (last_observer_notification_time_.is_null() ||
       (now - last_observer_notification_time_).InMillisecondsF() >=
       kNotifyIntervalMs) {

ash/wm/user_activity_detector.h

@@ -20,7 +20,11 @@ class UserActivityObserver;
 class ASH_EXPORT UserActivityDetector : public ui::EventHandler {
  public:
   // Minimum amount of time between notifications to observers.
-  static const double kNotifyIntervalMs;
+  static const int kNotifyIntervalMs;
+
+  // Amount of time that mouse events should be ignored after notification
+  // is received that displays' power states are being changed.
+  static const int kDisplayPowerChangeIgnoreMouseMs;
 
   UserActivityDetector();
   virtual ~UserActivityDetector();
@@ -31,8 +35,8 @@ class ASH_EXPORT UserActivityDetector : public ui::EventHandler {
   void AddObserver(UserActivityObserver* observer);
   void RemoveObserver(UserActivityObserver* observer);
 
-  // Called when chrome has received a request to turn of all displays.
-  void OnAllOutputsTurnedOff();
+  // Called when displays are about to be turned on or off.
+  void OnDisplayPowerChanging();
 
   // ui::EventHandler implementation.
   virtual void OnKeyEvent(ui::KeyEvent* event) OVERRIDE;
@@ -42,6 +46,9 @@ class ASH_EXPORT UserActivityDetector : public ui::EventHandler {
   virtual void OnGestureEvent(ui::GestureEvent* event) OVERRIDE;
 
  private:
+  // Returns |now_for_test_| if set or base::TimeTicks::Now() otherwise.
+  base::TimeTicks GetCurrentTime() const;
+
   // Notifies observers if enough time has passed since the last notification.
   void MaybeNotify();
 
@@ -54,10 +61,10 @@ class ASH_EXPORT UserActivityDetector : public ui::EventHandler {
   // simulate the passage of time.
   base::TimeTicks now_for_test_;
 
-  // When this is true, the next mouse event is ignored. This is to
-  // avoid mis-detecting a mouse enter event that occurs when turning
-  // off display as a user activity.
-  bool ignore_next_mouse_event_;
+  // If set, mouse events will be ignored until this time is reached. This
+  // is to avoid reporting mouse events that occur when displays are turned
+  // on or off as user activity.
+  base::TimeTicks honor_mouse_events_time_;
 
   DISALLOW_COPY_AND_ASSIGN(UserActivityDetector);
 };

ash/wm/user_activity_detector_unittest.cc

@@ -97,8 +97,8 @@ TEST_F(UserActivityDetectorTest, Basic) {
   EXPECT_EQ(1, observer_->num_invocations());
   observer_->reset_stats();
 
-  base::TimeDelta advance_delta =
-      base::TimeDelta::FromSeconds(UserActivityDetector::kNotifyIntervalMs);
+  base::TimeDelta advance_delta = base::TimeDelta::FromMilliseconds(
+      UserActivityDetector::kNotifyIntervalMs);
   AdvanceTime(advance_delta);
   ui::MouseEvent mouse_event(
       ui::ET_MOUSE_MOVED, gfx::Point(), gfx::Point(), ui::EF_NONE);
@@ -108,14 +108,29 @@ TEST_F(UserActivityDetectorTest, Basic) {
   EXPECT_EQ(1, observer_->num_invocations());
   observer_->reset_stats();
 
-  // Ignore one mouse event when all displays are turned off.
-  detector_->OnAllOutputsTurnedOff();
-  AdvanceTime(advance_delta);
+  // Temporarily ignore mouse events when displays are turned on or off.
+  detector_->OnDisplayPowerChanging();
   detector_->OnMouseEvent(&mouse_event);
   EXPECT_FALSE(mouse_event.handled());
   EXPECT_EQ(0, observer_->num_invocations());
   observer_->reset_stats();
 
+  const base::TimeDelta kIgnoreMouseTime =
+      base::TimeDelta::FromMilliseconds(
+          UserActivityDetector::kDisplayPowerChangeIgnoreMouseMs);
+  AdvanceTime(kIgnoreMouseTime / 2);
+  detector_->OnMouseEvent(&mouse_event);
+  EXPECT_FALSE(mouse_event.handled());
+  EXPECT_EQ(0, observer_->num_invocations());
+  observer_->reset_stats();
+
+  // After enough time has passed, mouse events should be reported again.
+  AdvanceTime(std::max(kIgnoreMouseTime, advance_delta));
+  detector_->OnMouseEvent(&mouse_event);
+  EXPECT_FALSE(mouse_event.handled());
+  EXPECT_EQ(1, observer_->num_invocations());
+  observer_->reset_stats();
+
   AdvanceTime(advance_delta);
   ui::TouchEvent touch_event(
       ui::ET_TOUCH_PRESSED, gfx::Point(), 0, base::TimeDelta());

chrome/browser/chromeos/dbus/cros_dbus_service.cc

@@ -8,6 +8,7 @@
 #include "base/chromeos/chromeos_version.h"
 #include "base/stl_util.h"
 #include "base/threading/platform_thread.h"
+#include "chrome/browser/chromeos/dbus/display_power_service_provider.h"
 #include "chrome/browser/chromeos/dbus/liveness_service_provider.h"
 #include "chrome/browser/chromeos/dbus/printer_service_provider.h"
 #include "chrome/browser/chromeos/dbus/proxy_resolution_service_provider.h"
@@ -111,6 +112,7 @@ void CrosDBusService::Initialize() {
   if (base::chromeos::IsRunningOnChromeOS() && bus) {
     CrosDBusServiceImpl* service = new CrosDBusServiceImpl(bus);
     service->RegisterServiceProvider(ProxyResolutionServiceProvider::Create());
+    service->RegisterServiceProvider(new DisplayPowerServiceProvider);
     service->RegisterServiceProvider(new LivenessServiceProvider);
     service->RegisterServiceProvider(new PrinterServiceProvider);
     g_cros_dbus_service = service;

chrome/browser/chromeos/dbus/display_power_service_provider.cc

@@ -0,0 +1,66 @@
+// Copyright (c) 2013 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 "chrome/browser/chromeos/dbus/display_power_service_provider.h"
+
+#include "ash/shell.h"
+#include "ash/wm/user_activity_detector.h"
+#include "base/bind.h"
+#include "chromeos/display/output_configurator.h"
+#include "dbus/bus.h"
+#include "dbus/message.h"
+#include "third_party/cros_system_api/dbus/service_constants.h"
+
+namespace chromeos {
+
+DisplayPowerServiceProvider::DisplayPowerServiceProvider()
+    : weak_ptr_factory_(this) {
+}
+
+DisplayPowerServiceProvider::~DisplayPowerServiceProvider() {}
+
+void DisplayPowerServiceProvider::Start(
+    scoped_refptr<dbus::ExportedObject> exported_object) {
+  exported_object->ExportMethod(
+      kLibCrosServiceInterface,
+      kSetDisplayPower,
+      base::Bind(&DisplayPowerServiceProvider::SetDisplayPower,
+                 weak_ptr_factory_.GetWeakPtr()),
+      base::Bind(&DisplayPowerServiceProvider::OnExported,
+                 weak_ptr_factory_.GetWeakPtr()));
+}
+
+void DisplayPowerServiceProvider::OnExported(const std::string& interface_name,
+                                             const std::string& method_name,
+                                             bool success) {
+  if (!success) {
+    LOG(ERROR) << "Failed to export " << interface_name << "."
+               << method_name;
+  }
+}
+
+void DisplayPowerServiceProvider::SetDisplayPower(
+    dbus::MethodCall* method_call,
+    dbus::ExportedObject::ResponseSender response_sender) {
+  dbus::MessageReader reader(method_call);
+  int int_state = 0;
+  if (reader.PopInt32(&int_state)) {
+    // Turning displays off when the device becomes idle or on just before
+    // we suspend may trigger a mouse move, which would then be incorrectly
+    // reported as user activity.  Let the UserActivityDetector
+    // know so that it can ignore such events.
+    ash::Shell::GetInstance()->user_activity_detector()->
+        OnDisplayPowerChanging();
+
+    DisplayPowerState state = static_cast<DisplayPowerState>(int_state);
+    ash::Shell::GetInstance()->output_configurator()->SetDisplayPower(
+        state, false);
+  } else {
+    LOG(ERROR) << "Unable to parse " << kSetDisplayPower << " request";
+  }
+
+  response_sender.Run(dbus::Response::FromMethodCall(method_call));
+}
+
+}  // namespace chromeos

chrome/browser/chromeos/dbus/display_power_service_provider.h

@@ -0,0 +1,56 @@
+// Copyright (c) 2013 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.
+
+#ifndef CHROME_BROWSER_CHROMEOS_DBUS_DISPLAY_POWER_SERVICE_PROVIDER_H_
+#define CHROME_BROWSER_CHROMEOS_DBUS_DISPLAY_POWER_SERVICE_PROVIDER_H_
+
+#include <string>
+
+#include "base/basictypes.h"
+#include "base/compiler_specific.h"
+#include "base/memory/ref_counted.h"
+#include "base/memory/weak_ptr.h"
+#include "chrome/browser/chromeos/dbus/cros_dbus_service.h"
+#include "dbus/exported_object.h"
+
+namespace dbus {
+class MethodCall;
+class Response;
+}
+
+namespace chromeos {
+
+// This class exports a "SetDisplayPower" D-Bus method that the power
+// manager calls to instruct Chrome to turn various displays on or off.
+class DisplayPowerServiceProvider
+    : public CrosDBusService::ServiceProviderInterface {
+ public:
+  DisplayPowerServiceProvider();
+  virtual ~DisplayPowerServiceProvider();
+
+  // CrosDBusService::ServiceProviderInterface overrides:
+  virtual void Start(
+      scoped_refptr<dbus::ExportedObject> exported_object) OVERRIDE;
+
+ private:
+  // Called from ExportedObject when SetDisplayPower() is exported as a D-Bus
+  // method or failed to be exported.
+  void OnExported(const std::string& interface_name,
+                  const std::string& method_name,
+                  bool success);
+
+  // Called on UI thread in response to a D-Bus request.
+  void SetDisplayPower(dbus::MethodCall* method_call,
+                       dbus::ExportedObject::ResponseSender response_sender);
+
+  // Keep this last so that all weak pointers will be invalidated at the
+  // beginning of destruction.
+  base::WeakPtrFactory<DisplayPowerServiceProvider> weak_ptr_factory_;
+
+  DISALLOW_COPY_AND_ASSIGN(DisplayPowerServiceProvider);
+};
+
+}  // namespace chromeos
+
+#endif  // CHROME_BROWSER_CHROMEOS_DBUS_DISPLAY_POWER_SERVICE_PROVIDER_H_

chrome/browser/chromeos/dbus/org.chromium.LibCrosService.conf

@@ -22,15 +22,15 @@
   <!-- tlsdate needs to query proxy config. -->
   <policy user="proxystate">
     <allow send_destination="org.chromium.LibCrosService"/>
- </policy>
+  </policy>
+
+  <!-- powerd needs to change display power states. -->
+  <policy user="power">
+    <allow send_destination="org.chromium.LibCrosService"/>
+  </policy>
 
   <!-- update_engine uses this service to resolve the proxy config. -->
   <policy user="root">
     <allow send_destination="org.chromium.LibCrosService"/>
   </policy>
-
-  <policy context="default">
-    <deny send_type="method_call"
-          send_destination="org.chromium.LibCrosService"/>
-  </policy>
 </busconfig>

chrome/browser/chromeos/power/output_observer.cc

@@ -20,16 +20,24 @@ OutputObserver::~OutputObserver() {
 }
 
 void OutputObserver::ScreenPowerSet(bool power_on, bool all_displays) {
-  if (!power_on && all_displays) {
-    // All displays are turned off when the device becomes idle, which
-    // may trigger a mouse move. Let the UserActivityDetector know so
-    // that it can ignore such events.
-    ash::Shell::GetInstance()->user_activity_detector()->
-        OnAllOutputsTurnedOff();
-  }
+  // Turning displays off when the device becomes idle or on just before we
+  // suspend may trigger a mouse move, which would then be incorrectly
+  // reported as user activity.  Let the UserActivityDetector know so that
+  // it can ignore such events.
+  ash::Shell::GetInstance()->user_activity_detector()->OnDisplayPowerChanging();
 
-  ash::Shell::GetInstance()->output_configurator()->
-      ScreenPowerSet(power_on, all_displays);
+  DisplayPowerState state = DISPLAY_POWER_ALL_ON;
+  if (power_on && all_displays)
+    state = DISPLAY_POWER_ALL_ON;
+  else if (power_on && !all_displays)
+    state = DISPLAY_POWER_INTERNAL_ON_EXTERNAL_OFF;
+  else if (!power_on && all_displays)
+    state = DISPLAY_POWER_ALL_OFF;
+  else if (!power_on && !all_displays)
+    state = DISPLAY_POWER_INTERNAL_OFF_EXTERNAL_ON;
+
+  ash::Shell::GetInstance()->output_configurator()->SetDisplayPower(
+      state, false);
 }
 
 }  // namespace chromeos

chrome/browser/chromeos/power/output_observer.h

@@ -13,6 +13,8 @@ namespace chromeos {
 
 // This observer listens for when video outputs have been turned off so that the
 // corresponding CRTCs can be disabled to force the connected output off.
+// TODO(derat): Remove this class after powerd is calling the method
+// exported by DisplayPowerServiceProvider instead.
 class OutputObserver : public PowerManagerClient::Observer {
  public:
   // This class registers/unregisters itself as an observer in ctor/dtor.

chrome/browser/chromeos/power/resume_observer.cc

@@ -4,8 +4,10 @@
 
 #include "chrome/browser/chromeos/power/resume_observer.h"
 
+#include "ash/shell.h"
 #include "chrome/browser/extensions/api/system_private/system_private_api.h"
 #include "chromeos/dbus/dbus_thread_manager.h"
+#include "chromeos/display/output_configurator.h"
 
 namespace chromeos {
 
@@ -19,6 +21,7 @@ ResumeObserver::~ResumeObserver() {
 
 void ResumeObserver::SystemResumed(const base::TimeDelta& sleep_duration) {
   extensions::DispatchWokeUpEvent();
+  ash::Shell::GetInstance()->output_configurator()->ResumeDisplays();
 }
 
 }  // namespace chromeos

chrome/browser/chromeos/power/suspend_observer.cc

@@ -4,6 +4,8 @@
 
 #include "chrome/browser/chromeos/power/suspend_observer.h"
 
+#include "ash/shell.h"
+#include "ash/wm/user_activity_detector.h"
 #include "base/prefs/pref_service.h"
 #include "chrome/browser/chromeos/login/user_manager.h"
 #include "chrome/browser/extensions/api/system_private/system_private_api.h"
@@ -42,6 +44,7 @@ void SuspendObserver::SuspendImminent() {
     session_client_->RequestLockScreen();
   }
 
+  ash::Shell::GetInstance()->user_activity_detector()->OnDisplayPowerChanging();
   ash::Shell::GetInstance()->output_configurator()->SuspendDisplays();
 }
 

chrome/browser/chromeos/power/suspend_observer.h

@@ -5,7 +5,6 @@
 #ifndef CHROME_BROWSER_CHROMEOS_POWER_SUSPEND_OBSERVER_H_
 #define CHROME_BROWSER_CHROMEOS_POWER_SUSPEND_OBSERVER_H_
 
-#include "ash/shell.h"
 #include "base/basictypes.h"
 #include "base/callback.h"
 #include "base/compiler_specific.h"

chrome/chrome_browser_chromeos.gypi

@@ -193,6 +193,8 @@
         'browser/chromeos/display/primary_display_switch_observer.h',
         'browser/chromeos/dbus/cros_dbus_service.cc',
         'browser/chromeos/dbus/cros_dbus_service.h',
+        'browser/chromeos/dbus/display_power_service_provider.cc',
+        'browser/chromeos/dbus/display_power_service_provider.h',
         'browser/chromeos/dbus/liveness_service_provider.cc',
         'browser/chromeos/dbus/liveness_service_provider.h',
         'browser/chromeos/dbus/printer_service_provider.cc',

chromeos/dbus/power_manager_client.cc

@@ -69,6 +69,7 @@ class PowerManagerClientImpl : public PowerManagerClient {
         base::Bind(&PowerManagerClientImpl::SignalConnected,
                    weak_ptr_factory_.GetWeakPtr()));
 
+    // TODO(derat): Stop listening for this.
     power_manager_proxy_->ConnectToSignal(
         power_manager::kPowerManagerInterface,
         power_manager::kSetScreenPowerSignal,

chromeos/dbus/power_manager_client.h

@@ -61,6 +61,7 @@ class CHROMEOS_EXPORT PowerManagerClient {
     // |power_on| The new state of the power setting.
     // |all_displays| True if this applies to all displays or false if it is
     // the internal display only.
+    // TODO(derat): Remove this.
     virtual void ScreenPowerSet(bool power_on, bool all_displays) {}
 
     // Called when power supply polling takes place.  |status| is a data

chromeos/display/output_configurator.cc

@@ -538,6 +538,7 @@ OutputConfigurator::OutputConfigurator()
       connected_output_count_(0),
       xrandr_event_base_(0),
       output_state_(STATE_INVALID),
+      power_state_(DISPLAY_POWER_ALL_ON),
       mirror_mode_will_preserve_aspect_(false),
       mirror_mode_preserved_aspect_(false),
       last_enter_state_time_() {
@@ -594,10 +595,7 @@ void OutputConfigurator::Init(bool is_panel_fitting_enabled,
                                             STATE_INVALID,
                                             outputs);
   if (output_state_ != starting_state &&
-      EnterState(display,
-                 screen,
-                 window,
-                 starting_state,
+      EnterState(display, screen, window, starting_state, power_state_,
                  outputs)) {
     output_state_ = starting_state;
   }
@@ -643,7 +641,7 @@ bool OutputConfigurator::CycleDisplayMode() {
   OutputState original = InferCurrentState(display, screen, outputs);
   OutputState next_state = GetNextState(display, screen, original, outputs);
   if (original != next_state &&
-      EnterState(display, screen, window, next_state, outputs)) {
+      EnterState(display, screen, window, next_state, power_state_, outputs)) {
     did_change = true;
   }
   // We have seen cases where the XRandR data can get out of sync with our own
@@ -663,77 +661,41 @@ bool OutputConfigurator::CycleDisplayMode() {
   return did_change;
 }
 
-bool OutputConfigurator::ScreenPowerSet(bool power_on, bool all_displays) {
-  TRACE_EVENT0("chromeos", "OutputConfigurator::ScreenPowerSet");
-  VLOG(1) << "OutputConfigurator::SetScreensOn " << power_on
-          << " all displays " << all_displays;
+bool OutputConfigurator::SetDisplayPower(DisplayPowerState power_state,
+                                         bool force_probe) {
+  TRACE_EVENT0("chromeos", "OutputConfigurator::SetDisplayPower");
+  VLOG(1) << "OutputConfigurator::SetDisplayPower: power_state=" << power_state
+          << " force_probe=" << force_probe;
+
   if (!configure_display_)
     return false;
+  if (power_state == power_state_ && !force_probe)
+    return true;
 
-  bool success = false;
   Display* display = base::MessagePumpAuraX11::GetDefaultXDisplay();
-  CHECK(display != NULL);
+  CHECK(display);
   XGrabServer(display);
   Window window = DefaultRootWindow(display);
   XRRScreenResources* screen = GetScreenResourcesAndRecordUMA(display, window);
-  CHECK(screen != NULL);
-
+  CHECK(screen);
   std::vector<OutputSnapshot> outputs = GetDualOutputs(display, screen);
   connected_output_count_ = outputs.size();
 
-  if (all_displays && power_on) {
-    // Resume all displays using the current state.
-    if (EnterState(display, screen, window, output_state_, outputs)) {
-      // 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 (EnterState(display, screen, window, output_state_, power_state,
+                 outputs)) {
+    power_state_ = power_state;
+    if (power_state != DISPLAY_POWER_ALL_OFF)  {
+      // 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.
       CHECK(DPMSEnable(display));
       CHECK(DPMSForceLevel(display, DPMSModeOn));
-
-      XRRFreeScreenResources(screen);
-      XUngrabServer(display);
-      return true;
     }
   }
 
-  CrtcConfig config;
-  config.crtc = None;
-  // Set the CRTCs based on whether we want to turn the power on or off and
-  // select the outputs to operate on by name or all_displays.
-  for (int i = 0; i < connected_output_count_; ++i) {
-    if (all_displays || outputs[i].is_internal || power_on) {
-      config.x = 0;
-      config.y = outputs[i].y;
-      config.output = outputs[i].output;
-      config.mode = None;
-      if (power_on) {
-        config.mode = (output_state_ == STATE_DUAL_MIRROR) ?
-            outputs[i].mirror_mode : outputs[i].native_mode;
-      } else if (connected_output_count_ > 1 && !all_displays &&
-                 outputs[i].is_internal) {
-        // Workaround for crbug.com/148365: leave internal display in native
-        // mode so user can move cursor (and hence windows) onto internal
-        // display even when dimmed
-        config.mode = outputs[i].native_mode;
-      }
-      config.crtc = GetNextCrtcAfter(display, screen, config.output,
-                                     config.crtc);
-
-      ConfigureCrtc(display, screen, &config);
-      success = 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 (power_on) {
-    CHECK(DPMSEnable(display));
-    CHECK(DPMSForceLevel(display, DPMSModeOn));
-  }
-
   XRRFreeScreenResources(screen);
   XUngrabServer(display);
 
-  return success;
+  return true;
 }
 
 bool OutputConfigurator::SetDisplayMode(OutputState new_state) {
@@ -755,7 +717,7 @@ bool OutputConfigurator::SetDisplayMode(OutputState new_state) {
 
   std::vector<OutputSnapshot> outputs = GetDualOutputs(display, screen);
   connected_output_count_ = outputs.size();
-  if (EnterState(display, screen, window, new_state, outputs))
+  if (EnterState(display, screen, window, new_state, power_state_, outputs))
     output_state_ = new_state;
 
   XRRFreeScreenResources(screen);
@@ -829,7 +791,8 @@ void OutputConfigurator::ConfigureOutputs() {
   // When a display was swapped, the state moves from
   // STATE_DUAL_EXTENDED to STATE_DUAL_EXTENDED, so don't rely on
   // the state chagne to tell if it was successful.
-  bool success = EnterState(display, screen, window, new_state, outputs);
+  bool success =
+      EnterState(display, screen, window, new_state, power_state_, outputs);
   bool is_projecting = IsProjecting(outputs);
   XRRFreeScreenResources(screen);
   XUngrabServer(display);
@@ -859,16 +822,23 @@ bool OutputConfigurator::IsInternalOutputName(const std::string& name) {
 }
 
 void OutputConfigurator::SuspendDisplays() {
-  // Turn displays on before suspend. At this point, the backlight is off,
-  // so we turn on the internal display so that we can resume directly into
-  // "on" state. This greatly reduces resume times.
-  ScreenPowerSet(true, true);
+  // Turn internal displays on before suspend. At this point, the backlight
+  // is off, so we turn on the internal display so that we can resume
+  // directly into "on" state. This greatly reduces resume times.
+  SetDisplayPower(DISPLAY_POWER_INTERNAL_ON_EXTERNAL_OFF, false);
+
   // 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.
   XSync(base::MessagePumpAuraX11::GetDefaultXDisplay(), 0);
 }
 
+void OutputConfigurator::ResumeDisplays() {
+  // Force probing to ensure that we pick up any changes that were made
+  // while the system was suspended.
+  SetDisplayPower(DISPLAY_POWER_ALL_ON, true);
+}
+
 void OutputConfigurator::NotifyOnDisplayChanged() {
   TRACE_EVENT0("chromeos", "OutputConfigurator::NotifyOnDisplayChanged");
   FOR_EACH_OBSERVER(Observer, observers_, OnDisplayModeChanged());
@@ -1192,7 +1162,8 @@ bool OutputConfigurator::EnterState(
     Display* display,
     XRRScreenResources* screen,
     Window window,
-    OutputState new_state,
+    OutputState output_state,
+    DisplayPowerState power_state,
     const std::vector<OutputSnapshot>& outputs) {
   TRACE_EVENT0("chromeos", "OutputConfigurator::EnterState");
   switch (outputs.size()) {
@@ -1207,16 +1178,20 @@ bool OutputConfigurator::EnterState(
         return false;
       }
 
+      bool power_on = power_state == DISPLAY_POWER_ALL_ON ||
+          (power_state == DISPLAY_POWER_INTERNAL_OFF_EXTERNAL_ON &&
+           !outputs[0].is_internal) ||
+          (power_state == DISPLAY_POWER_INTERNAL_ON_EXTERNAL_OFF &&
+           outputs[0].is_internal);
       CrtcConfig config(
           GetNextCrtcAfter(display, screen, outputs[0].output, None),
-          0, 0, outputs[0].native_mode, outputs[0].output);
+          0, 0, power_on ? outputs[0].native_mode : None, outputs[0].output);
 
-      int width = mode_info->width;
-      int height = mode_info->height;
-      CreateFrameBuffer(display, screen, window, width, height, &config, NULL);
+      CreateFrameBuffer(display, screen, window, mode_info->width,
+                        mode_info->height, &config, NULL);
 
-      // Re-attach native mode for the CRTC.
       ConfigureCrtc(display, screen, &config);
+
       // Restore identity transformation for single monitor in native mode.
       if (outputs[0].touch_device_id != None) {
         CoordinateTransformation ctm;  // Defaults to identity
@@ -1230,25 +1205,36 @@ bool OutputConfigurator::EnterState(
       RRCrtc secondary_crtc =
           GetNextCrtcAfter(display, screen, outputs[1].output, primary_crtc);
 
-      if (new_state == STATE_DUAL_MIRROR) {
+      // Workaround for crbug.com/148365: leave internal display on for
+      // internal-off, external-on so user can move cursor (and hence
+      // windows) onto internal display even when it's off.
+      bool primary_power_on = power_state == DISPLAY_POWER_ALL_ON ||
+          (power_state == DISPLAY_POWER_INTERNAL_ON_EXTERNAL_OFF &&
+           outputs[0].is_internal);
+      bool secondary_power_on = power_state == DISPLAY_POWER_ALL_ON ||
+          (power_state == DISPLAY_POWER_INTERNAL_ON_EXTERNAL_OFF &&
+           outputs[1].is_internal);
+
+      if (output_state == STATE_DUAL_MIRROR) {
         XRRModeInfo* mode_info = ModeInfoForID(screen, outputs[0].mirror_mode);
         if (mode_info == NULL) {
           UMA_HISTOGRAM_COUNTS("Display.EnterState.mirror_failures", 1);
           return false;
         }
 
-        CrtcConfig config1(primary_crtc, 0, 0, outputs[0].mirror_mode,
+        CrtcConfig config1(primary_crtc, 0, 0,
+                           primary_power_on ? outputs[0].mirror_mode : None,
                            outputs[0].output);
-        CrtcConfig config2(secondary_crtc, 0, 0, outputs[1].mirror_mode,
+        CrtcConfig config2(secondary_crtc, 0, 0,
+                           secondary_power_on ? outputs[1].mirror_mode : None,
                            outputs[1].output);
 
-        int width = mode_info->width;
-        int height = mode_info->height;
-        CreateFrameBuffer(display, screen, window, width, height,
-                          &config1, &config2);
+        CreateFrameBuffer(display, screen, window, mode_info->width,
+                          mode_info->height, &config1, &config2);
 
         ConfigureCrtc(display, screen, &config1);
         ConfigureCrtc(display, screen, &config2);
+
         for (size_t i = 0; i < outputs.size(); i++) {
           if (outputs[i].touch_device_id == None)
             continue;
@@ -1274,19 +1260,20 @@ bool OutputConfigurator::EnterState(
 
         int primary_height = primary_mode_info->height;
         int secondary_height = secondary_mode_info->height;
-        CrtcConfig config1(primary_crtc, 0, 0, outputs[0].native_mode,
+        CrtcConfig config1(primary_crtc, 0, 0,
+                           primary_power_on ? outputs[0].native_mode : None,
                            outputs[0].output);
-        CrtcConfig config2(secondary_crtc, 0, 0, outputs[1].native_mode,
+        CrtcConfig config2(secondary_crtc, 0, 0,
+                           secondary_power_on ? outputs[1].native_mode : None,
                            outputs[1].output);
 
-        if (new_state == STATE_DUAL_EXTENDED)
+        if (output_state == STATE_DUAL_EXTENDED)
           config2.y = primary_height + kVerticalGap;
         else
           config1.y = secondary_height + kVerticalGap;
 
-
-        int width =
-            std::max<int>(primary_mode_info->width, secondary_mode_info->width);
+        int width = std::max<int>(
+            primary_mode_info->width, secondary_mode_info->width);
         int height = primary_height + secondary_height + kVerticalGap;
 
         CreateFrameBuffer(display, screen, window, width, height, &config1,
@@ -1305,7 +1292,8 @@ bool OutputConfigurator::EnterState(
         }
         if (outputs[1].touch_device_id != None) {
           CoordinateTransformation ctm;
-          ctm.x_scale = static_cast<float>(secondary_mode_info->width) / width;
+          ctm.x_scale = static_cast<float>(secondary_mode_info->width) /
+              width;
           ctm.x_offset = static_cast<float>(config2.x) / width;
           ctm.y_scale = static_cast<float>(secondary_height) / height;
           ctm.y_offset = static_cast<float>(config2.y) / height;

chromeos/display/output_configurator.h

@@ -14,6 +14,7 @@
 #include "base/message_loop.h"
 #include "base/timer.h"
 #include "chromeos/chromeos_export.h"
+#include "third_party/cros_system_api/dbus/service_constants.h"
 
 // Forward declarations for Xlib and Xrandr.
 // This is so unused X definitions don't pollute the namespace.
@@ -86,7 +87,9 @@ class CHROMEOS_EXPORT OutputConfigurator : public MessageLoop::Dispatcher {
   // Called when powerd notifies us that some set of displays should be turned
   // on or off.  This requires enabling or disabling the CRTC associated with
   // the display(s) in question so that the low power state is engaged.
-  bool ScreenPowerSet(bool power_on, bool all_displays);
+  // If |force_probe| is true, the displays will be configured even if
+  // |power_state| matches |power_state_|.
+  bool SetDisplayPower(DisplayPowerState power_state, bool force_probe);
 
   // Force switching the display mode to |new_state|.  This method is used when
   // the user explicitly changes the display mode in the options UI.  Returns
@@ -106,11 +109,15 @@ class CHROMEOS_EXPORT OutputConfigurator : public MessageLoop::Dispatcher {
   // Tells if the output specified by |name| is for internal display.
   static bool IsInternalOutputName(const std::string& name);
 
-  // Set all the displays into pre-suspend mode; usually this means configure
-  // them for their resume state. This allows faster resume on machines where
-  // display configuration is slow.
+  // Sets all the displays into pre-suspend mode; usually this means
+  // configure them for their resume state. This allows faster resume on
+  // machines where display configuration is slow.
   void SuspendDisplays();
 
+  // Reprobes displays to handle changes made while the system was
+  // suspended.
+  void ResumeDisplays();
+
  private:
   // Configure outputs.
   void ConfigureOutputs();
@@ -156,15 +163,15 @@ class CHROMEOS_EXPORT OutputConfigurator : public MessageLoop::Dispatcher {
                        XRRScreenResources* screen,
                        std::vector<OutputSnapshot>& outputs);
 
-  // Configures X to the state specified in |new_state|.
-  // |display|, |screen| and |window| are used to change X configuration.
-  // |new_state| is the state to enter.
-  // |outputs| contains information on the currently configured state,
-  // as well as how to apply the new state.
+  // Configures X to the state specified in |output_state| and
+  // |power_state|.  |display|, |screen| and |window| are used to change X
+  // configuration.  |outputs| contains information on the currently
+  // configured state, as well as how to apply the new state.
   bool EnterState(Display* display,
                   XRRScreenResources* screen,
                   Window window,
-                  OutputState new_state,
+                  OutputState output_state,
+                  DisplayPowerState power_state,
                   const std::vector<OutputSnapshot>& outputs);
 
   // Outputs UMA metrics of previous state (the state that is being left).
@@ -199,6 +206,9 @@ class CHROMEOS_EXPORT OutputConfigurator : public MessageLoop::Dispatcher {
   // This is used for rotating display modes.
   OutputState output_state_;
 
+  // The current power state as set via SetDisplayPower().
+  DisplayPowerState power_state_;
+
   ObserverList<Observer> observers_;
 
   // The timer to delay configuring outputs. See also the comments in