aw: Use functor callback for lifetime management

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

Committed: https://crrev.com/c748c90944d0a21be20b4a0d8844e51bc828f659
Cr-Commit-Position: refs/heads/master@{#387926}

[boliu, 2016-04-16 00:08:30]

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

[boliu, 2016-04-16 00:08:31]

I wrote this, then realized that the attach/detach stuff is actually ok for now
and didn't really need to be removed. Meh.

I did check that ref counting works as expected. It's just not really needed yet

[boliu, 2016-04-16 00:25:11]

Description was changed from

==========
aw: Use functor 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
==========

to

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

[boliu, 2016-04-18 15:47:19]

Description was changed from

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

to

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

[boliu, 2016-04-18 15:47:41]

tbr-ing tobiasjs, who has already looked over this

[boliu, 2016-04-18 15:47:51]

The CQ bit was checked by boliu@chromium.org

[commit-bot: I haz the power, 2016-04-18 15:48:05]

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

[commit-bot: I haz the power, 2016-04-18 16:29:55]

Description was changed from

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

to

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

[commit-bot: I haz the power, 2016-04-18 16:29:55]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-04-18 16:31:12]

Description was changed from

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

to

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

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

[commit-bot: I haz the power, 2016-04-18 16:31:13]

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

[Torne, 2016-04-19 14:03:01]

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

[Torne, 2016-04-19 14:03:01]

This seems to have broken running current ToT webview on N. Looks like this
landed before the corresponding framework change? :/

[Torne, 2016-04-19 14:03:04]

This seems to have broken running current ToT webview on N. Looks like this
landed before the corresponding framework change? :/

[boliu, 2016-04-19 14:03:52]

On 2016/04/19 14:03:04, Torne wrote:
> This seems to have broken running current ToT webview on N. Looks like this
> landed before the corresponding framework change? :/

Hmm, what framework change, all implementations in WebViewDelegateFactory are
no-op or throws..?

[Torne, 2016-04-19 14:06:48]

On 2016/04/19 14:03:52, boliu wrote:
> On 2016/04/19 14:03:04, Torne wrote:
> > This seems to have broken running current ToT webview on N. Looks like this
> > landed before the corresponding framework change? :/
> 
> Hmm, what framework change, all implementations in WebViewDelegateFactory are
> no-op or throws..?

Yes - it's now calling one of the methods that throws immediately on first use,
and crashing 100% of the time.

[boliu, 2016-04-19 14:07:21]

On 2016/04/19 14:03:52, boliu wrote:
> On 2016/04/19 14:03:04, Torne wrote:
> > This seems to have broken running current ToT webview on N. Looks like this
> > landed before the corresponding framework change? :/
> 
> Hmm, what framework change, all implementations in WebViewDelegateFactory are
> no-op or throws..?

Oh.. you are right. For now, you can get around that by setting
DrawGLFunctor.sSupportFunctorReleasedCallback false

[Torne, 2016-04-19 14:08:33]

On 2016/04/19 14:07:21, boliu wrote:
> On 2016/04/19 14:03:52, boliu wrote:
> > On 2016/04/19 14:03:04, Torne wrote:
> > > This seems to have broken running current ToT webview on N. Looks like
this
> > > landed before the corresponding framework change? :/
> > 
> > Hmm, what framework change, all implementations in WebViewDelegateFactory
are
> > no-op or throws..?
> 
> Oh.. you are right. For now, you can get around that by setting
> DrawGLFunctor.sSupportFunctorReleasedCallback false

I just reverted this change for now and now it's fine, but this probably wasn't
a good idea to land as-is then :)

[boliu, 2016-04-19 14:10:11]

On 2016/04/19 14:08:33, Torne wrote:
> On 2016/04/19 14:07:21, boliu wrote:
> > On 2016/04/19 14:03:52, boliu wrote:
> > > On 2016/04/19 14:03:04, Torne wrote:
> > > > This seems to have broken running current ToT webview on N. Looks like
> this
> > > > landed before the corresponding framework change? :/
> > > 
> > > Hmm, what framework change, all implementations in WebViewDelegateFactory
> are
> > > no-op or throws..?
> > 
> > Oh.. you are right. For now, you can get around that by setting
> > DrawGLFunctor.sSupportFunctorReleasedCallback false
> 
> I just reverted this change for now and now it's fine, but this probably
wasn't
> a good idea to land as-is then :)

I'll revert the N bit before we roll frameworks jar downtream. The N test bot
didn't blow up because.. it doesn't run tests on the real webview. Meh x2

[boliu, 2016-04-19 14:12:56]

On 2016/04/19 14:10:11, boliu wrote:
> On 2016/04/19 14:08:33, Torne wrote:
> > On 2016/04/19 14:07:21, boliu wrote:
> > > On 2016/04/19 14:03:52, boliu wrote:
> > > > On 2016/04/19 14:03:04, Torne wrote:
> > > > > This seems to have broken running current ToT webview on N. Looks like
> > this
> > > > > landed before the corresponding framework change? :/
> > > > 
> > > > Hmm, what framework change, all implementations in
WebViewDelegateFactory
> > are
> > > > no-op or throws..?
> > > 
> > > Oh.. you are right. For now, you can get around that by setting
> > > DrawGLFunctor.sSupportFunctorReleasedCallback false
> > 
> > I just reverted this change for now and now it's fine, but this probably
> wasn't
> > a good idea to land as-is then :)
> 
> I'll revert the N bit before we roll frameworks jar downtream. The N test bot
> didn't blow up because.. it doesn't run tests on the real webview. Meh x2

