aw: Do not destroy functor in destroy

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
Committed: https://crrev.com/9e1b6d426ecb1302df0aca23fa3cc96bdc8d83bd
Cr-Commit-Position: refs/heads/master@{#388394}

[boliu, 2016-04-19 22:57:31]

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

[boliu, 2016-04-19 22:57:31]

description should be self explanatory..hopefully

[boliu, 2016-04-19 22:57:39]

The CQ bit was checked by boliu@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-04-19 22:58:02]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1900323002/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1900323002/1

[commit-bot: I haz the power, 2016-04-19 23:36:29]

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

[commit-bot: I haz the power, 2016-04-19 23:36:29]

Dry run: This issue passed the CQ dry run.

[boliu, 2016-04-20 00:35:57]

The CQ bit was checked by boliu@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-04-20 00:36:20]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1900323002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1900323002/20001

[hush (inactive), 2016-04-20 00:54:37]

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

[hush (inactive), 2016-04-20 00:54:37]

lgtm

[hush (inactive), 2016-04-20 00:56:24]

The CQ bit was unchecked by hush@chromium.org

[hush (inactive), 2016-04-20 00:56:25]

The CQ bit was checked by hush@chromium.org

[commit-bot: I haz the power, 2016-04-20 00:56:49]

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

[commit-bot: I haz the power, 2016-04-20 01:15:57]

Committed patchset #2 (id:20001)

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

On 2016/04/20 01:15:57, commit-bot: I haz the power wrote:
> Committed patchset #2 (id:20001)

Yeah, I spent an hour talking to hush to convince him this is a good idea. So
review is pretty coerced :p

[hush (inactive), 2016-04-20 18:34:26]

On 2016/04/20 01:16:48, boliu wrote:
> On 2016/04/20 01:15:57, commit-bot: I haz the power wrote:
> > Committed patchset #2 (id:20001)
> 
> Yeah, I spent an hour talking to hush to convince him this is a good idea. So
> review is pretty coerced :p

The following is the graph of object ownership relationships for your
information.

              CleanupReference (GC root here)
            ↑ ⇩                 ↓
       AwContents → AwContent#DestroyRunnable

          ↑ ↓                ↓

       AwGLFunctor → AwGLFunctor#DestroyRunnable
              ↓          ↑  ⇧
             
              CleanupReference (GC root here)

⇧: weak reference
↑: strong reference

[boliu, 2016-04-20 18:36:55]

On 2016/04/20 18:34:26, hush wrote:
> On 2016/04/20 01:16:48, boliu wrote:
> > On 2016/04/20 01:15:57, commit-bot: I haz the power wrote:
> > > Committed patchset #2 (id:20001)
> > 
> > Yeah, I spent an hour talking to hush to convince him this is a good idea.
So
> > review is pretty coerced :p
> 
> The following is the graph of object ownership relationships for your
> information.
> 
>               CleanupReference (GC root here)
>             ↑ ⇩                 ↓
>        AwContents → AwContent#DestroyRunnable
> 
>           ↑ ↓                ↓
> 
>        AwGLFunctor → AwGLFunctor#DestroyRunnable
>               ↓          ↑  ⇧
>              
>               CleanupReference (GC root here)
> 
> ⇧: weak reference
> ↑: strong reference

Spaces got dropped in email, but looks ok on code review.

So... spot the bug yet?

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

Description was changed from

==========
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
==========

to

==========
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

Committed: https://crrev.com/9e1b6d426ecb1302df0aca23fa3cc96bdc8d83bd
Cr-Commit-Position: refs/heads/master@{#388394}
==========

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

Patchset 2 (id:??) landed as
https://crrev.com/9e1b6d426ecb1302df0aca23fa3cc96bdc8d83bd
Cr-Commit-Position: refs/heads/master@{#388394}