diff options
author | dnicoara@chromium.org <dnicoara@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-02-26 23:39:18 +0000 |
---|---|---|
committer | dnicoara@chromium.org <dnicoara@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-02-26 23:39:18 +0000 |
commit | 3d1cd60ccb3f7176fc206268300cca16115bf0c8 (patch) | |
tree | cf020f2374da11c83796683637558fa66804813c | |
parent | 1763a9bf43129c9f103145350d1c0abf04921182 (diff) | |
download | chromium_src-3d1cd60ccb3f7176fc206268300cca16115bf0c8.zip chromium_src-3d1cd60ccb3f7176fc206268300cca16115bf0c8.tar.gz chromium_src-3d1cd60ccb3f7176fc206268300cca16115bf0c8.tar.bz2 |
Extract NativeDisplayDelegate and move display event handling to NativeDisplayDelegate
Display configuration events are platform specific, thus move display event processing to NativeDisplayDelegate. OutputConfigurator will then register itself as an observer of NativeDisplayDelegate which will notify observers when display configuration changes occur.
The NativeDisplayDelegate interface has also been modified to take in OutputSnapshot objects as a first step to abstracting display state. In a future CL NativeDisplayDelegate and OutputSnapshot will be updated such that X11 specific state will not be present in the interface.
BUG=333413
Review URL: https://codereview.chromium.org/173713003
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@253631 0039d316-1c4b-4281-b951-d872f2087c98
-rw-r--r-- | ash/display/display_manager.cc | 2 | ||||
-rw-r--r-- | ash/shell.cc | 9 | ||||
-rw-r--r-- | chromeos/chromeos.gyp | 5 | ||||
-rw-r--r-- | chromeos/display/native_display_delegate.h | 82 | ||||
-rw-r--r-- | chromeos/display/native_display_delegate_x11.cc | 165 | ||||
-rw-r--r-- | chromeos/display/native_display_delegate_x11.h | 74 | ||||
-rw-r--r-- | chromeos/display/native_display_event_dispatcher_x11.cc | 73 | ||||
-rw-r--r-- | chromeos/display/native_display_event_dispatcher_x11.h | 42 | ||||
-rw-r--r-- | chromeos/display/native_display_event_dispatcher_x11_unittest.cc | 232 | ||||
-rw-r--r-- | chromeos/display/native_display_observer.h | 21 | ||||
-rw-r--r-- | chromeos/display/output_configurator.cc | 158 | ||||
-rw-r--r-- | chromeos/display/output_configurator.h | 107 | ||||
-rw-r--r-- | chromeos/display/output_configurator_unittest.cc | 212 |
13 files changed, 731 insertions, 451 deletions
diff --git a/ash/display/display_manager.cc b/ash/display/display_manager.cc index b911bdd..709d3a5 100644 --- a/ash/display/display_manager.cc +++ b/ash/display/display_manager.cc @@ -456,7 +456,7 @@ void DisplayManager::SetDisplayResolution(int64 display_id, display_modes_[display_id] = *iter; #if defined(OS_CHROMEOS) && defined(USE_X11) if (base::SysInfo::IsRunningOnChromeOS()) - Shell::GetInstance()->output_configurator()->ScheduleConfigureOutputs(); + Shell::GetInstance()->output_configurator()->OnConfigurationChanged(); #endif } diff --git a/ash/shell.cc b/ash/shell.cc index 8fc65a7..c70e584 100644 --- a/ash/shell.cc +++ b/ash/shell.cc @@ -607,12 +607,6 @@ Shell::Shell(ShellDelegate* delegate) // TODO: Move this initialization into Shell::Init(). output_configurator_->Init(!gpu_support_->IsPanelFittingDisabled()); user_metrics_recorder_.reset(new UserMetricsRecorder); - - base::MessagePumpX11::Current()->AddDispatcherForRootWindow( - output_configurator()); - // We can't do this with a root window listener because XI_HierarchyChanged - // messages don't have a target window. - base::MessagePumpX11::Current()->AddObserver(output_configurator()); #endif // defined(OS_CHROMEOS) #if defined(OS_CHROMEOS) @@ -751,9 +745,6 @@ Shell::~Shell() { output_configurator_->RemoveObserver(display_error_observer_.get()); if (projecting_observer_) output_configurator_->RemoveObserver(projecting_observer_.get()); - base::MessagePumpX11::Current()->RemoveDispatcherForRootWindow( - output_configurator()); - base::MessagePumpX11::Current()->RemoveObserver(output_configurator()); display_change_observer_.reset(); #endif // defined(OS_CHROMEOS) diff --git a/chromeos/chromeos.gyp b/chromeos/chromeos.gyp index b7daed7..2dda381 100644 --- a/chromeos/chromeos.gyp +++ b/chromeos/chromeos.gyp @@ -206,8 +206,12 @@ 'dbus/volume_state.h', 'disks/disk_mount_manager.cc', 'disks/disk_mount_manager.h', + 'display/native_display_delegate.h', 'display/native_display_delegate_x11.cc', 'display/native_display_delegate_x11.h', + 'display/native_display_event_dispatcher_x11.cc', + 'display/native_display_event_dispatcher_x11.h', + 'display/native_display_observer.h', 'display/output_configurator.cc', 'display/output_configurator.h', 'display/output_util.cc', @@ -482,6 +486,7 @@ 'dbus/shill_profile_client_unittest.cc', 'dbus/shill_service_client_unittest.cc', 'disks/disk_mount_manager_unittest.cc', + 'display/native_display_event_dispatcher_x11_unittest.cc', 'display/output_configurator_unittest.cc', 'display/output_util_unittest.cc', 'ime/component_extension_ime_manager_unittest.cc', diff --git a/chromeos/display/native_display_delegate.h b/chromeos/display/native_display_delegate.h new file mode 100644 index 0000000..c595056 --- /dev/null +++ b/chromeos/display/native_display_delegate.h @@ -0,0 +1,82 @@ +// 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. + +#ifndef CHROMEOS_DISPLAY_NATIVE_DISPLAY_DELEGATE_H_ +#define CHROMEOS_DISPLAY_NATIVE_DISPLAY_DELEGATE_H_ + +#include "chromeos/display/output_configurator.h" +#include "ui/display/display_constants.h" + +namespace chromeos { + +class NativeDisplayObserver; + +// Interface for classes that perform display configuration actions on behalf +// of OutputConfigurator. +// TODO(dnicoara) This interface will eventually be platform independent, +// however it first requires refactoring the datatypes: crbug.com/333413 +class NativeDisplayDelegate { + public: + virtual ~NativeDisplayDelegate() {} + + // Initializes the XRandR extension, saving the base event ID to |event_base|. + virtual void Initialize() = 0; + + // Grabs the X server and refreshes XRandR-related resources. While + // the server is grabbed, other clients are blocked. Must be balanced + // by a call to UngrabServer(). + virtual void GrabServer() = 0; + + // Ungrabs the server and frees XRandR-related resources. + virtual void UngrabServer() = 0; + + // Flushes all pending requests and waits for replies. + virtual void SyncWithServer() = 0; + + // Sets the window's background color to |color_argb|. + virtual void SetBackgroundColor(uint32 color_argb) = 0; + + // Enables DPMS and forces it to the "on" state. + virtual void ForceDPMSOn() = 0; + + // Returns information about the current outputs. This method may block for + // 60 milliseconds or more. The returned outputs are not fully initialized; + // the rest of the work happens in + // OutputConfigurator::UpdateCachedOutputs(). + virtual std::vector<OutputConfigurator::OutputSnapshot> GetOutputs() = 0; + + // Adds |mode| to |output|. + virtual void AddMode(const OutputConfigurator::OutputSnapshot& output, + RRMode mode) = 0; + + // Calls XRRSetCrtcConfig() with the given options but some of our default + // output count and rotation arguments. Returns true on success. + virtual bool Configure(const OutputConfigurator::OutputSnapshot& output, + RRMode mode, + int x, + int y) = 0; + + // Called to set the frame buffer (underlying XRR "screen") size. Has + // a side-effect of disabling all CRTCs. + virtual void CreateFrameBuffer( + int width, + int height, + const std::vector<OutputConfigurator::OutputSnapshot>& outputs) = 0; + + // Gets HDCP state of output. + virtual bool GetHDCPState(const OutputConfigurator::OutputSnapshot& output, + ui::HDCPState* state) = 0; + + // Sets HDCP state of output. + virtual bool SetHDCPState(const OutputConfigurator::OutputSnapshot& output, + ui::HDCPState state) = 0; + + virtual void AddObserver(NativeDisplayObserver* observer) = 0; + + virtual void RemoveObserver(NativeDisplayObserver* observer) = 0; +}; + +} // namespace chromeos + +#endif // CHROMEOS_DISPLAY_NATIVE_DISPLAY_DELEGATE_H_ diff --git a/chromeos/display/native_display_delegate_x11.cc b/chromeos/display/native_display_delegate_x11.cc index 19f1ef8..dbd905d 100644 --- a/chromeos/display/native_display_delegate_x11.cc +++ b/chromeos/display/native_display_delegate_x11.cc @@ -8,13 +8,17 @@ #include <X11/Xlib.h> #include <X11/extensions/dpms.h> #include <X11/extensions/Xrandr.h> +#include <X11/extensions/XInput2.h> #include <utility> #include "base/logging.h" +#include "base/message_loop/message_loop.h" #include "base/message_loop/message_pump_x11.h" #include "base/x11/edid_parser_x11.h" #include "base/x11/x11_error_tracker.h" +#include "chromeos/display/native_display_event_dispatcher_x11.h" +#include "chromeos/display/native_display_observer.h" #include "chromeos/display/output_util.h" namespace chromeos { @@ -47,21 +51,112 @@ RRMode GetOutputNativeMode(const XRROutputInfo* output_info) { } // namespace +//////////////////////////////////////////////////////////////////////////////// +// NativeDisplayDelegateX11::HelperDelegateX11 + +class NativeDisplayDelegateX11::HelperDelegateX11 + : public NativeDisplayDelegateX11::HelperDelegate { + public: + HelperDelegateX11(NativeDisplayDelegateX11* delegate) + : delegate_(delegate) {} + virtual ~HelperDelegateX11() {} + + // NativeDisplayDelegateX11::HelperDelegate overrides: + virtual void UpdateXRandRConfiguration( + const base::NativeEvent& event) OVERRIDE { + XRRUpdateConfiguration(event); + } + virtual const std::vector<OutputConfigurator::OutputSnapshot>& + GetCachedOutputs() const OVERRIDE { + return delegate_->cached_outputs_; + } + virtual void NotifyDisplayObservers() OVERRIDE { + FOR_EACH_OBSERVER(NativeDisplayObserver, + delegate_->observers_, + OnConfigurationChanged()); + } + + private: + NativeDisplayDelegateX11* delegate_; + + DISALLOW_COPY_AND_ASSIGN(HelperDelegateX11); +}; + +//////////////////////////////////////////////////////////////////////////////// +// NativeDisplayDelegateX11::MessagePumpObserverX11 + +class NativeDisplayDelegateX11::MessagePumpObserverX11 + : public base::MessagePumpObserver { + public: + MessagePumpObserverX11(HelperDelegate* delegate); + virtual ~MessagePumpObserverX11(); + + // base::MessagePumpObserver overrides: + virtual base::EventStatus WillProcessEvent( + const base::NativeEvent& event) OVERRIDE; + virtual void DidProcessEvent(const base::NativeEvent& event) OVERRIDE; + + private: + HelperDelegate* delegate_; // Not owned. + + DISALLOW_COPY_AND_ASSIGN(MessagePumpObserverX11); +}; + +NativeDisplayDelegateX11::MessagePumpObserverX11::MessagePumpObserverX11( + HelperDelegate* delegate) : delegate_(delegate) {} + +NativeDisplayDelegateX11::MessagePumpObserverX11::~MessagePumpObserverX11() {} + +base::EventStatus +NativeDisplayDelegateX11::MessagePumpObserverX11::WillProcessEvent( + const base::NativeEvent& event) { + // XI_HierarchyChanged events are special. There is no window associated with + // these events. So process them directly from here. + if (event->type == GenericEvent && + event->xgeneric.evtype == XI_HierarchyChanged) { + VLOG(1) << "Received XI_HierarchyChanged event"; + // Defer configuring outputs to not stall event processing. + // This also takes care of same event being received twice. + delegate_->NotifyDisplayObservers(); + } + + return base::EVENT_CONTINUE; +} + +void NativeDisplayDelegateX11::MessagePumpObserverX11::DidProcessEvent( + const base::NativeEvent& event) { +} + +//////////////////////////////////////////////////////////////////////////////// +// NativeDisplayDelegateX11 implementation: + NativeDisplayDelegateX11::NativeDisplayDelegateX11() : display_(base::MessagePumpX11::GetDefaultXDisplay()), window_(DefaultRootWindow(display_)), screen_(NULL) {} -NativeDisplayDelegateX11::~NativeDisplayDelegateX11() {} - -void NativeDisplayDelegateX11::InitXRandRExtension(int* event_base) { - int error_base_ignored = 0; - XRRQueryExtension(display_, event_base, &error_base_ignored); +NativeDisplayDelegateX11::~NativeDisplayDelegateX11() { + base::MessagePumpX11::Current()->RemoveDispatcherForRootWindow( + message_pump_dispatcher_.get()); + base::MessagePumpX11::Current()->RemoveObserver(message_pump_observer_.get()); } -void NativeDisplayDelegateX11::UpdateXRandRConfiguration( - const base::NativeEvent& event) { - XRRUpdateConfiguration(event); +void NativeDisplayDelegateX11::Initialize() { + int error_base_ignored = 0; + int xrandr_event_base = 0; + XRRQueryExtension(display_, &xrandr_event_base, &error_base_ignored); + + helper_delegate_.reset(new HelperDelegateX11(this)); + message_pump_dispatcher_.reset(new NativeDisplayEventDispatcherX11( + helper_delegate_.get(), xrandr_event_base)); + message_pump_observer_.reset(new MessagePumpObserverX11( + helper_delegate_.get())); + + base::MessagePumpX11::Current()->AddDispatcherForRootWindow( + message_pump_dispatcher_.get()); + // We can't do this with a root window listener because XI_HierarchyChanged + // messages don't have a target window. + base::MessagePumpX11::Current()->AddObserver(message_pump_observer_.get()); } void NativeDisplayDelegateX11::GrabServer() { @@ -108,37 +203,47 @@ std::vector<OutputConfigurator::OutputSnapshot> NativeDisplayDelegateX11::GetOutputs() { CHECK(screen_) << "Server not grabbed"; - std::vector<OutputConfigurator::OutputSnapshot> outputs; + cached_outputs_.clear(); RRCrtc last_used_crtc = None; - for (int i = 0; i < screen_->noutput && outputs.size() < 2; ++i) { + for (int i = 0; i < screen_->noutput && cached_outputs_.size() < 2; ++i) { RROutput output_id = screen_->outputs[i]; XRROutputInfo* output_info = XRRGetOutputInfo(display_, screen_, output_id); if (output_info->connection == RR_Connected) { OutputConfigurator::OutputSnapshot output = InitOutputSnapshot(output_id, output_info, &last_used_crtc, i); - VLOG(2) << "Found display " << outputs.size() << ":" + VLOG(2) << "Found display " << cached_outputs_.size() << ":" << " output=" << output.output << " crtc=" << output.crtc << " current_mode=" << output.current_mode; - outputs.push_back(output); + cached_outputs_.push_back(output); } XRRFreeOutputInfo(output_info); } - return outputs; + return cached_outputs_; } -void NativeDisplayDelegateX11::AddOutputMode(RROutput output, RRMode mode) { +void NativeDisplayDelegateX11::AddMode( + const OutputConfigurator::OutputSnapshot& output, RRMode mode) { CHECK(screen_) << "Server not grabbed"; - VLOG(1) << "AddOutputMode: output=" << output << " mode=" << mode; - XRRAddOutputMode(display_, output, mode); + VLOG(1) << "AddOutputMode: output=" << output.output << " mode=" << mode; + XRRAddOutputMode(display_, output.output, mode); } -bool NativeDisplayDelegateX11::ConfigureCrtc(RRCrtc crtc, - RRMode mode, - RROutput output, - int x, - int y) { +bool NativeDisplayDelegateX11::Configure( + const OutputConfigurator::OutputSnapshot& output, + RRMode mode, + int x, + int y) { + return ConfigureCrtc(output.crtc, mode, output.output, x, y); +} + +bool NativeDisplayDelegateX11::ConfigureCrtc( + RRCrtc crtc, + RRMode mode, + RROutput output, + int x, + int y) { CHECK(screen_) << "Server not grabbed"; VLOG(1) << "ConfigureCrtc: crtc=" << crtc << " mode=" << mode << " output=" << output << " x=" << x << " y=" << y; @@ -271,7 +376,8 @@ OutputConfigurator::OutputSnapshot NativeDisplayDelegateX11::InitOutputSnapshot( return output; } -bool NativeDisplayDelegateX11::GetHDCPState(RROutput id, ui::HDCPState* state) { +bool NativeDisplayDelegateX11::GetHDCPState( + const OutputConfigurator::OutputSnapshot& output, ui::HDCPState* state) { unsigned char* values = NULL; int actual_format = 0; unsigned long nitems = 0; @@ -285,7 +391,7 @@ bool NativeDisplayDelegateX11::GetHDCPState(RROutput id, ui::HDCPState* state) { // TODO(kcwu): Move this to x11_util (similar method calls in this file and // output_util.cc) success = XRRGetOutputProperty(display_, - id, + output.output, prop, 0, 100, @@ -328,7 +434,8 @@ bool NativeDisplayDelegateX11::GetHDCPState(RROutput id, ui::HDCPState* state) { return ok; } -bool NativeDisplayDelegateX11::SetHDCPState(RROutput id, ui::HDCPState state) { +bool NativeDisplayDelegateX11::SetHDCPState( + const OutputConfigurator::OutputSnapshot& output, ui::HDCPState state) { Atom name = XInternAtom(display_, kContentProtectionAtomName, False); Atom value = None; switch (state) { @@ -345,7 +452,7 @@ bool NativeDisplayDelegateX11::SetHDCPState(RROutput id, ui::HDCPState state) { base::X11ErrorTracker err_tracker; unsigned char* data = reinterpret_cast<unsigned char*>(&value); XRRChangeOutputProperty( - display_, id, name, XA_ATOM, 32, PropModeReplace, data, 1); + display_, output.output, name, XA_ATOM, 32, PropModeReplace, data, 1); if (err_tracker.FoundNewError()) { LOG(ERROR) << "XRRChangeOutputProperty failed"; return false; @@ -453,4 +560,12 @@ bool NativeDisplayDelegateX11::IsOutputAspectPreservingScaling(RROutput id) { return ret; } +void NativeDisplayDelegateX11::AddObserver(NativeDisplayObserver* observer) { + observers_.AddObserver(observer); +} + +void NativeDisplayDelegateX11::RemoveObserver(NativeDisplayObserver* observer) { + observers_.RemoveObserver(observer); +} + } // namespace chromeos diff --git a/chromeos/display/native_display_delegate_x11.h b/chromeos/display/native_display_delegate_x11.h index b204731..9e7393b 100644 --- a/chromeos/display/native_display_delegate_x11.h +++ b/chromeos/display/native_display_delegate_x11.h @@ -9,7 +9,7 @@ #include "base/basictypes.h" #include "base/compiler_specific.h" -#include "chromeos/display/output_configurator.h" +#include "chromeos/display/native_display_delegate.h" typedef XID Window; @@ -22,36 +22,63 @@ typedef _XRRScreenResources XRRScreenResources; namespace chromeos { -class NativeDisplayDelegateX11 - : public OutputConfigurator::NativeDisplayDelegate { +class NativeDisplayEventDispatcherX11; + +class NativeDisplayDelegateX11 : public NativeDisplayDelegate { public: + // Helper class that allows NativeDisplayEventDispatcherX11 and + // NativeDisplayDelegateX11::MessagePumpObserverX11 to interact with this + // class or with mocks in tests. + class HelperDelegate { + public: + virtual ~HelperDelegate() {} + + // Tells XRandR to update its configuration in response to |event|, an + // RRScreenChangeNotify event. + virtual void UpdateXRandRConfiguration(const base::NativeEvent& event) = 0; + + // Returns the list of current outputs. This is used to discard duplicate + // events. + virtual const std::vector<OutputConfigurator::OutputSnapshot>& + GetCachedOutputs() const = 0; + + // Notify |observers_| that a change in configuration has occurred. + virtual void NotifyDisplayObservers() = 0; + }; + NativeDisplayDelegateX11(); virtual ~NativeDisplayDelegateX11(); // OutputConfigurator::Delegate overrides: - virtual void InitXRandRExtension(int* event_base) OVERRIDE; - virtual void UpdateXRandRConfiguration(const base::NativeEvent& event) - OVERRIDE; + virtual void Initialize() OVERRIDE; virtual void GrabServer() OVERRIDE; virtual void UngrabServer() OVERRIDE; virtual void SyncWithServer() OVERRIDE; virtual void SetBackgroundColor(uint32 color_argb) OVERRIDE; virtual void ForceDPMSOn() OVERRIDE; virtual std::vector<OutputConfigurator::OutputSnapshot> GetOutputs() OVERRIDE; - virtual void AddOutputMode(RROutput output, RRMode mode) OVERRIDE; - virtual bool ConfigureCrtc(RRCrtc crtc, - RRMode mode, - RROutput output, - int x, - int y) OVERRIDE; + virtual void AddMode(const OutputConfigurator::OutputSnapshot& output, + RRMode mode) OVERRIDE; + virtual bool Configure(const OutputConfigurator::OutputSnapshot& output, + RRMode mode, + int x, + int y) OVERRIDE; virtual void CreateFrameBuffer( int width, int height, const std::vector<OutputConfigurator::OutputSnapshot>& outputs) OVERRIDE; - virtual bool GetHDCPState(RROutput id, ui::HDCPState* state) OVERRIDE; - virtual bool SetHDCPState(RROutput id, ui::HDCPState state) OVERRIDE; + virtual bool GetHDCPState(const OutputConfigurator::OutputSnapshot& output, + ui::HDCPState* state) OVERRIDE; + virtual bool SetHDCPState(const OutputConfigurator::OutputSnapshot& output, + ui::HDCPState state) OVERRIDE; + + virtual void AddObserver(NativeDisplayObserver* observer) OVERRIDE; + virtual void RemoveObserver(NativeDisplayObserver* observer) OVERRIDE; private: + class HelperDelegateX11; + class MessagePumpObserverX11; + // Initializes |mode_info| to contain details corresponding to |mode|. Returns // true on success. bool InitModeInfo(RRMode mode, OutputConfigurator::ModeInfo* mode_info); @@ -70,6 +97,8 @@ class NativeDisplayDelegateX11 void DestroyUnusedCrtcs( const std::vector<OutputConfigurator::OutputSnapshot>& outputs); + bool ConfigureCrtc(RRCrtc crtc, RRMode mode, RROutput output, int x, int y); + // Returns whether |id| is configured to preserve aspect when scaling. bool IsOutputAspectPreservingScaling(RROutput id); @@ -79,6 +108,23 @@ class NativeDisplayDelegateX11 // Initialized when the server is grabbed and freed when it's ungrabbed. XRRScreenResources* screen_; + // Every time GetOutputs() is called we cache the updated list of outputs in + // |cached_outputs_| so that we can check for duplicate events rather than + // propagate them. + std::vector<OutputConfigurator::OutputSnapshot> cached_outputs_; + + scoped_ptr<HelperDelegate> helper_delegate_; + + // Processes X11 display events associated with the root window and notifies + // |observers_| when a display change has occurred. + scoped_ptr<NativeDisplayEventDispatcherX11> message_pump_dispatcher_; + + // Processes X11 display events that have no X11 window associated with it. + scoped_ptr<MessagePumpObserverX11> message_pump_observer_; + + // List of observers waiting for display configuration change events. + ObserverList<NativeDisplayObserver> observers_; + DISALLOW_COPY_AND_ASSIGN(NativeDisplayDelegateX11); }; diff --git a/chromeos/display/native_display_event_dispatcher_x11.cc b/chromeos/display/native_display_event_dispatcher_x11.cc new file mode 100644 index 0000000..01e91bc --- /dev/null +++ b/chromeos/display/native_display_event_dispatcher_x11.cc @@ -0,0 +1,73 @@ +// 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 "chromeos/display/native_display_event_dispatcher_x11.h" + +#include <X11/extensions/Xrandr.h> + +namespace chromeos { + +NativeDisplayEventDispatcherX11::NativeDisplayEventDispatcherX11( + NativeDisplayDelegateX11::HelperDelegate* delegate, int xrandr_event_base) + : delegate_(delegate), + xrandr_event_base_(xrandr_event_base) {} + +NativeDisplayEventDispatcherX11::~NativeDisplayEventDispatcherX11() {} + +uint32_t NativeDisplayEventDispatcherX11::Dispatch( + const base::NativeEvent& event) { + if (event->type - xrandr_event_base_ == RRScreenChangeNotify) { + VLOG(1) << "Received RRScreenChangeNotify event"; + delegate_->UpdateXRandRConfiguration(event); + return POST_DISPATCH_PERFORM_DEFAULT; + } + + // Bail out early for everything except RRNotify_OutputChange events + // about an output getting connected or disconnected. + if (event->type - xrandr_event_base_ != RRNotify) + return POST_DISPATCH_PERFORM_DEFAULT; + const XRRNotifyEvent* notify_event = reinterpret_cast<XRRNotifyEvent*>(event); + if (notify_event->subtype != RRNotify_OutputChange) + return POST_DISPATCH_PERFORM_DEFAULT; + const XRROutputChangeNotifyEvent* output_change_event = + reinterpret_cast<XRROutputChangeNotifyEvent*>(event); + const int action = output_change_event->connection; + if (action != RR_Connected && action != RR_Disconnected) + return POST_DISPATCH_PERFORM_DEFAULT; + + const bool connected = (action == RR_Connected); + VLOG(1) << "Received RRNotify_OutputChange event:" + << " output=" << output_change_event->output + << " crtc=" << output_change_event->crtc + << " mode=" << output_change_event->mode + << " action=" << (connected ? "connected" : "disconnected"); + + bool found_changed_output = false; + const std::vector<OutputConfigurator::OutputSnapshot>& cached_outputs = + delegate_->GetCachedOutputs(); + for (std::vector<OutputConfigurator::OutputSnapshot>::const_iterator + it = cached_outputs.begin(); + it != cached_outputs.end(); ++it) { + if (it->output == output_change_event->output) { + if (connected && it->crtc == output_change_event->crtc && + it->current_mode == output_change_event->mode) { + VLOG(1) << "Ignoring event describing already-cached state"; + return POST_DISPATCH_PERFORM_DEFAULT; + } + found_changed_output = true; + break; + } + } + + if (!connected && !found_changed_output) { + VLOG(1) << "Ignoring event describing already-disconnected output"; + return POST_DISPATCH_PERFORM_DEFAULT; + } + + delegate_->NotifyDisplayObservers(); + + return POST_DISPATCH_PERFORM_DEFAULT; +} + +} // namespace chromeos diff --git a/chromeos/display/native_display_event_dispatcher_x11.h b/chromeos/display/native_display_event_dispatcher_x11.h new file mode 100644 index 0000000..631f71a --- /dev/null +++ b/chromeos/display/native_display_event_dispatcher_x11.h @@ -0,0 +1,42 @@ +// 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. + +#ifndef CHROMEOS_DISPLAY_NATIVE_DISPLAY_EVENT_DISPATCHER_X11_H_ +#define CHROMEOS_DISPLAY_NATIVE_DISPLAY_EVENT_DISPATCHER_X11_H_ + +#include "base/message_loop/message_pump_dispatcher.h" +#include "chromeos/display/native_display_delegate_x11.h" + +namespace chromeos { + +class CHROMEOS_EXPORT NativeDisplayEventDispatcherX11 + : public base::MessagePumpDispatcher { + public: + NativeDisplayEventDispatcherX11( + NativeDisplayDelegateX11::HelperDelegate* delegate, + int xrandr_event_base); + virtual ~NativeDisplayEventDispatcherX11(); + + // base::MessagePumpDispatcher overrides: + // + // Called when an RRNotify event is received. The implementation is + // interested in the cases of RRNotify events which correspond to output + // add/remove events. Note that Output add/remove events are sent in response + // to our own reconfiguration operations so spurious events are common. + // Spurious events will have no effect. + virtual uint32_t Dispatch(const base::NativeEvent& event) OVERRIDE; + + private: + NativeDisplayDelegateX11::HelperDelegate* delegate_; // Not owned. + + // The base of the event numbers used to represent XRandr events used in + // decoding events regarding output add/remove. + int xrandr_event_base_; + + DISALLOW_COPY_AND_ASSIGN(NativeDisplayEventDispatcherX11); +}; + +} // namespace chromeos + +#endif // CHROMEOS_DISPLAY_NATIVE_DISPLAY_EVENT_DISPATCHER_X11_H_ diff --git a/chromeos/display/native_display_event_dispatcher_x11_unittest.cc b/chromeos/display/native_display_event_dispatcher_x11_unittest.cc new file mode 100644 index 0000000..9659148 --- /dev/null +++ b/chromeos/display/native_display_event_dispatcher_x11_unittest.cc @@ -0,0 +1,232 @@ +// 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 <X11/extensions/Xrandr.h> + +#undef Bool +#undef None + +#include "chromeos/display/native_display_delegate_x11.h" +#include "chromeos/display/native_display_event_dispatcher_x11.h" +#include "testing/gtest/include/gtest/gtest.h" + +namespace chromeos { + +namespace { + +OutputConfigurator::OutputSnapshot CreateOutput( + RROutput output, RRCrtc crtc, RRMode mode) { + OutputConfigurator::OutputSnapshot snapshot; + snapshot.output = output; + snapshot.crtc = crtc; + snapshot.current_mode = mode; + + return snapshot; +} + +class TestHelperDelegate : public NativeDisplayDelegateX11::HelperDelegate { + public: + TestHelperDelegate(); + virtual ~TestHelperDelegate(); + + int num_calls_update_xrandr_config() const { + return num_calls_update_xrandr_config_; + } + + int num_calls_notify_observers() const { + return num_calls_notify_observers_; + } + + void set_cached_outputs( + const std::vector<OutputConfigurator::OutputSnapshot>& outputs) { + cached_outputs_ = outputs; + } + + // NativeDisplayDelegateX11::HelperDelegate overrides: + virtual void UpdateXRandRConfiguration( + const base::NativeEvent& event) OVERRIDE; + virtual const std::vector<OutputConfigurator::OutputSnapshot>& + GetCachedOutputs() const OVERRIDE; + virtual void NotifyDisplayObservers() OVERRIDE; + + private: + int num_calls_update_xrandr_config_; + int num_calls_notify_observers_; + + std::vector<OutputConfigurator::OutputSnapshot> cached_outputs_; + + DISALLOW_COPY_AND_ASSIGN(TestHelperDelegate); +}; + +TestHelperDelegate::TestHelperDelegate() + : num_calls_update_xrandr_config_(0), + num_calls_notify_observers_(0) {} + +TestHelperDelegate::~TestHelperDelegate() {} + +void TestHelperDelegate::UpdateXRandRConfiguration( + const base::NativeEvent& event) { + ++num_calls_update_xrandr_config_; +} + +const std::vector<OutputConfigurator::OutputSnapshot>& +TestHelperDelegate::GetCachedOutputs() const { + return cached_outputs_; +} + +void TestHelperDelegate::NotifyDisplayObservers() { + ++num_calls_notify_observers_; +} + +//////////////////////////////////////////////////////////////////////////////// +// NativeDisplayEventDispatcherX11Test + +class NativeDisplayEventDispatcherX11Test : public testing::Test { + public: + NativeDisplayEventDispatcherX11Test(); + virtual ~NativeDisplayEventDispatcherX11Test(); + + protected: + void DispatchScreenChangeEvent(); + void DispatchOutputChangeEvent( + RROutput output, RRCrtc crtc, RRMode mode, bool connected); + + int xrandr_event_base_; + scoped_ptr<TestHelperDelegate> helper_delegate_; + scoped_ptr<NativeDisplayEventDispatcherX11> dispatcher_; + + private: + DISALLOW_COPY_AND_ASSIGN(NativeDisplayEventDispatcherX11Test); +}; + +NativeDisplayEventDispatcherX11Test::NativeDisplayEventDispatcherX11Test() + : xrandr_event_base_(10), + helper_delegate_(new TestHelperDelegate()), + dispatcher_(new NativeDisplayEventDispatcherX11(helper_delegate_.get(), + xrandr_event_base_)) { +} + +NativeDisplayEventDispatcherX11Test::~NativeDisplayEventDispatcherX11Test() {} + +void NativeDisplayEventDispatcherX11Test::DispatchScreenChangeEvent() { + XRRScreenChangeNotifyEvent event = {0}; + event.type = xrandr_event_base_ + RRScreenChangeNotify; + + dispatcher_->Dispatch(reinterpret_cast<const base::NativeEvent>(&event)); +} + +void NativeDisplayEventDispatcherX11Test::DispatchOutputChangeEvent( + RROutput output, RRCrtc crtc, RRMode mode, bool connected) { + XRROutputChangeNotifyEvent event = {0}; + event.type = xrandr_event_base_ + RRNotify; + event.subtype = RRNotify_OutputChange; + event.output = output; + event.crtc = crtc; + event.mode = mode; + event.connection = connected ? RR_Connected : RR_Disconnected; + + dispatcher_->Dispatch(reinterpret_cast<const base::NativeEvent>(&event)); +} + +} // namespace + +TEST_F(NativeDisplayEventDispatcherX11Test, OnScreenChangedEvent) { + DispatchScreenChangeEvent(); + EXPECT_EQ(1, helper_delegate_->num_calls_update_xrandr_config()); + EXPECT_EQ(0, helper_delegate_->num_calls_notify_observers()); +} + +TEST_F(NativeDisplayEventDispatcherX11Test, CheckNotificationOnFirstEvent) { + DispatchOutputChangeEvent(1, 10, 20, true); + EXPECT_EQ(0, helper_delegate_->num_calls_update_xrandr_config()); + EXPECT_EQ(1, helper_delegate_->num_calls_notify_observers()); +} + +TEST_F(NativeDisplayEventDispatcherX11Test, CheckNotificationAfterSecondEvent) { + DispatchOutputChangeEvent(1, 10, 20, true); + + // Simulate addition of the first output to the cached output list. + std::vector<OutputConfigurator::OutputSnapshot> outputs; + outputs.push_back(CreateOutput(1, 10, 20)); + helper_delegate_->set_cached_outputs(outputs); + + DispatchOutputChangeEvent(2, 11, 20, true); + EXPECT_EQ(2, helper_delegate_->num_calls_notify_observers()); +} + +TEST_F(NativeDisplayEventDispatcherX11Test, AvoidNotificationOnDuplicateEvent) { + std::vector<OutputConfigurator::OutputSnapshot> outputs; + outputs.push_back(CreateOutput(1, 10, 20)); + helper_delegate_->set_cached_outputs(outputs); + + DispatchOutputChangeEvent(1, 10, 20, true); + EXPECT_EQ(0, helper_delegate_->num_calls_notify_observers()); +} + +TEST_F(NativeDisplayEventDispatcherX11Test, CheckNotificationOnDisconnect) { + std::vector<OutputConfigurator::OutputSnapshot> outputs; + outputs.push_back(CreateOutput(1, 10, 20)); + helper_delegate_->set_cached_outputs(outputs); + + DispatchOutputChangeEvent(1, 10, 20, false); + EXPECT_EQ(1, helper_delegate_->num_calls_notify_observers()); +} + +TEST_F(NativeDisplayEventDispatcherX11Test, CheckNotificationOnModeChange) { + std::vector<OutputConfigurator::OutputSnapshot> outputs; + outputs.push_back(CreateOutput(1, 10, 20)); + helper_delegate_->set_cached_outputs(outputs); + + DispatchOutputChangeEvent(1, 10, 21, true); + EXPECT_EQ(1, helper_delegate_->num_calls_notify_observers()); +} + +TEST_F(NativeDisplayEventDispatcherX11Test, CheckNotificationOnSecondOutput) { + std::vector<OutputConfigurator::OutputSnapshot> outputs; + outputs.push_back(CreateOutput(1, 10, 20)); + helper_delegate_->set_cached_outputs(outputs); + + DispatchOutputChangeEvent(2, 11, 20, true); + EXPECT_EQ(1, helper_delegate_->num_calls_notify_observers()); +} + +TEST_F(NativeDisplayEventDispatcherX11Test, CheckNotificationOnDifferentCrtc) { + std::vector<OutputConfigurator::OutputSnapshot> outputs; + outputs.push_back(CreateOutput(1, 10, 20)); + helper_delegate_->set_cached_outputs(outputs); + + DispatchOutputChangeEvent(1, 11, 20, true); + EXPECT_EQ(1, helper_delegate_->num_calls_notify_observers()); +} + +TEST_F(NativeDisplayEventDispatcherX11Test, + CheckNotificationOnSecondOutputDisconnect) { + std::vector<OutputConfigurator::OutputSnapshot> outputs; + outputs.push_back(CreateOutput(1, 10, 20)); + outputs.push_back(CreateOutput(2, 11, 20)); + helper_delegate_->set_cached_outputs(outputs); + + DispatchOutputChangeEvent(2, 11, 20, false); + EXPECT_EQ(1, helper_delegate_->num_calls_notify_observers()); +} + +TEST_F(NativeDisplayEventDispatcherX11Test, + AvoidDuplicateNotificationOnSecondOutputDisconnect) { + std::vector<OutputConfigurator::OutputSnapshot> outputs; + outputs.push_back(CreateOutput(1, 10, 20)); + outputs.push_back(CreateOutput(2, 11, 20)); + helper_delegate_->set_cached_outputs(outputs); + + DispatchOutputChangeEvent(2, 11, 20, false); + EXPECT_EQ(1, helper_delegate_->num_calls_notify_observers()); + + // Simulate removal of second output from cached output list. + outputs.erase(outputs.begin() + 1); + helper_delegate_->set_cached_outputs(outputs); + + DispatchOutputChangeEvent(2, 11, 20, false); + EXPECT_EQ(1, helper_delegate_->num_calls_notify_observers()); +} + +} // namespace chromeos diff --git a/chromeos/display/native_display_observer.h b/chromeos/display/native_display_observer.h new file mode 100644 index 0000000..de15060 --- /dev/null +++ b/chromeos/display/native_display_observer.h @@ -0,0 +1,21 @@ +// 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. + +#ifndef CHROMEOS_DISPLAY_NATIVE_DISPLAY_OBSERVER_H_ +#define CHROMEOS_DISPLAY_NATIVE_DISPLAY_OBSERVER_H_ + +namespace chromeos { + +// Observer class used by NativeDisplayDelegate to announce when the display +// configuration changes. +class NativeDisplayObserver { + public: + virtual ~NativeDisplayObserver() {} + + virtual void OnConfigurationChanged() = 0; +}; + +} // namespace chromeos + +#endif // CHROMEOS_DISPLAY_NATIVE_DISPLAY_OBSERVER_H_ diff --git a/chromeos/display/output_configurator.cc b/chromeos/display/output_configurator.cc index 553bb1a..3bf0791 100644 --- a/chromeos/display/output_configurator.cc +++ b/chromeos/display/output_configurator.cc @@ -6,7 +6,6 @@ #include <X11/Xlib.h> #include <X11/extensions/Xrandr.h> -#include <X11/extensions/XInput2.h> #include "base/bind.h" #include "base/logging.h" @@ -150,26 +149,6 @@ OutputConfigurator::OutputSnapshot::OutputSnapshot() OutputConfigurator::OutputSnapshot::~OutputSnapshot() {} -void OutputConfigurator::TestApi::SendScreenChangeEvent() { - XRRScreenChangeNotifyEvent event = {0}; - event.type = xrandr_event_base_ + RRScreenChangeNotify; - configurator_->Dispatch(reinterpret_cast<const base::NativeEvent>(&event)); -} - -void OutputConfigurator::TestApi::SendOutputChangeEvent(RROutput output, - RRCrtc crtc, - RRMode mode, - bool connected) { - XRROutputChangeNotifyEvent event = {0}; - event.type = xrandr_event_base_ + RRNotify; - event.subtype = RRNotify_OutputChange; - event.output = output; - event.crtc = crtc; - event.mode = mode; - event.connection = connected ? RR_Connected : RR_Disconnected; - configurator_->Dispatch(reinterpret_cast<const base::NativeEvent>(&event)); -} - bool OutputConfigurator::TestApi::TriggerConfigureTimeout() { if (configurator_->configure_timer_.get() && configurator_->configure_timer_->IsRunning()) { @@ -236,21 +215,28 @@ OutputConfigurator::OutputConfigurator() mirroring_controller_(NULL), is_panel_fitting_enabled_(false), configure_display_(base::SysInfo::IsRunningOnChromeOS()), - xrandr_event_base_(0), output_state_(ui::OUTPUT_STATE_INVALID), power_state_(DISPLAY_POWER_ALL_ON), next_output_protection_client_id_(1) {} -OutputConfigurator::~OutputConfigurator() {} +OutputConfigurator::~OutputConfigurator() { + if (native_display_delegate_) + native_display_delegate_->RemoveObserver(this); +} void OutputConfigurator::SetNativeDisplayDelegateForTesting( scoped_ptr<NativeDisplayDelegate> delegate) { + DCHECK(!native_display_delegate_); + native_display_delegate_ = delegate.Pass(); + native_display_delegate_->AddObserver(this); configure_display_ = true; } void OutputConfigurator::SetTouchscreenDelegateForTesting( scoped_ptr<TouchscreenDelegate> delegate) { + DCHECK(!touchscreen_delegate_); + touchscreen_delegate_ = delegate.Pass(); } @@ -264,8 +250,10 @@ void OutputConfigurator::Init(bool is_panel_fitting_enabled) { if (!configure_display_) return; - if (!native_display_delegate_) + if (!native_display_delegate_) { native_display_delegate_.reset(new NativeDisplayDelegateX11()); + native_display_delegate_->AddObserver(this); + } if (!touchscreen_delegate_) touchscreen_delegate_.reset(new TouchscreenDelegateX11()); @@ -276,7 +264,7 @@ void OutputConfigurator::Start(uint32 background_color_argb) { return; native_display_delegate_->GrabServer(); - native_display_delegate_->InitXRandRExtension(&xrandr_event_base_); + native_display_delegate_->Initialize(); UpdateCachedOutputs(); if (cached_outputs_.size() > 1 && background_color_argb) @@ -295,7 +283,6 @@ void OutputConfigurator::Start(uint32 background_color_argb) { bool OutputConfigurator::ApplyProtections(const DisplayProtections& requests) { for (std::vector<OutputSnapshot>::const_iterator it = cached_outputs_.begin(); it != cached_outputs_.end(); ++it) { - RROutput this_id = it->output; uint32_t all_desired = 0; DisplayProtections::const_iterator request_it = requests.find( it->display_id); @@ -312,7 +299,7 @@ bool OutputConfigurator::ApplyProtections(const DisplayProtections& requests) { (all_desired & ui::OUTPUT_PROTECTION_METHOD_HDCP) ? ui::HDCP_STATE_DESIRED : ui::HDCP_STATE_UNDESIRED; - if (!native_display_delegate_->SetHDCPState(this_id, new_desired_state)) + if (!native_display_delegate_->SetHDCPState(*it, new_desired_state)) return false; break; } @@ -369,7 +356,6 @@ bool OutputConfigurator::QueryOutputProtectionStatus( *link_mask = 0; for (std::vector<OutputSnapshot>::const_iterator it = cached_outputs_.begin(); it != cached_outputs_.end(); ++it) { - RROutput this_id = it->output; if (it->display_id != display_id) continue; *link_mask |= it->type; @@ -381,7 +367,7 @@ bool OutputConfigurator::QueryOutputProtectionStatus( case ui::OUTPUT_TYPE_DVI: case ui::OUTPUT_TYPE_HDMI: { ui::HDCPState state; - if (!native_display_delegate_->GetHDCPState(this_id, &state)) + if (!native_display_delegate_->GetHDCPState(*it, &state)) return false; if (state == ui::HDCP_STATE_ENABLED) enabled |= ui::OUTPUT_PROTECTION_METHOD_HDCP; @@ -453,6 +439,13 @@ bool OutputConfigurator::EnableOutputProtection( void OutputConfigurator::Stop() { configure_display_ = false; + + // Since we need to stop processing configuration updates, we can just destroy + // the display delegate. + if (native_display_delegate_) { + native_display_delegate_->RemoveObserver(this); + native_display_delegate_.reset(); + } } bool OutputConfigurator::SetDisplayPower(DisplayPowerState power_state, @@ -520,78 +513,19 @@ bool OutputConfigurator::SetDisplayMode(ui::OutputState new_state) { return success; } -uint32_t OutputConfigurator::Dispatch(const base::NativeEvent& event) { - if (!configure_display_) - return POST_DISPATCH_PERFORM_DEFAULT; - - if (event->type - xrandr_event_base_ == RRScreenChangeNotify) { - VLOG(1) << "Received RRScreenChangeNotify event"; - native_display_delegate_->UpdateXRandRConfiguration(event); - return POST_DISPATCH_PERFORM_DEFAULT; - } - - // Bail out early for everything except RRNotify_OutputChange events - // about an output getting connected or disconnected. - if (event->type - xrandr_event_base_ != RRNotify) - return POST_DISPATCH_PERFORM_DEFAULT; - const XRRNotifyEvent* notify_event = reinterpret_cast<XRRNotifyEvent*>(event); - if (notify_event->subtype != RRNotify_OutputChange) - return POST_DISPATCH_PERFORM_DEFAULT; - const XRROutputChangeNotifyEvent* output_change_event = - reinterpret_cast<XRROutputChangeNotifyEvent*>(event); - const int action = output_change_event->connection; - if (action != RR_Connected && action != RR_Disconnected) - return POST_DISPATCH_PERFORM_DEFAULT; - - const bool connected = (action == RR_Connected); - VLOG(1) << "Received RRNotify_OutputChange event:" - << " output=" << output_change_event->output - << " crtc=" << output_change_event->crtc - << " mode=" << output_change_event->mode - << " action=" << (connected ? "connected" : "disconnected"); - - bool found_changed_output = false; - for (std::vector<OutputSnapshot>::const_iterator it = cached_outputs_.begin(); - it != cached_outputs_.end(); ++it) { - if (it->output == output_change_event->output) { - if (connected && it->crtc == output_change_event->crtc && - it->current_mode == output_change_event->mode) { - VLOG(1) << "Ignoring event describing already-cached state"; - return POST_DISPATCH_PERFORM_DEFAULT; - } - found_changed_output = true; - break; - } - } - - if (!connected && !found_changed_output) { - VLOG(1) << "Ignoring event describing already-disconnected output"; - return POST_DISPATCH_PERFORM_DEFAULT; - } - - // Connecting/disconnecting a display may generate multiple events. Defer - // configuring outputs to avoid grabbing X and configuring displays - // multiple times. - ScheduleConfigureOutputs(); - return POST_DISPATCH_PERFORM_DEFAULT; -} - -base::EventStatus OutputConfigurator::WillProcessEvent( - const base::NativeEvent& event) { - // XI_HierarchyChanged events are special. There is no window associated with - // these events. So process them directly from here. - if (configure_display_ && event->type == GenericEvent && - event->xgeneric.evtype == XI_HierarchyChanged) { - VLOG(1) << "Received XI_HierarchyChanged event"; - // Defer configuring outputs to not stall event processing. - // This also takes care of same event being received twice. - ScheduleConfigureOutputs(); +void OutputConfigurator::OnConfigurationChanged() { + // Configure outputs with |kConfigureDelayMs| delay, + // so that time-consuming ConfigureOutputs() won't be called multiple times. + if (configure_timer_.get()) { + configure_timer_->Reset(); + } else { + configure_timer_.reset(new base::OneShotTimer<OutputConfigurator>()); + configure_timer_->Start( + FROM_HERE, + base::TimeDelta::FromMilliseconds(kConfigureDelayMs), + this, + &OutputConfigurator::ConfigureOutputs); } - - return base::EVENT_CONTINUE; -} - -void OutputConfigurator::DidProcessEvent(const base::NativeEvent& event) { } void OutputConfigurator::AddObserver(Observer* observer) { @@ -625,19 +559,6 @@ void OutputConfigurator::ResumeDisplays() { SetDisplayPower(power_state_, kSetDisplayPowerForceProbe); } -void OutputConfigurator::ScheduleConfigureOutputs() { - if (configure_timer_.get()) { - configure_timer_->Reset(); - } else { - configure_timer_.reset(new base::OneShotTimer<OutputConfigurator>()); - configure_timer_->Start( - FROM_HERE, - base::TimeDelta::FromMilliseconds(kConfigureDelayMs), - this, - &OutputConfigurator::ConfigureOutputs); - } -} - void OutputConfigurator::UpdateCachedOutputs() { cached_outputs_ = native_display_delegate_->GetOutputs(); touchscreen_delegate_->AssociateTouchscreens(&cached_outputs_); @@ -750,7 +671,7 @@ bool OutputConfigurator::FindMirrorMode(OutputSnapshot* internal_output, !external_info.interlaced; if (can_fit) { RRMode mode = external_it->first; - native_display_delegate_->AddOutputMode(internal_output->output, mode); + native_display_delegate_->AddMode(*internal_output, mode); internal_output->mode_infos.insert(std::make_pair(mode, external_info)); internal_output->mirror_mode = mode; external_output->mirror_mode = mode; @@ -944,11 +865,10 @@ bool OutputConfigurator::EnterState(ui::OutputState output_state, bool configure_succeeded =false; while (true) { - if (native_display_delegate_->ConfigureCrtc(output.crtc, - output.current_mode, - output.output, - output.x, - output.y)) { + if (native_display_delegate_->Configure(output, + output.current_mode, + output.x, + output.y)) { configure_succeeded = true; break; } diff --git a/chromeos/display/output_configurator.h b/chromeos/display/output_configurator.h index e6661e1..89b2e53 100644 --- a/chromeos/display/output_configurator.h +++ b/chromeos/display/output_configurator.h @@ -12,11 +12,10 @@ #include "base/basictypes.h" #include "base/event_types.h" #include "base/memory/scoped_ptr.h" -#include "base/message_loop/message_loop.h" -#include "base/message_loop/message_pump_dispatcher.h" #include "base/observer_list.h" #include "base/timer/timer.h" #include "chromeos/chromeos_export.h" +#include "chromeos/display/native_display_observer.h" #include "third_party/cros_system_api/dbus/service_constants.h" #include "ui/display/display_constants.h" @@ -29,11 +28,12 @@ typedef XID RRMode; namespace chromeos { +class NativeDisplayDelegate; + // This class interacts directly with the underlying Xrandr API to manipulate // CTRCs and Outputs. class CHROMEOS_EXPORT OutputConfigurator - : public base::MessagePumpDispatcher, - public base::MessagePumpObserver { + : public NativeDisplayObserver { public: typedef uint64_t OutputProtectionClientId; static const OutputProtectionClientId kInvalidClientId = 0; @@ -156,68 +156,6 @@ class CHROMEOS_EXPORT OutputConfigurator virtual void SetSoftwareMirroring(bool enabled) = 0; }; - // Interface for classes that perform display configuration actions on behalf - // of OutputConfigurator. - class NativeDisplayDelegate { - public: - virtual ~NativeDisplayDelegate() {} - - // Initializes the XRandR extension, saving the base event ID to - // |event_base|. - virtual void InitXRandRExtension(int* event_base) = 0; - - // Tells XRandR to update its configuration in response to |event|, an - // RRScreenChangeNotify event. - virtual void UpdateXRandRConfiguration(const base::NativeEvent& event) = 0; - - // Grabs the X server and refreshes XRandR-related resources. While - // the server is grabbed, other clients are blocked. Must be balanced - // by a call to UngrabServer(). - virtual void GrabServer() = 0; - - // Ungrabs the server and frees XRandR-related resources. - virtual void UngrabServer() = 0; - - // Flushes all pending requests and waits for replies. - virtual void SyncWithServer() = 0; - - // Sets the window's background color to |color_argb|. - virtual void SetBackgroundColor(uint32 color_argb) = 0; - - // Enables DPMS and forces it to the "on" state. - virtual void ForceDPMSOn() = 0; - - // Returns information about the current outputs. This method may block for - // 60 milliseconds or more. The returned outputs are not fully initialized; - // the rest of the work happens in - // OutputConfigurator::UpdateCachedOutputs(). - virtual std::vector<OutputSnapshot> GetOutputs() = 0; - - // Adds |mode| to |output|. - virtual void AddOutputMode(RROutput output, RRMode mode) = 0; - - // Calls XRRSetCrtcConfig() with the given options but some of our default - // output count and rotation arguments. Returns true on success. - virtual bool ConfigureCrtc(RRCrtc crtc, - RRMode mode, - RROutput output, - int x, - int y) = 0; - - // Called to set the frame buffer (underlying XRR "screen") size. Has - // a side-effect of disabling all CRTCs. - virtual void CreateFrameBuffer( - int width, - int height, - const std::vector<OutputConfigurator::OutputSnapshot>& outputs) = 0; - - // Gets HDCP state of output. - virtual bool GetHDCPState(RROutput id, ui::HDCPState* state) = 0; - - // Sets HDCP state of output. - virtual bool SetHDCPState(RROutput id, ui::HDCPState state) = 0; - }; - class TouchscreenDelegate { public: virtual ~TouchscreenDelegate() {} @@ -242,24 +180,14 @@ class CHROMEOS_EXPORT OutputConfigurator // Helper class used by tests. class TestApi { public: - TestApi(OutputConfigurator* configurator, int xrandr_event_base) - : configurator_(configurator), - xrandr_event_base_(xrandr_event_base) {} + TestApi(OutputConfigurator* configurator) + : configurator_(configurator) {} ~TestApi() {} const std::vector<OutputSnapshot>& cached_outputs() const { return configurator_->cached_outputs_; } - // Dispatches an RRScreenChangeNotify event to |configurator_|. - void SendScreenChangeEvent(); - - // Dispatches an RRNotify_OutputChange event to |configurator_|. - void SendOutputChangeEvent(RROutput output, - RRCrtc crtc, - RRMode mode, - bool connected); - // If |configure_timer_| is started, stops the timer, runs // ConfigureOutputs(), and returns true; returns false otherwise. bool TriggerConfigureTimeout(); @@ -267,8 +195,6 @@ class CHROMEOS_EXPORT OutputConfigurator private: OutputConfigurator* configurator_; // not owned - int xrandr_event_base_; - DISALLOW_COPY_AND_ASSIGN(TestApi); }; @@ -348,17 +274,8 @@ class CHROMEOS_EXPORT OutputConfigurator // current set of connected outputs). bool SetDisplayMode(ui::OutputState new_state); - // Called when an RRNotify event is received. The implementation is - // interested in the cases of RRNotify events which correspond to output - // add/remove events. Note that Output add/remove events are sent in response - // to our own reconfiguration operations so spurious events are common. - // Spurious events will have no effect. - virtual uint32_t Dispatch(const base::NativeEvent& event) OVERRIDE; - - // Overridden from base::MessagePumpObserver: - virtual base::EventStatus WillProcessEvent( - const base::NativeEvent& event) OVERRIDE; - virtual void DidProcessEvent(const base::NativeEvent& event) OVERRIDE; + // NativeDisplayDelegate::Observer overrides: + virtual void OnConfigurationChanged() OVERRIDE; void AddObserver(Observer* observer); void RemoveObserver(Observer* observer); @@ -376,10 +293,6 @@ class CHROMEOS_EXPORT OutputConfigurator return mirrored_display_area_ratio_map_; } - // Configure outputs with |kConfigureDelayMs| delay, - // so that time-consuming ConfigureOutputs() won't be called multiple times. - void ScheduleConfigureOutputs(); - // Registers a client for output protection and requests a client id. Returns // 0 if requesting failed. OutputProtectionClientId RegisterOutputProtectionClient(); @@ -506,10 +419,6 @@ class CHROMEOS_EXPORT OutputConfigurator // configuration to immediately fail without changing the state. bool configure_display_; - // The base of the event numbers used to represent XRandr events used in - // decoding events regarding output add/remove. - int xrandr_event_base_; - // The current display state. ui::OutputState output_state_; diff --git a/chromeos/display/output_configurator_unittest.cc b/chromeos/display/output_configurator_unittest.cc index b269c7b..4258dd1 100644 --- a/chromeos/display/output_configurator_unittest.cc +++ b/chromeos/display/output_configurator_unittest.cc @@ -14,6 +14,7 @@ #include "base/compiler_specific.h" #include "base/message_loop/message_loop.h" #include "base/strings/stringprintf.h" +#include "chromeos/display/native_display_delegate.h" #include "testing/gtest/include/gtest/gtest.h" namespace chromeos { @@ -23,7 +24,6 @@ namespace { // Strings returned by TestNativeDisplayDelegate::GetActionsAndClear() to // describe various actions that were performed. const char kInitXRandR[] = "init"; -const char kUpdateXRandR[] = "update"; const char kGrab[] = "grab"; const char kUngrab[] = "ungrab"; const char kSync[] = "sync"; @@ -45,7 +45,7 @@ std::string GetAddOutputModeAction(RROutput output, RRMode mode) { return base::StringPrintf("add_mode(output=%lu,mode=%lu)", output, mode); } -// Returns a string describing a TestNativeDisplayDelegate::ConfigureCrtc() +// Returns a string describing a TestNativeDisplayDelegate::Configure() // call. std::string GetCrtcAction(RRCrtc crtc, int x, @@ -154,11 +154,8 @@ class TestTouchscreenDelegate : public OutputConfigurator::TouchscreenDelegate { DISALLOW_COPY_AND_ASSIGN(TestTouchscreenDelegate); }; -class TestNativeDisplayDelegate - : public OutputConfigurator::NativeDisplayDelegate { +class TestNativeDisplayDelegate : public NativeDisplayDelegate { public: - static const int kXRandREventBase = 10; - // Ownership of |log| remains with the caller. explicit TestNativeDisplayDelegate(ActionLogger* log) : max_configurable_pixels_(0), @@ -181,13 +178,8 @@ class TestNativeDisplayDelegate void set_hdcp_state(ui::HDCPState state) { hdcp_state_ = state; } // OutputConfigurator::Delegate overrides: - virtual void InitXRandRExtension(int* event_base) OVERRIDE { + virtual void Initialize() OVERRIDE { log_->AppendAction(kInitXRandR); - *event_base = kXRandREventBase; - } - virtual void UpdateXRandRConfiguration(const base::NativeEvent& event) - OVERRIDE { - log_->AppendAction(kUpdateXRandR); } virtual void GrabServer() OVERRIDE { log_->AppendAction(kGrab); } virtual void UngrabServer() OVERRIDE { log_->AppendAction(kUngrab); } @@ -200,20 +192,21 @@ class TestNativeDisplayDelegate OVERRIDE { return outputs_; } - virtual void AddOutputMode(RROutput output, RRMode mode) OVERRIDE { - log_->AppendAction(GetAddOutputModeAction(output, mode)); + virtual void AddMode(const OutputConfigurator::OutputSnapshot& output, + RRMode mode) OVERRIDE { + log_->AppendAction(GetAddOutputModeAction(output.output, mode)); } - virtual bool ConfigureCrtc(RRCrtc crtc, - RRMode mode, - RROutput output, - int x, - int y) OVERRIDE { - log_->AppendAction(GetCrtcAction(crtc, x, y, mode, output)); + virtual bool Configure(const OutputConfigurator::OutputSnapshot& output, + RRMode mode, + int x, + int y) OVERRIDE { + log_->AppendAction(GetCrtcAction(output.crtc, x, y, mode, output.output)); if (max_configurable_pixels_ == 0) return true; - OutputConfigurator::OutputSnapshot* snapshot = GetOutputFromId(output); + OutputConfigurator::OutputSnapshot* snapshot = GetOutputFromId( + output.output); if (!snapshot) return false; @@ -235,16 +228,22 @@ class TestNativeDisplayDelegate outputs.size() >= 1 ? outputs[0].crtc : 0, outputs.size() >= 2 ? outputs[1].crtc : 0)); } - virtual bool GetHDCPState(RROutput id, ui::HDCPState* state) OVERRIDE { + virtual bool GetHDCPState(const OutputConfigurator::OutputSnapshot& output, + ui::HDCPState* state) OVERRIDE { *state = hdcp_state_; return true; } - virtual bool SetHDCPState(RROutput id, ui::HDCPState state) OVERRIDE { - log_->AppendAction(GetSetHDCPStateAction(id, state)); + virtual bool SetHDCPState(const OutputConfigurator::OutputSnapshot& output, + ui::HDCPState state) OVERRIDE { + log_->AppendAction(GetSetHDCPStateAction(output.output, state)); return true; } + virtual void AddObserver(NativeDisplayObserver* observer) OVERRIDE {} + + virtual void RemoveObserver(NativeDisplayObserver* observer) OVERRIDE {} + private: OutputConfigurator::OutputSnapshot* GetOutputFromId(RROutput output_id) { for (unsigned int i = 0; i < outputs_.size(); i++) { @@ -258,10 +257,10 @@ class TestNativeDisplayDelegate std::vector<OutputConfigurator::OutputSnapshot> outputs_; // |max_configurable_pixels_| represents the maximum number of pixels that - // ConfigureCrtc will support. Tests can use this to force ConfigureCrtc + // Configure will support. Tests can use this to force Configure // to fail if attempting to set a resolution that is higher than what // a device might support under a given circumstance. - // A value of 0 means that no limit is enforced and ConfigureCrtc will + // A value of 0 means that no limit is enforced and Configure will // return success regardless of the resolution. int max_configurable_pixels_; @@ -384,7 +383,7 @@ class OutputConfiguratorTest : public testing::Test { OutputConfiguratorTest() : observer_(&configurator_), - test_api_(&configurator_, TestNativeDisplayDelegate::kXRandREventBase) { + test_api_(&configurator_) { } virtual ~OutputConfiguratorTest() {} @@ -393,8 +392,7 @@ class OutputConfiguratorTest : public testing::Test { native_display_delegate_ = new TestNativeDisplayDelegate(log_.get()); configurator_.SetNativeDisplayDelegateForTesting( - scoped_ptr<OutputConfigurator::NativeDisplayDelegate>( - native_display_delegate_)); + scoped_ptr<NativeDisplayDelegate>(native_display_delegate_)); touchscreen_delegate_ = new TestTouchscreenDelegate(log_.get()); configurator_.SetTouchscreenDelegateForTesting( @@ -454,13 +452,7 @@ class OutputConfiguratorTest : public testing::Test { native_display_delegate_->set_outputs(outputs); if (send_events) { - test_api_.SendScreenChangeEvent(); - for (size_t i = 0; i < arraysize(outputs_); ++i) { - const OutputConfigurator::OutputSnapshot output = outputs_[i]; - bool connected = i < num_outputs; - test_api_.SendOutputChangeEvent( - output.output, output.crtc, output.current_mode, connected); - } + configurator_.OnConfigurationChanged(); test_api_.TriggerConfigureTimeout(); } } @@ -585,7 +577,6 @@ TEST_F(OutputConfiguratorTest, ConnectSecondOutput) { kSmallModeHeight + OutputConfigurator::kVerticalGap + kBigModeHeight; EXPECT_EQ( JoinActions( - kUpdateXRandR, kGrab, GetFramebufferAction( kBigModeWidth, kDualHeight, outputs_[0].crtc, outputs_[1].crtc) @@ -627,7 +618,6 @@ TEST_F(OutputConfiguratorTest, ConnectSecondOutput) { UpdateOutputs(1, true); EXPECT_EQ( JoinActions( - kUpdateXRandR, kGrab, GetFramebufferAction( kSmallModeWidth, kSmallModeHeight, outputs_[0].crtc, 0).c_str(), @@ -645,7 +635,6 @@ TEST_F(OutputConfiguratorTest, ConnectSecondOutput) { UpdateOutputs(2, true); EXPECT_EQ( JoinActions( - kUpdateXRandR, kGrab, GetFramebufferAction( kBigModeWidth, kDualHeight, outputs_[0].crtc, outputs_[1].crtc) @@ -689,7 +678,6 @@ TEST_F(OutputConfiguratorTest, ConnectSecondOutput) { UpdateOutputs(1, true); EXPECT_EQ( JoinActions( - kUpdateXRandR, kGrab, GetFramebufferAction( kSmallModeWidth, kSmallModeHeight, outputs_[0].crtc, 0).c_str(), @@ -710,7 +698,6 @@ TEST_F(OutputConfiguratorTest, SetDisplayPower) { UpdateOutputs(2, true); EXPECT_EQ( JoinActions( - kUpdateXRandR, kGrab, GetFramebufferAction(kSmallModeWidth, kSmallModeHeight, @@ -800,7 +787,6 @@ TEST_F(OutputConfiguratorTest, SetDisplayPower) { kSmallModeHeight + OutputConfigurator::kVerticalGap + kBigModeHeight; EXPECT_EQ( JoinActions( - kUpdateXRandR, kGrab, GetFramebufferAction( kBigModeWidth, kDualHeight, outputs_[0].crtc, outputs_[1].crtc) @@ -959,7 +945,6 @@ TEST_F(OutputConfiguratorTest, SuspendAndResume) { UpdateOutputs(2, true); EXPECT_EQ( JoinActions( - kUpdateXRandR, kGrab, GetFramebufferAction(kSmallModeWidth, kSmallModeHeight, @@ -1031,7 +1016,6 @@ TEST_F(OutputConfiguratorTest, Headless) { UpdateOutputs(1, true); EXPECT_EQ( JoinActions( - kUpdateXRandR, kGrab, GetFramebufferAction( kBigModeWidth, kBigModeHeight, outputs_[0].crtc, 0).c_str(), @@ -1119,144 +1103,6 @@ TEST_F(OutputConfiguratorTest, GetOutputStateForDisplaysWithId) { EXPECT_EQ(ui::OUTPUT_STATE_DUAL_MIRROR, configurator_.output_state()); } -TEST_F(OutputConfiguratorTest, AvoidUnnecessaryProbes) { - InitWithSingleOutput(); - - // X sends several events just after the configurator starts. Check that - // the output change events don't trigger an additional probe, which can - // block the UI thread. - test_api_.SendScreenChangeEvent(); - EXPECT_EQ(kUpdateXRandR, log_->GetActionsAndClear()); - - test_api_.SendOutputChangeEvent( - outputs_[0].output, outputs_[0].crtc, outputs_[0].current_mode, true); - test_api_.SendOutputChangeEvent( - outputs_[1].output, outputs_[1].crtc, outputs_[1].current_mode, false); - EXPECT_FALSE(test_api_.TriggerConfigureTimeout()); - EXPECT_EQ(kNoActions, log_->GetActionsAndClear()); - - // Send an event stating that the second output is connected and check - // that it gets updated. - state_controller_.set_state(ui::OUTPUT_STATE_DUAL_MIRROR); - UpdateOutputs(2, false); - test_api_.SendOutputChangeEvent( - outputs_[1].output, outputs_[1].crtc, outputs_[1].current_mode, true); - EXPECT_TRUE(test_api_.TriggerConfigureTimeout()); - EXPECT_EQ( - JoinActions( - kGrab, - GetFramebufferAction(kSmallModeWidth, - kSmallModeHeight, - outputs_[0].crtc, - outputs_[1].crtc).c_str(), - GetCrtcAction( - outputs_[0].crtc, 0, 0, kSmallModeId, outputs_[0].output).c_str(), - GetCrtcAction( - outputs_[1].crtc, 0, 0, kSmallModeId, outputs_[1].output).c_str(), - kUngrab, - NULL), - log_->GetActionsAndClear()); - - // An event about the second output changing modes should trigger another - // reconfigure. - test_api_.SendOutputChangeEvent( - outputs_[1].output, outputs_[1].crtc, outputs_[1].native_mode, true); - EXPECT_TRUE(test_api_.TriggerConfigureTimeout()); - EXPECT_EQ( - JoinActions( - kGrab, - GetFramebufferAction(kSmallModeWidth, - kSmallModeHeight, - outputs_[0].crtc, - outputs_[1].crtc).c_str(), - GetCrtcAction( - outputs_[0].crtc, 0, 0, kSmallModeId, outputs_[0].output).c_str(), - GetCrtcAction( - outputs_[1].crtc, 0, 0, kSmallModeId, outputs_[1].output).c_str(), - kUngrab, - NULL), - log_->GetActionsAndClear()); - - // Disconnect the second output. - UpdateOutputs(1, true); - EXPECT_EQ( - JoinActions( - kUpdateXRandR, - kGrab, - GetFramebufferAction( - kSmallModeWidth, kSmallModeHeight, outputs_[0].crtc, 0).c_str(), - GetCrtcAction( - outputs_[0].crtc, 0, 0, kSmallModeId, outputs_[0].output).c_str(), - kUngrab, - NULL), - log_->GetActionsAndClear()); - - // An additional event about the second output being disconnected should - // be ignored. - test_api_.SendOutputChangeEvent( - outputs_[1].output, outputs_[1].crtc, outputs_[1].current_mode, false); - EXPECT_FALSE(test_api_.TriggerConfigureTimeout()); - EXPECT_EQ(kNoActions, log_->GetActionsAndClear()); - - // Lower the limit for which the delegate will succeed, which should result - // in the second output sticking with its native mode. - native_display_delegate_->set_max_configurable_pixels(1); - UpdateOutputs(2, true); - EXPECT_EQ( - JoinActions( - kUpdateXRandR, - kGrab, - GetFramebufferAction(kSmallModeWidth, - kSmallModeHeight, - outputs_[0].crtc, - outputs_[1].crtc).c_str(), - GetCrtcAction( - outputs_[0].crtc, 0, 0, kSmallModeId, outputs_[0].output).c_str(), - GetCrtcAction( - outputs_[1].crtc, 0, 0, kSmallModeId, outputs_[1].output).c_str(), - GetFramebufferAction(kBigModeWidth, - kSmallModeHeight + kBigModeHeight + - OutputConfigurator::kVerticalGap, - outputs_[0].crtc, - outputs_[1].crtc).c_str(), - GetCrtcAction( - outputs_[0].crtc, 0, 0, kSmallModeId, outputs_[0].output).c_str(), - GetCrtcAction(outputs_[1].crtc, - 0, - kSmallModeHeight + OutputConfigurator::kVerticalGap, - kBigModeId, - outputs_[1].output).c_str(), - GetCrtcAction(outputs_[1].crtc, - 0, - kSmallModeHeight + OutputConfigurator::kVerticalGap, - kSmallModeId, - outputs_[1].output).c_str(), - kUngrab, - NULL), - log_->GetActionsAndClear()); - - // A change event reporting a mode change on the second output should - // trigger another reconfigure. - native_display_delegate_->set_max_configurable_pixels(0); - test_api_.SendOutputChangeEvent( - outputs_[1].output, outputs_[1].crtc, outputs_[1].mirror_mode, true); - EXPECT_TRUE(test_api_.TriggerConfigureTimeout()); - EXPECT_EQ( - JoinActions( - kGrab, - GetFramebufferAction(kSmallModeWidth, - kSmallModeHeight, - outputs_[0].crtc, - outputs_[1].crtc).c_str(), - GetCrtcAction( - outputs_[0].crtc, 0, 0, kSmallModeId, outputs_[0].output).c_str(), - GetCrtcAction( - outputs_[1].crtc, 0, 0, kSmallModeId, outputs_[1].output).c_str(), - kUngrab, - NULL), - log_->GetActionsAndClear()); -} - TEST_F(OutputConfiguratorTest, UpdateCachedOutputsEvenAfterFailure) { InitWithSingleOutput(); const std::vector<OutputConfigurator::OutputSnapshot>* cached = @@ -1511,7 +1357,6 @@ TEST_F(OutputConfiguratorTest, HandleConfigureCrtcFailure) { EXPECT_EQ( JoinActions( - kUpdateXRandR, kGrab, GetFramebufferAction(2560, 1600, outputs_[0].crtc, 0).c_str(), GetCrtcAction(outputs_[0].crtc, 0, 0, kFirstMode, outputs_[0].output) @@ -1536,7 +1381,6 @@ TEST_F(OutputConfiguratorTest, HandleConfigureCrtcFailure) { EXPECT_EQ( JoinActions( - kUpdateXRandR, kGrab, GetFramebufferAction(outputs_[0].mode_infos[kFirstMode].width, outputs_[0].mode_infos[kFirstMode].height, |