[Chromecast] Reuse the Aura window manager across receiver apps.

[Chromecast] Reuse the Aura window manager across receiver apps.

Always makes the root window transparent for seeing the video plane
beneath it.

Allows tearing down and rebuilding the window manager.

BUG=internal b/34103918, internal b/33835219, internal b/33046596
TEST=manually test all categories of receiver apps and mirroring.

Change-Id: Ic84079e6eae012896b2a1d8bdf39dc64578f2307
Review-Url: https://codereview.chromium.org/2643553002
Cr-Commit-Position: refs/heads/master@{#445424}
Committed: https://chromium.googlesource.com/chromium/src/+/bc99b5bd4d84c497eee78e13e2c786e409cc80dd

[Joshua LeVasseur, 2017-01-17 22:48:46]

jlevasseur@chromium.org changed reviewers:
+ derekjchow@chromium.org, halliwell@chromium.org

[Joshua LeVasseur, 2017-01-17 22:49:15]

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

[commit-bot: I haz the power, 2017-01-17 22:49:56]

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

[alokp, 2017-01-17 23:08:17]

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

[alokp, 2017-01-17 23:08:17]

Very high-level drive-by comment.

https://codereview.chromium.org/2643553002/diff/1/chromecast/graphics/cast_wi...
File chromecast/graphics/cast_window_manager.h (right):

https://codereview.chromium.org/2643553002/diff/1/chromecast/graphics/cast_wi...
chromecast/graphics/cast_window_manager.h:4: #ifndef
CHROMECAST_GRAPHICS_CAST_WINDOW_MANAGER_H_
nit: The name of this directory is now wrong. We should rename this directory to
ui/. It can be done in a separate patch.

[commit-bot: I haz the power, 2017-01-17 23:47:32]

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

[commit-bot: I haz the power, 2017-01-17 23:47:32]

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

[Joshua LeVasseur, 2017-01-18 00:42:20]

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

[commit-bot: I haz the power, 2017-01-18 00:43:08]

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

[halliwell, 2017-01-18 00:50:40]

I started reviewing, but it seems like it needs rebasing on the other patch? 
Negates the point of splitting these up ... :)

https://codereview.chromium.org/2643553002/diff/1/chromecast/browser/BUILD.gn
File chromecast/browser/BUILD.gn (left):

https://codereview.chromium.org/2643553002/diff/1/chromecast/browser/BUILD.gn...
chromecast/browser/BUILD.gn:134: "//ui/base/ime",
does this need rebasing?

https://codereview.chromium.org/2643553002/diff/1/chromecast/graphics/cast_wi...
File chromecast/graphics/cast_window_manager.h (right):

https://codereview.chromium.org/2643553002/diff/1/chromecast/graphics/cast_wi...
chromecast/graphics/cast_window_manager.h:34: virtual void
AddAndShowWindow(gfx::NativeView window) = 0;
why does this warrant a separate interface entry?  Is there any scenario where
this wouldn't just be AddWindow followed by Show on the window?

[Joshua LeVasseur, 2017-01-18 00:55:38]

https://codereview.chromium.org/2643553002/diff/1/chromecast/browser/BUILD.gn
File chromecast/browser/BUILD.gn (left):

https://codereview.chromium.org/2643553002/diff/1/chromecast/browser/BUILD.gn...
chromecast/browser/BUILD.gn:134: "//ui/base/ime",
On 2017/01/18 00:50:40, halliwell wrote:
> does this need rebasing?

No, this is correct: I've moved the IME logic from chromecast/browser to
chromecast/graphics.

https://codereview.chromium.org/2643553002/diff/1/chromecast/graphics/cast_wi...
File chromecast/graphics/cast_window_manager.h (right):

https://codereview.chromium.org/2643553002/diff/1/chromecast/graphics/cast_wi...
chromecast/graphics/cast_window_manager.h:4: #ifndef
CHROMECAST_GRAPHICS_CAST_WINDOW_MANAGER_H_
On 2017/01/17 23:08:17, alokp wrote:
> nit: The name of this directory is now wrong. We should rename this directory
to
> ui/. It can be done in a separate patch.