codereview.chromium.org authentication expired, and I'm still home o_o. I'll
just fix downstream then :p

[Torne, 2016-04-19 14:14:36]

On 2016/04/19 14:12:56, boliu wrote:
> On 2016/04/19 14:10:11, boliu wrote:
> > On 2016/04/19 14:08:33, Torne wrote:
> > > On 2016/04/19 14:07:21, boliu wrote:
> > > > On 2016/04/19 14:03:52, boliu wrote:
> > > > > On 2016/04/19 14:03:04, Torne wrote:
> > > > > > This seems to have broken running current ToT webview on N. Looks
like
> > > this
> > > > > > landed before the corresponding framework change? :/
> > > > > 
> > > > > Hmm, what framework change, all implementations in
> WebViewDelegateFactory
> > > are
> > > > > no-op or throws..?
> > > > 
> > > > Oh.. you are right. For now, you can get around that by setting
> > > > DrawGLFunctor.sSupportFunctorReleasedCallback false
> > > 
> > > I just reverted this change for now and now it's fine, but this probably
> > wasn't
> > > a good idea to land as-is then :)
> > 
> > I'll revert the N bit before we roll frameworks jar downtream. The N test
bot
> > didn't blow up because.. it doesn't run tests on the real webview. Meh x2
> 
> http://codereview.chromium.org authentication expired, and I'm still home o_o.
I'll
> just fix downstream then :p

I don't think this has anything to do with rolling the framework jar. I haven't
done that, and it's still broken for me.

[boliu, 2016-04-19 14:19:39]

On 2016/04/19 14:14:36, Torne wrote:
> On 2016/04/19 14:12:56, boliu wrote:
> > On 2016/04/19 14:10:11, boliu wrote:
> > > On 2016/04/19 14:08:33, Torne wrote:
> > > > On 2016/04/19 14:07:21, boliu wrote:
> > > > > On 2016/04/19 14:03:52, boliu wrote:
> > > > > > On 2016/04/19 14:03:04, Torne wrote:
> > > > > > > This seems to have broken running current ToT webview on N. Looks
> like
> > > > this
> > > > > > > landed before the corresponding framework change? :/
> > > > > > 
> > > > > > Hmm, what framework change, all implementations in
> > WebViewDelegateFactory
> > > > are
> > > > > > no-op or throws..?
> > > > > 
> > > > > Oh.. you are right. For now, you can get around that by setting
> > > > > DrawGLFunctor.sSupportFunctorReleasedCallback false
> > > > 
> > > > I just reverted this change for now and now it's fine, but this probably
> > > wasn't
> > > > a good idea to land as-is then :)
> > > 
> > > I'll revert the N bit before we roll frameworks jar downtream. The N test
> bot
> > > didn't blow up because.. it doesn't run tests on the real webview. Meh x2
> > 
> > http://codereview.chromium.org authentication expired, and I'm still home
o_o.
> I'll
> > just fix downstream then :p
> 
> I don't think this has anything to do with rolling the framework jar. I
haven't
> done that, and it's still broken for me.

Right now the implementation says "this api is available on N, so go ahead and
call it", but in fact, it's not available and just throws as you found not. I'm
waiting on changing the throw to an actual call waiting on a framework jar roll,
but that's waiting on my android CL getting a +2.

[Torne, 2016-04-20 11:57:38]

On 2016/04/19 14:19:39, boliu wrote:
> On 2016/04/19 14:14:36, Torne wrote:
> > On 2016/04/19 14:12:56, boliu wrote:
> > > On 2016/04/19 14:10:11, boliu wrote:
> > > > On 2016/04/19 14:08:33, Torne wrote:
> > > > > On 2016/04/19 14:07:21, boliu wrote:
> > > > > > On 2016/04/19 14:03:52, boliu wrote:
> > > > > > > On 2016/04/19 14:03:04, Torne wrote:
> > > > > > > > This seems to have broken running current ToT webview on N.
Looks
> > like
> > > > > this
> > > > > > > > landed before the corresponding framework change? :/
> > > > > > > 
> > > > > > > Hmm, what framework change, all implementations in
> > > WebViewDelegateFactory
> > > > > are
> > > > > > > no-op or throws..?
> > > > > > 
> > > > > > Oh.. you are right. For now, you can get around that by setting
> > > > > > DrawGLFunctor.sSupportFunctorReleasedCallback false
> > > > > 
> > > > > I just reverted this change for now and now it's fine, but this
probably
> > > > wasn't
> > > > > a good idea to land as-is then :)
> > > > 
> > > > I'll revert the N bit before we roll frameworks jar downtream. The N
test
> > bot
> > > > didn't blow up because.. it doesn't run tests on the real webview. Meh
x2
> > > 
> > > http://codereview.chromium.org authentication expired, and I'm still home
> o_o.
> > I'll
> > > just fix downstream then :p
> > 
> > I don't think this has anything to do with rolling the framework jar. I
> haven't
> > done that, and it's still broken for me.
> 
> Right now the implementation says "this api is available on N, so go ahead and
> call it", but in fact, it's not available and just throws as you found not.
I'm
> waiting on changing the throw to an actual call waiting on a framework jar
roll,
> but that's waiting on my android CL getting a +2.

Ah, understood. Thanks :)