Android JNI gen: Don't emit code for empty RegisterNatives()

Android JNI gen: Don't emit code for empty RegisterNatives()

BUG=603936
TBR=halliwell@chromium.org

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

[no sievers, 2016-08-01 23:04:58]

Description was changed from

==========
Android JNI gen: Don't emit code for empty RegisterNatives()

BUG=603936
==========

to

==========
Android JNI gen: Don't emit code for empty RegisterNatives()

BUG=603936
==========

[no sievers, 2016-08-01 23:04:59]

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

[no sievers, 2016-08-01 23:05:17]

When everything else is landed, then I'd attempt to land this. ptal.

[Torne, 2016-08-02 12:31:40]

lgtm, but maybe don't do the fiddly thing with the comment?

https://codereview.chromium.org/2199973003/diff/1/base/android/jni_generator/...
File base/android/jni_generator/jni_generator.py (right):

https://codereview.chromium.org/2199973003/diff/1/base/android/jni_generator/...
base/android/jni_generator/jni_generator.py:743: $STEP3_COMMENT
I'd really just leave the comment there; it's an autogenerated file, it's okay
for there to be empty sections. I don't think this is worth the complexity.

[no sievers, 2016-08-02 17:38:14]

The CQ bit was checked by sievers@chromium.org

[commit-bot: I haz the power, 2016-08-02 17:38:44]

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

[no sievers, 2016-08-02 17:39:00]

The CQ bit was unchecked by sievers@chromium.org

[no sievers, 2016-08-02 23:12:17]

https://codereview.chromium.org/2199973003/diff/1/base/android/jni_generator/...
File base/android/jni_generator/jni_generator.py (right):

https://codereview.chromium.org/2199973003/diff/1/base/android/jni_generator/...
base/android/jni_generator/jni_generator.py:743: $STEP3_COMMENT
On 2016/08/02 12:31:40, Torne wrote:
> I'd really just leave the comment there; it's an autogenerated file, it's okay
> for there to be empty sections. I don't think this is worth the complexity.

Done.

[no sievers, 2016-08-02 23:14:22]

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

[commit-bot: I haz the power, 2016-08-02 23:14:52]

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, 2016-08-02 23:33:01]

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

[commit-bot: I haz the power, 2016-08-02 23:33:02]

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...)

[no sievers, 2016-08-03 18:25:41]

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

[commit-bot: I haz the power, 2016-08-03 18:26:19]

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, 2016-08-03 18:51:04]

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

[commit-bot: I haz the power, 2016-08-03 18:51:05]

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

[no sievers, 2016-08-03 21:14:56]

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

[no sievers, 2016-08-03 21:15:12]

Description was changed from

==========
Android JNI gen: Don't emit code for empty RegisterNatives()

BUG=603936
==========

to

==========
Android JNI gen: Don't emit code for empty RegisterNatives()

BUG=603936
TBR=halliwell@chromium.org
==========

[commit-bot: I haz the power, 2016-08-03 21:15:35]

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

[no sievers, 2016-08-03 21:16:20]

Patchset #4 (id:60001) has been deleted

[no sievers, 2016-08-03 21:19:53]

sievers@chromium.org changed reviewers:
+ halliwell@chromium.org

[no sievers, 2016-08-03 21:20:05]

+halliwell@ FYI, see comment

https://codereview.chromium.org/2199973003/diff/80001/chromecast/base/cast_sy...
File chromecast/base/cast_sys_info_android.h (right):

https://codereview.chromium.org/2199973003/diff/80001/chromecast/base/cast_sy...
chromecast/base/cast_sys_info_android.h:45: void DeviceNameChanged(JNIEnv* env,
jobject obj, jstring device_name);
hmm this method is unused (from Java)

[halliwell, 2016-08-03 21:26:19]

halliwell@chromium.org changed reviewers:
+ sanfin@chromium.org

[halliwell, 2016-08-03 21:27:37]

On 2016/08/03 21:20:05, sievers wrote:
> +halliwell@ FYI, see comment
> 
>
https://codereview.chromium.org/2199973003/diff/80001/chromecast/base/cast_sy...
> File chromecast/base/cast_sys_info_android.h (right):
> 
>
https://codereview.chromium.org/2199973003/diff/80001/chromecast/base/cast_sy...
> chromecast/base/cast_sys_info_android.h:45: void DeviceNameChanged(JNIEnv*
env,
> jobject obj, jstring device_name);
> hmm this method is unused (from Java)

+sanfin knows this code better; but I believe DeviceNameChanged is called from
Java, just from internal code.

[no sievers, 2016-08-03 21:43:14]

Description was changed from

==========
Android JNI gen: Don't emit code for empty RegisterNatives()

BUG=603936
TBR=halliwell@chromium.org
==========

to

==========
Android JNI gen: Don't emit code for empty RegisterNatives()

BUG=603936
==========

[no sievers, 2016-08-03 22:07:35]

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

[commit-bot: I haz the power, 2016-08-03 22:08:58]

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

[no sievers, 2016-08-03 22:09:40]

Description was changed from

==========
Android JNI gen: Don't emit code for empty RegisterNatives()

BUG=603936
==========

to

