Assert that AwGLFunctor instances do not leak.

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

[Tobias Sargeant, 2016-04-22 10:19:38]

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

[boliu, 2016-04-22 15:51:53]

did you repeat the gc tests like at least 50 times locally to make sure this
doesn't flake?

https://codereview.chromium.org/1914433002/diff/1/android_webview/javatests/s...
File
android_webview/javatests/src/org/chromium/android_webview/test/AwContentsGarbageCollectionTest.java
(right):

https://codereview.chromium.org/1914433002/diff/1/android_webview/javatests/s...
android_webview/javatests/src/org/chromium/android_webview/test/AwContentsGarbageCollectionTest.java:243:
return count_awc <= MAX_IDLE_INSTANCES && count_awglf <= count_awc;
second half is never going to true, for now anyway? awglf is destroyed *after*
awc? Also name it maybe count_aw_functor or osmething, awglf is going to far..

https://codereview.chromium.org/1914433002/diff/1/android_webview/native/aw_g...
File android_webview/native/aw_gl_functor.cc (right):

https://codereview.chromium.org/1914433002/diff/1/android_webview/native/aw_g...
android_webview/native/aw_gl_functor.cc:39: ++g_instance_count;
should add threadchecks to where g_instance_count is used

actually should just add threadchecks everywhere in this file

[Tobias Sargeant, 2016-04-22 16:43:28]

how's this?

[boliu, 2016-04-22 16:48:08]

https://codereview.chromium.org/1914433002/diff/20001/android_webview/javates...
File
android_webview/javatests/src/org/chromium/android_webview/test/AwContentsGarbageCollectionTest.java
(right):

https://codereview.chromium.org/1914433002/diff/20001/android_webview/javates...
android_webview/javatests/src/org/chromium/android_webview/test/AwContentsGarbageCollectionTest.java:244:
&& count_aw_functor == count_aw_contents;
yeah you are right, == is a stronger condition, which might not old if we decide
to destroy aw_functor first, just do <= MAX_IDLE_INSTANCES instead..

[Tobias Sargeant, 2016-04-22 17:01:33]

https://codereview.chromium.org/1914433002/diff/20001/android_webview/javates...
File
android_webview/javatests/src/org/chromium/android_webview/test/AwContentsGarbageCollectionTest.java
(right):

https://codereview.chromium.org/1914433002/diff/20001/android_webview/javates...
android_webview/javatests/src/org/chromium/android_webview/test/AwContentsGarbageCollectionTest.java:244:
&& count_aw_functor == count_aw_contents;
On 2016/04/22 16:48:08, boliu wrote:
> yeah you are right, == is a stronger condition, which might not old if we
decide
> to destroy aw_functor first, just do <= MAX_IDLE_INSTANCES instead..

Done.

[boliu, 2016-04-22 17:03:18]

lgtm % make sure test is not flaky

[Tobias Sargeant, 2016-04-25 08:31:13]

On 2016/04/22 17:03:18, boliu wrote:
> lgtm % make sure test is not flaky

50 runs without failure.

[Tobias Sargeant, 2016-04-25 08:31:16]

The CQ bit was checked by tobiasjs@chromium.org

[commit-bot: I haz the power, 2016-04-25 08:31:35]

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

[commit-bot: I haz the power, 2016-04-25 08:33:33]

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

[commit-bot: I haz the power, 2016-04-25 08:33:34]

Try jobs failed on following builders:
  ios_rel_device_gn on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_gn...)
  ios_rel_device_ninja on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ni...)

[Tobias Sargeant, 2016-04-25 09:20:09]

The CQ bit was checked by tobiasjs@chromium.org

[Tobias Sargeant, 2016-04-25 09:20:10]

The patchset sent to the CQ was uploaded after l-g-t-m from boliu@chromium.org
Link to the patchset: https://codereview.chromium.org/1914433002/#ps60001
(title: "rebase")

[commit-bot: I haz the power, 2016-04-25 09:20:25]

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

[commit-bot: I haz the power, 2016-04-25 10:20:08]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-04-25 10:21:47]

Description was changed from

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

to

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

Committed: https://crrev.com/453c3b298d6dc0197d674a29ed59c67a716fe1f1
Cr-Commit-Position: refs/heads/master@{#389438}
==========

[commit-bot: I haz the power, 2016-04-25 10:21:48]

Patchset 4 (id:??) landed as
https://crrev.com/453c3b298d6dc0197d674a29ed59c67a716fe1f1
Cr-Commit-Position: refs/heads/master@{#389438}