android: Use hw acceleration in android_webview_shell

android: Use hw acceleration in android_webview_shell

This creates a (GL)SurfaceView overlay on top of the shell 
container view which runs its own thread for rendering.
It calls into the native draw functor through a small
dynamic library (drawgl.so).

Committed previously: https://chromium.googlesource.com/chromium/src/+/09acbf59e7e8f1de574498ee5ba8eae0b0775438

Committed: https://crrev.com/625a78652e170011fd3590ec9791919f8c1093f0
Cr-Commit-Position: refs/heads/master@{#292761}

[no sievers, 2014-07-22 20:53:18]

I build android_webview_apk and install it and then run:

adb shell am start -n
org.chromium.android_webview.shell/org.chromium.android_webview.shell.AwShellActivity
-d http://www.some.site

https://codereview.chromium.org/414503004/diff/1/android_webview/browser/hard...
File android_webview/browser/hardware_renderer.cc (right):

https://codereview.chromium.org/414503004/diff/1/android_webview/browser/hard...
android_webview/browser/hardware_renderer.cc:190: LOG(INFO) << "no layer";
The SurfaceView might start drawing before the child ever produced a frame.
Can probably inhibit the draw at a higher level instead.

https://codereview.chromium.org/414503004/diff/1/android_webview/native/aw_co...
File android_webview/native/aw_contents.cc (right):

https://codereview.chromium.org/414503004/diff/1/android_webview/native/aw_co...
android_webview/native/aw_contents.cc:349: draw_info.mode =
AwDrawGLInfo::kModeDraw;
So I think since the SurfaceView never moves (when you make it a sibling), we
wouldn't have to ever touch the transform or clip rect here.
Not sure if always using kModeDraw causes issues?

https://codereview.chromium.org/414503004/diff/1/android_webview/test/shell/s...
File
android_webview/test/shell/src/org/chromium/android_webview/shell/AwShellActivity.java
(right):

https://codereview.chromium.org/414503004/diff/1/android_webview/test/shell/s...
android_webview/test/shell/src/org/chromium/android_webview/shell/AwShellActivity.java:61:
WindowManager.LayoutParams.FLAG_HARDWARE_ACCELERATED);
I forgot if this is necassary. The canvas for the SurfaceView will probably
accelerated regardless and punches a hole in any window, but not sure if we
check whether the view itself is accelerated in some other place.

https://codereview.chromium.org/414503004/diff/1/android_webview/test/shell/s...
File
android_webview/test/shell/src/org/chromium/android_webview/test/AwTestContainerView.java
(right):

