jni_generator: Make all object-returning natives return ScopedJavaLocalRef.

jni_generator: Make all object-returning natives return ScopedJavaLocalRef.

Instead of only expecting C++ methods to return ScopedJavaLocalRefs for
object return values, apply this to all native functions. This further
reduces the difference between methods and nonmethods. The vast majority
of native nonmethod functions already have a ScopedJavaLocalRef and
currently write "return foo.Release()" to return it, so this is a fairly
small change in actual behaviour; only a few functions need to create
one.

Also, remove the namespace qualifiers from the existing generated
references to ScopedJavaLocalRef, since jni_generator_helper.h already
includes a "using" declaration for it.

BUG=379897
Committed: https://crrev.com/be33e691030b6de0351d9a7eeaaca218685dd5bb
Cr-Commit-Position: refs/heads/master@{#345147}

[Torne, 2015-08-18 11:17:08]

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

[Torne, 2015-08-18 11:17:08]

OK, broke this up into incremental steps to make this somewhat saner to review:

PS1 is just the changes to the generator and the regeneration of the generator
test files.
PS2 is all the functions that already just called Release() (the trivial cases)
PS3 is all the nontrivial cases
PS4 just fixes the include order the presubmit pointed out ;)

This adds a total of 564 bytes to libchrome.so so the inlining is behaving as
expected in all the trivial cases.

[rmcilroy, 2015-08-18 13:26:10]

lgtm, thanks!

https://codereview.chromium.org/1288183004/diff/40001/chrome/browser/dom_dist...
File chrome/browser/dom_distiller/dom_distiller_service_factory_android.h
(right):

https://codereview.chromium.org/1288183004/diff/40001/chrome/browser/dom_dist...
chrome/browser/dom_distiller/dom_distiller_service_factory_android.h:19:
GetForProfile(JNIEnv* env, jclass clazz, jobject j_profile);
nit - add newline below for clarity

[Torne, 2015-08-18 13:58:02]

torne@chromium.org changed reviewers:
+ mlerman@chromium.org, mmenke@chromium.org, nyquist@chromium.org,
yfriedman@google.com

[Torne, 2015-08-18 13:58:03]

First round of OWNERS:

yfriedman:
chrome/browser/android
chrome/browser/ui/android
content/browser/android
sync/android

nyquist:
chrome/browser/dom_distiller
chrome/browser/invalidation
components/dom_distiller

mmenke:
components/cronet
net

mlerman:
chrome/browser/profiles
chrome/browser/signin

[mmenke, 2015-08-18 14:04:44]

On 2015/08/18 13:58:03, Torne wrote:
> First round of OWNERS:
> 
> yfriedman:
> chrome/browser/android
> chrome/browser/ui/android
> content/browser/android
> sync/android
> 
> nyquist:
> chrome/browser/dom_distiller
> chrome/browser/invalidation
> components/dom_distiller
> 
> mmenke:
> components/cronet
> net
> 
> mlerman:
> chrome/browser/profiles
> chrome/browser/signin

net/ and cronet/ LGTM.


May want to update your description - this doesn't make all natives return
ScopedJavaLocalRefs, just the ones that return objects.

[Mike Lerman, 2015-08-18 14:17:39]

c/b/profiles and c/b/signin lgtm

https://codereview.chromium.org/1288183004/diff/80001/chrome/browser/signin/o...
File chrome/browser/signin/oauth2_token_service_delegate_android.h (right):

https://codereview.chromium.org/1288183004/diff/80001/chrome/browser/signin/o...
chrome/browser/signin/oauth2_token_service_delegate_android.h:40:
GetForProfile(JNIEnv* env, jclass clazz, jobject j_profile_android);
nit: indent

[Torne, 2015-08-18 14:32:28]

> net/ and cronet/ LGTM.
> 
> 
> May want to update your description - this doesn't make all natives return
> ScopedJavaLocalRefs, just the ones that return objects.

Retitled.

[Torne, 2015-08-18 14:33:15]

