Transfer DrawGLFunctor ownership from AwContents to AwGLFunctor

Transfer DrawGLFunctor ownership from AwContents to AwGLFunctor

BUG=597167
Committed: https://crrev.com/dbdd755abfd49e973f683967a5ad54edc5dc7274
Cr-Commit-Position: refs/heads/master@{#388757}

[Tobias Sargeant, 2016-04-20 15:16:38]

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

[boliu, 2016-04-20 15:38:32]

close

so next step would be to think about removing the cleanupreference from
DrawGLFunctor. Do that in follow up.

https://codereview.chromium.org/1904453004/diff/1/android_webview/glue/java/s...
File
android_webview/glue/java/src/com/android/webview/chromium/DrawGLFunctor.java
(right):

https://codereview.chromium.org/1904453004/diff/1/android_webview/glue/java/s...
android_webview/glue/java/src/com/android/webview/chromium/DrawGLFunctor.java:30:
View mContainerView;
clean this stuff up again while you are at this

https://codereview.chromium.org/1904453004/diff/1/android_webview/glue/java/s...
android_webview/glue/java/src/com/android/webview/chromium/DrawGLFunctor.java:57:
if (!mWebViewDelegate.canInvokeDrawGlFunctor(containerView)) {
yeah, don't need this check here :)

https://codereview.chromium.org/1904453004/diff/1/android_webview/java/src/or...
File android_webview/java/src/org/chromium/android_webview/AwContents.java
(right):

https://codereview.chromium.org/1904453004/diff/1/android_webview/java/src/or...
android_webview/java/src/org/chromium/android_webview/AwContents.java:198:
boolean requestDrawGL(View containerView, Canvas canvas, Runnable
releasedCallback);
this doesn't need container view, right?

[Tobias Sargeant, 2016-04-20 16:00:31]

https://codereview.chromium.org/1904453004/diff/1/android_webview/glue/java/s...
File
android_webview/glue/java/src/com/android/webview/chromium/DrawGLFunctor.java
(right):

https://codereview.chromium.org/1904453004/diff/1/android_webview/glue/java/s...
android_webview/glue/java/src/com/android/webview/chromium/DrawGLFunctor.java:57:
if (!mWebViewDelegate.canInvokeDrawGlFunctor(containerView)) {
On 2016/04/20 15:38:32, boliu wrote:
> yeah, don't need this check here :)

The name kind of gave it away, but it *was* done in either case previously, so I
wasn't sure. If it can be removed, great.

https://codereview.chromium.org/1904453004/diff/1/android_webview/java/src/or...
File android_webview/java/src/org/chromium/android_webview/AwContents.java
(right):

https://codereview.chromium.org/1904453004/diff/1/android_webview/java/src/or...
android_webview/java/src/org/chromium/android_webview/AwContents.java:198:
boolean requestDrawGL(View containerView, Canvas canvas, Runnable
releasedCallback);
On 2016/04/20 15:38:32, boliu wrote:
> this doesn't need container view, right?

Contingent upon not needing the check, no, it doesn't.

https://codereview.chromium.org/1904453004/diff/1/android_webview/java/src/or...
android_webview/java/src/org/chromium/android_webview/AwContents.java:203:
boolean requestDrawGL(View containerView, boolean waitForCompletion);
I considered that maybe this could be called requestInvokeGL. WDYT?

[boliu, 2016-04-20 16:01:26]

https://codereview.chromium.org/1904453004/diff/1/android_webview/glue/java/s...
File
android_webview/glue/java/src/com/android/webview/chromium/DrawGLFunctor.java
(right):

https://codereview.chromium.org/1904453004/diff/1/android_webview/glue/java/s...
android_webview/glue/java/src/com/android/webview/chromium/DrawGLFunctor.java:57:
if (!mWebViewDelegate.canInvokeDrawGlFunctor(containerView)) {
On 2016/04/20 15:38:32, boliu wrote:
> yeah, don't need this check here :)

You can replace this check on the chromium side with whether the view is
attached, actually. Same thing.

https://codereview.chromium.org/1904453004/diff/1/android_webview/glue/java/s...
android_webview/glue/java/src/com/android/webview/chromium/DrawGLFunctor.java:78:
if (!mWebViewDelegate.canInvokeDrawGlFunctor(containerView)) {
We can actually remove this on N, ie where sSupportFunctorReleasedCallback is
true. N promises that invokeDrawGLFunctor is safe to call at any time, since we
need to call it after view has been detached.

[boliu, 2016-04-20 16:02:23]

https://codereview.chromium.org/1904453004/diff/1/android_webview/java/src/or...
File android_webview/java/src/org/chromium/android_webview/AwContents.java
(right):

https://codereview.chromium.org/1904453004/diff/1/android_webview/java/src/or...
android_webview/java/src/org/chromium/android_webview/AwContents.java:203:
boolean requestDrawGL(View containerView, boolean waitForCompletion);
On 2016/04/20 16:00:31, Tobias Sargeant wrote:
> I considered that maybe this could be called requestInvokeGL. WDYT?

sgtm

[boliu, 2016-04-20 16:49:09]

lgtm

https://codereview.chromium.org/1904453004/diff/40001/android_webview/browser...
File android_webview/browser/render_thread_manager.cc (right):

https://codereview.chromium.org/1904453004/diff/40001/android_webview/browser...
android_webview/browser/render_thread_manager.cc:147: if
(!client_->RequestInvokeGL(false)) {
Hmm... this should *almost* never happen now. Should consider getting this thing
altogether. Ok for now though.

https://codereview.chromium.org/1904453004/diff/40001/android_webview/glue/ja...
File
android_webview/glue/java/src/com/android/webview/chromium/DrawGLFunctor.java
(right):

https://codereview.chromium.org/1904453004/diff/40001/android_webview/glue/ja...
android_webview/glue/java/src/com/android/webview/chromium/DrawGLFunctor.java:28:
private final long mNativeDrawGLFunctor;
this looks unused

https://codereview.chromium.org/1904453004/diff/40001/android_webview/glue/ja...
android_webview/glue/java/src/com/android/webview/chromium/DrawGLFunctor.java:66:
if (mDestroyRunnable.mNativeDrawGLFunctor == 0) {
This can't ever happen. There is no explicit destroy call anymore

https://codereview.chromium.org/1904453004/diff/40001/android_webview/java/sr...
File android_webview/java/src/org/chromium/android_webview/AwGLFunctor.java
(right):

https://codereview.chromium.org/1904453004/diff/40001/android_webview/java/sr...
android_webview/java/src/org/chromium/android_webview/AwGLFunctor.java:38:
private final AwContents.NativeDrawGLFunctor mNativeDrawGLFunctor;
needs a rebase now

[boliu, 2016-04-20 17:19:55]

still lgtm

[Tobias Sargeant, 2016-04-20 17:20:24]

The CQ bit was checked by tobiasjs@chromium.org

[commit-bot: I haz the power, 2016-04-20 17:20:37]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1904453004/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1904453004/60001

[commit-bot: I haz the power, 2016-04-20 19:14:05]

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

[commit-bot: I haz the power, 2016-04-20 19:14:06]

Try jobs failed on following builders:
  linux_android_rel_ng on tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[boliu, 2016-04-20 20:51:14]

This assertion do not hold for full screen video tests

f9f3f:  04-20 18:55:09.439 29667 29667 W System.err: java.lang.AssertionError
f9f3f:  04-20 18:55:09.439 29667 29667 W System.err:  at
org.chromium.android_webview.test.AwTestContainerView$NativeDrawGLFunctor.detach(AwTestContainerView.java:453)
f9f3f:  04-20 18:55:09.439 29667 29667 W System.err:  at
org.chromium.android_webview.AwGLFunctor.detachFunctorFromView(AwGLFunctor.java:87)
f9f3f:  04-20 18:55:09.439 29667 29667 W System.err:  at
org.chromium.android_webview.AwGLFunctor.nativeDeleteHardwareRenderer(Native
Method)
f9f3f:  04-20 18:55:09.439 29667 29667 W System.err:  at
org.chromium.android_webview.AwGLFunctor.deleteHardwareRenderer(AwGLFunctor.java:92)
f9f3f:  04-20 18:55:09.439 29667 29667 W System.err:  at
org.chromium.android_webview.AwContents$AwViewMethodsImpl.onDetachedFromWindow(AwContents.java:3113)
f9f3f:  04-20 18:55:09.449 29667 29667 W System.err:  at
org.chromium.android_webview.AwContents.onDetachedFromWindow(AwContents.java:2348)
f9f3f:  04-20 18:55:09.449 29667 29667 W System.err:  at
org.chromium.android_webview.AwContents.enterFullScreen(AwContents.java:820)
f9f3f:  04-20 18:55:09.449 29667 29667 W System.err:  at
org.chromium.android_webview.AwContentViewClient.enterFullscreen(AwContentViewClient.java:128)
f9f3f:  04-20 18:55:09.449 29667 29667 W System.err:  at
org.chromium.android_webview.AwWebContentsDelegateAdapter.toggleFullscreenModeForTab(AwWebContentsDelegateAdapter.java:266)
f9f3f:  04-20 18:55:09.449 29667 29667 W System.err:  at
org.chromium.base.SystemMessageHandler.nativeDoRunLoopOnce(Native Method)
f9f3f:  04-20 18:55:09.449 29667 29667 W System.err:  at
org.chromium.base.SystemMessageHandler.handleMessage(SystemMessageHandler.java:39)
f9f3f:  04-20 18:55:09.449 29667 29667 W System.err:  at
android.os.Handler.dispatchMessage(Handler.java:102)
f9f3f:  04-20 18:55:09.449 29667 29667 W System.err:  at
android.os.Looper.loop(Looper.java:136)
f9f3f:  04-20 18:55:09.449 29667 29667 W System.err:  at
android.app.ActivityThread.main(ActivityThread.java:5001)
f9f3f:  04-20 18:55:09.449 29667 29667 W System.err:  at
java.lang.reflect.Method.invokeNative(Native Method)
f9f3f:  04-20 18:55:09.449 29667 29667 W System.err:  at
java.lang.reflect.Method.invoke(Method.java:515)
f9f3f:  04-20 18:55:09.449 29667 29667 W System.err:  at
com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:785)
f9f3f:  04-20 18:55:09.449 29667 29667 W System.err:  at
com.android.internal.os.ZygoteInit.main(ZygoteInit.java:601)
f9f3f:  04-20 18:55:09.449 29667 29667 W System.err:  at
dalvik.system.NativeStart.main(Native Method)

[Tobias Sargeant, 2016-04-21 09:09:20]

The CQ bit was checked by tobiasjs@chromium.org

[Tobias Sargeant, 2016-04-21 09:09:20]

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

[commit-bot: I haz the power, 2016-04-21 09:09:30]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1904453004/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1904453004/80001

[commit-bot: I haz the power, 2016-04-21 09:14:22]

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

[commit-bot: I haz the power, 2016-04-21 09:14:23]

Try jobs failed on following builders:
  android_chromium_gn_compile_dbg on tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_chro...)
  linux_chromium_compile_dbg_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Tobias Sargeant, 2016-04-21 09:29:42]

The CQ bit was checked by tobiasjs@chromium.org

[commit-bot: I haz the power, 2016-04-21 09:29:57]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1904453004/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1904453004/80001

[commit-bot: I haz the power, 2016-04-21 09:35:14]

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

[commit-bot: I haz the power, 2016-04-21 09:35:15]

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

[Tobias Sargeant, 2016-04-21 10:14:04]

The CQ bit was checked by tobiasjs@chromium.org

[commit-bot: I haz the power, 2016-04-21 10:14:08]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1904453004/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1904453004/80001

[commit-bot: I haz the power, 2016-04-21 11:32:53]

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

[commit-bot: I haz the power, 2016-04-21 11:32:53]

Try jobs failed on following builders:
  linux_android_rel_ng on tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[Tobias Sargeant, 2016-04-21 13:16:46]

The CQ bit was checked by tobiasjs@chromium.org

[commit-bot: I haz the power, 2016-04-21 13:16:51]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1904453004/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1904453004/80001

[commit-bot: I haz the power, 2016-04-21 13:21:03]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-04-22 19:34:09]

Description was changed from

==========
Transfer DrawGLFunctor ownership from AwContents to AwGLFunctor

BUG=597167
==========

to

==========
Transfer DrawGLFunctor ownership from AwContents to AwGLFunctor

BUG=597167

Committed: https://crrev.com/dbdd755abfd49e973f683967a5ad54edc5dc7274
Cr-Commit-Position: refs/heads/master@{#388757}
==========

[commit-bot: I haz the power, 2016-04-22 19:34:10]

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