https://codereview.chromium.org/414503004/diff/1/android_webview/test/shell/s...
android_webview/test/shell/src/org/chromium/android_webview/test/AwTestContainerView.java:117:
mRootView.addView(mSurfaceView,
you can also call 'addView' instead of 'mRootView.addView' which adds the
SurfaceView as a child to the container view. however, i think because the
SurfaceView then actually moves when the container view is scrolled, this messes
things up more.

https://codereview.chromium.org/414503004/diff/1/gpu/command_buffer/service/a...
File gpu/command_buffer/service/async_pixel_transfer_manager_android.cc (right):

https://codereview.chromium.org/414503004/diff/1/gpu/command_buffer/service/a...
gpu/command_buffer/service/async_pixel_transfer_manager_android.cc:62:
context->HasExtension("GL_OES_EGL_image") && false &&
using egl image here might cause issues with mailbox sync, so disabled it which
will use idle uploader.

https://codereview.chromium.org/414503004/diff/1/gpu/command_buffer/service/i...
File gpu/command_buffer/service/in_process_command_buffer.cc (right):

https://codereview.chromium.org/414503004/diff/1/gpu/command_buffer/service/i...
gpu/command_buffer/service/in_process_command_buffer.cc:301:
capabilities_.map_image = false;
force async upload path. i tried using GLImageShm also, but got cmdbuffer parse
errors - did not investigate further yet.

[no sievers, 2014-07-22 21:00:36]

https://codereview.chromium.org/414503004/diff/1/android_webview/browser/hard...
File android_webview/browser/hardware_renderer.cc (right):

https://codereview.chromium.org/414503004/diff/1/android_webview/browser/hard...
android_webview/browser/hardware_renderer.cc:203: // 
transform.Translate(scroll_offset_.x(), scroll_offset_.y());
So here I don't understand why we scroll in the parent compositor and not in the
child. It actually causes problems in this setup.

[no sievers, 2014-08-27 22:53:55]

sievers@chromium.org changed reviewers:
+ mkosiba@chromium.org

[no sievers, 2014-08-27 22:54:03]

ptal

[boliu, 2014-08-28 23:15:06]

boliu@chromium.org changed reviewers:
+ hush@chromium.org

[boliu, 2014-08-28 23:15:06]

+hush too

https://codereview.chromium.org/414503004/diff/80001/android_webview/test/she...
File android_webview/test/shell/src/draw_gl.cc (right):

https://codereview.chromium.org/414503004/diff/80001/android_webview/test/she...
android_webview/test/shell/src/draw_gl.cc:7: #include "draw_gl.h"
use full path instead of adding a custom include path?

https://codereview.chromium.org/414503004/diff/80001/android_webview/test/she...
android_webview/test/shell/src/draw_gl.cc:29: reinterpret_cast<void (*)(long,
AwDrawGLInfo*, void*)>(draw_gl);
AwDrawGLFunction (typedef in draw_gl.h), also can you move this below to just
before where it's used?

https://codereview.chromium.org/414503004/diff/80001/android_webview/test/she...
android_webview/test/shell/src/draw_gl.cc:31: draw_info.mode =
static_cast<AwDrawGLInfo::Mode>(mode);
draw_info.version = kAwDrawGLInfoVersion;

https://codereview.chromium.org/414503004/diff/80001/android_webview/test/she...
android_webview/test/shell/src/draw_gl.cc:32: draw_info.is_layer = true;
Not a big deal but most common case is is_layer = false.

https://codereview.chromium.org/414503004/diff/80001/android_webview/test/she...
File
android_webview/test/shell/src/org/chromium/android_webview/shell/AwShellActivity.java
(right):

https://codereview.chromium.org/414503004/diff/80001/android_webview/test/she...
android_webview/test/shell/src/org/chromium/android_webview/shell/AwShellActivity.java:58:
getWindow().setFlags(
You can change android_webview/test/shell/AndroidManifest.xml so whole
application to be hardware accelerated.

Although according to
http://developer.android.com/guide/topics/graphics/hardware-accel.html#determ...,
you can't tell if a window is accelerated until a view is attached to it, so
still have to hardcode that part below :/

https://codereview.chromium.org/414503004/diff/80001/android_webview/test/she...
File
android_webview/test/shell/src/org/chromium/android_webview/test/AwTestContainerView.java
(right):

https://codereview.chromium.org/414503004/diff/80001/android_webview/test/she...
android_webview/test/shell/src/org/chromium/android_webview/test/AwTestContainerView.java:140:
while (mNeedsDrawGL) {
We can pass waitForCompletion to here, and only do this while loop if it's true?

https://codereview.chromium.org/414503004/diff/80001/android_webview/test/she...
android_webview/test/shell/src/org/chromium/android_webview/test/AwTestContainerView.java:165:
mSyncLock.notifyAll();
MODE_SYNC doesn't run GL. You want to move this to after MODE_DRAW or
MODE_PROCESS.

https://codereview.chromium.org/414503004/diff/80001/android_webview/test/she...
android_webview/test/shell/src/org/chromium/android_webview/test/AwTestContainerView.java:236:
assert !mAttachedContents;
Hmm, what if onDetachedFromWindow is called before the surface is ready?

https://codereview.chromium.org/414503004/diff/80001/android_webview/test/she...
android_webview/test/shell/src/org/chromium/android_webview/test/AwTestContainerView.java:322:
if (mHardwareView != null) {
More of a question for Martin I guess. I'm not sure what happens right now if
scroll changes in onDraw (eg if the draw has a compositor activation). I think
this is still correct, and the scroll will only be taken into account by android
compositor in the next frame

https://codereview.chromium.org/414503004/diff/80001/android_webview/test/she...
android_webview/test/shell/src/org/chromium/android_webview/test/AwTestContainerView.java:331:
// Huh?
Me too, huh?

[boliu, 2014-08-28 23:54:32]

https://codereview.chromium.org/414503004/diff/80001/android_webview/test/she...
File
android_webview/test/shell/src/org/chromium/android_webview/test/AwTestContainerView.java
(right):

https://codereview.chromium.org/414503004/diff/80001/android_webview/test/she...
android_webview/test/shell/src/org/chromium/android_webview/test/AwTestContainerView.java:411:
if (mHardwareView != null) {
So this is a pretty big-ish hack to counter-act the HardwareView being
translated due to the AwTestContainerView being scrolled, but I think it
actually makes total sense to do this to basically simulate the "screen".

Worth a comment though.

[no sievers, 2014-08-29 01:30:44]

https://codereview.chromium.org/414503004/diff/80001/android_webview/test/she...
File android_webview/test/shell/src/draw_gl.cc (right):

https://codereview.chromium.org/414503004/diff/80001/android_webview/test/she...
android_webview/test/shell/src/draw_gl.cc:7: #include "draw_gl.h"
On 2014/08/28 23:15:05, boliu wrote:
> use full path instead of adding a custom include path?

So using relative paths doesn't work well with DEPS.

But I had the include path point to the base directory instead and then added a
DEPS file for draw_gl to further keep anyone from adding code or dependencies to
this.

https://codereview.chromium.org/414503004/diff/80001/android_webview/test/she...
android_webview/test/shell/src/draw_gl.cc:29: reinterpret_cast<void (*)(long,
AwDrawGLInfo*, void*)>(draw_gl);
On 2014/08/28 23:15:05, boliu wrote:
> AwDrawGLFunction (typedef in draw_gl.h), also can you move this below to just
> before where it's used?

Done.

https://codereview.chromium.org/414503004/diff/80001/android_webview/test/she...
android_webview/test/shell/src/draw_gl.cc:31: draw_info.mode =
static_cast<AwDrawGLInfo::Mode>(mode);
On 2014/08/28 23:15:05, boliu wrote:
> draw_info.version = kAwDrawGLInfoVersion;

Done.

https://codereview.chromium.org/414503004/diff/80001/android_webview/test/she...
android_webview/test/shell/src/draw_gl.cc:32: draw_info.is_layer = true;
On 2014/08/28 23:15:05, boliu wrote:
> Not a big deal but most common case is is_layer = false.

Done.

https://codereview.chromium.org/414503004/diff/80001/android_webview/test/she...
File
android_webview/test/shell/src/org/chromium/android_webview/shell/AwShellActivity.java
(right):

https://codereview.chromium.org/414503004/diff/80001/android_webview/test/she...
android_webview/test/shell/src/org/chromium/android_webview/shell/AwShellActivity.java:58:
getWindow().setFlags(
On 2014/08/28 23:15:06, boliu wrote:
> You can change android_webview/test/shell/AndroidManifest.xml so whole
> application to be hardware accelerated.
> 
> Although according to
>
http://developer.android.com/guide/topics/graphics/hardware-accel.html#determ...,
> you can't tell if a window is accelerated until a view is attached to it, so
> still have to hardcode that part below :/

Done. I made it an argument to the test container view so that we can eventually
run tests in both hw and sw mode.

https://codereview.chromium.org/414503004/diff/80001/android_webview/test/she...
File
android_webview/test/shell/src/org/chromium/android_webview/test/AwTestContainerView.java
(right):

https://codereview.chromium.org/414503004/diff/80001/android_webview/test/she...
android_webview/test/shell/src/org/chromium/android_webview/test/AwTestContainerView.java:140:
while (mNeedsDrawGL) {
On 2014/08/28 23:15:06, boliu wrote:
> We can pass waitForCompletion to here, and only do this while loop if it's
true?

Don't we only request draw after we have committed a new frame nowadays, at
which point we need to wait for the SYNC here after onDraw?

https://codereview.chromium.org/414503004/diff/80001/android_webview/test/she...
android_webview/test/shell/src/org/chromium/android_webview/test/AwTestContainerView.java:165:
mSyncLock.notifyAll();
On 2014/08/28 23:15:06, boliu wrote:
> MODE_SYNC doesn't run GL. You want to move this to after MODE_DRAW or
> MODE_PROCESS.

The lock was only meant to block the UI thread until a new frame has been
committed. Do we really need to wait for any GL?

https://codereview.chromium.org/414503004/diff/80001/android_webview/test/she...
android_webview/test/shell/src/org/chromium/android_webview/test/AwTestContainerView.java:236:
assert !mAttachedContents;
On 2014/08/28 23:15:06, boliu wrote:
> Hmm, what if onDetachedFromWindow is called before the surface is ready?

Ok, added a call to reset the callback in onDetached() below.

https://codereview.chromium.org/414503004/diff/80001/android_webview/test/she...
android_webview/test/shell/src/org/chromium/android_webview/test/AwTestContainerView.java:331:
// Huh?
On 2014/08/28 23:15:06, boliu wrote:
> Me too, huh?

Removed. This happened somewhere to me, maybe on K.

https://codereview.chromium.org/414503004/diff/80001/android_webview/test/she...
android_webview/test/shell/src/org/chromium/android_webview/test/AwTestContainerView.java:411:
if (mHardwareView != null) {
On 2014/08/28 23:54:32, boliu wrote:
> So this is a pretty big-ish hack to counter-act the HardwareView being
> translated due to the AwTestContainerView being scrolled, but I think it
> actually makes total sense to do this to basically simulate the "screen".
> 
> Worth a comment though.

Done.

[boliu, 2014-08-29 02:36:04]

https://codereview.chromium.org/414503004/diff/120001/android_webview/test/sh...
File
android_webview/test/shell/src/org/chromium/android_webview/test/AwTestContainerView.java
(right):

https://codereview.chromium.org/414503004/diff/120001/android_webview/test/sh...
android_webview/test/shell/src/org/chromium/android_webview/test/AwTestContainerView.java:249:
mHardwareView.setReadyToRenderCallback(null);
Need to update the assert in setReadyToRenderCallback to

assert !isReadyToRender() || runner == null;

https://codereview.chromium.org/414503004/diff/120001/android_webview/test/sh...
android_webview/test/shell/src/org/chromium/android_webview/test/AwTestContainerView.java:360:
public boolean requestDrawGL(Canvas canvas, boolean waitForCompletion,
So as discussed, waitForCompletion is important for detach to work correctly, so
can't be ignored.

[no sievers, 2014-08-29 20:41:28]

https://codereview.chromium.org/414503004/diff/120001/android_webview/test/sh...
File
android_webview/test/shell/src/org/chromium/android_webview/test/AwTestContainerView.java
(right):

https://codereview.chromium.org/414503004/diff/120001/android_webview/test/sh...
android_webview/test/shell/src/org/chromium/android_webview/test/AwTestContainerView.java:249:
mHardwareView.setReadyToRenderCallback(null);
On 2014/08/29 02:36:04, boliu wrote:
> Need to update the assert in setReadyToRenderCallback to
> 
> assert !isReadyToRender() || runner == null;

Done.

https://codereview.chromium.org/414503004/diff/120001/android_webview/test/sh...
android_webview/test/shell/src/org/chromium/android_webview/test/AwTestContainerView.java:360:
public boolean requestDrawGL(Canvas canvas, boolean waitForCompletion,
On 2014/08/29 02:36:04, boliu wrote:
> So as discussed, waitForCompletion is important for detach to work correctly,
so
> can't be ignored.

Done.

[no sievers, 2014-08-29 20:48:09]

https://codereview.chromium.org/414503004/diff/140001/android_webview/test/sh...
File android_webview/test/shell/AndroidManifest.xml (right):

https://codereview.chromium.org/414503004/diff/140001/android_webview/test/sh...
android_webview/test/shell/AndroidManifest.xml:13:
android:label="AwShellApplication" android:hardwareAccelerated="true">
This breaks all the tests since it applies to AwTestRunnerActivity also.
Go back to what I had before?

[no sievers, 2014-08-29 20:49:04]

https://codereview.chromium.org/414503004/diff/140001/android_webview/test/sh...
File android_webview/test/shell/AndroidManifest.xml (right):

https://codereview.chromium.org/414503004/diff/140001/android_webview/test/sh...
android_webview/test/shell/AndroidManifest.xml:13:
android:label="AwShellApplication" android:hardwareAccelerated="true">
On 2014/08/29 20:48:08, sievers wrote:
> This breaks all the tests since it applies to AwTestRunnerActivity also.
> Go back to what I had before?

Ah wait, it says you can put this at the activity level also. Trying...

[no sievers, 2014-08-29 20:52:08]

https://codereview.chromium.org/414503004/diff/140001/android_webview/test/sh...
File android_webview/test/shell/AndroidManifest.xml (right):

https://codereview.chromium.org/414503004/diff/140001/android_webview/test/sh...
android_webview/test/shell/AndroidManifest.xml:13:
android:label="AwShellApplication" android:hardwareAccelerated="true">
On 2014/08/29 20:49:04, sievers wrote:
> On 2014/08/29 20:48:08, sievers wrote:
> > This breaks all the tests since it applies to AwTestRunnerActivity also.
> > Go back to what I had before?
> 
> Ah wait, it says you can put this at the activity level also. Trying...

Seems to work. Updated.

[boliu, 2014-08-29 22:33:04]

lgtm

Hmm, how did the tests fail exactly?

[no sievers, 2014-08-29 23:15:32]

The CQ bit was checked by sievers@chromium.org

[no sievers, 2014-08-29 23:17:26]

The CQ bit was unchecked by sievers@chromium.org

[no sievers, 2014-08-29 23:17:48]

The CQ bit was checked by sievers@chromium.org

[commit-bot: I haz the power, 2014-08-29 23:19:43]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/sievers@chromium.org/414503004/160001

[no sievers, 2014-08-29 23:33:29]

The CQ bit was unchecked by sievers@chromium.org

[commit-bot: I haz the power, 2014-08-29 23:38:46]

Committed patchset #9 (id:160001) as 09acbf59e7e8f1de574498ee5ba8eae0b0775438

[no sievers, 2014-08-29 23:59:31]

A revert of this CL (patchset #9 id:160001) has been created in
https://codereview.chromium.org/524933002/ by sievers@chromium.org.

The reason for reverting is: clang.

[no sievers, 2014-08-30 01:11:03]

The CQ bit was checked by sievers@chromium.org

[commit-bot: I haz the power, 2014-08-30 01:11:53]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/sievers@chromium.org/414503004/220001

[commit-bot: I haz the power, 2014-08-30 02:19:46]

Committed patchset #12 (id:220001) as d62a79f70786be78405c9342acec12eadd26261a

[commit-bot: I haz the power, 2014-09-10 03:11:46]

Patchset 12 (id:??) landed as
https://crrev.com/386f9301889efde34b202a4600496b1e10aa06c8
Cr-Commit-Position: refs/heads/master@{#292718}

[commit-bot: I haz the power, 2014-09-10 03:13:23]

Patchset 12 (id:??) landed as
https://crrev.com/625a78652e170011fd3590ec9791919f8c1093f0
Cr-Commit-Position: refs/heads/master@{#292761}