New issue
Advanced search Search tips

Issue 597167 link

Starred by 4 users
Status: Assigned
Owner:
tobiasjs@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 3
Type: Task



Sign in to add a comment

[Android WebVIew] Async functor destroy

Project Member Reported by boliu@chromium.org, Mar 23 2016 
Live design doc: https://docs.google.com/document/d/1ElwPzFflLAnuFW26tM2jVydzg5WbSz6wYiJba_iIfu4/edit?usp=sharing

Right now, BrowserViewRenderer lives on the UI thread, SharedRendererState straddles UI and Render Thread, and HardwareRenderer lives entirely on RenderThread. BVR owns SRS owns HR. You can get a sense of how the functor should behave, at least at the BVR level, by studying the test harness for browser_view_renderer_unittest.cc

First step is move ownership of SRS to native AwContents, and add APIs to BVR to attach and detach SRS so that SRS and BVR can (in theory) have independent lifetimes.
Project Member

Comment 1 by bugdroid1@chromium.org, Mar 30 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/af7b098d0d2cd8b567d19fbf8a233bb00532b0ae

commit af7b098d0d2cd8b567d19fbf8a233bb00532b0ae
Author: tobiasjs <tobiasjs@chromium.org>
Date: Wed Mar 30 18:33:06 2016

Move SharedRendererState ownership to AwContents

In order to support asynchronous functor destruction, we need
to decouple the lifetimes of SharedRendererState and
BrowserViewRenderer. AwContents becomes responsible for
informing BVR of the SharedRendererState instance to use,
and SRS talks to AwC instead of BVR.

BUG=597167

Review URL: https://codereview.chromium.org/1816283005

