diff options
author | derat@chromium.org <derat@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2013-03-07 05:24:07 +0000 |
---|---|---|
committer | derat@chromium.org <derat@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2013-03-07 05:24:07 +0000 |
commit | 8405db533878bac39d622c421e8c1baa9d21c0fe (patch) | |
tree | 2c60a7f67b3a5d20c358e6f10c57a66e574e8bbc | |
parent | dce8642ab3068e41c3ce0a46571498dbd270921d (diff) | |
download | chromium_src-8405db533878bac39d622c421e8c1baa9d21c0fe.zip chromium_src-8405db533878bac39d622c421e8c1baa9d21c0fe.tar.gz chromium_src-8405db533878bac39d622c421e8c1baa9d21c0fe.tar.bz2 |
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
19 files changed, 316 insertions, 134 deletions
diff --git a/ash/system/chromeos/tray_display.cc b/ash/system/chromeos/tray_display.cc index 19ca0fc..a7f73c1 100644 --- a/ash/system/chromeos/tray_display.cc +++ b/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 diff --git a/ash/system/chromeos/tray_display.h b/ash/system/chromeos/tray_display.h index d471df8..524ffe8 100644 --- a/ash/system/chromeos/tray_display.h +++ b/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_; diff --git a/ash/wm/user_activity_detector.cc b/ash/wm/user_activity_detector.cc index fccc6d8..b6b3bde 100644 --- a/ash/wm/user_activity_detector.cc +++ b/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) { diff --git a/ash/wm/user_activity_detector.h b/ash/wm/user_activity_detector.h index 55a685c..18b3409 100644 --- a/ash/wm/user_activity_detector.h +++ b/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); }; diff --git a/ash/wm/user_activity_detector_unittest.cc b/ash/wm/user_activity_detector_unittest.cc index e0108d4..6c88660 100644 --- a/ash/wm/user_activity_detector_unittest.cc +++ b/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()); diff --git a/chrome/browser/chromeos/dbus/cros_dbus_service.cc b/chrome/browser/chromeos/dbus/cros_dbus_service.cc index a6f8b63..6ab8262 100644 --- a/chrome/browser/chromeos/dbus/cros_dbus_service.cc +++ b/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; diff --git a/chrome/browser/chromeos/dbus/display_power_service_provider.cc b/chrome/browser/chromeos/dbus/display_power_service_provider.cc new file mode 100644 index 0000000..6b92d44 --- /dev/null +++ b/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 diff --git a/chrome/browser/chromeos/dbus/display_power_service_provider.h b/chrome/browser/chromeos/dbus/display_power_service_provider.h new file mode 100644 index 0000000..196d870 --- /dev/null +++ b/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_ diff --git a/chrome/browser/chromeos/dbus/org.chromium.LibCrosService.conf b/chrome/browser/chromeos/dbus/org.chromium.LibCrosService.conf index 454a021..28be79c 100644 --- a/chrome/browser/chromeos/dbus/org.chromium.LibCrosService.conf +++ b/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> - <!-- update_engine uses this service to resolve the proxy config. --> - <policy user="root"> + <!-- powerd needs to change display power states. --> + <policy user="power"> <allow send_destination="org.chromium.LibCrosService"/> </policy> - <policy context="default"> - <deny send_type="method_call" - send_destination="org.chromium.LibCrosService"/> + <!-- update_engine uses this service to resolve the proxy config. --> + <policy user="root"> + <allow send_destination="org.chromium.LibCrosService"/> </policy> </busconfig> diff --git a/chrome/browser/chromeos/power/output_observer.cc b/chrome/browser/chromeos/power/output_observer.cc index 24e573e..0cbe05d 100644 --- a/chrome/browser/chromeos/power/output_observer.cc +++ b/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(); - } - - ash::Shell::GetInstance()->output_configurator()-> - ScreenPowerSet(power_on, all_displays); + // 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 = 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 diff --git a/chrome/browser/chromeos/power/output_observer.h b/chrome/browser/chromeos/power/output_observer.h index a9b80f2..da0b90e 100644 --- a/chrome/browser/chromeos/power/output_observer.h +++ b/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. diff --git a/chrome/browser/chromeos/power/resume_observer.cc b/chrome/browser/chromeos/power/resume_observer.cc index f28f14c..16d4a42 100644 --- a/chrome/browser/chromeos/power/resume_observer.cc +++ b/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 diff --git a/chrome/browser/chromeos/power/suspend_observer.cc b/chrome/browser/chromeos/power/suspend_observer.cc index d177ded..95cabe9 100644 --- a/chrome/browser/chromeos/power/suspend_observer.cc +++ b/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(); } diff --git a/chrome/browser/chromeos/power/suspend_observer.h b/chrome/browser/chromeos/power/suspend_observer.h index 47e7924..273b7dc 100644 --- a/chrome/browser/chromeos/power/suspend_observer.h +++ b/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" diff --git a/chrome/chrome_browser_chromeos.gypi b/chrome/chrome_browser_chromeos.gypi index b3c7c52..f2f7aeb 100644 --- a/chrome/chrome_browser_chromeos.gypi +++ b/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', diff --git a/chromeos/dbus/power_manager_client.cc b/chromeos/dbus/power_manager_client.cc index a00296a..26b881a 100644 --- a/chromeos/dbus/power_manager_client.cc +++ b/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, diff --git a/chromeos/dbus/power_manager_client.h b/chromeos/dbus/power_manager_client.h index 99ffd52..ffd3d12 100644 --- a/chromeos/dbus/power_manager_client.h +++ b/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 diff --git a/chromeos/display/output_configurator.cc b/chromeos/display/output_configurator.cc index c460114..cb8210f 100644 --- a/chromeos/display/output_configurator.cc +++ b/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; diff --git a/chromeos/display/output_configurator.h b/chromeos/display/output_configurator.h index 804f856..4f93598 100644 --- a/chromeos/display/output_configurator.h +++ b/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 |