Acknowledged.

https://codereview.chromium.org/2643553002/diff/1/chromecast/graphics/cast_wi...
chromecast/graphics/cast_window_manager.h:34: virtual void
AddAndShowWindow(gfx::NativeView window) = 0;
On 2017/01/18 00:50:40, halliwell wrote:
> why does this warrant a separate interface entry?  Is there any scenario where
> this wouldn't just be AddWindow followed by Show on the window?

I was just keeping the flow similar to what was there before.  It currently is
unnecessary to have this interface.

[commit-bot: I haz the power, 2017-01-18 01:38:01]

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

[commit-bot: I haz the power, 2017-01-18 01:38:02]

Dry run: This issue passed the CQ dry run.

[derekjchow1, 2017-01-18 02:49:13]

https://codereview.chromium.org/2643553002/diff/20001/chromecast/browser/andr...
File chromecast/browser/android/cast_content_window_android.cc (right):

https://codereview.chromium.org/2643553002/diff/20001/chromecast/browser/andr...
chromecast/browser/android/cast_content_window_android.cc:63: CastWindowManager*
window_manager) {
DCHECK(window_manager).

Even if it's not used, it should be part of the API contract that window_manager
should not be nullptr.

https://codereview.chromium.org/2643553002/diff/20001/chromecast/browser/cast...
File chromecast/browser/cast_content_window.h (left):

https://codereview.chromium.org/2643553002/diff/20001/chromecast/browser/cast...
chromecast/browser/cast_content_window.h:47: // |web_contents| should outlive
this CastContentWindow.
Update command (window_manager should also outlive CastContentWindow).

https://codereview.chromium.org/2643553002/diff/20001/chromecast/browser/serv...
File chromecast/browser/service/cast_service_simple.cc (right):

https://codereview.chromium.org/2643553002/diff/20001/chromecast/browser/serv...
chromecast/browser/service/cast_service_simple.cc:44:
window_manager_(window_manager) {}
DCHECK(window_manager_);

https://codereview.chromium.org/2643553002/diff/20001/chromecast/graphics/cas...
File chromecast/graphics/cast_window_manager.h (right):

https://codereview.chromium.org/2643553002/diff/20001/chromecast/graphics/cas...
chromecast/graphics/cast_window_manager.h:1: // Copyright 2016 The Chromium
Authors. All rights reserved.
2016 + 1

https://codereview.chromium.org/2643553002/diff/20001/chromecast/graphics/cas...
chromecast/graphics/cast_window_manager.h:34: virtual void
AddAndShowWindow(gfx::NativeView window) = 0;
How is a window removed? How is a window hidden?

https://codereview.chromium.org/2643553002/diff/20001/chromecast/graphics/cas...
File chromecast/graphics/cast_window_manager_aura.cc (right):

https://codereview.chromium.org/2643553002/diff/20001/chromecast/graphics/cas...
chromecast/graphics/cast_window_manager_aura.cc:164: LOG(INFO) << "Adding
window: " << child->id() << ": " << child->GetName();
DLOG perhaps? Do you expect to need this debugging the features in production?

https://codereview.chromium.org/2643553002/diff/20001/chromecast/graphics/cas...
File chromecast/graphics/cast_window_manager_aura.h (right):

https://codereview.chromium.org/2643553002/diff/20001/chromecast/graphics/cas...
chromecast/graphics/cast_window_manager_aura.h:18: }
}  // namespace client
}  // namespace aura

Helps keep track of the nesting.