Cr-Commit-Position: refs/heads/master@{#384031}

[modify] https://crrev.com/af7b098d0d2cd8b567d19fbf8a233bb00532b0ae/android_webview/browser/browser_view_renderer.cc
[modify] https://crrev.com/af7b098d0d2cd8b567d19fbf8a233bb00532b0ae/android_webview/browser/browser_view_renderer.h
[modify] https://crrev.com/af7b098d0d2cd8b567d19fbf8a233bb00532b0ae/android_webview/browser/browser_view_renderer_client.h
[modify] https://crrev.com/af7b098d0d2cd8b567d19fbf8a233bb00532b0ae/android_webview/browser/browser_view_renderer_unittest.cc
[modify] https://crrev.com/af7b098d0d2cd8b567d19fbf8a233bb00532b0ae/android_webview/browser/hardware_renderer.cc
[modify] https://crrev.com/af7b098d0d2cd8b567d19fbf8a233bb00532b0ae/android_webview/browser/shared_renderer_state.cc
[modify] https://crrev.com/af7b098d0d2cd8b567d19fbf8a233bb00532b0ae/android_webview/browser/shared_renderer_state.h
[add] https://crrev.com/af7b098d0d2cd8b567d19fbf8a233bb00532b0ae/android_webview/browser/shared_renderer_state_client.h
[modify] https://crrev.com/af7b098d0d2cd8b567d19fbf8a233bb00532b0ae/android_webview/browser/test/fake_window.cc
[modify] https://crrev.com/af7b098d0d2cd8b567d19fbf8a233bb00532b0ae/android_webview/browser/test/fake_window.h
[modify] https://crrev.com/af7b098d0d2cd8b567d19fbf8a233bb00532b0ae/android_webview/browser/test/rendering_test.cc
[modify] https://crrev.com/af7b098d0d2cd8b567d19fbf8a233bb00532b0ae/android_webview/browser/test/rendering_test.h
[modify] https://crrev.com/af7b098d0d2cd8b567d19fbf8a233bb00532b0ae/android_webview/native/aw_contents.cc
[modify] https://crrev.com/af7b098d0d2cd8b567d19fbf8a233bb00532b0ae/android_webview/native/aw_contents.h
Project Member

Comment 2 by bugdroid1@chromium.org, Mar 30 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/af7b098d0d2cd8b567d19fbf8a233bb00532b0ae

commit af7b098d0d2cd8b567d19fbf8a233bb00532b0ae
Author: tobiasjs <tobiasjs@chromium.org>
Date: Wed Mar 30 18:33:06 2016

Move SharedRendererState ownership to AwContents

In order to support asynchronous functor destruction, we need
to decouple the lifetimes of SharedRendererState and
BrowserViewRenderer. AwContents becomes responsible for
informing BVR of the SharedRendererState instance to use,
and SRS talks to AwC instead of BVR.

BUG=597167

Review URL: https://codereview.chromium.org/1816283005

Cr-Commit-Position: refs/heads/master@{#384031}

[modify] https://crrev.com/af7b098d0d2cd8b567d19fbf8a233bb00532b0ae/android_webview/browser/browser_view_renderer.cc
[modify] https://crrev.com/af7b098d0d2cd8b567d19fbf8a233bb00532b0ae/android_webview/browser/browser_view_renderer.h
[modify] https://crrev.com/af7b098d0d2cd8b567d19fbf8a233bb00532b0ae/android_webview/browser/browser_view_renderer_client.h
[modify] https://crrev.com/af7b098d0d2cd8b567d19fbf8a233bb00532b0ae/android_webview/browser/browser_view_renderer_unittest.cc
[modify] https://crrev.com/af7b098d0d2cd8b567d19fbf8a233bb00532b0ae/android_webview/browser/hardware_renderer.cc
[modify] https://crrev.com/af7b098d0d2cd8b567d19fbf8a233bb00532b0ae/android_webview/browser/shared_renderer_state.cc
[modify] https://crrev.com/af7b098d0d2cd8b567d19fbf8a233bb00532b0ae/android_webview/browser/shared_renderer_state.h
[add] https://crrev.com/af7b098d0d2cd8b567d19fbf8a233bb00532b0ae/android_webview/browser/shared_renderer_state_client.h
[modify] https://crrev.com/af7b098d0d2cd8b567d19fbf8a233bb00532b0ae/android_webview/browser/test/fake_window.cc
[modify] https://crrev.com/af7b098d0d2cd8b567d19fbf8a233bb00532b0ae/android_webview/browser/test/fake_window.h
[modify] https://crrev.com/af7b098d0d2cd8b567d19fbf8a233bb00532b0ae/android_webview/browser/test/rendering_test.cc
[modify] https://crrev.com/af7b098d0d2cd8b567d19fbf8a233bb00532b0ae/android_webview/browser/test/rendering_test.h
[modify] https://crrev.com/af7b098d0d2cd8b567d19fbf8a233bb00532b0ae/android_webview/native/aw_contents.cc
[modify] https://crrev.com/af7b098d0d2cd8b567d19fbf8a233bb00532b0ae/android_webview/native/aw_contents.h
Project Member

Comment 3 by bugdroid1@chromium.org, Apr 5 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1c63c2f902014ca9e36532a8cc82d048c2c940fa

commit 1c63c2f902014ca9e36532a8cc82d048c2c940fa
Author: tobiasjs <tobiasjs@chromium.org>
Date: Tue Apr 05 11:30:03 2016

Rename SharedRendererState to RenderThreadManager

BUG=597167

Review URL: https://codereview.chromium.org/1858853002

Cr-Commit-Position: refs/heads/master@{#385150}

[modify] https://crrev.com/1c63c2f902014ca9e36532a8cc82d048c2c940fa/android_webview/BUILD.gn
[modify] https://crrev.com/1c63c2f902014ca9e36532a8cc82d048c2c940fa/android_webview/android_webview.gyp
[modify] https://crrev.com/1c63c2f902014ca9e36532a8cc82d048c2c940fa/android_webview/browser/browser_view_renderer.cc
[modify] https://crrev.com/1c63c2f902014ca9e36532a8cc82d048c2c940fa/android_webview/browser/browser_view_renderer.h
[modify] https://crrev.com/1c63c2f902014ca9e36532a8cc82d048c2c940fa/android_webview/browser/browser_view_renderer_unittest.cc
[modify] https://crrev.com/1c63c2f902014ca9e36532a8cc82d048c2c940fa/android_webview/browser/deferred_gpu_command_service.cc
[modify] https://crrev.com/1c63c2f902014ca9e36532a8cc82d048c2c940fa/android_webview/browser/gl_view_renderer_manager.h
[modify] https://crrev.com/1c63c2f902014ca9e36532a8cc82d048c2c940fa/android_webview/browser/hardware_renderer.cc
[modify] https://crrev.com/1c63c2f902014ca9e36532a8cc82d048c2c940fa/android_webview/browser/hardware_renderer.h
[rename] https://crrev.com/1c63c2f902014ca9e36532a8cc82d048c2c940fa/android_webview/browser/render_thread_manager.cc
[rename] https://crrev.com/1c63c2f902014ca9e36532a8cc82d048c2c940fa/android_webview/browser/render_thread_manager.h
[rename] https://crrev.com/1c63c2f902014ca9e36532a8cc82d048c2c940fa/android_webview/browser/render_thread_manager_client.h
[modify] https://crrev.com/1c63c2f902014ca9e36532a8cc82d048c2c940fa/android_webview/browser/test/fake_window.cc
[modify] https://crrev.com/1c63c2f902014ca9e36532a8cc82d048c2c940fa/android_webview/browser/test/fake_window.h
[modify] https://crrev.com/1c63c2f902014ca9e36532a8cc82d048c2c940fa/android_webview/browser/test/rendering_test.cc
[modify] https://crrev.com/1c63c2f902014ca9e36532a8cc82d048c2c940fa/android_webview/browser/test/rendering_test.h
[modify] https://crrev.com/1c63c2f902014ca9e36532a8cc82d048c2c940fa/android_webview/native/aw_contents.cc
[modify] https://crrev.com/1c63c2f902014ca9e36532a8cc82d048c2c940fa/android_webview/native/aw_contents.h
Project Member

Comment 4 by bugdroid1@chromium.org, Apr 7 2016 

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/clank/internal/apps/+/1b10198cdbf446ea66243d697c4dcfc51bc1d765

commit 1b10198cdbf446ea66243d697c4dcfc51bc1d765
Author: Bo Liu <boliu@google.com>
Date: Thu Apr 07 22:55:05 2016
Project Member

Comment 6 by bugdroid1@chromium.org, Apr 8 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/53910e537e70af7b491ffaff4f581ea9c43fdb71

commit 53910e537e70af7b491ffaff4f581ea9c43fdb71
Author: tobiasjs <tobiasjs@chromium.org>
Date: Fri Apr 08 12:34:32 2016

Control the lifetime of RenderThreadManager from Java.

At the moment the lifetime of the RenderThreadManager instance stays
the same as it was previously, but we can now delay its destruction by
keeping the corresponding Java AwGLFunctor object alive, independently
of AwContents.

BUG=597167

Review URL: https://codereview.chromium.org/1844343005

Cr-Commit-Position: refs/heads/master@{#386053}

[modify] https://crrev.com/53910e537e70af7b491ffaff4f581ea9c43fdb71/android_webview/BUILD.gn
[modify] https://crrev.com/53910e537e70af7b491ffaff4f581ea9c43fdb71/android_webview/java/src/org/chromium/android_webview/AwContents.java
[add] https://crrev.com/53910e537e70af7b491ffaff4f581ea9c43fdb71/android_webview/java/src/org/chromium/android_webview/AwGLFunctor.java
[modify] https://crrev.com/53910e537e70af7b491ffaff4f581ea9c43fdb71/android_webview/native/BUILD.gn
[modify] https://crrev.com/53910e537e70af7b491ffaff4f581ea9c43fdb71/android_webview/native/android_webview_jni_registrar.cc
[modify] https://crrev.com/53910e537e70af7b491ffaff4f581ea9c43fdb71/android_webview/native/aw_contents.cc
[modify] https://crrev.com/53910e537e70af7b491ffaff4f581ea9c43fdb71/android_webview/native/aw_contents.h
[add] https://crrev.com/53910e537e70af7b491ffaff4f581ea9c43fdb71/android_webview/native/aw_gl_functor.cc
[add] https://crrev.com/53910e537e70af7b491ffaff4f581ea9c43fdb71/android_webview/native/aw_gl_functor.h
[modify] https://crrev.com/53910e537e70af7b491ffaff4f581ea9c43fdb71/android_webview/native/webview_native.gyp
Project Member

Comment 8 by bugdroid1@chromium.org, Apr 8 2016 

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/clank/internal/apps/+/566117ac5e43db1e7f0aefb9453f70b788c1ab31

commit 566117ac5e43db1e7f0aefb9453f70b788c1ab31
Author: boliu <boliu@chromium.org>
Date: Fri Apr 08 10:13:06 2016
Project Member

Comment 9 by bugdroid1@chromium.org, Apr 8 2016 

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/clank/internal/apps/+/a4e9eb175464147a2fd869d18a7d6dd6f25ce089

commit a4e9eb175464147a2fd869d18a7d6dd6f25ce089
Author: boliu <boliu@chromium.org>
Date: Fri Apr 08 17:23:47 2016
Project Member

Comment 10 by bugdroid1@chromium.org, Apr 8 2016 

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/clank/internal/apps/+/2b98808ad757faa4fc8d4a22b696289e31bde902

commit 2b98808ad757faa4fc8d4a22b696289e31bde902
Author: Bo Liu <boliu@google.com>
Date: Fri Apr 08 02:48:49 2016

Comment 11 by boliu@chromium.org, Apr 12 2016 

Issue 600782 has been merged into this issue.

Comment 12 by boliu@chromium.org, Apr 12 2016

Labels: ReleaseBlock-Stable
I'll think about maybe a small fix to merge back to m51 instead of the whole thing.
Project Member

Comment 13 by bugdroid1@chromium.org, Apr 12 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ee6d581cf7f60fc79ed8b7df472fa4e58790ec22

commit ee6d581cf7f60fc79ed8b7df472fa4e58790ec22
Author: boliu <boliu@chromium.org>
Date: Tue Apr 12 22:13:19 2016

aw: Quick workaround for functor detach crash/hang

Two quick fixes:
* On N where the functor detach callback is supported use it
  instead along with destroy to destroy AwGLFunctor
* In RenderThreadManager::DrawGL, make sure that
  HardwareRenderer is not re-created after window detach
  but before functor detach/destroy. Do this by checking
  if there is a frame for HardwareRenderer to consume.

Fixes above should ensure there are no crashes for using a
regular webview, but are still incomplete. These cases are
still wrong and should be fixed by the complete functor
lifetime refactor:
* Full screen video, which uses a different view (may crash)
* The pop up window flow still potentially uses the old
  functor (existing bug)
* Functor playback after window detach will not draw
  anything, which may cause flicker.

BUG=597167

Review URL: https://codereview.chromium.org/1882623002

Cr-Commit-Position: refs/heads/master@{#386831}

[modify] https://crrev.com/ee6d581cf7f60fc79ed8b7df472fa4e58790ec22/android_webview/browser/render_thread_manager.cc
[modify] https://crrev.com/ee6d581cf7f60fc79ed8b7df472fa4e58790ec22/android_webview/browser/render_thread_manager.h
[modify] https://crrev.com/ee6d581cf7f60fc79ed8b7df472fa4e58790ec22/android_webview/java/src/org/chromium/android_webview/AwContents.java

Comment 14 by boliu@chromium.org, Apr 13 2016

Labels: Merge-Request-51
Request merging #13 to m51. Then this will no longer need to block m51.

Comment 15 by tin...@google.com, Apr 13 2016

Labels: -Merge-Request-51 Merge-Approved-51 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M51 (branch: 2704)
Project Member

Comment 16 by bugdroid1@chromium.org, Apr 13 2016

Labels: -merge-approved-51 merge-merged-2704
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/cb6539bde8f3ae00e3ceed9334d8883c856d729c

commit cb6539bde8f3ae00e3ceed9334d8883c856d729c
Author: Bo Liu <boliu@chromium.org>
Date: Wed Apr 13 17:04:17 2016

[Merge M51] aw: Quick workaround for functor detach crash/hang

Two quick fixes:
* On N where the functor detach callback is supported use it
  instead along with destroy to destroy AwGLFunctor
* In RenderThreadManager::DrawGL, make sure that
  HardwareRenderer is not re-created after window detach
  but before functor detach/destroy. Do this by checking
  if there is a frame for HardwareRenderer to consume.

Fixes above should ensure there are no crashes for using a
regular webview, but are still incomplete. These cases are
still wrong and should be fixed by the complete functor
lifetime refactor:
* Full screen video, which uses a different view (may crash)
* The pop up window flow still potentially uses the old
  functor (existing bug)
* Functor playback after window detach will not draw
  anything, which may cause flicker.

BUG=597167

Review URL: https://codereview.chromium.org/1882623002

Cr-Commit-Position: refs/heads/master@{#386831}
(cherry picked from commit ee6d581cf7f60fc79ed8b7df472fa4e58790ec22)

Review URL: https://codereview.chromium.org/1885103002 .

Cr-Commit-Position: refs/branch-heads/2704@{#28}
Cr-Branched-From: 6e53600def8f60d8c632fadc70d7c1939ccea347-refs/heads/master@{#386251}

[modify] https://crrev.com/cb6539bde8f3ae00e3ceed9334d8883c856d729c/android_webview/browser/render_thread_manager.cc
[modify] https://crrev.com/cb6539bde8f3ae00e3ceed9334d8883c856d729c/android_webview/browser/render_thread_manager.h
[modify] https://crrev.com/cb6539bde8f3ae00e3ceed9334d8883c856d729c/android_webview/java/src/org/chromium/android_webview/AwContents.java

Comment 17 by boliu@chromium.org, Apr 13 2016

Labels: -ReleaseBlock-Stable -M-51 M-52

Comment 18 by boliu@chromium.org, Apr 13 2016

Labels: -M-52 ReleaseBlock-Stable M-51
Well, requirement just changed. See comment #56 on b/27709981
Project Member

Comment 19 by bugdroid1@chromium.org, Apr 13 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8e2e091e5c565b90e76ceb85dd4c848013c5cf94

commit 8e2e091e5c565b90e76ceb85dd4c848013c5cf94
Author: boliu <boliu@chromium.org>
Date: Wed Apr 13 19:50:40 2016

aw: Ensure WebView stays alive until functor detach

Android will not guarantee that the WebView is not garbage colllected
before functor detached is called. So we need to add a gc root while the
functor detached callback is pending.

BUG=597167

Review URL: https://codereview.chromium.org/1880243004

Cr-Commit-Position: refs/heads/master@{#387071}

[modify] https://crrev.com/8e2e091e5c565b90e76ceb85dd4c848013c5cf94/android_webview/java/src/org/chromium/android_webview/AwContents.java

Comment 20 by boliu@chromium.org, Apr 13 2016

Labels: Merge-Request-51
Request merging #19 to M51.

Comment 21 by tin...@google.com, Apr 13 2016

Labels: -Merge-Request-51 Merge-Approved-51
Your change meets the bar and is auto-approved for M51 (branch: 2704)

Comment 22 by boliu@chromium.org, Apr 13 2016 

Err, #19 leaks on N

Comment 23 by boliu@chromium.org, Apr 13 2016

Labels: -Merge-Approved-51
Not merging, but still RB-Stable for now, pending discussions in b/27709981.
Project Member

Comment 24 by bugdroid1@chromium.org, Apr 16 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3400c0da62249a66b9e6f5f0f6ef27dd89c50926

commit 3400c0da62249a66b9e6f5f0f6ef27dd89c50926
Author: boliu <boliu@chromium.org>
Date: Sat Apr 16 00:58:24 2016

Revert of aw: Ensure WebView stays alive until functor detach (patchset #1 id:1 of https://codereview.chromium.org/1880243004/ )

Reason for revert:
API changed

Original issue's description:
> aw: Ensure WebView stays alive until functor detach
>
> Android will not guarantee that the WebView is not garbage colllected
> before functor detached is called. So we need to add a gc root while the
> functor detached callback is pending.
>
> BUG=597167
>
> Committed: https://crrev.com/8e2e091e5c565b90e76ceb85dd4c848013c5cf94
> Cr-Commit-Position: refs/heads/master@{#387071}

TBR=tobiasjs@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG=597167

Review URL: https://codereview.chromium.org/1897583002

Cr-Commit-Position: refs/heads/master@{#387773}

[modify] https://crrev.com/3400c0da62249a66b9e6f5f0f6ef27dd89c50926/android_webview/java/src/org/chromium/android_webview/AwContents.java
Project Member

Comment 25 by bugdroid1@chromium.org, Apr 18 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c748c90944d0a21be20b4a0d8844e51bc828f659

commit c748c90944d0a21be20b4a0d8844e51bc828f659
Author: boliu <boliu@chromium.org>
Date: Mon Apr 18 16:29:43 2016

aw: Use functor released callback for lifetime management

This is a smaller CL for merging to m51 branch. Has similar caveats to
r386831 so not restating them here.

This CL does these things:
* Expose the new callback from android on the callDrawGlFunction
  function.
* Use the callback only as a strong reference to keep view
  CleanupReferences alive. The actual callback is a no-op for now.
* AwGLFunctor will always hold a the NativeGLDelegate and ContainerView
  This is important in case destroy comes after detach.
* AwContents.DestroyRunnable only holds the DestroyRunnable of
  AwGLFunctor instead of AwGLFunctor itself to prevent GC leaks.

BUG=597167
TBR=tobiasjs@chromium.org

Review URL: https://codereview.chromium.org/1890343003

Cr-Commit-Position: refs/heads/master@{#387926}

[modify] https://crrev.com/c748c90944d0a21be20b4a0d8844e51bc828f659/android_webview/glue/java/src/com/android/webview/chromium/DrawGLFunctor.java
[modify] https://crrev.com/c748c90944d0a21be20b4a0d8844e51bc828f659/android_webview/glue/java/src/com/android/webview/chromium/WebViewChromium.java
[modify] https://crrev.com/c748c90944d0a21be20b4a0d8844e51bc828f659/android_webview/glue/java/src/com/android/webview/chromium/WebViewDelegateFactory.java
[modify] https://crrev.com/c748c90944d0a21be20b4a0d8844e51bc828f659/android_webview/java/src/org/chromium/android_webview/AwContents.java
[modify] https://crrev.com/c748c90944d0a21be20b4a0d8844e51bc828f659/android_webview/java/src/org/chromium/android_webview/AwGLFunctor.java
[modify] https://crrev.com/c748c90944d0a21be20b4a0d8844e51bc828f659/android_webview/test/shell/src/org/chromium/android_webview/test/AwTestContainerView.java
Project Member

Comment 26 by bugdroid1@chromium.org, Apr 18 2016 

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/clank/internal/apps/+/7237d99e8134a67ed739993c0b142a6f1c962670

commit 7237d99e8134a67ed739993c0b142a6f1c962670
Author: boliu <boliu@chromium.org>
Date: Mon Apr 18 16:29:43 2016
Project Member

Comment 27 by bugdroid1@chromium.org, Apr 19 2016 

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/clank/internal/apps/+/74095fd782a89d381466a75f551f3e483d6c1175

commit 74095fd782a89d381466a75f551f3e483d6c1175
Author: Bo Liu <boliu@google.com>
Date: Tue Apr 19 14:13:55 2016

Comment 28 by boliu@chromium.org, Apr 19 2016 

#25 is not enough. It doesn't cover the explicit destroy case :/

Comment 29 by boliu@chromium.org, Apr 19 2016 

Oops, internal https://chrome-internal-review.googlesource.com/255858 referrs to this bug, but forgot to put it in the description
Project Member

Comment 30 by bugdroid1@chromium.org, Apr 20 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9e1b6d426ecb1302df0aca23fa3cc96bdc8d83bd

commit 9e1b6d426ecb1302df0aca23fa3cc96bdc8d83bd
Author: boliu <boliu@chromium.org>
Date: Wed Apr 20 01:15:47 2016

aw: Do not destroy functor in destroy

Functor has to be kept alive as long as the functor release callback is
still pending. This CL adds back the CleanupReference in AwGLFunctor
and remove the call from AwContents.DestroyRunnable.

Native AwContents is still destroyed before AwGLFunctor by ensuring that
AwContents.DestroyRunnable holds a reference to th AwGLFunctor referent,
which is only the AwGLFunctor.DestroyRunnable instead of AwGLFunctor
itself. This avoids strong references to the container View from any
DestroyRunnable.

Also remove destroying glue DrawGLFunctor from destroy. Now
DrawGLFunctor is destroyed only by its CleanupReference, and
DrawGLFucntor is kept alive by the (transitively) functor release
callback. Luckily it doesn't have any destruction ordering requirements
with other native objects.

BUG=597167

Review URL: https://codereview.chromium.org/1900323002

Cr-Commit-Position: refs/heads/master@{#388394}

[modify] https://crrev.com/9e1b6d426ecb1302df0aca23fa3cc96bdc8d83bd/android_webview/glue/java/src/com/android/webview/chromium/DrawGLFunctor.java
[modify] https://crrev.com/9e1b6d426ecb1302df0aca23fa3cc96bdc8d83bd/android_webview/glue/java/src/com/android/webview/chromium/WebViewChromium.java
[modify] https://crrev.com/9e1b6d426ecb1302df0aca23fa3cc96bdc8d83bd/android_webview/java/src/org/chromium/android_webview/AwContents.java
[modify] https://crrev.com/9e1b6d426ecb1302df0aca23fa3cc96bdc8d83bd/android_webview/java/src/org/chromium/android_webview/AwGLFunctor.java

Comment 31 by boliu@chromium.org, Apr 20 2016

Labels: Merge-Request-51
Merge request:
#25 (both upstream and downstream)
#29 (only downstream)
#30 (both upstream and downstream)

So 5 cherry-picks no total.

Comment 32 by amineer@chromium.org, Apr 20 2016

Labels: -Merge-Request-51 Merge-Approved-51
Merge approved for M51 branch 2704.
Project Member

Comment 33 by bugdroid1@chromium.org, Apr 20 2016

Labels: -merge-approved-51
The following revision refers to this bug:
  https://chrome-internal.googlesource.com/clank/internal/apps/+/e184eb3750a59adad1b385eafbb0f9475bbd0544

commit e184eb3750a59adad1b385eafbb0f9475bbd0544
Author: boliu <boliu@chromium.org>
Date: Mon Apr 18 16:29:43 2016
Project Member

Comment 34 by bugdroid1@chromium.org, Apr 20 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/74af09d91fec7a5d27e3d7def6805414b0a16a81

commit 74af09d91fec7a5d27e3d7def6805414b0a16a81
Author: Bo Liu <boliu@chromium.org>
Date: Wed Apr 20 01:57:00 2016

[Merge M51] aw: Use functor released callback for lifetime management

This is a smaller CL for merging to m51 branch. Has similar caveats to
r386831 so not restating them here.

This CL does these things:
* Expose the new callback from android on the callDrawGlFunction
  function.
* Use the callback only as a strong reference to keep view
  CleanupReferences alive. The actual callback is a no-op for now.
* AwGLFunctor will always hold a the NativeGLDelegate and ContainerView
  This is important in case destroy comes after detach.
* AwContents.DestroyRunnable only holds the DestroyRunnable of
  AwGLFunctor instead of AwGLFunctor itself to prevent GC leaks.

BUG=597167
TBR=tobiasjs@chromium.org

Review URL: https://codereview.chromium.org/1890343003

Cr-Commit-Position: refs/heads/master@{#387926}
(cherry picked from commit c748c90944d0a21be20b4a0d8844e51bc828f659)

Review URL: https://codereview.chromium.org/1899383002 .

Cr-Commit-Position: refs/branch-heads/2704@{#137}
Cr-Branched-From: 6e53600def8f60d8c632fadc70d7c1939ccea347-refs/heads/master@{#386251}

[modify] https://crrev.com/74af09d91fec7a5d27e3d7def6805414b0a16a81/android_webview/glue/java/src/com/android/webview/chromium/DrawGLFunctor.java
[modify] https://crrev.com/74af09d91fec7a5d27e3d7def6805414b0a16a81/android_webview/glue/java/src/com/android/webview/chromium/WebViewChromium.java
[modify] https://crrev.com/74af09d91fec7a5d27e3d7def6805414b0a16a81/android_webview/glue/java/src/com/android/webview/chromium/WebViewDelegateFactory.java
[modify] https://crrev.com/74af09d91fec7a5d27e3d7def6805414b0a16a81/android_webview/java/src/org/chromium/android_webview/AwContents.java
[modify] https://crrev.com/74af09d91fec7a5d27e3d7def6805414b0a16a81/android_webview/java/src/org/chromium/android_webview/AwGLFunctor.java
[modify] https://crrev.com/74af09d91fec7a5d27e3d7def6805414b0a16a81/android_webview/test/shell/src/org/chromium/android_webview/test/AwTestContainerView.java
Project Member

Comment 35 by bugdroid1@chromium.org, Apr 20 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/64b8b7f87e81a57c5127f4d2de1684c6d5c1dde1

commit 64b8b7f87e81a57c5127f4d2de1684c6d5c1dde1
Author: Bo Liu <boliu@chromium.org>
Date: Wed Apr 20 02:05:51 2016

[Merge M50] aw: Do not destroy functor in destroy

Functor has to be kept alive as long as the functor release callback is
still pending. This CL adds back the CleanupReference in AwGLFunctor
and remove the call from AwContents.DestroyRunnable.

Native AwContents is still destroyed before AwGLFunctor by ensuring that
AwContents.DestroyRunnable holds a reference to th AwGLFunctor referent,
which is only the AwGLFunctor.DestroyRunnable instead of AwGLFunctor
itself. This avoids strong references to the container View from any
DestroyRunnable.

Also remove destroying glue DrawGLFunctor from destroy. Now
DrawGLFunctor is destroyed only by its CleanupReference, and
DrawGLFucntor is kept alive by the (transitively) functor release
callback. Luckily it doesn't have any destruction ordering requirements
with other native objects.

BUG=597167

Review URL: https://codereview.chromium.org/1900323002

Cr-Commit-Position: refs/heads/master@{#388394}
(cherry picked from commit 9e1b6d426ecb1302df0aca23fa3cc96bdc8d83bd)

Review URL: https://codereview.chromium.org/1903673003 .

Cr-Commit-Position: refs/branch-heads/2704@{#138}
Cr-Branched-From: 6e53600def8f60d8c632fadc70d7c1939ccea347-refs/heads/master@{#386251}

[modify] https://crrev.com/64b8b7f87e81a57c5127f4d2de1684c6d5c1dde1/android_webview/glue/java/src/com/android/webview/chromium/DrawGLFunctor.java
[modify] https://crrev.com/64b8b7f87e81a57c5127f4d2de1684c6d5c1dde1/android_webview/glue/java/src/com/android/webview/chromium/WebViewChromium.java
[modify] https://crrev.com/64b8b7f87e81a57c5127f4d2de1684c6d5c1dde1/android_webview/java/src/org/chromium/android_webview/AwContents.java
[modify] https://crrev.com/64b8b7f87e81a57c5127f4d2de1684c6d5c1dde1/android_webview/java/src/org/chromium/android_webview/AwGLFunctor.java
Project Member

Comment 36 by bugdroid1@chromium.org, Apr 20 2016 

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/clank/internal/apps/+/932bc57be4cf5b3aaeaf827676ce5253ee8605c7

commit 932bc57be4cf5b3aaeaf827676ce5253ee8605c7
Author: Bo Liu <boliu@google.com>
Date: Wed Apr 20 02:07:11 2016

Comment 37 by boliu@chromium.org, Apr 20 2016

Labels: -ReleaseBlock-Stable -M-51 M-52
#35 should say Merge M51. And https://chrome-internal-review.googlesource.com/256205 should have pointed to this bug.

Other than that, this should hopefully be it for m51 merges \o/
Project Member

Comment 38 by bugdroid1@chromium.org, Apr 20 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5160fb439dc0ba515c6708a7d96dc88ebd14bb11

commit 5160fb439dc0ba515c6708a7d96dc88ebd14bb11
Author: boliu <boliu@chromium.org>
Date: Wed Apr 20 16:35:22 2016

aw: Fix AwGLFunctor lifetime object

CleanupReference can't use the DestroyRunnable as the referent, since
CleanupReference itself is a gc root that will keep DestroyRunnable
alive, in which case the referent is never garbage collected.

BUG=597167

Review URL: https://codereview.chromium.org/1905623002

Cr-Commit-Position: refs/heads/master@{#388518}

[modify] https://crrev.com/5160fb439dc0ba515c6708a7d96dc88ebd14bb11/android_webview/java/src/org/chromium/android_webview/AwGLFunctor.java

Comment 39 by boliu@chromium.org, Apr 20 2016

Labels: -M-52 Merge-Request-51 M-51
Oops, did miss something yesterday :(

Request merging #38 to M51. Not super rushed this time.

Comment 40 by tin...@google.com, Apr 20 2016

Labels: -Merge-Request-51 Merge-Approved-51
Your change meets the bar and is auto-approved for M51 (branch: 2704)
Project Member

Comment 41 by bugdroid1@chromium.org, Apr 20 2016

Labels: -merge-approved-51
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0f00382fbdc076d93bd0a04b7b7dfba4116a97fd

commit 0f00382fbdc076d93bd0a04b7b7dfba4116a97fd
Author: Bo Liu <boliu@chromium.org>
Date: Wed Apr 20 16:59:48 2016

[Merge M51] aw: Fix AwGLFunctor lifetime object

CleanupReference can't use the DestroyRunnable as the referent, since
CleanupReference itself is a gc root that will keep DestroyRunnable
alive, in which case the referent is never garbage collected.

BUG=597167

Review URL: https://codereview.chromium.org/1905623002

Cr-Commit-Position: refs/heads/master@{#388518}
(cherry picked from commit 5160fb439dc0ba515c6708a7d96dc88ebd14bb11)

Review URL: https://codereview.chromium.org/1898393003 .

Cr-Commit-Position: refs/branch-heads/2704@{#146}
Cr-Branched-From: 6e53600def8f60d8c632fadc70d7c1939ccea347-refs/heads/master@{#386251}

[modify] https://crrev.com/0f00382fbdc076d93bd0a04b7b7dfba4116a97fd/android_webview/java/src/org/chromium/android_webview/AwGLFunctor.java
Project Member

Comment 42 by bugdroid1@chromium.org, Apr 20 2016 

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/clank/internal/apps/+/e784b4e81b83efed606fee07dc04f9316079bd0a

commit e784b4e81b83efed606fee07dc04f9316079bd0a
Author: boliu <boliu@chromium.org>
Date: Wed Apr 20 01:15:47 2016

Comment 43 by boliu@chromium.org, Apr 20 2016

Labels: -M-51 M-52
Project Member

Comment 44 by bugdroid1@chromium.org, Apr 21 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/dbdd755abfd49e973f683967a5ad54edc5dc7274

commit dbdd755abfd49e973f683967a5ad54edc5dc7274
Author: tobiasjs <tobiasjs@chromium.org>
Date: Thu Apr 21 13:20:53 2016

Transfer DrawGLFunctor ownership from AwContents to AwGLFunctor

BUG=597167

Review URL: https://codereview.chromium.org/1904453004

Cr-Commit-Position: refs/heads/master@{#388757}

[modify] https://crrev.com/dbdd755abfd49e973f683967a5ad54edc5dc7274/android_webview/browser/deferred_gpu_command_service.cc
[modify] https://crrev.com/dbdd755abfd49e973f683967a5ad54edc5dc7274/android_webview/browser/render_thread_manager.cc
[modify] https://crrev.com/dbdd755abfd49e973f683967a5ad54edc5dc7274/android_webview/browser/render_thread_manager.h
[modify] https://crrev.com/dbdd755abfd49e973f683967a5ad54edc5dc7274/android_webview/browser/render_thread_manager_client.h
[modify] https://crrev.com/dbdd755abfd49e973f683967a5ad54edc5dc7274/android_webview/browser/test/fake_window.cc
[modify] https://crrev.com/dbdd755abfd49e973f683967a5ad54edc5dc7274/android_webview/browser/test/fake_window.h
[modify] https://crrev.com/dbdd755abfd49e973f683967a5ad54edc5dc7274/android_webview/browser/test/rendering_test.cc
[modify] https://crrev.com/dbdd755abfd49e973f683967a5ad54edc5dc7274/android_webview/browser/test/rendering_test.h
[modify] https://crrev.com/dbdd755abfd49e973f683967a5ad54edc5dc7274/android_webview/glue/java/src/com/android/webview/chromium/DrawGLFunctor.java
[modify] https://crrev.com/dbdd755abfd49e973f683967a5ad54edc5dc7274/android_webview/glue/java/src/com/android/webview/chromium/WebViewChromium.java
[modify] https://crrev.com/dbdd755abfd49e973f683967a5ad54edc5dc7274/android_webview/java/src/org/chromium/android_webview/AwContents.java
[modify] https://crrev.com/dbdd755abfd49e973f683967a5ad54edc5dc7274/android_webview/java/src/org/chromium/android_webview/AwGLFunctor.java
[modify] https://crrev.com/dbdd755abfd49e973f683967a5ad54edc5dc7274/android_webview/javatests/src/org/chromium/android_webview/test/AwTestBase.java
[modify] https://crrev.com/dbdd755abfd49e973f683967a5ad54edc5dc7274/android_webview/native/aw_gl_functor.cc
[modify] https://crrev.com/dbdd755abfd49e973f683967a5ad54edc5dc7274/android_webview/native/aw_gl_functor.h
[modify] https://crrev.com/dbdd755abfd49e973f683967a5ad54edc5dc7274/android_webview/test/shell/src/org/chromium/android_webview/shell/AwShellActivity.java
[modify] https://crrev.com/dbdd755abfd49e973f683967a5ad54edc5dc7274/android_webview/test/shell/src/org/chromium/android_webview/test/AwTestContainerView.java
Project Member

Comment 45 by bugdroid1@chromium.org, Apr 21 2016 

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/clank/internal/apps/+/6f6dd39365707eeaa87677d74ae2db0d32e76012

commit 6f6dd39365707eeaa87677d74ae2db0d32e76012
Author: tobiasjs <tobiasjs@chromium.org>
Date: Thu Apr 21 13:20:53 2016
Project Member

Comment 46 by bugdroid1@chromium.org, Apr 22 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ba29ba7e5d6dc86a7b316ac706f784072271c45b

commit ba29ba7e5d6dc86a7b316ac706f784072271c45b
Author: tobiasjs <tobiasjs@chromium.org>
Date: Fri Apr 22 17:05:59 2016

Refactor BrowserViewRenderer-RenderThreadManager relationship.

Make BrowserViewRenderer a CompositorFrameProducer and
RenderThreadManager a CompositorFrameConsumer, that implicitly
have a 1:many relationship. Each end can notify the other when it
is about to be destroyed, so that the relationship can be cleaned
up appropriately.

Both BVR and RTM must then cope with not having a valid
association with the other.

BUG=597167

Review URL: https://codereview.chromium.org/1911433002

Cr-Commit-Position: refs/heads/master@{#389147}

[modify] https://crrev.com/ba29ba7e5d6dc86a7b316ac706f784072271c45b/android_webview/browser/browser_view_renderer.cc
[modify] https://crrev.com/ba29ba7e5d6dc86a7b316ac706f784072271c45b/android_webview/browser/browser_view_renderer.h
[modify] https://crrev.com/ba29ba7e5d6dc86a7b316ac706f784072271c45b/android_webview/browser/browser_view_renderer_unittest.cc
[add] https://crrev.com/ba29ba7e5d6dc86a7b316ac706f784072271c45b/android_webview/browser/compositor_frame_consumer.h
[add] https://crrev.com/ba29ba7e5d6dc86a7b316ac706f784072271c45b/android_webview/browser/compositor_frame_producer.h
[modify] https://crrev.com/ba29ba7e5d6dc86a7b316ac706f784072271c45b/android_webview/browser/render_thread_manager.cc
[modify] https://crrev.com/ba29ba7e5d6dc86a7b316ac706f784072271c45b/android_webview/browser/render_thread_manager.h
[modify] https://crrev.com/ba29ba7e5d6dc86a7b316ac706f784072271c45b/android_webview/browser/render_thread_manager_client.h
[modify] https://crrev.com/ba29ba7e5d6dc86a7b316ac706f784072271c45b/android_webview/browser/test/rendering_test.cc
[modify] https://crrev.com/ba29ba7e5d6dc86a7b316ac706f784072271c45b/android_webview/browser/test/rendering_test.h
[modify] https://crrev.com/ba29ba7e5d6dc86a7b316ac706f784072271c45b/android_webview/native/aw_contents.cc
[modify] https://crrev.com/ba29ba7e5d6dc86a7b316ac706f784072271c45b/android_webview/native/aw_contents.h
[modify] https://crrev.com/ba29ba7e5d6dc86a7b316ac706f784072271c45b/android_webview/native/aw_gl_functor.cc
[modify] https://crrev.com/ba29ba7e5d6dc86a7b316ac706f784072271c45b/android_webview/native/aw_gl_functor.h
Project Member

Comment 47 by bugdroid1@chromium.org, Apr 25 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/453c3b298d6dc0197d674a29ed59c67a716fe1f1

commit 453c3b298d6dc0197d674a29ed59c67a716fe1f1
Author: tobiasjs <tobiasjs@chromium.org>
Date: Mon Apr 25 10:19:58 2016

Assert that AwGLFunctor instances do not leak.

We should ensure that AwGLFunctor references are cleaned up correctly.
In general there could be more native AwGLFunctor instances than
AwContents instance (when in fullscreen mode) but in non-fullscreen
tests it is safe to require that the number of AwGLFunctor instances is
eventually less than or equal to the number of AwContents instances.

AwGLFunctor instances may temporarily outlive AwContents while the native
DrawGL functor remains needed by the render thread, but this should not
be the case indefinitely.

BUG=597167

Review URL: https://codereview.chromium.org/1914433002

Cr-Commit-Position: refs/heads/master@{#389438}

[modify] https://crrev.com/453c3b298d6dc0197d674a29ed59c67a716fe1f1/android_webview/java/src/org/chromium/android_webview/AwGLFunctor.java
[modify] https://crrev.com/453c3b298d6dc0197d674a29ed59c67a716fe1f1/android_webview/javatests/src/org/chromium/android_webview/test/AwContentsGarbageCollectionTest.java
[modify] https://crrev.com/453c3b298d6dc0197d674a29ed59c67a716fe1f1/android_webview/native/aw_gl_functor.cc
Project Member

Comment 48 by bugdroid1@chromium.org, Apr 25 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/161c8946c7474131ed93447dec861bb4c70a0b81

commit 161c8946c7474131ed93447dec861bb4c70a0b81
Author: tobiasjs <tobiasjs@chromium.org>
Date: Mon Apr 25 15:32:45 2016

Remove thread DCHECK from RegisterAwGLFunctor

BUG=597167
NOTRY=true

Review URL: https://codereview.chromium.org/1915053002

Cr-Commit-Position: refs/heads/master@{#389481}

[modify] https://crrev.com/161c8946c7474131ed93447dec861bb4c70a0b81/android_webview/native/aw_gl_functor.cc
Project Member

Comment 49 by bugdroid1@chromium.org, Apr 25 2016

Labels: merge-merged-2716
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/453c3b298d6dc0197d674a29ed59c67a716fe1f1

commit 453c3b298d6dc0197d674a29ed59c67a716fe1f1
Author: tobiasjs <tobiasjs@chromium.org>
Date: Mon Apr 25 10:19:58 2016

Assert that AwGLFunctor instances do not leak.

We should ensure that AwGLFunctor references are cleaned up correctly.
In general there could be more native AwGLFunctor instances than
AwContents instance (when in fullscreen mode) but in non-fullscreen
tests it is safe to require that the number of AwGLFunctor instances is
eventually less than or equal to the number of AwContents instances.

AwGLFunctor instances may temporarily outlive AwContents while the native
DrawGL functor remains needed by the render thread, but this should not
be the case indefinitely.

BUG=597167

Review URL: https://codereview.chromium.org/1914433002

Cr-Commit-Position: refs/heads/master@{#389438}

[modify] https://crrev.com/453c3b298d6dc0197d674a29ed59c67a716fe1f1/android_webview/java/src/org/chromium/android_webview/AwGLFunctor.java
[modify] https://crrev.com/453c3b298d6dc0197d674a29ed59c67a716fe1f1/android_webview/javatests/src/org/chromium/android_webview/test/AwContentsGarbageCollectionTest.java
[modify] https://crrev.com/453c3b298d6dc0197d674a29ed59c67a716fe1f1/android_webview/native/aw_gl_functor.cc
Project Member

Comment 50 by bugdroid1@chromium.org, Apr 25 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/161c8946c7474131ed93447dec861bb4c70a0b81

commit 161c8946c7474131ed93447dec861bb4c70a0b81
Author: tobiasjs <tobiasjs@chromium.org>
Date: Mon Apr 25 15:32:45 2016

Remove thread DCHECK from RegisterAwGLFunctor

BUG=597167
NOTRY=true

Review URL: https://codereview.chromium.org/1915053002

Cr-Commit-Position: refs/heads/master@{#389481}

[modify] https://crrev.com/161c8946c7474131ed93447dec861bb4c70a0b81/android_webview/native/aw_gl_functor.cc
Project Member

Comment 51 by bugdroid1@chromium.org, Apr 25 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2dacfffced52ae60725186d6061756d0609f01e8

commit 2dacfffced52ae60725186d6061756d0609f01e8
Author: boliu <boliu@chromium.org>
Date: Mon Apr 25 23:44:05 2016

aw: Fix dcheck failure on start up

This method is called before BrowserThreads is initialized in the real
webview, so crashing there.

BUG=597167,  606543 

Review URL: https://codereview.chromium.org/1920023004

Cr-Commit-Position: refs/heads/master@{#389617}

[modify] https://crrev.com/2dacfffced52ae60725186d6061756d0609f01e8/android_webview/native/aw_gl_functor.cc
Project Member

Comment 53 by bugdroid1@chromium.org, Apr 27 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/fece06881fde15ddb181b3af43c3912457fca427

commit fece06881fde15ddb181b3af43c3912457fca427
Author: tobiasjs <tobiasjs@chromium.org>
Date: Wed Apr 27 18:13:42 2016

Remove DrawGLFunctor CleanupReference.

Instead of having Java DrawGLFunctor register a CleanupReference to
destroy its native counterpart, it provides a method to get a destroy
runnable that performs this function, and it is up to the owner of the
Java DrawGLFunctor object to call it at the appropriate time (i.e. when
the native DrawGLFunctor is no longer referenced by the render thread
display list).

The AwGLFunctor DestroyRunnable holds a reference to the DrawGLFunctor
DestroyRunnable, so that it can call it once AwGLFunctor is GC'ed.

BUG=597167

Review-Url: https://codereview.chromium.org/1927543003
Cr-Commit-Position: refs/heads/master@{#390130}

[modify] https://crrev.com/fece06881fde15ddb181b3af43c3912457fca427/android_webview/glue/java/src/com/android/webview/chromium/DrawGLFunctor.java
[modify] https://crrev.com/fece06881fde15ddb181b3af43c3912457fca427/android_webview/java/src/org/chromium/android_webview/AwContents.java
[modify] https://crrev.com/fece06881fde15ddb181b3af43c3912457fca427/android_webview/java/src/org/chromium/android_webview/AwGLFunctor.java
[modify] https://crrev.com/fece06881fde15ddb181b3af43c3912457fca427/android_webview/test/shell/src/org/chromium/android_webview/test/AwTestContainerView.java
Project Member

Comment 54 by bugdroid1@chromium.org, Apr 28 2016 

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/clank/internal/apps/+/7d19e812d17caa14b02c90f4e77c1a92551998f5

commit 7d19e812d17caa14b02c90f4e77c1a92551998f5
Author: tobiasjs <tobiasjs@chromium.org>
Date: Wed Apr 27 18:13:42 2016
Project Member

Comment 55 by bugdroid1@chromium.org, May 4 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9e8cbda24ddf31063e5aace07f31c2c9ffe3f8aa

commit 9e8cbda24ddf31063e5aace07f31c2c9ffe3f8aa
Author: tobiasjs <tobiasjs@chromium.org>
Date: Wed May 04 10:47:08 2016

Avoid recreating hardware_renderer_ if a draw occurs during cleanup.

This exposed the fact that uncommitted frames were not being
returned when the RTM was deleted, so this CL fixes that too.

BUG=597167

Review-Url: https://codereview.chromium.org/1946583002
Cr-Commit-Position: refs/heads/master@{#391474}

[modify] https://crrev.com/9e8cbda24ddf31063e5aace07f31c2c9ffe3f8aa/android_webview/browser/browser_view_renderer.cc
[modify] https://crrev.com/9e8cbda24ddf31063e5aace07f31c2c9ffe3f8aa/android_webview/browser/render_thread_manager.cc
Project Member

Comment 56 by bugdroid1@chromium.org, May 18 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6f2a192ab2b7fe5bb6bdb7f58e0d0015816ba121

commit 6f2a192ab2b7fe5bb6bdb7f58e0d0015816ba121
Author: tobiasjs <tobiasjs@chromium.org>
Date: Wed May 18 11:53:40 2016

WIP Handle AwContents needing multiple live functors.

AwContents now needs to keep a functor per view. Either functor may be
asked to draw its current frame at any time. New frames produced by
the compositor go to the current RTM, and the BVR is interested in
the geometry of the current RTM. Trim memory signal must go to all
RTMs, and all RTMs must be able to hand back resources to the right
compositor at any time, and not be forced to do so when a different
RTM becomes current.

BUG=597167

Review-Url: https://codereview.chromium.org/1943963003
Cr-Commit-Position: refs/heads/master@{#394387}

[modify] https://crrev.com/6f2a192ab2b7fe5bb6bdb7f58e0d0015816ba121/android_webview/browser/browser_view_renderer.cc
[modify] https://crrev.com/6f2a192ab2b7fe5bb6bdb7f58e0d0015816ba121/android_webview/browser/browser_view_renderer.h
[modify] https://crrev.com/6f2a192ab2b7fe5bb6bdb7f58e0d0015816ba121/android_webview/browser/browser_view_renderer_unittest.cc
[modify] https://crrev.com/6f2a192ab2b7fe5bb6bdb7f58e0d0015816ba121/android_webview/browser/compositor_frame_consumer.h
[modify] https://crrev.com/6f2a192ab2b7fe5bb6bdb7f58e0d0015816ba121/android_webview/browser/compositor_frame_producer.h
[modify] https://crrev.com/6f2a192ab2b7fe5bb6bdb7f58e0d0015816ba121/android_webview/browser/render_thread_manager.cc
[modify] https://crrev.com/6f2a192ab2b7fe5bb6bdb7f58e0d0015816ba121/android_webview/browser/test/fake_window.cc
[modify] https://crrev.com/6f2a192ab2b7fe5bb6bdb7f58e0d0015816ba121/android_webview/browser/test/fake_window.h
[modify] https://crrev.com/6f2a192ab2b7fe5bb6bdb7f58e0d0015816ba121/android_webview/browser/test/rendering_test.cc
[modify] https://crrev.com/6f2a192ab2b7fe5bb6bdb7f58e0d0015816ba121/android_webview/browser/test/rendering_test.h
[modify] https://crrev.com/6f2a192ab2b7fe5bb6bdb7f58e0d0015816ba121/android_webview/java/src/org/chromium/android_webview/AwContents.java
[modify] https://crrev.com/6f2a192ab2b7fe5bb6bdb7f58e0d0015816ba121/android_webview/java/src/org/chromium/android_webview/AwGLFunctor.java
[modify] https://crrev.com/6f2a192ab2b7fe5bb6bdb7f58e0d0015816ba121/android_webview/native/aw_contents.cc
[modify] https://crrev.com/6f2a192ab2b7fe5bb6bdb7f58e0d0015816ba121/android_webview/test/shell/src/org/chromium/android_webview/test/AwTestContainerView.java

Comment 57 by boliu@chromium.org, Jun 8 2016 

in case you are bored, there is more work here for this bug: don't let functor callback prevent AwContents from gc-ed

Comment 58 by tobiasjs@chromium.org, Apr 10 2017

Cc: -hush@chromium.org
Labels: -Pri-1 Pri-3
Not time critical any more, but still tracking the remaining work.

Comment 59 by tobiasjs@chromium.org, Oct 1

Labels: -Type-Bug WebView-triaged webview-graphics Type-Task
►

Sign in to add a comment