Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1052)

Issue 1308363003: Revert of jni_generator: Make all object-returning natives return ScopedJavaLocalRef. (Closed)

Created:
5 years, 4 months ago by nasko
Modified:
5 years, 4 months ago
CC:
chromium-reviews
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Revert of jni_generator: Make all object-returning natives return ScopedJavaLocalRef. (patchset #5 id:80001 of https://codereview.chromium.org/1288183004/ ) Reason for revert: 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. Original issue's description: > 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} TBR=aurimas@chromium.org,jshin@chromium.org,jwd@chromium.org,mlerman@chromium.org,mmenke@chromium.org,nyquist@chromium.org,rmcilroy@chromium.org,sievers@chromium.org,sky@chromium.org,solb@chromium.org,yfriedman@google.com,lambroslambrou@chromium.org,yfriedman@chromium.org,torne@chromium.org NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=379897 Committed: https://crrev.com/48100928ada82f247379600164bef90502b82a4e Cr-Commit-Position: refs/heads/master@{#345171}

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+664 lines, -692 lines) Patch
M android_webview/native/android_protocol_handler.cc View 1 chunk +10 lines, -8 lines 0 comments Download
M android_webview/native/aw_contents_statics.cc View 2 chunks +4 lines, -4 lines 0 comments Download
M android_webview/native/aw_settings.cc View 1 chunk +2 lines, -3 lines 0 comments Download
M android_webview/native/cookie_manager.cc View 1 chunk +3 lines, -4 lines 0 comments Download
M base/android/command_line_android.cc View 1 chunk +4 lines, -5 lines 0 comments Download
M base/android/field_trial_list.cc View 1 chunk +5 lines, -4 lines 0 comments Download
M base/android/jni_generator/golden_sample_for_tests_jni.h View 5 chunks +8 lines, -9 lines 0 comments Download
M base/android/jni_generator/jni_generator.py View 4 chunks +8 lines, -10 lines 0 comments Download
M base/android/jni_generator/sample_for_tests.cc View 1 chunk +2 lines, -2 lines 0 comments Download
M base/android/jni_generator/testCalledByNatives.golden View 17 chunks +43 lines, -41 lines 0 comments Download
M base/android/jni_generator/testConstantsFromJavaP.golden View 16 chunks +45 lines, -40 lines 0 comments Download
M base/android/jni_generator/testEagerCalledByNativesOption.golden View 1 chunk +3 lines, -2 lines 0 comments Download
M base/android/jni_generator/testFromJavaP.golden View 2 chunks +5 lines, -4 lines 0 comments Download
M base/android/jni_generator/testNativeExportsOption.golden View 4 chunks +4 lines, -4 lines 0 comments Download
M base/android/jni_generator/testNativeExportsOptionalOption.golden View 4 chunks +4 lines, -4 lines 0 comments Download
M base/android/jni_generator/testNatives.golden View 3 chunks +10 lines, -13 lines 0 comments Download
M base/android/library_loader/library_loader_hooks.cc View 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/browser/android/chrome_application.cc View 1 chunk +2 lines, -3 lines 0 comments Download
M chrome/browser/android/download/chrome_download_delegate.cc View 1 chunk +4 lines, -5 lines 0 comments Download
M chrome/browser/android/omnibox/autocomplete_controller_android.cc View 2 chunks +6 lines, -7 lines 0 comments Download
M chrome/browser/android/password_ui_view_android.cc View 1 chunk +3 lines, -3 lines 0 comments Download
M chrome/browser/android/preferences/autofill/autofill_profile_bridge.cc View 3 chunks +10 lines, -11 lines 0 comments Download
M chrome/browser/android/preferences/pref_service_bridge.cc View 4 chunks +46 lines, -47 lines 0 comments Download
M chrome/browser/android/profiles/profile_downloader_android.cc View 4 chunks +18 lines, -20 lines 0 comments Download
M chrome/browser/android/tab_state.h View 1 chunk +6 lines, -6 lines 0 comments Download
M chrome/browser/android/tab_state.cc View 6 chunks +36 lines, -39 lines 0 comments Download
M chrome/browser/android/url_utilities.cc View 3 chunks +15 lines, -15 lines 0 comments Download
M chrome/browser/android/web_contents_factory.cc View 1 chunk +3 lines, -6 lines 0 comments Download
M chrome/browser/autofill/android/personal_data_manager_android.cc View 1 chunk +5 lines, -6 lines 0 comments Download
M chrome/browser/dom_distiller/dom_distiller_service_factory_android.h View 2 chunks +1 line, -4 lines 0 comments Download
M chrome/browser/dom_distiller/dom_distiller_service_factory_android.cc View 1 chunk +7 lines, -10 lines 0 comments Download
M chrome/browser/dom_distiller/tab_utils_android.cc View 2 chunks +7 lines, -7 lines 0 comments Download
M chrome/browser/invalidation/invalidation_service_factory_android.h View 3 chunks +3 lines, -8 lines 0 comments Download
M chrome/browser/invalidation/invalidation_service_factory_android.cc View 1 chunk +14 lines, -19 lines 0 comments Download
M chrome/browser/profiles/profile_android.h View 1 chunk +1 line, -3 lines 0 comments Download
M chrome/browser/profiles/profile_android.cc View 2 chunks +5 lines, -6 lines 0 comments Download
M chrome/browser/signin/oauth2_token_service_delegate_android.h View 2 chunks +3 lines, -3 lines 0 comments Download
M chrome/browser/signin/oauth2_token_service_delegate_android.cc View 2 chunks +8 lines, -8 lines 0 comments Download
M chrome/browser/ui/android/certificate_viewer_android.cc View 1 chunk +30 lines, -31 lines 0 comments Download
M chrome/browser/ui/android/connection_info_popup_android.cc View 2 chunks +6 lines, -4 lines 0 comments Download
M chrome/browser/ui/android/javascript_app_modal_dialog_android.cc View 1 chunk +3 lines, -3 lines 0 comments Download
M chrome/browser/ui/android/navigation_popup.cc View 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/browser/ui/android/omnibox/omnibox_url_emphasizer.cc View 2 chunks +5 lines, -5 lines 0 comments Download
M chrome/browser/ui/android/omnibox/omnibox_view_util.cc View 1 chunk +2 lines, -4 lines 0 comments Download
M components/cronet/android/chromium_url_request.cc View 5 chunks +29 lines, -28 lines 0 comments Download
M components/cronet/android/chromium_url_request_context.cc View 1 chunk +4 lines, -4 lines 0 comments Download
M components/cronet/android/cronet_histogram_manager.cc View 1 chunk +3 lines, -4 lines 0 comments Download
M components/cronet/android/cronet_library_loader.cc View 1 chunk +2 lines, -2 lines 0 comments Download
M components/cronet/android/test/mock_url_request_job_factory.cc View 1 chunk +12 lines, -13 lines 0 comments Download
M components/cronet/android/test/native_test_server.cc View 1 chunk +30 lines, -34 lines 0 comments Download
M components/cronet/android/test/quic_test_server.cc View 1 chunk +2 lines, -2 lines 0 comments Download
M components/dom_distiller/core/url_utils_android.cc View 2 chunks +24 lines, -21 lines 0 comments Download
M components/variations/android/variations_associated_data_android.cc View 1 chunk +5 lines, -5 lines 0 comments Download
M content/browser/android/content_video_view.cc View 1 chunk +3 lines, -4 lines 0 comments Download
M content/browser/android/content_view_core_impl.cc View 1 chunk +8 lines, -6 lines 0 comments Download
M content/browser/android/content_view_statics.cc View 1 chunk +3 lines, -5 lines 0 comments Download
M content/browser/android/tracing_controller_android.cc View 1 chunk +4 lines, -3 lines 0 comments Download
M content/browser/web_contents/web_contents_android.cc View 1 chunk +8 lines, -8 lines 0 comments Download
M content/shell/browser/shell_mojo_test_utils_android.cc View 2 chunks +4 lines, -5 lines 0 comments Download
M mojo/android/system/core_impl.cc View 16 chunks +82 lines, -78 lines 0 comments Download
M net/android/gurl_utils.cc View 1 chunk +6 lines, -4 lines 0 comments Download
M net/cert/x509_util_android.cc View 1 chunk +2 lines, -3 lines 0 comments Download
M remoting/client/jni/chromoting_jni_runtime.cc View 1 chunk +13 lines, -12 lines 0 comments Download
M sync/android/model_type_helper.cc View 1 chunk +5 lines, -3 lines 0 comments Download
M ui/base/l10n/l10n_util_android.cc View 1 chunk +2 lines, -4 lines 0 comments Download

Messages

Total messages: 4 (0 generated)
nasko
Created Revert of jni_generator: Make all object-returning natives return ScopedJavaLocalRef.
5 years, 4 months ago (2015-08-24 20:45:14 UTC) #1
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1308363003/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1308363003/1
5 years, 4 months ago (2015-08-24 20:45:37 UTC) #2
commit-bot: I haz the power
Committed patchset #1 (id:1)
5 years, 4 months ago (2015-08-24 20:46:47 UTC) #3
commit-bot: I haz the power
5 years, 4 months ago (2015-08-24 20:47:33 UTC) #4
Message was sent while issue was closed.
Patchset 1 (id:??) landed as
https://crrev.com/48100928ada82f247379600164bef90502b82a4e
Cr-Commit-Position: refs/heads/master@{#345171}

Powered by Google App Engine
This is Rietveld 408576698