blimp: Hook up the layer holding the tab content into Chrome.

blimp: Hook up the layer holding the tab content into Chrome.

Introduce a BlimpContentsView to blimp which abstracts the platform
dependent view needs. Have it own the ViewAndroid and expose it to the
embedder using the BlimpContents. Have Chrome attach the cc::Layer
holding the delegated content from Blimp to the tab's content layer.

BUG=611105
Committed: https://crrev.com/030bd21d33b79c73c3f5376f7ed0a976660e8b33
Cr-Commit-Position: refs/heads/master@{#415061}

[Khushal, 2016-08-27 02:04:55]

khushalsagar@chromium.org changed reviewers:
+ dtrainor@chromium.org

[David Trainor- moved to gerrit, 2016-08-29 05:17:51]

lgtm % nit.

https://codereview.chromium.org/2279403003/diff/20001/blimp/client/core/conte...
File blimp/client/core/contents/BUILD.gn (right):

https://codereview.chromium.org/2279403003/diff/20001/blimp/client/core/conte...
blimp/client/core/contents/BUILD.gn:63: "blimp_contents_view_aura.h",
Should this be _aura or _linux?  I don't think we use _aura on the linux client
right?

[Khushal, 2016-08-29 05:54:19]

https://codereview.chromium.org/2279403003/diff/20001/blimp/client/core/conte...
File blimp/client/core/contents/BUILD.gn (right):

https://codereview.chromium.org/2279403003/diff/20001/blimp/client/core/conte...
blimp/client/core/contents/BUILD.gn:63: "blimp_contents_view_aura.h",
On 2016/08/29 05:17:51, David Trainor wrote:
> Should this be _aura or _linux?  I don't think we use _aura on the linux
client
> right?

Yeah, we don't use aura on the linux client but all the native widget stuff for
linux is hidden behind aura. The rest of the code talks in terms of
aura::Windows, and uses a WindowTreeHost which has an implementation for X11,
https://cs.chromium.org/chromium/src/ui/aura/window_tree_host_x11.h, but we just
use a PlatformWindow directly. May be we should be using it too? For now since
the widget in the linux case is an aura::Window so I just named it that. Kept it
consistent with content/ too.

[David Trainor- moved to gerrit, 2016-08-29 15:47:26]

On 2016/08/29 05:54:19, Khushal wrote:
>
https://codereview.chromium.org/2279403003/diff/20001/blimp/client/core/conte...
> File blimp/client/core/contents/BUILD.gn (right):
> 
>
https://codereview.chromium.org/2279403003/diff/20001/blimp/client/core/conte...
> blimp/client/core/contents/BUILD.gn:63: "blimp_contents_view_aura.h",
> On 2016/08/29 05:17:51, David Trainor wrote:
> > Should this be _aura or _linux?  I don't think we use _aura on the linux
> client
> > right?
> 
> Yeah, we don't use aura on the linux client but all the native widget stuff
for
> linux is hidden behind aura. The rest of the code talks in terms of
> aura::Windows, and uses a WindowTreeHost which has an implementation for X11,
> https://cs.chromium.org/chromium/src/ui/aura/window_tree_host_x11.h, but we
just
> use a PlatformWindow directly. May be we should be using it too? For now since
> the widget in the linux case is an aura::Window so I just named it that. Kept
it
> consistent with content/ too.

Ah yeah they don't have any support for a linux NativeView that isn't Aura...
hmm ok!  Yeah we should probably be using that.  I think the widget is still an
unsigned long for x11, do you mean the NativeView?

[Khushal, 2016-08-29 16:03:00]

On 2016/08/29 15:47:26, David Trainor wrote:
> On 2016/08/29 05:54:19, Khushal wrote:
> >
>
https://codereview.chromium.org/2279403003/diff/20001/blimp/client/core/conte...
> > File blimp/client/core/contents/BUILD.gn (right):
> > 
> >
>
https://codereview.chromium.org/2279403003/diff/20001/blimp/client/core/conte...
> > blimp/client/core/contents/BUILD.gn:63: "blimp_contents_view_aura.h",
> > On 2016/08/29 05:17:51, David Trainor wrote:
> > > Should this be _aura or _linux?  I don't think we use _aura on the linux
> > client
> > > right?
> > 
> > Yeah, we don't use aura on the linux client but all the native widget stuff
> for
> > linux is hidden behind aura. The rest of the code talks in terms of
> > aura::Windows, and uses a WindowTreeHost which has an implementation for
X11,
> > https://cs.chromium.org/chromium/src/ui/aura/window_tree_host_x11.h, but we
> just
> > use a PlatformWindow directly. May be we should be using it too? For now
since
> > the widget in the linux case is an aura::Window so I just named it that.
Kept
> it
> > consistent with content/ too.
> 
> Ah yeah they don't have any support for a linux NativeView that isn't Aura...
> hmm ok!  Yeah we should probably be using that.  I think the widget is still
an
> unsigned long for x11, do you mean the NativeView?

Oops yeah, I meant the NativeView and Window.

[Khushal, 2016-08-29 16:03:11]

The CQ bit was checked by khushalsagar@chromium.org

[commit-bot: I haz the power, 2016-08-29 16:04:31]

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-08-29 16:10:43]

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

