Makes WindowTreeHost::InitHost() not Show the window

Makes WindowTreeHost::InitHost() not Show the window

Users of WindowTreeHost may want to show it at different times, and
should be able to do that.

BUG=672150
TEST=covered by tests
R=ben@chromium.org, sadrul@chromium.org

Committed: https://crrev.com/746d0103f209a2b2bf1765d3ac3ec365b99e32d5
Cr-Commit-Position: refs/heads/master@{#438363}

[sky, 2016-12-13 18:50:59]

The CQ bit was checked by sky@chromium.org to run a CQ dry run

[sky, 2016-12-13 18:51:09]

sadrul: ui/aura
ben: the rest (which is just adding a bunch of calls to Show() after calls to
InitHost()).

[commit-bot: I haz the power, 2016-12-13 18:51:46]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[mfomitchev, 2016-12-13 19:33:53]

mfomitchev@chromium.org changed reviewers:
+ mfomitchev@chromium.org

[mfomitchev, 2016-12-13 19:33:53]

https://codereview.chromium.org/2567293004/diff/20001/ui/aura/window_tree_hos...
File ui/aura/window_tree_host.cc (left):

https://codereview.chromium.org/2567293004/diff/20001/ui/aura/window_tree_hos...
ui/aura/window_tree_host.cc:66: window()->Show();
Why not add this to WindowTreeHost::Show? Otherwise clients have to remember to
call both WTH::Show() and WTH::window()::Show(), which makes the interface a
little more complex.

[commit-bot: I haz the power, 2016-12-13 20:05:54]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-12-13 20:05:55]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Ben Goodger (Google), 2016-12-13 21:34:58]

lgtm with mfomitchev's suggestion which seems good also

[sky, 2016-12-13 21:38:46]

https://codereview.chromium.org/2567293004/diff/20001/ui/aura/window_tree_hos...
File ui/aura/window_tree_host.cc (left):

https://codereview.chromium.org/2567293004/diff/20001/ui/aura/window_tree_hos...
ui/aura/window_tree_host.cc:66: window()->Show();
On 2016/12/13 19:33:53, mfomitchev wrote:
> Why not add this to WindowTreeHost::Show? Otherwise clients have to remember
to
> call both WTH::Show() and WTH::window()::Show(), which makes the interface a
> little more complex.

Good idea, done.

I could see the argument for Hide() also Hide()ing the window, but I'm worried
that may have weird side-effects, so I'm keeping this at just Show() calling
Show() on the window and not changing Hide().

[sky, 2016-12-13 21:38:48]

The CQ bit was checked by sky@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-13 21:39:40]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[mfomitchev, 2016-12-13 22:30:32]

https://codereview.chromium.org/2567293004/diff/40001/ash/display/mirror_wind...
File ash/display/mirror_window_controller.cc (right):

https://codereview.chromium.org/2567293004/diff/40001/ash/display/mirror_wind...
ash/display/mirror_window_controller.cc:195: host->window()->Show();
We are already calling host->Show() below, so we shouldn't have to call Show()
on the window here.
I think it would be better if here and in all the other places we either called
host->Show() instead of host->window()->Show(), or didn't do anything (because
host->Show() is already called somewhere else).

[commit-bot: I haz the power, 2016-12-13 22:50:19]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-12-13 22:50:20]

Dry run: This issue passed the CQ dry run.

[sky, 2016-12-13 23:21:17]

On 2016/12/13 22:30:32, mfomitchev wrote:
>
https://codereview.chromium.org/2567293004/diff/40001/ash/display/mirror_wind...
> File ash/display/mirror_window_controller.cc (right):
> 
>
https://codereview.chromium.org/2567293004/diff/40001/ash/display/mirror_wind...
> ash/display/mirror_window_controller.cc:195: host->window()->Show();
> We are already calling host->Show() below, so we shouldn't have to call Show()
> on the window here.
> I think it would be better if here and in all the other places we either
called
> host->Show() instead of host->window()->Show(), or didn't do anything (because
> host->Show() is already called somewhere else).

I agree that in theory the Show() in may of these places is likely unnecessary,
but in hopes of not having to figure out all the call sites I would prefer to
leave to the owners of these places.

[mfomitchev, 2016-12-14 00:13:28]

On 2016/12/13 23:21:17, sky wrote:
> On 2016/12/13 22:30:32, mfomitchev wrote:
> >
>
https://codereview.chromium.org/2567293004/diff/40001/ash/display/mirror_wind...
> > File ash/display/mirror_window_controller.cc (right):
> > 
> >
>
https://codereview.chromium.org/2567293004/diff/40001/ash/display/mirror_wind...
> > ash/display/mirror_window_controller.cc:195: host->window()->Show();
> > We are already calling host->Show() below, so we shouldn't have to call
Show()
> > on the window here.
> > I think it would be better if here and in all the other places we either
> called
> > host->Show() instead of host->window()->Show(), or didn't do anything
(because
> > host->Show() is already called somewhere else).
> 
> I agree that in theory the Show() in may of these places is likely
unnecessary,
> but in hopes of not having to figure out all the call sites I would prefer to
> leave to the owners of these places.

LGTM. As discussed offline, getting rid from superfluous Show()'s doesn't need
to be part of this CL.

[sky, 2016-12-14 00:28:35]

The CQ bit was checked by sky@chromium.org

[sky, 2016-12-14 00:28:36]

The patchset sent to the CQ was uploaded after l-g-t-m from ben@chromium.org
Link to the patchset: https://codereview.chromium.org/2567293004/#ps40001
(title: "WTF::Show shows window")

[commit-bot: I haz the power, 2016-12-14 00:29:24]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-14 00:37:01]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1481675315857900,
"parent_rev": "af524209e761987d76b427167923e7d08fec6bf6", "commit_rev":
"52df6027025ae011ba8ac7bf907459227cfbdec7"}

[commit-bot: I haz the power, 2016-12-14 00:37:39]

Description was changed from

==========
Makes WindowTreeHost::InitHost() not Show the window

Users of WindowTreeHost may want to show it at different times, and
should be able to do that.

BUG=672150
TEST=covered by tests
R=ben@chromium.org, sadrul@chromium.org
==========

to

==========
Makes WindowTreeHost::InitHost() not Show the window

Users of WindowTreeHost may want to show it at different times, and
should be able to do that.

BUG=672150
TEST=covered by tests
R=ben@chromium.org, sadrul@chromium.org

Review-Url: https://codereview.chromium.org/2567293004
==========

[commit-bot: I haz the power, 2016-12-14 00:37:41]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-12-14 00:40:59]

Description was changed from

==========
Makes WindowTreeHost::InitHost() not Show the window

Users of WindowTreeHost may want to show it at different times, and
should be able to do that.

BUG=672150
TEST=covered by tests
R=ben@chromium.org, sadrul@chromium.org

Review-Url: https://codereview.chromium.org/2567293004
==========

to

==========
Makes WindowTreeHost::InitHost() not Show the window

Users of WindowTreeHost may want to show it at different times, and
should be able to do that.

BUG=672150
TEST=covered by tests
R=ben@chromium.org, sadrul@chromium.org

Committed: https://crrev.com/746d0103f209a2b2bf1765d3ac3ec365b99e32d5
Cr-Commit-Position: refs/heads/master@{#438363}
==========

[commit-bot: I haz the power, 2016-12-14 00:41:01]

Patchset 3 (id:??) landed as
https://crrev.com/746d0103f209a2b2bf1765d3ac3ec365b99e32d5
Cr-Commit-Position: refs/heads/master@{#438363}