Handles the failures of output state change.

Handles the failures of output state change.

This is relevant to show the warning message for the failures, but
I want to introduce the error handling code at first.  This also
starts fade-in in failure.

BUG=149061
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=159823

[Jun Mukai, 2012-10-01 17:57:50]

oshima-san, can you take a look at this?  This is anyway necessary to show the
warning message of mirroring failure, but independent from its UI.

[oshima, 2012-10-01 21:44:02]

http://codereview.chromium.org/10989084/diff/1/chromeos/display/output_config...
File chromeos/display/output_configurator.cc (right):

http://codereview.chromium.org/10989084/diff/1/chromeos/display/output_config...
chromeos/display/output_configurator.cc:741: return false;
is it possible to return output_state_ == new_state,
and simply test if output_state_ has changed later?

[Jun Mukai, 2012-10-01 21:49:35]

http://codereview.chromium.org/10989084/diff/1/chromeos/display/output_config...
File chromeos/display/output_configurator.cc (right):

http://codereview.chromium.org/10989084/diff/1/chromeos/display/output_config...
chromeos/display/output_configurator.cc:741: return false;
On 2012/10/01 21:44:02, oshima wrote:
> is it possible to return output_state_ == new_state,
> and simply test if output_state_ has changed later?

The header said it "returns false if it was called in a single-head or headless
mode."  Did you intend to change this too?
I think changing this would be fine since the existing callers don't care its
return value anyway for now.

[oshima, 2012-10-01 22:16:19]

http://codereview.chromium.org/10989084/diff/1/chromeos/display/output_config...
File chromeos/display/output_configurator.cc (right):

http://codereview.chromium.org/10989084/diff/1/chromeos/display/output_config...
chromeos/display/output_configurator.cc:741: return false;
On 2012/10/01 21:49:35, Jun Mukai wrote:
> On 2012/10/01 21:44:02, oshima wrote:
> > is it possible to return output_state_ == new_state,
> > and simply test if output_state_ has changed later?
> 
> The header said it "returns false if it was called in a single-head or
headless
> mode."  Did you intend to change this too?
> I think changing this would be fine since the existing callers don't care its
> return value anyway for now.

I was thinking to add

if (output_state_ == new_state)
   return true;   // switching to the same state is considered successful.

and this shouldn't require changes to header. Do you think there is a risk?

[Jun Mukai, 2012-10-01 22:36:56]

http://codereview.chromium.org/10989084/diff/1/chromeos/display/output_config...
File chromeos/display/output_configurator.cc (right):

http://codereview.chromium.org/10989084/diff/1/chromeos/display/output_config...
chromeos/display/output_configurator.cc:741: return false;
On 2012/10/01 22:16:19, oshima wrote:
> On 2012/10/01 21:49:35, Jun Mukai wrote:
> > On 2012/10/01 21:44:02, oshima wrote:
> > > is it possible to return output_state_ == new_state,
> > > and simply test if output_state_ has changed later?
> > 
> > The header said it "returns false if it was called in a single-head or
> headless
> > mode."  Did you intend to change this too?
> > I think changing this would be fine since the existing callers don't care
its
> > return value anyway for now.
> 
> I was thinking to add
> 
> if (output_state_ == new_state)
>    return true;   // switching to the same state is considered successful.
> 
> and this shouldn't require changes to header. Do you think there is a risk?

That would be safe.  Added.

[oshima, 2012-10-02 07:53:02]

lgtm with one request.

http://codereview.chromium.org/10989084/diff/8001/chromeos/display/output_con...
File chromeos/display/output_configurator.cc (right):

http://codereview.chromium.org/10989084/diff/8001/chromeos/display/output_con...
chromeos/display/output_configurator.cc:770: if (!did_change)
if (output_state_ != new_state)

[Jun Mukai, 2012-10-02 20:55:00]

http://codereview.chromium.org/10989084/diff/8001/chromeos/display/output_con...
File chromeos/display/output_configurator.cc (right):

http://codereview.chromium.org/10989084/diff/8001/chromeos/display/output_con...
chromeos/display/output_configurator.cc:770: if (!did_change)
On 2012/10/02 07:53:02, oshima wrote:
> if (output_state_ != new_state)

Done.

[Jun Mukai, 2012-10-02 20:55:35]

sky, could you have the OWNER review for ash/display?

[sky, 2012-10-02 21:35:11]

LGTM

[commit-bot: I haz the power, 2012-10-02 23:21:07]

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

[commit-bot: I haz the power, 2012-10-03 01:25:04]

Change committed as 159823