https://codereview.chromium.org/2643553002/diff/20001/chromecast/graphics/cas...
chromecast/graphics/cast_window_manager_aura.h:26: public
CastVSyncSettings::Observer {
I don't think we need to publicly inherit from CastVSyncSettings::Observer.

https://codereview.chromium.org/2643553002/diff/20001/chromecast/graphics/cas...
File chromecast/graphics/cast_window_manager_default.cc (right):

https://codereview.chromium.org/2643553002/diff/20001/chromecast/graphics/cas...
chromecast/graphics/cast_window_manager_default.cc:1: // Copyright 2016 The
Chromium Authors. All rights reserved.
2017

[halliwell, 2017-01-18 02:55:05]

On 2017/01/18 00:55:38, Joshua LeVasseur wrote:
> https://codereview.chromium.org/2643553002/diff/1/chromecast/browser/BUILD.gn
> File chromecast/browser/BUILD.gn (left):
> 
>
https://codereview.chromium.org/2643553002/diff/1/chromecast/browser/BUILD.gn...
> chromecast/browser/BUILD.gn:134: "//ui/base/ime",
> On 2017/01/18 00:50:40, halliwell wrote:
> > does this need rebasing?
> 
> No, this is correct: I've moved the IME logic from chromecast/browser to
> chromecast/graphics.
> 

My mistake.  Still, it has somewhat negated the point of splitting up the
original patch ... never mind anyway :)

[halliwell, 2017-01-18 02:55:18]

https://codereview.chromium.org/2643553002/diff/20001/chromecast/browser/serv...
File chromecast/browser/service/cast_service_simple.cc (right):

https://codereview.chromium.org/2643553002/diff/20001/chromecast/browser/serv...
chromecast/browser/service/cast_service_simple.cc:12: #include
"chromecast/graphics/cast_window_manager.h"
unnecessary

https://codereview.chromium.org/2643553002/diff/20001/chromecast/graphics/cas...
File chromecast/graphics/cast_window_manager_aura.cc (right):