On 2015/08/18 14:17:39, Mike Lerman wrote:
> c/b/profiles and c/b/signin lgtm
> 
>
https://codereview.chromium.org/1288183004/diff/80001/chrome/browser/signin/o...
> File chrome/browser/signin/oauth2_token_service_delegate_android.h (right):
> 
>
https://codereview.chromium.org/1288183004/diff/80001/chrome/browser/signin/o...
> chrome/browser/signin/oauth2_token_service_delegate_android.h:40:
> GetForProfile(JNIEnv* env, jclass clazz, jobject j_profile_android);
> nit: indent

This is all clang-format output, so if we don't like that wrapping style we
should fix clang-format..

[nyquist, 2015-08-19 10:12:47]

chrome/browser/dom_distiller, chrome/browser/invalidation,
components/dom_distiller lgtm

[Torne, 2015-08-19 11:10:26]

torne@chromium.org changed reviewers:
+ aurimas@chromium.org, jshin@chromium.org, jwd@chromium.org,
sievers@chromium.org, sky@chromium.org, solb@chromium.org

[Torne, 2015-08-19 11:10:27]

More OWNERS:

sievers:
content/browser/web_contents
content/shell/browser

solb:
remoting/client/jni

jshin:
ui/base/l10n

aurimas:
chrome/browser/autofill/android

jwd:
components/variations

sky:
mojo

[aurimas (slooooooooow), 2015-08-19 16:14:52]

LGTM for chrome/browser/autofill/android

[jwd, 2015-08-19 18:43:22]

components/variations LGTM

[sky, 2015-08-19 20:33:34]

LGTM

[solb, 2015-08-20 03:13:09]

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

[solb, 2015-08-20 03:15:08]

lgtm but please wait for lambroslambrou

[Lambros, 2015-08-20 16:04:46]

remoting/ lgtm

[no sievers, 2015-08-20 20:23:49]

content lgtm

[jungshik at Google, 2015-08-21 16:20:51]

LGTM: l10n_util

[Yaron, 2015-08-24 18:06:16]

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

[Yaron, 2015-08-24 18:06:16]

lgtm

[Torne, 2015-08-24 18:47:25]

The CQ bit was checked by torne@chromium.org

[Torne, 2015-08-24 18:47:26]

The patchset sent to the CQ was uploaded after l-g-t-m from
rmcilroy@chromium.org
Link to the patchset: https://codereview.chromium.org/1288183004/#ps80001
(title: "Add some newlines for readability")

[commit-bot: I haz the power, 2015-08-24 18:47:51]

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

[commit-bot: I haz the power, 2015-08-24 19:37:29]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2015-08-24 19:38:21]

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

[nasko, 2015-08-24 20:45:14]

