[Android WebView] Call AwGLFunctor.onDetachedFromWindow with correct order.

[Android WebView] Call AwGLFunctor.onDetachedFromWindow with correct order.

HardwareRenderer should destoryed before BrowserViewRenderer's onDetachedFromWindow,
which makes sure resources returned by parent compositor are released immediately after detach.

BUG=655434
TEST=

[Shouqun1, 2016-10-14 13:21:59]

Description was changed from

==========
[Android WebView] Call AwGLFunctor.onDetachedFromWindow with correct order.

HardwareRenderer should destoryed before BrowserViewRenderer's
onDetachedFromWindow,
which makes sure BVR can get the resources returned by parent compositor.

BUG=655434
TEST=
==========

to

==========
[Android WebView] Call AwGLFunctor.onDetachedFromWindow with correct order.

HardwareRenderer should destoryed before BrowserViewRenderer's
onDetachedFromWindow, which makes sure resources returned by parent compositor
are released immediately after detach.

BUG=655434
TEST=
==========

[Shouqun1, 2016-10-14 13:22:13]

Description was changed from

==========
[Android WebView] Call AwGLFunctor.onDetachedFromWindow with correct order.

HardwareRenderer should destoryed before BrowserViewRenderer's
onDetachedFromWindow, which makes sure resources returned by parent compositor
are released immediately after detach.

BUG=655434
TEST=
==========

to

==========
[Android WebView] Call AwGLFunctor.onDetachedFromWindow with correct order.

HardwareRenderer should destoryed before BrowserViewRenderer's
onDetachedFromWindow,
which makes sure resources returned by parent compositor are released
immediately after detach.

BUG=655434
TEST=
==========

[Shouqun1, 2016-10-14 13:22:29]

Patchset #1 (id:1) has been deleted

[Shouqun1, 2016-10-14 13:33:21]

liushouqun@xiaomi.com changed reviewers:
+ boliu@chromium.org

[Shouqun1, 2016-10-14 13:33:22]

PTAL

[boliu, 2016-10-14 17:30:42]

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

[boliu, 2016-10-14 17:30:42]

I'm gonna let toby take this one.

Without out this, it just means we are holding onto one frame of resources even
after detached. Isn't the end of the world, as long as that those resources are
eventually get returned.

[boliu, 2016-10-14 21:27:31]

boliu@chromium.org changed reviewers:
- boliu@chromium.org

[Shouqun1, 2016-10-17 03:06:18]

On 2016/10/14 17:30:42, boliu wrote:
> I'm gonna let toby take this one.
> 
> Without out this, it just means we are holding onto one frame of resources
even
> after detached. Isn't the end of the world, as long as that those resources
are
> eventually get returned.

Yes, but since the WebView is detached, the next frame of BVR (accordingly
BVR.ReturnResourceFromParent) will be processed util next attach.

If AwGLFunctor.onDetachedFromWindow is called after BVR.OnDetachedFromWindow,
the resources will be hold util next WebView attach or destroy, since BVR can't
get a chance to execute ReturnResourceFromParent until next attach.

For applications with the following logic: (create WebView -> detach WebView ->
<do sth for a while> -> attach WebView again), the resources will be hold during
<do sth> stage, in which case,  the memory footprint is increased accordingly.  

IMO, the ideal case is that when the WebView is in detached state, the resources
are recycled.

[boliu, 2016-10-18 04:08:40]

On 2016/10/17 03:06:18, Shouqun1 wrote:
> On 2016/10/14 17:30:42, boliu wrote:
> > I'm gonna let toby take this one.
> > 
> > Without out this, it just means we are holding onto one frame of resources
> even
> > after detached. Isn't the end of the world, as long as that those resources
> are
> > eventually get returned.
> 
> Yes, but since the WebView is detached, the next frame of BVR (accordingly
> BVR.ReturnResourceFromParent) will be processed util next attach.
> 
> If AwGLFunctor.onDetachedFromWindow is called after BVR.OnDetachedFromWindow,
> the resources will be hold util next WebView attach or destroy, since BVR
can't
> get a chance to execute ReturnResourceFromParent until next attach.
> 
> For applications with the following logic: (create WebView -> detach WebView
->
> <do sth for a while> -> attach WebView again), the resources will be hold
during
> <do sth> stage, in which case,  the memory footprint is increased accordingly.
 