[commit-bot: I haz the power, 2016-08-29 16:10:45]

Try jobs failed on following builders:
  android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)
  android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...)
  android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
  chromeos_x86-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_x86-ge...)
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_compile_dbg_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_clobber_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Khushal, 2016-08-29 16:46:18]

The CQ bit was checked by khushalsagar@chromium.org

[Khushal, 2016-08-29 16:46:18]

The patchset sent to the CQ was uploaded after l-g-t-m from
dtrainor@chromium.org
Link to the patchset: https://codereview.chromium.org/2279403003/#ps40001
(title: "make gn happy")

[commit-bot: I haz the power, 2016-08-29 16:51:20]

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

[Khushal, 2016-08-29 17:11:39]

The CQ bit was unchecked by khushalsagar@chromium.org

[Khushal, 2016-08-29 17:54:26]

The CQ bit was checked by khushalsagar@chromium.org

[Khushal, 2016-08-29 17:54:27]

The patchset sent to the CQ was uploaded after l-g-t-m from
dtrainor@chromium.org
Link to the patchset: https://codereview.chromium.org/2279403003/#ps60001
(title: "tab_android")

[commit-bot: I haz the power, 2016-08-29 18:10:05]

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

[nyquist, 2016-08-29 23:05:58]

The CQ bit was unchecked by nyquist@chromium.org

[nyquist, 2016-08-29 23:06:08]

The CQ bit was checked by nyquist@chromium.org

[nyquist, 2016-08-29 23:09:45]

https://codereview.chromium.org/2279403003/diff/60001/blimp/client/core/conte...
File blimp/client/core/contents/blimp_contents_impl.h (right):

https://codereview.chromium.org/2279403003/diff/60001/blimp/client/core/conte...
blimp/client/core/contents/blimp_contents_impl.h:78:
std::unique_ptr<BlimpContentsView> blimp_contents_view_;
Could you add an explanation as to what this is for?

https://codereview.chromium.org/2279403003/diff/60001/chrome/browser/android/...
File chrome/browser/android/tab_android.cc (right):

https://codereview.chromium.org/2279403003/diff/60001/chrome/browser/android/...
chrome/browser/android/tab_android.cc:436: cc::Layer* web_contents_layer =
web_contents_->GetNativeView()->GetLayer();
Are you sure web_contents_ is never nullptr here?

[commit-bot: I haz the power, 2016-08-29 23:53:12]

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

[Khushal, 2016-08-30 00:09:06]

The CQ bit was unchecked by khushalsagar@chromium.org

[Khushal, 2016-08-30 00:51:11]

https://codereview.chromium.org/2279403003/diff/60001/blimp/client/core/conte...
File blimp/client/core/contents/blimp_contents_impl.h (right):

https://codereview.chromium.org/2279403003/diff/60001/blimp/client/core/conte...
blimp/client/core/contents/blimp_contents_impl.h:78:
std::unique_ptr<BlimpContentsView> blimp_contents_view_;
On 2016/08/29 23:09:45, nyquist wrote:
> Could you add an explanation as to what this is for?

Done.

https://codereview.chromium.org/2279403003/diff/60001/chrome/browser/android/...
File chrome/browser/android/tab_android.cc (right):

https://codereview.chromium.org/2279403003/diff/60001/chrome/browser/android/...
chrome/browser/android/tab_android.cc:436: cc::Layer* web_contents_layer =
web_contents_->GetNativeView()->GetLayer();
On 2016/08/29 23:09:45, nyquist wrote:
> Are you sure web_contents_ is never nullptr here?

Thanks for catching it. Done.

[nyquist, 2016-08-30 01:27:44]

lgtm. thanks!

[Khushal, 2016-08-30 01:28:29]

The CQ bit was checked by khushalsagar@chromium.org

[Khushal, 2016-08-30 01:28:30]

The patchset sent to the CQ was uploaded after l-g-t-m from
dtrainor@chromium.org
Link to the patchset: https://codereview.chromium.org/2279403003/#ps80001
(title: "Addressed comments")

[commit-bot: I haz the power, 2016-08-30 01:57:14]

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-08-30 03:52:13]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-08-30 03:55:15]

Description was changed from

==========
blimp: Hook up the layer holding the tab content into Chrome.

Introduce a BlimpContentsView to blimp which abstracts the platform
dependent view needs. Have it own the ViewAndroid and expose it to the
embedder using the BlimpContents. Have Chrome attach the cc::Layer
holding the delegated content from Blimp to the tab's content layer.

BUG=611105
==========

to

==========
blimp: Hook up the layer holding the tab content into Chrome.

Introduce a BlimpContentsView to blimp which abstracts the platform
dependent view needs. Have it own the ViewAndroid and expose it to the
embedder using the BlimpContents. Have Chrome attach the cc::Layer
holding the delegated content from Blimp to the tab's content layer.

BUG=611105

Committed: https://crrev.com/030bd21d33b79c73c3f5376f7ed0a976660e8b33
Cr-Commit-Position: refs/heads/master@{#415061}
==========

[commit-bot: I haz the power, 2016-08-30 03:55:15]

Patchset 5 (id:??) landed as
https://crrev.com/030bd21d33b79c73c3f5376f7ed0a976660e8b33
Cr-Commit-Position: refs/heads/master@{#415061}