chromeos: Add testing support code to OutputConfigurator.

This makes a few changes to OutputConfigurator to support
upcoming tests:
- Adds a TestApi class for dispatching XRandR events
- Pulls the kVerticalGap constant out to be a static member
- Adds a SetDelegateForTesting() method
- Moves D-Bus code into the delegate

It also fixes a bug that was probably introduced by one of
my recent changes where the gap wasn't being applied between
outputs.  A test that I'm writing caught this. :-)

BUG=225536


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

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

4 files changed, 128 insertions, 57 deletions

diff --git a/chromeos/display/output_configurator.cc b/chromeos/display/output_configurator.cc
index a07a9de..4556442 100644
--- a/chromeos/display/output_configurator.cc
+++ b/chromeos/display/output_configurator.cc
@@ -12,8 +12,6 @@
 #include "base/logging.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/time.h"
-#include "chromeos/dbus/dbus_thread_manager.h"
-#include "chromeos/dbus/power_manager_client.h"
 #include "chromeos/display/real_output_configurator_delegate.h"
 
 namespace chromeos {
@@ -28,15 +26,6 @@ const char kInternal_eDP[] = "eDP";
 // in |Dispatch()|.
 const int64 kConfigureDelayMs = 500;
 
-// Gap between screens so cursor at bottom of active display doesn't partially
-// appear on top of inactive display. Higher numbers guard against larger
-// cursors, but also waste more memory.
-// For simplicity, this is hard-coded to 60 to avoid the complexity of always
-// determining the DPI of the screen and rationalizing which screen we need to
-// use for the DPI calculation.
-// See crbug.com/130188 for initial discussion.
-const int kVerticalGap = 60;
-
 // Returns a string describing |state|.
 std::string DisplayPowerStateToString(DisplayPowerState state) {
   switch (state) {
@@ -95,7 +84,8 @@ OutputState InferCurrentState(
             (secondary_mode == None);
         if (primary_native && secondary_native) {
           // Just check the relative locations.
-          int secondary_offset = outputs[0].height + kVerticalGap;
+          int secondary_offset = outputs[0].height +
+              OutputConfigurator::kVerticalGap;
           if (outputs[0].y == 0 && outputs[1].y == secondary_offset) {
             state = STATE_DUAL_EXTENDED;
           } else {
@@ -166,6 +156,30 @@ OutputConfigurator::CrtcConfig::CrtcConfig(RRCrtc crtc,
       mode(mode),
       output(output) {}
 
+bool OutputConfigurator::TestApi::SendOutputChangeEvents(bool connected) {
+  XRRScreenChangeNotifyEvent screen_event;
+  memset(&screen_event, 0, sizeof(screen_event));
+  screen_event.type = xrandr_event_base_ + RRScreenChangeNotify;
+  configurator_->Dispatch(
+      reinterpret_cast<const base::NativeEvent>(&screen_event));
+
+  XRROutputChangeNotifyEvent notify_event;
+  memset(&notify_event, 0, sizeof(notify_event));
+  notify_event.type = xrandr_event_base_ + RRNotify;
+  notify_event.subtype = RRNotify_OutputChange;
+  notify_event.connection = connected ? RR_Connected : RR_Disconnected;
+  configurator_->Dispatch(
+      reinterpret_cast<const base::NativeEvent>(&notify_event));
+
+  if (!configurator_->configure_timer_->IsRunning()) {
+    LOG(ERROR) << "ConfigureOutputs() timer not running";
+    return false;
+  }
+
+  configurator_->ConfigureOutputs();
+  return true;
+}
+
 // static
 bool OutputConfigurator::IsInternalOutputName(const std::string& name) {
   return name.find(kInternal_LVDS) == 0 || name.find(kInternal_eDP) == 0;
@@ -173,7 +187,6 @@ bool OutputConfigurator::IsInternalOutputName(const std::string& name) {
 
 OutputConfigurator::OutputConfigurator()
     : state_controller_(NULL),
-      delegate_(new RealOutputConfiguratorDelegate()),
       configure_display_(base::chromeos::IsRunningOnChromeOS()),
       connected_output_count_(0),
       xrandr_event_base_(0),
@@ -183,11 +196,20 @@ OutputConfigurator::OutputConfigurator()
 
 OutputConfigurator::~OutputConfigurator() {}
 
+void OutputConfigurator::SetDelegateForTesting(
+    scoped_ptr<Delegate> delegate) {
+  delegate_ = delegate.Pass();
+  configure_display_ = true;
+}
+
 void OutputConfigurator::Init(bool is_panel_fitting_enabled,
                               uint32 background_color_argb) {
   if (!configure_display_)
     return;
 
+  if (!delegate_)
+    delegate_.reset(new RealOutputConfiguratorDelegate());
+
   // Cache the initial output state.
   delegate_->SetPanelFittingEnabled(is_panel_fitting_enabled);
   delegate_->GrabServer();
@@ -198,8 +220,10 @@ void OutputConfigurator::Init(bool is_panel_fitting_enabled,
 }
 
 void OutputConfigurator::Start() {
+  if (!configure_display_)
+    return;
+
   delegate_->GrabServer();
-  // Detect our initial state.
   std::vector<OutputSnapshot> outputs = delegate_->GetOutputs();
   connected_output_count_ = outputs.size();
 
@@ -218,12 +242,8 @@ void OutputConfigurator::Start() {
   // Force the DPMS on chrome startup as the driver doesn't always detect
   // that all displays are on when signing out.
   delegate_->ForceDPMSOn();
-
-  // Relinquish X resources.
   delegate_->UngrabServer();
-
-  chromeos::DBusThreadManager::Get()->GetPowerManagerClient()->
-      SetIsProjecting(is_projecting);
+  delegate_->SendProjectingStateToPowerManager(is_projecting);
 }
 
 void OutputConfigurator::Stop() {
@@ -232,11 +252,11 @@ void OutputConfigurator::Stop() {
 
 bool OutputConfigurator::SetDisplayPower(DisplayPowerState power_state,
                                          int flags) {
-  VLOG(1) << "SetDisplayPower: power_state="
-          << DisplayPowerStateToString(power_state) << " flags=" << flags;
-
   if (!configure_display_)
     return false;
+
+  VLOG(1) << "SetDisplayPower: power_state="
+          << DisplayPowerStateToString(power_state) << " flags=" << flags;
   if (power_state == power_state_ && !(flags & kSetDisplayPowerForceProbe))
     return true;
 
@@ -262,6 +282,9 @@ bool OutputConfigurator::SetDisplayPower(DisplayPowerState power_state,
 }
 
 bool OutputConfigurator::SetDisplayMode(OutputState new_state) {
+  if (!configure_display_)
+    return false;
+
   if (output_state_ == STATE_INVALID ||
       output_state_ == STATE_HEADLESS ||
       output_state_ == STATE_SINGLE)
@@ -287,14 +310,17 @@ bool OutputConfigurator::SetDisplayMode(OutputState new_state) {
 }
 
 bool OutputConfigurator::Dispatch(const base::NativeEvent& event) {
-  if (event->type - xrandr_event_base_ == RRScreenChangeNotify)
+  if (!configure_display_)
+    return true;
+
+  if (event->type - xrandr_event_base_ == RRScreenChangeNotify) {
     delegate_->UpdateXRandRConfiguration(event);
-  // Ignore this event if the Xrandr extension isn't supported, or
-  // the device is being shutdown.
-  if (!configure_display_ ||
-      (event->type - xrandr_event_base_ != RRNotify)) {
     return true;
   }
+
+  if (event->type - xrandr_event_base_ != RRNotify)
+    return true;
+
   XEvent* xevent = static_cast<XEvent*>(event);
   XRRNotifyEvent* notify_event =
       reinterpret_cast<XRRNotifyEvent*>(xevent);
@@ -319,38 +345,9 @@ bool OutputConfigurator::Dispatch(const base::NativeEvent& event) {
     }
   }
 
-  // Ignore the case of RR_UnknownConnection.
   return true;
 }
 
-void OutputConfigurator::ConfigureOutputs() {
-  configure_timer_.reset();
-
-  delegate_->GrabServer();
-  std::vector<OutputSnapshot> outputs = delegate_->GetOutputs();
-  int new_output_count = outputs.size();
-  // Don't skip even if the output counts didn't change because
-  // a display might have been swapped during the suspend.
-  connected_output_count_ = new_output_count;
-  OutputState new_state = GetNextState(outputs);
-  // 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(new_state, power_state_, outputs);
-  bool is_projecting = IsProjecting(outputs);
-  delegate_->UngrabServer();
-
-  if (success) {
-    output_state_ = new_state;
-    NotifyOnDisplayChanged();
-  } else {
-    FOR_EACH_OBSERVER(
-        Observer, observers_, OnDisplayModeChangeFailed(new_state));
-  }
-  chromeos::DBusThreadManager::Get()->GetPowerManagerClient()->
-      SetIsProjecting(is_projecting);
-}
-
 void OutputConfigurator::AddObserver(Observer* observer) {
   observers_.AddObserver(observer);
 }
@@ -382,6 +379,33 @@ void OutputConfigurator::ResumeDisplays() {
   SetDisplayPower(power_state_, kSetDisplayPowerForceProbe);
 }
 
+void OutputConfigurator::ConfigureOutputs() {
+  configure_timer_.reset();
+
+  delegate_->GrabServer();
+  std::vector<OutputSnapshot> outputs = delegate_->GetOutputs();
+  int new_output_count = outputs.size();
+  // Don't skip even if the output counts didn't change because
+  // a display might have been swapped during the suspend.
+  connected_output_count_ = new_output_count;
+  OutputState new_state = GetNextState(outputs);
+  // 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(new_state, power_state_, outputs);
+  bool is_projecting = IsProjecting(outputs);
+  delegate_->UngrabServer();
+
+  if (success) {
+    output_state_ = new_state;
+    NotifyOnDisplayChanged();
+  } else {
+    FOR_EACH_OBSERVER(
+        Observer, observers_, OnDisplayModeChangeFailed(new_state));
+  }
+  delegate_->SendProjectingStateToPowerManager(is_projecting);
+}
+
 void OutputConfigurator::NotifyOnDisplayChanged() {
   FOR_EACH_OBSERVER(Observer, observers_, OnDisplayModeChanged());
 }
@@ -471,7 +495,7 @@ bool OutputConfigurator::EnterState(
           }
 
           configs[i] = CrtcConfig(
-              outputs[i].crtc, 0, height,
+              outputs[i].crtc, 0, (height ? height + kVerticalGap : 0),
               output_power[i] ? outputs[i].native_mode : None,
               outputs[i].output);
 
diff --git a/chromeos/display/output_configurator.h b/chromeos/display/output_configurator.h
index c59c076..bac694a 100644
--- a/chromeos/display/output_configurator.h
+++ b/chromeos/display/output_configurator.h
@@ -177,6 +177,31 @@ class CHROMEOS_EXPORT OutputConfigurator : public MessageLoop::Dispatcher {
     // size for the corresponding offset.
     virtual void ConfigureCTM(int touch_device_id,
                               const CoordinateTransformation& ctm) = 0;
+
+    // Sends a D-Bus message to the power manager telling it that the
+    // machine is or is not projecting.
+    virtual void SendProjectingStateToPowerManager(bool projecting) = 0;
+  };
+
+  // Helper class used by tests.
+  class TestApi {
+   public:
+    TestApi(OutputConfigurator* configurator, int xrandr_event_base)
+        : configurator_(configurator),
+          xrandr_event_base_(xrandr_event_base) {}
+    ~TestApi() {}
+
+    // Dispatches RRScreenChangeNotify and RRNotify_OutputChange events to
+    // |configurator_| and runs ConfigureOutputs().  Returns false if
+    // |configure_timer_| wasn't started.
+    bool SendOutputChangeEvents(bool connected);
+
+   private:
+    OutputConfigurator* configurator_;  // not owned
+
+    int xrandr_event_base_;
+
+    DISALLOW_COPY_AND_ASSIGN(TestApi);
   };
 
   // Flags that can be passed to SetDisplayPower().
@@ -187,6 +212,15 @@ class CHROMEOS_EXPORT OutputConfigurator : public MessageLoop::Dispatcher {
   // only connected display is external.
   static const int kSetDisplayPowerOnlyIfSingleInternalDisplay = 1 << 1;
 
+  // Gap between screens so cursor at bottom of active display doesn't
+  // partially appear on top of inactive display. Higher numbers guard
+  // against larger cursors, but also waste more memory.
+  // For simplicity, this is hard-coded to avoid the complexity of always
+  // determining the DPI of the screen and rationalizing which screen we
+  // need to use for the DPI calculation.
+  // See crbug.com/130188 for initial discussion.
+  static const int kVerticalGap = 60;
+
   // Returns true if an output named |name| is an internal display.
   static bool IsInternalOutputName(const std::string& name);
 
@@ -206,6 +240,10 @@ class CHROMEOS_EXPORT OutputConfigurator : public MessageLoop::Dispatcher {
     state_controller_ = controller;
   }
 
+  // Replaces |delegate_| with |delegate| and sets |configure_display_| to
+  // true.  Should be called before Init().
+  void SetDelegateForTesting(scoped_ptr<Delegate> delegate);
+
   // Initialization, must be called right after constructor.
   // |is_panel_fitting_enabled| indicates hardware panel fitting support.
   // If |background_color_argb| is non zero and there are multiple displays,
diff --git a/chromeos/display/real_output_configurator_delegate.cc b/chromeos/display/real_output_configurator_delegate.cc
index 46df404..c94a262 100644
--- a/chromeos/display/real_output_configurator_delegate.cc
+++ b/chromeos/display/real_output_configurator_delegate.cc
@@ -15,6 +15,8 @@
 
 #include "base/logging.h"
 #include "base/message_pump_aurax11.h"
+#include "chromeos/dbus/dbus_thread_manager.h"
+#include "chromeos/dbus/power_manager_client.h"
 
 namespace chromeos {
 
@@ -314,6 +316,12 @@ void RealOutputConfiguratorDelegate::ConfigureCTM(
   XIFreeDeviceInfo(info);
 }
 
+void RealOutputConfiguratorDelegate::SendProjectingStateToPowerManager(
+    bool projecting) {
+  chromeos::DBusThreadManager::Get()->GetPowerManagerClient()->
+      SetIsProjecting(projecting);
+}
+
 void RealOutputConfiguratorDelegate::DestroyUnusedCrtcs(
     OutputConfigurator::CrtcConfig* config1,
     OutputConfigurator::CrtcConfig* config2) {
diff --git a/chromeos/display/real_output_configurator_delegate.h b/chromeos/display/real_output_configurator_delegate.h
index 8ad68c8..b06f77d 100644
--- a/chromeos/display/real_output_configurator_delegate.h
+++ b/chromeos/display/real_output_configurator_delegate.h
@@ -52,6 +52,7 @@ class RealOutputConfiguratorDelegate : public OutputConfigurator::Delegate {
   virtual void ConfigureCTM(
       int touch_device_id,
       const OutputConfigurator::CoordinateTransformation& ctm) OVERRIDE;
+  virtual void SendProjectingStateToPowerManager(bool projecting) OVERRIDE;
 
  private:
   // Destroys unused CRTCs and parks used CRTCs in a way which allows a