Use primary display as hangout/gtalk broadcast window

Use primary display as hangout/gtalk broadcast window

BUG=242666
TEST=covered by test

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=207305

[Josh Horwich, 2013-06-14 22:44:02]

LGTM with nits / test-related suggestions + questions.

https://codereview.chromium.org/17099003/diff/2001/ash/display/display_contro...
File ash/display/display_controller_unittest.cc (right):

https://codereview.chromium.org/17099003/diff/2001/ash/display/display_contro...
ash/display/display_controller_unittest.cc:946: *secondary = root_windows[0] ==
*primary ? root_windows[1] : root_windows[0];
I know that right now the function will only be invoked when there are two
windows, but shouldn't we set *secondary to NULL if the root window list is one
element long (e.g. only one root window)?

I suppose if we do this, we could use it for the last (single display) test case
in XWindowNameForRootWindow and EXPECT_EQ(NULL, secondary); there.

https://codereview.chromium.org/17099003/diff/2001/ash/display/display_contro...
ash/display/display_controller_unittest.cc:971: EXPECT_EQ("aura_root_x",
GetXWindowName(secondary));
Would it make sense to save the pointers from before the SwapPrimaryDisplay
call, and verify the formerly-primary is now "aura_root_x" and the
formerly-secondary is now "aura_root_0", in addition to this test where we
verify the swap changes the names?

https://codereview.chromium.org/17099003/diff/2001/ui/aura/root_window_host_x...
File ui/aura/root_window_host_x11.cc (right):

https://codereview.chromium.org/17099003/diff/2001/ui/aura/root_window_host_x...
ui/aura/root_window_host_x11.cc:425: */
Why not just remove this code and the above comment?

[oshima, 2013-06-14 23:10:01]

uploaded new patch.PTAL.

https://codereview.chromium.org/17099003/diff/2001/ash/display/display_contro...
File ash/display/display_controller_unittest.cc (right):

https://codereview.chromium.org/17099003/diff/2001/ash/display/display_contro...
ash/display/display_controller_unittest.cc:946: *secondary = root_windows[0] ==
*primary ? root_windows[1] : root_windows[0];
On 2013/06/14 22:44:03, Josh Horwich wrote:
> I know that right now the function will only be invoked when there are two
> windows, but shouldn't we set *secondary to NULL if the root window list is
one
> element long (e.g. only one root window)?
> 
> I suppose if we do this, we could use it for the last (single display) test
case
> in XWindowNameForRootWindow and EXPECT_EQ(NULL, secondary); there.

For single display case, we should just use Shell::GetPrimaryDisplay().
There is already tests for multi displays scenarios, so the above case would be
a dup.

https://codereview.chromium.org/17099003/diff/2001/ash/display/display_contro...
ash/display/display_controller_unittest.cc:952: // ASSERT_TRUE(!!name);
I forgot to remove this as well. Removed.

https://codereview.chromium.org/17099003/diff/2001/ash/display/display_contro...
ash/display/display_controller_unittest.cc:954: }
I also moved this to anonymous namespace.

https://codereview.chromium.org/17099003/diff/2001/ash/display/display_contro...
ash/display/display_controller_unittest.cc:971: EXPECT_EQ("aura_root_x",
GetXWindowName(secondary));
On 2013/06/14 22:44:03, Josh Horwich wrote:
> Would it make sense to save the pointers from before the SwapPrimaryDisplay
> call, and verify the formerly-primary is now "aura_root_x" and the
> formerly-secondary is now "aura_root_0", in addition to this test where we
> verify the swap changes the names?

Swapping display actually doesn't change the root windows (but it's
implementation details from this test's point of view).
This is just to make sure that window name wasn't swapped somehow.
I did change so that it resets primary/secondary to make sure it doesn't simply
use old values.

https://codereview.chromium.org/17099003/diff/2001/ui/aura/root_window_host_x...
File ui/aura/root_window_host_x11.cc (right):

https://codereview.chromium.org/17099003/diff/2001/ui/aura/root_window_host_x...
ui/aura/root_window_host_x11.cc:425: */
On 2013/06/14 22:44:03, Josh Horwich wrote:
> Why not just remove this code and the above comment?

Doh, I thought I did, but looks like I didn't save it. Removed.

[Josh Horwich, 2013-06-14 23:12:13]

lgtm

[oshima, 2013-06-14 23:13:47]

ben -> ui/aura OWNERS

[oshima, 2013-06-18 18:13:50]

On 2013/06/14 23:13:47, oshima wrote:
> ben -> ui/aura OWNERS

ping?

[Ben Goodger (Google), 2013-06-18 21:30:47]

lgtm

[commit-bot: I haz the power, 2013-06-18 21:31:16]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/oshima@chromium.org/17099003/9001

[commit-bot: I haz the power, 2013-06-20 03:35:23]

Change committed as 207305