Remove JNI onload callback vector construction in favour of direct calls.

Remove JNI onload callback vector construction in favour of direct calls.

As part of cleaning up JNI onload and initialization, this CL removes
the current code that builds up a vector of callbacks, which are
eventually run by base::android::OnJNIOnLoad{RegisterJNI,Init} in
favour of calling the appropriate code directly from those functions
(and the functions that call them).

BUG=678579
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester

Review-Url: https://codereview.chromium.org/2620303004
Cr-Commit-Position: refs/heads/master@{#448955}
Committed: https://chromium.googlesource.com/chromium/src/+/b9e287e5af645fe02056c99c36b6ffc40925ff07

[Tobias Sargeant, 2017-01-11 17:06:04]

Description was changed from

==========
Remove JNI onload callback vector construction in favour of direct calls.

As part of cleaning up JNI onload and initialization, this CL removes
the current code that builds up a vector of callbacks, which are
eventually run by base::android::OnJNIOnLoad{RegisterJNI,Init} in
favour of calling the appropriate code directly from those functions
(and the functions that call them).

BUG=678579
==========

to

==========
Remove JNI onload callback vector construction in favour of direct calls.

As part of cleaning up JNI onload and initialization, this CL removes
the current code that builds up a vector of callbacks, which are
eventually run by base::android::OnJNIOnLoad{RegisterJNI,Init} in
favour of calling the appropriate code directly from those functions
(and the functions that call them).

BUG=678579
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester
==========

[Tobias Sargeant, 2017-01-11 17:06:59]

Description was changed from

==========
Remove JNI onload callback vector construction in favour of direct calls.

As part of cleaning up JNI onload and initialization, this CL removes
the current code that builds up a vector of callbacks, which are
eventually run by base::android::OnJNIOnLoad{RegisterJNI,Init} in
favour of calling the appropriate code directly from those functions
(and the functions that call them).

BUG=678579
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester
==========

to

==========
Remove JNI onload callback vector construction in favour of direct calls.

As part of cleaning up JNI onload and initialization, this CL removes
the current code that builds up a vector of callbacks, which are
eventually run by base::android::OnJNIOnLoad{RegisterJNI,Init} in
favour of calling the appropriate code directly from those functions
(and the functions that call them).

BUG=678579
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester
==========

[Tobias Sargeant, 2017-01-11 17:06:59]

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

[Tobias Sargeant, 2017-01-11 17:07:50]

Torne, could you PTAL at this, before I start adding reviewers for all the test
code that's been touched? Thanks.

[Torne, 2017-01-12 13:33:53]

https://codereview.chromium.org/2620303004/diff/1/base/android/base_jni_onloa...
File base/android/base_jni_onload.cc (right):

https://codereview.chromium.org/2620303004/diff/1/base/android/base_jni_onloa...
base/android/base_jni_onload.cc:16: base::android::InitVM(vm);
Rather than passing JavaVM down through the RegisterJNI sequence we should just
call InitVM and AttachCurrentThread in each JNI_OnLoad and then pass the JNIEnv
down through RegisterJNI instead.

We always need to do InitVM even if we don't need JNI registration, I think it
makes most sense to just do it at the top level even though that means
duplicating it, it's just a single line.

https://codereview.chromium.org/2620303004/diff/1/components/cronet/android/t...
File components/cronet/android/test/cronet_test_jni.cc (right):

https://codereview.chromium.org/2620303004/diff/1/components/cronet/android/t...
components/cronet/android/test/cronet_test_jni.cc:39: bool NativeInit() {
Seems inconsistent with other usage to do RegisterNativeMethods in here. I
realise the original file did it in this order, but it seems better to just
leave things arranged as they are here?

[Tobias Sargeant, 2017-01-12 14:22:43]

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

[commit-bot: I haz the power, 2017-01-12 14:22:52]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Tobias Sargeant, 2017-01-12 14:27:56]

https://codereview.chromium.org/2620303004/diff/1/base/android/base_jni_onloa...
File base/android/base_jni_onload.cc (right):

https://codereview.chromium.org/2620303004/diff/1/base/android/base_jni_onloa...
base/android/base_jni_onload.cc:16: base::android::InitVM(vm);
On 2017/01/12 13:33:52, Torne wrote:
> Rather than passing JavaVM down through the RegisterJNI sequence we should
just
> call InitVM and AttachCurrentThread in each JNI_OnLoad and then pass the
JNIEnv
> down through RegisterJNI instead.
> 
> We always need to do InitVM even if we don't need JNI registration, I think it
> makes most sense to just do it at the top level even though that means
> duplicating it, it's just a single line.

Done.

https://codereview.chromium.org/2620303004/diff/1/components/cronet/android/t...
File components/cronet/android/test/cronet_test_jni.cc (right):

https://codereview.chromium.org/2620303004/diff/1/components/cronet/android/t...
components/cronet/android/test/cronet_test_jni.cc:39: bool NativeInit() {
On 2017/01/12 13:33:53, Torne wrote:
> Seems inconsistent with other usage to do RegisterNativeMethods in here. I
> realise the original file did it in this order, but it seems better to just
> leave things arranged as they are here?

Done.

[Torne, 2017-01-12 14:36:12]

lgtm

[Tobias Sargeant, 2017-01-12 15:50:56]

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

[Tobias Sargeant, 2017-01-12 15:50:57]

Hi Brett,

I was wondering if you would you be prepared to review this? Torne has covered
the //base/android changes, and mostly what's left to LGTM is a bunch of
mechanical changes to tests. I don't mind asking a number of individual
reviewers, but I thought maybe I'd see if you were prepared to LGTM the lot
first.

[Tobias Sargeant, 2017-01-16 15:38:17]

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

[commit-bot: I haz the power, 2017-01-16 15:38:31]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-16 15:40:25]

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

[commit-bot: I haz the power, 2017-01-16 15:40:26]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)

