Only show start/stop mirroring button when there are multiple displays.

Only show start/stop mirroring button when there are multiple displays.

BUG=169124
TEST=manually on parrot

[hshi1, 2013-01-10 00:20:43]

Can you review this? Thanks!

[Jun Mukai, 2013-01-10 01:27:04]

lgtm but probably the display dialog itself should be hidden when there's only
one display...

[Jun Mukai, 2013-01-10 01:30:50]

On 2013/01/10 01:27:04, Jun Mukai wrote:
> lgtm but probably the display dialog itself should be hidden when there's only
> one display...

to do so, probably you can just call this.getDefaultPage() in onDisplayChanged_.
Asking oshima offline to make sure my thought.

[hshi1, 2013-01-10 01:31:16]

On 2013/01/10 01:27:04, Jun Mukai wrote:
> lgtm but probably the display dialog itself should be hidden when there's only
> one display...

You're correct that when there's only one display, the display setting dialog
will not show up on the settings page or be user-accessible via the status
notification area in the bottom-right corner.

This problem really only shows up when
(1) manually type "chrome://settings/display" to open the dialog; or
(2) start with multiple displays, open the dialog, then unplug the external
monitor.

[Jun Mukai, 2013-01-10 02:19:56]

On 2013/01/10 01:31:16, hshi1 wrote:
> On 2013/01/10 01:27:04, Jun Mukai wrote:
> > lgtm but probably the display dialog itself should be hidden when there's
only
> > one display...
> 
> You're correct that when there's only one display, the display setting dialog
> will not show up on the settings page or be user-accessible via the status
> notification area in the bottom-right corner.
> 
> This problem really only shows up when
> (1) manually type "chrome://settings/display" to open the dialog; or
> (2) start with multiple displays, open the dialog, then unplug the external
> monitor.

My second comment was slightly wrong, this.showDefaultPage().  I think it will
redirect the page to 'chrome://settings' from .../display, so we can stop the
problem even for those 2 cases you pointed.
Can you add that in onDisplayChanged_?

Hiding the mirroring button is looking good though, so please keep it. It would
be good especially in case that we decide to show the display settings page for
some reasons in the future.

[hshi1, 2013-01-10 02:34:17]

Updated with patch set #2 thanks!

[Jun Mukai, 2013-01-10 02:40:47]

lgtm!

[commit-bot: I haz the power, 2013-01-10 03:28:40]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/hshi@chromium.org/11818043/4002

[commit-bot: I haz the power, 2013-01-10 06:04:31]

Retried try job too often on win_aura for step(s) content_browsertests

[commit-bot: I haz the power, 2013-01-10 06:08:33]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/hshi@chromium.org/11818043/4002

[commit-bot: I haz the power, 2013-01-10 08:54:49]

Retried try job too often on linux_chromeos for step(s) browser_tests

[commit-bot: I haz the power, 2013-01-10 15:18:41]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/hshi@chromium.org/11818043/4002

[hshi1, 2013-01-10 18:59:24]

https://codereview.chromium.org/11818043/diff/4002/chrome/browser/resources/o...
File chrome/browser/resources/options/chromeos/display_options.js (right):

https://codereview.chromium.org/11818043/diff/4002/chrome/browser/resources/o...
chrome/browser/resources/options/chromeos/display_options.js:1057:
OptionsPage.showDefaultPage();
My bad. This is causing all the chrome://settings/xxx pages to be re-directed to
the main settings page when only one monitor is attached. CL was already
reverted. Need to re-consider this.

[Jun Mukai, 2013-01-10 19:04:37]

https://codereview.chromium.org/11818043/diff/4002/chrome/browser/resources/o...
File chrome/browser/resources/options/chromeos/display_options.js (right):

https://codereview.chromium.org/11818043/diff/4002/chrome/browser/resources/o...
chrome/browser/resources/options/chromeos/display_options.js:1057:
OptionsPage.showDefaultPage();
On 2013/01/10 18:59:24, hshi1 wrote:
> My bad. This is causing all the chrome://settings/xxx pages to be re-directed
to
> the main settings page when only one monitor is attached. CL was already
> reverted. Need to re-consider this.

Oops, I missed that point too :(  Sorry for my bad proposal.
Let's separate the patch, first create a CL of toggle-mirroring button, which is
apparently safe.

[hshi1, 2013-01-10 19:08:41]

Makes sense. I've uploaded the CL for toggle-mirroing button at
https://codereview.chromium.org/11786011/.