New issue
Advanced search Search tips

Issue 901554 link

Starred by 2 users

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 3
Type: Bug

Blocking:
issue 900965



Sign in to add a comment

Allow functor destroy to happen on render thread

Project Member Reported by boliu@chromium.org, Nov 2

Issue description

Support behavior where we get destroy callback for functor on RT and do not have access to synchronous invoke functor.

This is to support vulkan functor that probably will work this way, but should try converge GL and VK functor code paths, so should try to convert GL functor now and see how things shake out.

 
Cc: tobiasjs@chromium.org
Design in my head: Make RenderThreadManager RefCountedThreadSafe. In vulkan, have HardwareRenderer refcount RenderThreadManager.

For GL, conceptually, there is a refcount associated with each requestDrawGL (matched with the release callback), and there is a refcount for being attached to the window. This is the exact implementation in AwGLFunctor right now with a count on the UI thread.

The idea is for vulkan, the refcount for functor from requestDrawVK all the way to when the destroy callback for the functor happens, which means it spans UI and RT. So this needs RefCountedThreadSafe, and each ChildFrame probably will need a scoped_refptr to RTM. HR will be self owned and keep refcount to RTM for frames that it contains.

AwGLFunctor (and RTM) is a long lived object with the same lifetime as AwContents. That isn't really necessary, and we could just create it on demand and destroy it when the refcount above goes to 0. I think we already have logic for RTM and BVR destroyed in different order and to ensure no resource is dropped in either case. Then there is no need for the CleanupReference in AwGLFunctor I think? Toby: see any problems with this plan?

AwGLFunctor lifetime refactor is probably the more significant change than switching things to RefCountedThreadSafe.
Project Member

Comment 2 by bugdroid1@chromium.org, Nov 8

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

commit 523d01b80d212e827df97656698cf68a9e5bc64e
Author: Bo Liu <boliu@chromium.org>
Date: Thu Nov 08 17:26:53 2018

aw: Lazy allocate AwGLFunctor

Right now the lifetime of AwGLFunctor (and that of associated objects)
roughly matches the lifetime of AwContents. This is not necessary, and
we only need to ensure it's alive when functor is in use. This better
matches the APIthat the vulkan functor will have. It also has the up
side of converting CleanupReference usage to explicit destroy, which
is always nice.

Bug: 901554
Change-Id: Ib1126d1ddfeacc7b8c689aeb51bb897cf839b41c
Reviewed-on: https://chromium-review.googlesource.com/c/1320749
Reviewed-by: Tobias Sargeant <tobiasjs@chromium.org>
Reviewed-by: Chris Blume <cblume@chromium.org>
Commit-Queue: Bo <boliu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#606512}
[modify] https://crrev.com/523d01b80d212e827df97656698cf68a9e5bc64e/android_webview/glue/java/src/com/android/webview/chromium/DrawGLFunctor.java
[modify] https://crrev.com/523d01b80d212e827df97656698cf68a9e5bc64e/android_webview/java/src/org/chromium/android_webview/AwContents.java
[modify] https://crrev.com/523d01b80d212e827df97656698cf68a9e5bc64e/android_webview/java/src/org/chromium/android_webview/AwGLFunctor.java
[modify] https://crrev.com/523d01b80d212e827df97656698cf68a9e5bc64e/android_webview/test/shell/src/org/chromium/android_webview/test/AwTestContainerView.java

Sign in to add a comment