chromeos: Add testing support code to OutputConfigurator.

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
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=193155

[oshima, 2013-04-08 17:29:15]

https://codereview.chromium.org/13756002/diff/2001/chromeos/display/output_co...
File chromeos/display/output_configurator.cc (right):

https://codereview.chromium.org/13756002/diff/2001/chromeos/display/output_co...
chromeos/display/output_configurator.cc:201: delegate_.swap(delegate);
delegate_ = delegate.Pass();

is more common nowadays.

https://codereview.chromium.org/13756002/diff/2001/chromeos/display/output_co...
chromeos/display/output_configurator.cc:314: if (delegate_)
This is only place you check if it's NULL. How this can happen?

https://codereview.chromium.org/13756002/diff/2001/chromeos/display/output_co...
File chromeos/display/output_configurator.h (right):

https://codereview.chromium.org/13756002/diff/2001/chromeos/display/output_co...
chromeos/display/output_configurator.h:205: };
can you move testapi to chromeos/test ? I believe one of
ideas behind test API class to avoid linking test code.

[Daniel Erat, 2013-04-08 21:47:53]

https://codereview.chromium.org/13756002/diff/2001/chromeos/display/output_co...
File chromeos/display/output_configurator.cc (right):

https://codereview.chromium.org/13756002/diff/2001/chromeos/display/output_co...
chromeos/display/output_configurator.cc:201: delegate_.swap(delegate);
On 2013/04/08 17:29:15, oshima wrote:
> delegate_ = delegate.Pass();
> 
> is more common nowadays.

Done.

https://codereview.chromium.org/13756002/diff/2001/chromeos/display/output_co...
chromeos/display/output_configurator.cc:314: if (delegate_)
On 2013/04/08 17:29:15, oshima wrote:
> This is only place you check if it's NULL. How this can happen?

All of the other members return early on !configure_display_.  I'm preserving
this method's previous behavior of updating Xlib even if configure_display_ is
false, but I'll move the !configure_display_ test earlier if you think this
isn't needed.

https://codereview.chromium.org/13756002/diff/2001/chromeos/display/output_co...
File chromeos/display/output_configurator.h (right):

https://codereview.chromium.org/13756002/diff/2001/chromeos/display/output_co...
chromeos/display/output_configurator.h:205: };
On 2013/04/08 17:29:15, oshima wrote:
> can you move testapi to chromeos/test ? I believe one of
> ideas behind test API class to avoid linking test code.

There are a lot of existing examples of TestApi classes.  The big benefit is
that the class's implementation details stay within it instead of spilling out
to friends.  This class is tiny and I'd prefer to keep it here.

[oshima, 2013-04-08 23:21:44]

lgtm

https://codereview.chromium.org/13756002/diff/2001/chromeos/display/output_co...
File chromeos/display/output_configurator.cc (right):

https://codereview.chromium.org/13756002/diff/2001/chromeos/display/output_co...
chromeos/display/output_configurator.cc:314: if (delegate_)
On 2013/04/08 21:47:53, Daniel Erat wrote:
> On 2013/04/08 17:29:15, oshima wrote:
> > This is only place you check if it's NULL. How this can happen?
> 
> All of the other members return early on !configure_display_.  I'm preserving
> this method's previous behavior of updating Xlib even if configure_display_ is
> false, but I'll move the !configure_display_ test earlier if you think this
> isn't needed.

I see. I think it's safe to move after configure_display_ because it can happen
only when we're shutting down and syncing state isn't important at this stage.

https://codereview.chromium.org/13756002/diff/2001/chromeos/display/output_co...
File chromeos/display/output_configurator.h (right):

https://codereview.chromium.org/13756002/diff/2001/chromeos/display/output_co...
chromeos/display/output_configurator.h:205: };
On 2013/04/08 21:47:53, Daniel Erat wrote:
> On 2013/04/08 17:29:15, oshima wrote:
> > can you move testapi to chromeos/test ? I believe one of
> > ideas behind test API class to avoid linking test code.
> 
> There are a lot of existing examples of TestApi classes.  The big benefit is
> that the class's implementation details stay within it instead of spilling out
> to friends.  This class is tiny and I'd prefer to keep it here.

Ok, I was asked to separate TestApi in the past to avoid linking
test code, but I don't feel strongly about it given that it's small,
so you may keep it as is.

[Daniel Erat, 2013-04-09 00:21:34]

https://codereview.chromium.org/13756002/diff/2001/chromeos/display/output_co...
File chromeos/display/output_configurator.cc (right):

https://codereview.chromium.org/13756002/diff/2001/chromeos/display/output_co...
chromeos/display/output_configurator.cc:314: if (delegate_)
On 2013/04/08 23:21:44, oshima wrote:
> On 2013/04/08 21:47:53, Daniel Erat wrote:
> > On 2013/04/08 17:29:15, oshima wrote:
> > > This is only place you check if it's NULL. How this can happen?
> > 
> > All of the other members return early on !configure_display_.  I'm
preserving
> > this method's previous behavior of updating Xlib even if configure_display_
is
> > false, but I'll move the !configure_display_ test earlier if you think this
> > isn't needed.
> 
> I see. I think it's safe to move after configure_display_ because it can
happen
> only when we're shutting down and syncing state isn't important at this stage.

Done.

[commit-bot: I haz the power, 2013-04-09 14:14:51]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/derat@chromium.org/13756002/11001

[commit-bot: I haz the power, 2013-04-09 18:33:57]

Change committed as 193155