Fix display switching for extended displays.

Fix display switching for extended displays.
Currently when we switch between displays with extended displays turned on, the sequence is,
Mirrored -> Extended -> Extended (reversed) -> Mirrorer.

Change this to switch only between extended and mirrored displays if the extended display flag is enabled,
i.e.,
Mirrorer -> Extended -> Mirrored.


R=oshima@chromium.org
BUG=137727
TEST=Tested on a ChromeOS device with a secondary display to confirm that the inteded sequence is followed on switching displays.


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

[oshima, 2012-07-25 23:44:20]

+josh

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

http://codereview.chromium.org/10829026/diff/1/chromeos/display/output_config...
chromeos/display/output_configurator.cc:669: if
(ash::internal::DisplayController::IsExtendedDesktopEnabled()) {
/chromeos can't depend on /ash.Can you let ash switch the mode in output
configurator instead? (like SetEnbableSecondaryOnly())

[rkc, 2012-07-25 23:59:37]

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

http://codereview.chromium.org/10829026/diff/1/chromeos/display/output_config...
chromeos/display/output_configurator.cc:669: if
(ash::internal::DisplayController::IsExtendedDesktopEnabled()) {
On 2012/07/25 23:44:20, oshima wrote:
> /chromeos can't depend on /ash.Can you let ash switch the mode in output
> configurator instead? (like SetEnbableSecondaryOnly())

Done.

[oshima, 2012-07-26 00:11:06]

lgtm

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

http://codereview.chromium.org/10829026/diff/5001/chromeos/display/output_con...
chromeos/display/output_configurator.cc:683: //  
mirror->primary->secondary->mirror.
can you update the comment?

[rkc, 2012-07-26 00:12:48]

Also added sky@ for the owner's review.

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

http://codereview.chromium.org/10829026/diff/5001/chromeos/display/output_con...
chromeos/display/output_configurator.cc:683: //  
mirror->primary->secondary->mirror.
On 2012/07/26 00:11:06, oshima wrote:
> can you update the comment?

Done.

[Josh Horwich, 2012-07-26 00:18:25]

drive-by comment

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

http://codereview.chromium.org/10829026/diff/5001/chromeos/display/output_con...
chromeos/display/output_configurator.cc:676: }
Would this be easier to follow if you just put it into the block for 'case
STATE_DUAL_PRIMARY_ONLY', e.g.
    case STATE_DUAL_PRIMARY_ONLY:
      new_state = extended_desktop_enabled ?
          (mirror_supported_ ? STATE_DUAL_MIRROR : STATE_INVALID) :
          STATE_DUAL_SECONDAY_ONLY;

[sky, 2012-07-26 00:30:02]

LGTM

[rkc, 2012-07-26 00:32:52]

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

http://codereview.chromium.org/10829026/diff/5001/chromeos/display/output_con...
chromeos/display/output_configurator.cc:676: }
The ?:'s seemed kinda ugly so I moved it in the switch but with if's.


On 2012/07/26 00:18:26, Josh Horwich wrote:
> Would this be easier to follow if you just put it into the block for 'case
> STATE_DUAL_PRIMARY_ONLY', e.g.
>     case STATE_DUAL_PRIMARY_ONLY:
>       new_state = extended_desktop_enabled ?
>           (mirror_supported_ ? STATE_DUAL_MIRROR : STATE_INVALID) :
>           STATE_DUAL_SECONDAY_ONLY;

[commit-bot: I haz the power, 2012-07-27 20:19:35]

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

[commit-bot: I haz the power, 2012-07-27 23:33:24]

Change committed as 148842