https://codereview.chromium.org/2643553002/diff/20001/chromecast/graphics/cas...
chromecast/graphics/cast_window_manager_aura.cc:117: if (window_tree_host_) {
Do we expect clients to get Setup/Teardown sequence wrong?  i.e. should this be
DCHECK(!window_tree_host_) ?

https://codereview.chromium.org/2643553002/diff/20001/chromecast/graphics/cas...
chromecast/graphics/cast_window_manager_aura.cc:120: LOG(INFO) << "Starting
window manager.";
how important is this?  maybe at least combine with screen size logging below?

https://codereview.chromium.org/2643553002/diff/20001/chromecast/graphics/cas...
chromecast/graphics/cast_window_manager_aura.cc:150: if (!window_tree_host_) {
ditto question on calling sequence

https://codereview.chromium.org/2643553002/diff/20001/chromecast/graphics/cas...
chromecast/graphics/cast_window_manager_aura.cc:165: Setup();
I guess this would lead to multiple Setup calls.  Wonder if it's better to be an
error to add windows when it's not set up?  It kind of seems like this class
doesn't make its mind up between needing explicit setup/teardown management vs
just handling that automatically.

[Joshua LeVasseur, 2017-01-18 05:52:33]

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

[commit-bot: I haz the power, 2017-01-18 05:52:50]

Dry run: 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, 2017-01-18 05:55:43]

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

[commit-bot: I haz the power, 2017-01-18 05:55:44]

Dry run: This issue passed the CQ dry run.

[Joshua LeVasseur, 2017-01-18 06:00:57]

https://codereview.chromium.org/2643553002/diff/20001/chromecast/graphics/cas...
File chromecast/graphics/cast_window_manager.h (right):

https://codereview.chromium.org/2643553002/diff/20001/chromecast/graphics/cas...
chromecast/graphics/cast_window_manager.h:34: virtual void
AddAndShowWindow(gfx::NativeView window) = 0;
On 2017/01/18 02:49:12, derekjchow1 wrote:
> How is a window removed? How is a window hidden?

That is platform dependent.  For Aura, you directly operate on the window to
hide/unhide or remove (our code removes these windows by deleting the windows).

https://codereview.chromium.org/2643553002/diff/20001/chromecast/graphics/cas...
File chromecast/graphics/cast_window_manager_aura.cc (right):

https://codereview.chromium.org/2643553002/diff/20001/chromecast/graphics/cas...
chromecast/graphics/cast_window_manager_aura.cc:165: Setup();
On 2017/01/18 02:55:18, halliwell wrote:
> I guess this would lead to multiple Setup calls.  Wonder if it's better to be
an
> error to add windows when it's not set up?  It kind of seems like this class
> doesn't make its mind up between needing explicit setup/teardown management vs
> just handling that automatically.

I set it up for on-demand initialization to mimic the initialization sequence
that already existed.  I'm fine with converting it to use an explicit setup
step, but then the Cast shell will initialize in a slightly different order.

[Joshua LeVasseur, 2017-01-18 18:26:35]

https://codereview.chromium.org/2643553002/diff/20001/chromecast/browser/andr...
File chromecast/browser/android/cast_content_window_android.cc (right):

https://codereview.chromium.org/2643553002/diff/20001/chromecast/browser/andr...
chromecast/browser/android/cast_content_window_android.cc:63: CastWindowManager*
window_manager) {
On 2017/01/18 02:49:12, derekjchow1 wrote:
> DCHECK(window_manager).
> 
> Even if it's not used, it should be part of the API contract that
window_manager
> should not be nullptr.

Done.

https://codereview.chromium.org/2643553002/diff/20001/chromecast/browser/cast...
File chromecast/browser/cast_content_window.h (left):

https://codereview.chromium.org/2643553002/diff/20001/chromecast/browser/cast...
chromecast/browser/cast_content_window.h:47: // |web_contents| should outlive
this CastContentWindow.
On 2017/01/18 02:49:12, derekjchow1 wrote:
> Update command (window_manager should also outlive CastContentWindow).

Done.

https://codereview.chromium.org/2643553002/diff/20001/chromecast/browser/serv...
File chromecast/browser/service/cast_service_simple.cc (right):

https://codereview.chromium.org/2643553002/diff/20001/chromecast/browser/serv...
chromecast/browser/service/cast_service_simple.cc:12: #include
"chromecast/graphics/cast_window_manager.h"
On 2017/01/18 02:55:18, halliwell wrote:
> unnecessary

Done.

https://codereview.chromium.org/2643553002/diff/20001/chromecast/browser/serv...
chromecast/browser/service/cast_service_simple.cc:44:
window_manager_(window_manager) {}
On 2017/01/18 02:49:12, derekjchow1 wrote:
> DCHECK(window_manager_);

Done.

https://codereview.chromium.org/2643553002/diff/20001/chromecast/graphics/cas...
File chromecast/graphics/cast_window_manager.h (right):

https://codereview.chromium.org/2643553002/diff/20001/chromecast/graphics/cas...
chromecast/graphics/cast_window_manager.h:1: // Copyright 2016 The Chromium
Authors. All rights reserved.
On 2017/01/18 02:49:12, derekjchow1 wrote:
> 2016 + 1

Done.

https://codereview.chromium.org/2643553002/diff/20001/chromecast/graphics/cas...
File chromecast/graphics/cast_window_manager_aura.cc (right):

https://codereview.chromium.org/2643553002/diff/20001/chromecast/graphics/cas...
chromecast/graphics/cast_window_manager_aura.cc:120: LOG(INFO) << "Starting
window manager.";
On 2017/01/18 02:55:18, halliwell wrote:
> how important is this?  maybe at least combine with screen size logging below?

Done.

https://codereview.chromium.org/2643553002/diff/20001/chromecast/graphics/cas...
chromecast/graphics/cast_window_manager_aura.cc:164: LOG(INFO) << "Adding
window: " << child->id() << ": " << child->GetName();
On 2017/01/18 02:49:13, derekjchow1 wrote:
> DLOG perhaps? Do you expect to need this debugging the features in production?

I'm not sure.  I figured it would be good to have in the log to help debug
unexpected behaviors.

https://codereview.chromium.org/2643553002/diff/20001/chromecast/graphics/cas...
File chromecast/graphics/cast_window_manager_aura.h (right):

https://codereview.chromium.org/2643553002/diff/20001/chromecast/graphics/cas...
chromecast/graphics/cast_window_manager_aura.h:18: }
On 2017/01/18 02:49:13, derekjchow1 wrote:
> }  // namespace client
> }  // namespace aura
> 
> Helps keep track of the nesting.