> 
> IMO, the ideal case is that when the WebView is in detached state, the
resources
> are recycled.

All true. I'm just saying this issue is not that severe, since memory won't grow
unboundedly. An alternative solution is to always directly return resources, and
never hold returned resources in RenderThreadManager. Anyway I'll let toby
decide where this one goes.

[Tobias Sargeant, 2016-10-19 18:47:29]

On 2016/10/18 04:08:40, boliu wrote:
> On 2016/10/17 03:06:18, Shouqun1 wrote:
> > On 2016/10/14 17:30:42, boliu wrote:
> > > I'm gonna let toby take this one.
> > > 
> > > Without out this, it just means we are holding onto one frame of resources
> > even
> > > after detached. Isn't the end of the world, as long as that those
resources
> > are
> > > eventually get returned.
> > 
> > Yes, but since the WebView is detached, the next frame of BVR (accordingly
> > BVR.ReturnResourceFromParent) will be processed util next attach.
> > 
> > If AwGLFunctor.onDetachedFromWindow is called after
BVR.OnDetachedFromWindow,
> > the resources will be hold util next WebView attach or destroy, since BVR
> can't
> > get a chance to execute ReturnResourceFromParent until next attach.
> > 
> > For applications with the following logic: (create WebView -> detach WebView
> ->
> > <do sth for a while> -> attach WebView again), the resources will be hold
> during
> > <do sth> stage, in which case,  the memory footprint is increased
accordingly.
>  
> > 
> > IMO, the ideal case is that when the WebView is in detached state, the
> resources
> > are recycled.
> 
> All true. I'm just saying this issue is not that severe, since memory won't
grow
> unboundedly. An alternative solution is to always directly return resources,
and
> never hold returned resources in RenderThreadManager. Anyway I'll let toby
> decide where this one goes.

The only concern I can see with changing the order is that if we detach the
WebView and return child_frame_ before any frame is drawn, then it seems that
DeleteHardwareRendererOnUI will have the wrong value for hardware_initialized
(HasFrameOnUI will return false, even though the functor is attached).

[boliu, 2016-10-20 20:15:33]

On 2016/10/19 18:47:29, Tobias Sargeant wrote:
> On 2016/10/18 04:08:40, boliu wrote:
> > On 2016/10/17 03:06:18, Shouqun1 wrote:
> > > On 2016/10/14 17:30:42, boliu wrote:
> > > > I'm gonna let toby take this one.
> > > > 
> > > > Without out this, it just means we are holding onto one frame of
resources
> > > even
> > > > after detached. Isn't the end of the world, as long as that those
> resources
> > > are
> > > > eventually get returned.
> > > 
> > > Yes, but since the WebView is detached, the next frame of BVR (accordingly
> > > BVR.ReturnResourceFromParent) will be processed util next attach.
> > > 
> > > If AwGLFunctor.onDetachedFromWindow is called after
> BVR.OnDetachedFromWindow,
> > > the resources will be hold util next WebView attach or destroy, since BVR
> > can't
> > > get a chance to execute ReturnResourceFromParent until next attach.
> > > 
> > > For applications with the following logic: (create WebView -> detach
WebView
> > ->
> > > <do sth for a while> -> attach WebView again), the resources will be hold
> > during
> > > <do sth> stage, in which case,  the memory footprint is increased
> accordingly.
> >  
> > > 
> > > IMO, the ideal case is that when the WebView is in detached state, the
> > resources
> > > are recycled.
> > 
> > All true. I'm just saying this issue is not that severe, since memory won't
> grow
> > unboundedly. An alternative solution is to always directly return resources,
> and
> > never hold returned resources in RenderThreadManager. Anyway I'll let toby
> > decide where this one goes.
> 
> The only concern I can see with changing the order is that if we detach the
> WebView and return child_frame_ before any frame is drawn, then it seems that
> DeleteHardwareRendererOnUI will have the wrong value for hardware_initialized
> (HasFrameOnUI will return false, even though the functor is attached).

please work with shouqun to resolve that then