A revert of this CL (patchset #5 id:80001) has been created in
https://codereview.chromium.org/1308363003/ by nasko@chromium.org.

The reason for reverting is: Broke Android GN build:

http://build.chromium.org/p/chromium.linux/builders/Android%20GN/builds/29644

FAILED: /b/build/goma/gomacc
../../third_party/android_tools/ndk/toolchains/arm-linux-androideabi-4.9/prebuilt/linux-x86_64/bin/arm-linux-androideabi-g++
-MMD -MF obj/mojo/android/libmojo_java_unittests/validation_test_util.o.d
-DUNIT_TEST -DV8_DEPRECATION_WARNINGS -DCLD_VERSION=1 -DENABLE_NOTIFICATIONS
-DENABLE_BROWSER_CDMS -DENABLE_PRINTING=1 -DENABLE_BASIC_PRINTING=1
-DENABLE_SPELLCHECK=1 -DUSE_BROWSER_SPELLCHECKER=1 -DDONT_EMBED_BUILD_METADATA
-DUSE_OPENSSL=1 -DUSE_OPENSSL_CERTS=1 -DNO_TCMALLOC -DDISABLE_NACL
-DENABLE_CONFIGURATION_POLICY -DENABLE_SUPERVISED_USERS=1
-DENABLE_AUTOFILL_DIALOG=1 -DUSE_PROPRIETARY_CODECS
-DV8_USE_EXTERNAL_STARTUP_DATA -DVIDEO_HOLE=1 -DMOBILE_SAFE_BROWSING
-DSAFE_BROWSING_SERVICE -DCHROMIUM_BUILD -DENABLE_MEDIA_ROUTER=1
-DFIELDTRIAL_TESTING_ENABLED -D_FILE_OFFSET_BITS=64 -DANDROID -DHAVE_SYS_UIO_H
-D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D_FORTIFY_SOURCE=2
-D__GNU_SOURCE=1 -DNDEBUG -DNVALGRIND -DDYNAMIC_ANNOTATIONS_ENABLED=0
-DMOJO_USE_SYSTEM_IMPL -DGTEST_HAS_POSIX_RE=0 -DGTEST_LANG_CXX11=0
-DGTEST_HAS_RTTI=0 -DGTEST_USE_OWN_TR1_TUPLE=1 -DGTEST_HAS_TR1_TUPLE=1
-DU_USING_ICU_NAMESPACE=0 -DU_ENABLE_DYLOAD=0 -DU_STATIC_IMPLEMENTATION -I../..
-Igen -Igen/mojo/android/jni_headers -Igen/mojo/android/jni_headers/mojo
-Igen/mojo/android/system_java_jni_headers
-Igen/mojo/android/system_java_jni_headers/mojo -I../../testing/gtest/include
-I../../third_party/icu/source/common -I../../third_party/icu/source/i18n
-I../../third_party/mojo/src -Igen/third_party/mojo/src -fno-strict-aliasing
--param=ssp-buffer-size=4 -fstack-protector -march=armv7-a -mfloat-abi=softfp
-mtune=generic-armv7-a -mthumb -mthumb-interwork -fno-tree-sra -fno-caller-saves
-funwind-tables -fPIC -pipe -ffunction-sections -funwind-tables -fno-short-enums
-finline-limit=64 -mfpu=vfpv3-d16 -Wall -Wsign-compare -Wendif-labels -Werror
-Wno-missing-field-initializers -Wno-unused-parameter -Wno-psabi -Wno-extra
-Wno-ignored-qualifiers -Wno-type-limits -Wno-unused-local-typedefs
-isystem../../third_party/android_tools/ndk/sources/cxx-stl/llvm-libc++/libcxx/include
-isystem../../third_party/android_tools/ndk/sources/cxx-stl/llvm-libc++abi/libcxxabi/include
-isystem../../third_party/android_tools/ndk/sources/android/support/include
-fvisibility=hidden
--sysroot=/b/build/slave/Android_GN/build/src/third_party/android_tools/ndk/platforms/android-16/arch-arm
-Os -fno-ident -fdata-sections -ffunction-sections -fomit-frame-pointer -g2
-fno-threadsafe-statics -fvisibility-inlines-hidden -std=gnu++11 -Wno-narrowing
-Wno-literal-suffix -Wno-error=c++0x-compat -Wno-non-virtual-dtor
-Wno-sign-promo -fno-rtti -fno-exceptions -c
../../mojo/android/javatests/validation_test_util.cc -o
obj/mojo/android/libmojo_java_unittests/validation_test_util.o
../../mojo/android/javatests/validation_test_util.cc: In function '_jobject*
mojo::android::ParseData(JNIEnv*, jclass, jstring)':
../../mojo/android/javatests/validation_test_util.cc:21:70:error: ambiguating
new declaration of '_jobject* mojo::android::ParseData(JNIEnv*, jclass,
jstring)'
 jobject ParseData(JNIEnv* env, jclass jcaller, jstring data_as_string) {
                                                                      ^
In file included from ../../mojo/android/javatests/validation_test_util.cc:11:0:
gen/mojo/android/jni_headers/mojo/jni/ValidationTestUtil_jni.h:34:36: note: old
declaration 'base::android::ScopedJavaLocalRef<_jobject*>
mojo::android::ParseData(JNIEnv*, jclass, jstring)'
 static ScopedJavaLocalRef<jobject> ParseData(JNIEnv* env, jclass jcaller,
                                    ^
gen/mojo/android/jni_headers/mojo/jni/ValidationTestUtil_jni.h: At global scope:
gen/mojo/android/jni_headers/mojo/jni/ValidationTestUtil_jni.h:34:36:error:
'base::android::ScopedJavaLocalRef<_jobject*> mojo::android::ParseData(JNIEnv*,
jclass, jstring)' declared 'static' but never defined [-Werror=unused-function]
cc1plus: all warnings being treated as errors
ninja: build stopped: subcommand failed.
.