==========
Android JNI gen: Don't emit code for empty RegisterNatives()

BUG=603936
TBR=halliwell@chromium.org
==========

[Simeon, 2016-08-03 22:20:34]

On 2016/08/03 21:27:37, halliwell wrote:
> On 2016/08/03 21:20:05, sievers wrote:
> > +halliwell@ FYI, see comment
> > 
> >
>
https://codereview.chromium.org/2199973003/diff/80001/chromecast/base/cast_sy...
> > File chromecast/base/cast_sys_info_android.h (right):
> > 
> >
>
https://codereview.chromium.org/2199973003/diff/80001/chromecast/base/cast_sy...
> > chromecast/base/cast_sys_info_android.h:45: void DeviceNameChanged(JNIEnv*
> env,
> > jobject obj, jstring device_name);
> > hmm this method is unused (from Java)
> 
> +sanfin knows this code better; but I believe DeviceNameChanged is called from
> Java, just from internal code.

lgtm; we do call a DeviceNameChanged in the internal code, but it's not THAT
one. The one we actually use is defined in an internal class, and
CastSysInfoAndroid::DeviceNameChanged() is never implemented or called.

[commit-bot: I haz the power, 2016-08-03 23:04:56]

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

[commit-bot: I haz the power, 2016-08-03 23:04:57]

Dry run: This issue passed the CQ dry run.

[no sievers, 2016-08-03 23:14:18]

The CQ bit was checked by sievers@chromium.org

[no sievers, 2016-08-03 23:14:18]

The patchset sent to the CQ was uploaded after l-g-t-m from torne@chromium.org
Link to the patchset: https://codereview.chromium.org/2199973003/#ps100001
(title: "Android JNI gen: Don't emit code for empty RegisterNatives()")

[commit-bot: I haz the power, 2016-08-03 23:15:22]

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

[halliwell, 2016-08-03 23:17:20]

On 2016/08/03 23:15:22, commit-bot: I haz the power wrote:
> CQ is trying da patch. Follow status at
>  
>
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

lgtm

[commit-bot: I haz the power, 2016-08-03 23:20:56]

Description was changed from

==========
Android JNI gen: Don't emit code for empty RegisterNatives()

BUG=603936
TBR=halliwell@chromium.org
==========

to

==========
Android JNI gen: Don't emit code for empty RegisterNatives()

BUG=603936
TBR=halliwell@chromium.org
==========

[commit-bot: I haz the power, 2016-08-03 23:20:57]

Committed patchset #5 (id:100001)

[commit-bot: I haz the power, 2016-08-03 23:23:39]

Description was changed from

==========
Android JNI gen: Don't emit code for empty RegisterNatives()

BUG=603936
TBR=halliwell@chromium.org
==========

to

==========
Android JNI gen: Don't emit code for empty RegisterNatives()

BUG=603936
TBR=halliwell@chromium.org

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

[commit-bot: I haz the power, 2016-08-03 23:23:40]

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

[mikecase (-- gone --), 2016-08-04 15:19:06]

On 2016/08/03 at 23:23:40, commit-bot wrote:
> Patchset 5 (id:??) landed as
https://crrev.com/da13ab25380b86601db01fc11ad04e4c515b2209
> Cr-Commit-Position: refs/heads/master@{#409657}

This CL seems to be causing a compile failure on the cronet bots.

https://build.chromium.org/p/chromium.android/builders/Android%20Cronet%20ARM...

../../net/base/net_string_util_icu_alternatives_android.cc: In function 'bool
net::RegisterNetStringUtils(JNIEnv*)':
../../net/base/net_string_util_icu_alternatives_android.cc:117:10: error:
'RegisterNativesImpl' is not a member of 'net::android'
   return android::RegisterNativesImpl(env);
          ^
../../net/base/net_string_util_icu_alternatives_android.cc:118:1: error: control
reaches end of non-void function [-Werror=return-type]
 }

[Torne, 2016-08-04 16:09:05]

On 2016/08/04 15:19:06, mikecase wrote:
> On 2016/08/03 at 23:23:40, commit-bot wrote:
> > Patchset 5 (id:??) landed as
> https://crrev.com/da13ab25380b86601db01fc11ad04e4c515b2209
> > Cr-Commit-Position: refs/heads/master@{#409657}
> 
> This CL seems to be causing a compile failure on the cronet bots.
> 
>
https://build.chromium.org/p/chromium.android/builders/Android%20Cronet%20ARM...
> 
> ../../net/base/net_string_util_icu_alternatives_android.cc: In function 'bool
> net::RegisterNetStringUtils(JNIEnv*)':
> ../../net/base/net_string_util_icu_alternatives_android.cc:117:10: error:
> 'RegisterNativesImpl' is not a member of 'net::android'
>    return android::RegisterNativesImpl(env);
>           ^
> ../../net/base/net_string_util_icu_alternatives_android.cc:118:1: error:
control
> reaches end of non-void function [-Werror=return-type]
>  }

Yup, sorry, the CQ doesn't cover the config where
use_platform_icu_alternatives=true (cronet bots) and so this got overlooked. CL
to remove the right code from that codepath here:
https://codereview.chromium.org/2215863002/