Done.

https://codereview.chromium.org/2643553002/diff/20001/chromecast/graphics/cas...
chromecast/graphics/cast_window_manager_aura.h:26: public
CastVSyncSettings::Observer {
On 2017/01/18 02:49:13, derekjchow1 wrote:
> I don't think we need to publicly inherit from CastVSyncSettings::Observer.

Done.

https://codereview.chromium.org/2643553002/diff/20001/chromecast/graphics/cas...
File chromecast/graphics/cast_window_manager_default.cc (right):

https://codereview.chromium.org/2643553002/diff/20001/chromecast/graphics/cas...
chromecast/graphics/cast_window_manager_default.cc:1: // Copyright 2016 The
Chromium Authors. All rights reserved.
On 2017/01/18 02:49:13, derekjchow1 wrote:
> 2017

Done.

[halliwell, 2017-01-18 21:11:22]

https://codereview.chromium.org/2643553002/diff/40001/chromecast/graphics/cas...
File chromecast/graphics/cast_window_manager.h (right):

https://codereview.chromium.org/2643553002/diff/40001/chromecast/graphics/cas...
chromecast/graphics/cast_window_manager.h:3: // found in the LICENSE file.
blank line

https://codereview.chromium.org/2643553002/diff/40001/chromecast/graphics/cas...
chromecast/graphics/cast_window_manager.h:27: virtual void Setup() = 0;
not clear what the point of this is given that AddWindow implicitly sets up.

https://codereview.chromium.org/2643553002/diff/40001/chromecast/graphics/cas...
chromecast/graphics/cast_window_manager.h:34: virtual void
AddAndShowWindow(gfx::NativeView window) = 0;
Still seems unnecessary (just AddWindow should be enough)

https://codereview.chromium.org/2643553002/diff/40001/chromecast/graphics/cas...
chromecast/graphics/cast_window_manager.h:37: // visibility.
comment that this implicitly sets up?

https://codereview.chromium.org/2643553002/diff/40001/chromecast/graphics/cas...
File chromecast/graphics/cast_window_manager_aura.cc (right):

https://codereview.chromium.org/2643553002/diff/40001/chromecast/graphics/cas...
chromecast/graphics/cast_window_manager_aura.cc:1: // Copyright 2016 The
Chromium Authors. All rights reserved.
2017

https://codereview.chromium.org/2643553002/diff/40001/chromecast/graphics/cas...
chromecast/graphics/cast_window_manager_aura.cc:106: return base::WrapUnique(new
CastWindowManagerAura(enable_input));
include ptr util header

https://codereview.chromium.org/2643553002/diff/40001/chromecast/graphics/cas...
File chromecast/graphics/cast_window_manager_aura.h (right):

https://codereview.chromium.org/2643553002/diff/40001/chromecast/graphics/cas...
chromecast/graphics/cast_window_manager_aura.h:1: // Copyright 2016 The Chromium
Authors. All rights reserved.
2017

https://codereview.chromium.org/2643553002/diff/40001/chromecast/graphics/cas...
chromecast/graphics/cast_window_manager_aura.h:3: // found in the LICENSE file.
blank line

https://codereview.chromium.org/2643553002/diff/40001/chromecast/graphics/cas...
File chromecast/graphics/cast_window_manager_default.h (right):

https://codereview.chromium.org/2643553002/diff/40001/chromecast/graphics/cas...
chromecast/graphics/cast_window_manager_default.h:3: // found in the LICENSE
file.
blank line

[Joshua LeVasseur, 2017-01-19 00:56:33]

https://codereview.chromium.org/2643553002/diff/40001/chromecast/graphics/cas...
File chromecast/graphics/cast_window_manager.h (right):

https://codereview.chromium.org/2643553002/diff/40001/chromecast/graphics/cas...
chromecast/graphics/cast_window_manager.h:3: // found in the LICENSE file.
On 2017/01/18 21:11:22, halliwell wrote:
> blank line

Done.

https://codereview.chromium.org/2643553002/diff/40001/chromecast/graphics/cas...
chromecast/graphics/cast_window_manager.h:27: virtual void Setup() = 0;
On 2017/01/18 21:11:22, halliwell wrote:
> not clear what the point of this is given that AddWindow implicitly sets up.

I've removed it.  I'm going to keep implicit setup for now, because it isn't
clear to me where to safely invoke setup on some of the internal code paths that
recover the GPU process.  I'm thinking that we can have a follow-up CL that
converts it to an explicit approach once we clarify those code paths.

https://codereview.chromium.org/2643553002/diff/40001/chromecast/graphics/cas...
chromecast/graphics/cast_window_manager.h:34: virtual void
AddAndShowWindow(gfx::NativeView window) = 0;
On 2017/01/18 21:11:22, halliwell wrote:
> Still seems unnecessary (just AddWindow should be enough)

Removed.

https://codereview.chromium.org/2643553002/diff/40001/chromecast/graphics/cas...
chromecast/graphics/cast_window_manager.h:37: // visibility.
On 2017/01/18 21:11:22, halliwell wrote:
> comment that this implicitly sets up?

Done.

https://codereview.chromium.org/2643553002/diff/40001/chromecast/graphics/cas...
File chromecast/graphics/cast_window_manager_aura.cc (right):

https://codereview.chromium.org/2643553002/diff/40001/chromecast/graphics/cas...
chromecast/graphics/cast_window_manager_aura.cc:1: // Copyright 2016 The
Chromium Authors. All rights reserved.
On 2017/01/18 21:11:22, halliwell wrote:
> 2017

Done.

https://codereview.chromium.org/2643553002/diff/40001/chromecast/graphics/cas...
chromecast/graphics/cast_window_manager_aura.cc:106: return base::WrapUnique(new
CastWindowManagerAura(enable_input));
On 2017/01/18 21:11:22, halliwell wrote:
> include ptr util header

Done.

https://codereview.chromium.org/2643553002/diff/40001/chromecast/graphics/cas...
File chromecast/graphics/cast_window_manager_aura.h (right):

https://codereview.chromium.org/2643553002/diff/40001/chromecast/graphics/cas...
chromecast/graphics/cast_window_manager_aura.h:1: // Copyright 2016 The Chromium
Authors. All rights reserved.
On 2017/01/18 21:11:22, halliwell wrote:
> 2017

Done.

https://codereview.chromium.org/2643553002/diff/40001/chromecast/graphics/cas...
chromecast/graphics/cast_window_manager_aura.h:3: // found in the LICENSE file.
On 2017/01/18 21:11:22, halliwell wrote:
> blank line

Done.

https://codereview.chromium.org/2643553002/diff/40001/chromecast/graphics/cas...
File chromecast/graphics/cast_window_manager_default.h (right):

https://codereview.chromium.org/2643553002/diff/40001/chromecast/graphics/cas...
chromecast/graphics/cast_window_manager_default.h:3: // found in the LICENSE
file.
On 2017/01/18 21:11:22, halliwell wrote:
> blank line

Done.

[Joshua LeVasseur, 2017-01-19 00:57:43]

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

[commit-bot: I haz the power, 2017-01-19 00:58:05]

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

[halliwell, 2017-01-19 01:10:07]

On 2017/01/19 00:56:33, Joshua LeVasseur wrote:
>
https://codereview.chromium.org/2643553002/diff/40001/chromecast/graphics/cas...
> File chromecast/graphics/cast_window_manager.h (right):
> 
>
https://codereview.chromium.org/2643553002/diff/40001/chromecast/graphics/cas...
> chromecast/graphics/cast_window_manager.h:3: // found in the LICENSE file.
> On 2017/01/18 21:11:22, halliwell wrote:
> > blank line
> 
> Done.
> 
>
https://codereview.chromium.org/2643553002/diff/40001/chromecast/graphics/cas...
> chromecast/graphics/cast_window_manager.h:27: virtual void Setup() = 0;
> On 2017/01/18 21:11:22, halliwell wrote:
> > not clear what the point of this is given that AddWindow implicitly sets up.
> 
> I've removed it.  I'm going to keep implicit setup for now, because it isn't
> clear to me where to safely invoke setup on some of the internal code paths
that
> recover the GPU process.  I'm thinking that we can have a follow-up CL that
> converts it to an explicit approach once we clarify those code paths.
> 
>
https://codereview.chromium.org/2643553002/diff/40001/chromecast/graphics/cas...
> chromecast/graphics/cast_window_manager.h:34: virtual void
> AddAndShowWindow(gfx::NativeView window) = 0;
> On 2017/01/18 21:11:22, halliwell wrote:
> > Still seems unnecessary (just AddWindow should be enough)
> 
> Removed.
> 
>
https://codereview.chromium.org/2643553002/diff/40001/chromecast/graphics/cas...
> chromecast/graphics/cast_window_manager.h:37: // visibility.
> On 2017/01/18 21:11:22, halliwell wrote:
> > comment that this implicitly sets up?
> 
> Done.
> 
>
https://codereview.chromium.org/2643553002/diff/40001/chromecast/graphics/cas...
> File chromecast/graphics/cast_window_manager_aura.cc (right):
> 
>
https://codereview.chromium.org/2643553002/diff/40001/chromecast/graphics/cas...
> chromecast/graphics/cast_window_manager_aura.cc:1: // Copyright 2016 The
> Chromium Authors. All rights reserved.
> On 2017/01/18 21:11:22, halliwell wrote:
> > 2017
> 
> Done.
> 
>
https://codereview.chromium.org/2643553002/diff/40001/chromecast/graphics/cas...
> chromecast/graphics/cast_window_manager_aura.cc:106: return
base::WrapUnique(new
> CastWindowManagerAura(enable_input));
> On 2017/01/18 21:11:22, halliwell wrote:
> > include ptr util header
> 
> Done.
> 
>
https://codereview.chromium.org/2643553002/diff/40001/chromecast/graphics/cas...
> File chromecast/graphics/cast_window_manager_aura.h (right):
> 
>
https://codereview.chromium.org/2643553002/diff/40001/chromecast/graphics/cas...
> chromecast/graphics/cast_window_manager_aura.h:1: // Copyright 2016 The
Chromium
> Authors. All rights reserved.
> On 2017/01/18 21:11:22, halliwell wrote:
> > 2017
> 
> Done.
> 
>
https://codereview.chromium.org/2643553002/diff/40001/chromecast/graphics/cas...
> chromecast/graphics/cast_window_manager_aura.h:3: // found in the LICENSE
file.
> On 2017/01/18 21:11:22, halliwell wrote:
> > blank line
> 
> Done.
> 
>
https://codereview.chromium.org/2643553002/diff/40001/chromecast/graphics/cas...
> File chromecast/graphics/cast_window_manager_default.h (right):
> 
>
https://codereview.chromium.org/2643553002/diff/40001/chromecast/graphics/cas...
> chromecast/graphics/cast_window_manager_default.h:3: // found in the LICENSE
> file.
> On 2017/01/18 21:11:22, halliwell wrote:
> > blank line
> 
> Done.

lgtm

[derekjchow1, 2017-01-19 01:16:28]

lgtm with one question/nit

https://codereview.chromium.org/2643553002/diff/20001/chromecast/graphics/cas...
File chromecast/graphics/cast_window_manager.h (right):

https://codereview.chromium.org/2643553002/diff/20001/chromecast/graphics/cas...
chromecast/graphics/cast_window_manager.h:34: virtual void
AddAndShowWindow(gfx::NativeView window) = 0;
On 2017/01/18 06:00:57, Joshua LeVasseur wrote:
> On 2017/01/18 02:49:12, derekjchow1 wrote:
> > How is a window removed? How is a window hidden?
> 
> That is platform dependent.  For Aura, you directly operate on the window to
> hide/unhide or remove (our code removes these windows by deleting the
windows).

Any comment on z-order? Or will the issue be solved in next CL?

[Joshua LeVasseur, 2017-01-19 01:28:16]

On 2017/01/19 01:16:28, derekjchow1 wrote:
> lgtm with one question/nit
> 
>
https://codereview.chromium.org/2643553002/diff/20001/chromecast/graphics/cas...
> File chromecast/graphics/cast_window_manager.h (right):
> 
>
https://codereview.chromium.org/2643553002/diff/20001/chromecast/graphics/cas...
> chromecast/graphics/cast_window_manager.h:34: virtual void
> AddAndShowWindow(gfx::NativeView window) = 0;
> On 2017/01/18 06:00:57, Joshua LeVasseur wrote:
> > On 2017/01/18 02:49:12, derekjchow1 wrote:
> > > How is a window removed? How is a window hidden?
> > 
> > That is platform dependent.  For Aura, you directly operate on the window to
> > hide/unhide or remove (our code removes these windows by deleting the
> windows).
> 
> Any comment on z-order? Or will the issue be solved in next CL?

The next CL handles z order and input focus.

[Joshua LeVasseur, 2017-01-19 01:35:15]

jlevasseur@chromium.org changed reviewers:
+ shuchen@chromium.org

[Joshua LeVasseur, 2017-01-19 01:35:15]

Hi Shu Chen, this CL moves our use of ui/base/ime from chromecast/browser to
chromecast/graphics, as we are relocating the code.   Otherwise the code is the
same as when you reviewed https://codereview.chromium.org/2428383007/

[Shu Chen, 2017-01-19 01:38:56]

lgtm

[commit-bot: I haz the power, 2017-01-19 02:09:22]

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

[commit-bot: I haz the power, 2017-01-19 02:09:23]

Dry run: This issue passed the CQ dry run.

[Joshua LeVasseur, 2017-01-23 17:03:08]

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

[commit-bot: I haz the power, 2017-01-23 17:03:47]

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

[Joshua LeVasseur, 2017-01-23 18:54:47]

The CQ bit was checked by jlevasseur@chromium.org

[commit-bot: I haz the power, 2017-01-23 18:55:18]

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, 2017-01-23 19:01:42]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1485197687630560,
"parent_rev": "9fe4892d60cfb278970d6acb517e581a241ab012", "commit_rev":
"bc99b5bd4d84c497eee78e13e2c786e409cc80dd"}

[commit-bot: I haz the power, 2017-01-23 19:04:14]

Description was changed from

==========
[Chromecast] Reuse the Aura window manager across receiver apps.

Always makes the root window transparent for seeing the video plane
beneath it.

Allows tearing down and rebuilding the window manager.

BUG=internal b/34103918, internal b/33835219, internal b/33046596
TEST=manually test all categories of receiver apps and mirroring.

Change-Id: Ic84079e6eae012896b2a1d8bdf39dc64578f2307
==========

to

==========
[Chromecast] Reuse the Aura window manager across receiver apps.

Always makes the root window transparent for seeing the video plane
beneath it.

Allows tearing down and rebuilding the window manager.

BUG=internal b/34103918, internal b/33835219, internal b/33046596
TEST=manually test all categories of receiver apps and mirroring.

Change-Id: Ic84079e6eae012896b2a1d8bdf39dc64578f2307
Review-Url: https://codereview.chromium.org/2643553002
Cr-Commit-Position: refs/heads/master@{#445424}
Committed:
https://chromium.googlesource.com/chromium/src/+/bc99b5bd4d84c497eee78e13e2c7...
==========

[commit-bot: I haz the power, 2017-01-23 19:04:16]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/bc99b5bd4d84c497eee78e13e2c7...