[Tobias Sargeant, 2017-01-17 09:57:21]

tobiasjs@chromium.org changed reviewers:
+ jochen@chromium.org, kelvinp@chromium.org, rockot@chromium.org,
rsleevi@chromium.org, slan@chromium.org, yfriedman@chromium.org

[Tobias Sargeant, 2017-01-19 11:42:26]

Gently pinging this CL.

[Lambros, 2017-01-19 17:53:36]

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

[Lambros, 2017-01-19 17:53:37]

lgtm for remoting/

[slan, 2017-01-19 18:08:58]

+sanfin, +tsunghung for FYI

cast/ lgtm

[Ryan Sleevi, 2017-01-19 18:49:59]

//net LGTM

[Tobias Sargeant, 2017-02-03 10:22:44]

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

[Tobias Sargeant, 2017-02-03 10:24:18]

On 2017/01/19 11:42:26, Tobias Sargeant wrote:
> Gently pinging this CL.

Another ping...

[commit-bot: I haz the power, 2017-02-03 10:26:21]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-03 10:43:46]

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

[commit-bot: I haz the power, 2017-02-03 10:43:48]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[Ken Rockot(use gerrit already), 2017-02-03 11:35:38]

lgtm - please consider using IM to ping anyone who still needs to review this

[Ken Rockot(use gerrit already), 2017-02-03 11:35:38]

lgtm - please consider using IM to ping anyone who still needs to review this

[Tobias Sargeant, 2017-02-03 14:45:26]

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

[commit-bot: I haz the power, 2017-02-03 14:45:35]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-03 15:36:42]

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

[commit-bot: I haz the power, 2017-02-03 15:36:43]

Dry run: This issue passed the CQ dry run.

[AndyWu, 2017-02-03 18:05:53]

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

[AndyWu, 2017-02-03 18:05:54]

lgtm

[kelvinp, 2017-02-03 18:08:24]

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

[kelvinp, 2017-02-03 18:09:47]

On 2017/02/03 18:05:54, AndyWu wrote:
> lgtm

Removing myself from the reviewer list as Lambros (our TL for Android) has
already lgtm'ed the change.

[brettw, 2017-02-08 05:11:43]

I don't understand the Java big picture here but the code looks a lot cleaner.
I'm assuming one of the previous reviewers understand the Java initialization,
so owners LGTM.

[Tobias Sargeant, 2017-02-08 10:13:10]

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

[commit-bot: I haz the power, 2017-02-08 10:13:30]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-08 10:58:47]

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

[commit-bot: I haz the power, 2017-02-08 10:58:48]

Dry run: This issue passed the CQ dry run.

[Tobias Sargeant, 2017-02-08 11:05:06]

The CQ bit was checked by tobiasjs@chromium.org

[Tobias Sargeant, 2017-02-08 11:05:07]

The patchset sent to the CQ was uploaded after l-g-t-m from torne@chromium.org,
slan@chromium.org, rsleevi@chromium.org, lambroslambrou@chromium.org,
rockot@chromium.org
Link to the patchset: https://codereview.chromium.org/2620303004/#ps120001
(title: "rebase compile errors")

[commit-bot: I haz the power, 2017-02-08 11:05:23]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-08 11:09:57]

CQ is committing da patch.

Bot data: {"patchset_id": 120001, "attempt_start_ts": 1486551906529350,
"parent_rev": "598f19378351e13bda80475bff9ce5a07eec4363", "commit_rev":
"b9e287e5af645fe02056c99c36b6ffc40925ff07"}

[commit-bot: I haz the power, 2017-02-08 11:10:40]

Description was changed from

==========
Remove JNI onload callback vector construction in favour of direct calls.

As part of cleaning up JNI onload and initialization, this CL removes
the current code that builds up a vector of callbacks, which are
eventually run by base::android::OnJNIOnLoad{RegisterJNI,Init} in
favour of calling the appropriate code directly from those functions
(and the functions that call them).

BUG=678579
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester
==========

to

==========
Remove JNI onload callback vector construction in favour of direct calls.

As part of cleaning up JNI onload and initialization, this CL removes
the current code that builds up a vector of callbacks, which are
eventually run by base::android::OnJNIOnLoad{RegisterJNI,Init} in
favour of calling the appropriate code directly from those functions
(and the functions that call them).

BUG=678579
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester

Review-Url: https://codereview.chromium.org/2620303004
Cr-Commit-Position: refs/heads/master@{#448955}
Committed:
https://chromium.googlesource.com/chromium/src/+/b9e287e5af645fe02056c99c36b6...
==========

[commit-bot: I haz the power, 2017-02-08 11:10:42]

Committed patchset #7 (id:120001) as
https://chromium.googlesource.com/chromium/src/+/b9e287e5af645fe02056c99c36b6...