The JNI generator should generate code that doesn't require manual kJavaLangClass and similar |
||
Issue descriptionDuring the jumbo work I was hit by clashing kJavaLangClass and similar in content/browser/android/java/java_method.cc as well as in content/browser/android/java/gin_java_bound_object.cc const char kGetName[] = "getName"; const char kGetDeclaringClass[] = "getDeclaringClass"; const char kGetModifiers[] = "getModifiers"; const char kGetParameterTypes[] = "getParameterTypes"; const char kGetReturnType[] = "getReturnType"; const char kIntegerReturningBoolean[] = "(I)Z"; const char kIsStatic[] = "isStatic"; const char kJavaLangClass[] = "java/lang/Class"; const char kJavaLangReflectMethod[] = "java/lang/reflect/Method"; const char kJavaLangReflectModifier[] = "java/lang/reflect/Modifier"; const char kReturningInteger[] = "()I"; const char kReturningJavaLangClass[] = "()Ljava/lang/Class;"; const char kReturningJavaLangClassArray[] = "()[Ljava/lang/Class;"; const char kReturningJavaLangString[] = "()Ljava/lang/String;"; torne commented that "I think the ideal solution here would be to use the JNI generator to generate stubs for the reflection classes we use here instead of hardcoding a bunch of calls to CallObjectMethod and friends.." It's a bit outside my track so I'm writing this bug report to track it.
,
Feb 15 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1257be13887e153bc570c0a3de05e743c651c8a2 commit 1257be13887e153bc570c0a3de05e743c651c8a2 Author: Torne (Richard Coles) <torne@google.com> Date: Thu Feb 15 19:23:34 2018 Fix JNI generator handling of generics. Instead of hardcoding startswith('Class') to try to deal with generics (which doesn't work when disassembling a class file as the type is java.lang.Class there), just use the existing _StripGenerics function to remove generics from types before converting them to C types. Bug: 787557 Change-Id: Iae2b760bcb8f1407e1e13ba9d692cd463a5a3ad2 Reviewed-on: https://chromium-review.googlesource.com/922501 Reviewed-by: agrieve <agrieve@chromium.org> Commit-Queue: Richard Coles <torne@chromium.org> Cr-Commit-Position: refs/heads/master@{#537096} [modify] https://crrev.com/1257be13887e153bc570c0a3de05e743c651c8a2/base/android/jni_generator/jni_generator.py [modify] https://crrev.com/1257be13887e153bc570c0a3de05e743c651c8a2/base/android/jni_generator/jni_generator_tests.py [modify] https://crrev.com/1257be13887e153bc570c0a3de05e743c651c8a2/base/android/jni_generator/testFromJavaPGenerics.golden
,
Feb 16 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8571d251b4312c9a8303f12155f2626555b5560b commit 8571d251b4312c9a8303f12155f2626555b5560b Author: Torne (Richard Coles) <torne@google.com> Date: Fri Feb 16 20:17:33 2018 Convert Java bridge to use JNI generator. Have the JNI generator generate stubs for the classes used to do reflection in the Java bridge, and call those instead of using the Java bridge's own jni_helper.h functions that reimplement much of the same logic in a different way. Bug: 787557 Change-Id: I72930937ca30abb1139d025ed359f77ea6986cc2 Reviewed-on: https://chromium-review.googlesource.com/922803 Reviewed-by: agrieve <agrieve@chromium.org> Reviewed-by: Tao Bai <michaelbai@chromium.org> Reviewed-by: Bo <boliu@chromium.org> Commit-Queue: Richard Coles <torne@chromium.org> Cr-Commit-Position: refs/heads/master@{#537397} [modify] https://crrev.com/8571d251b4312c9a8303f12155f2626555b5560b/content/browser/BUILD.gn [modify] https://crrev.com/8571d251b4312c9a8303f12155f2626555b5560b/content/browser/android/java/gin_java_bound_object.cc [modify] https://crrev.com/8571d251b4312c9a8303f12155f2626555b5560b/content/browser/android/java/gin_java_bound_object.h [modify] https://crrev.com/8571d251b4312c9a8303f12155f2626555b5560b/content/browser/android/java/gin_java_bridge_dispatcher_host.cc [modify] https://crrev.com/8571d251b4312c9a8303f12155f2626555b5560b/content/browser/android/java/gin_java_method_invocation_helper.cc [modify] https://crrev.com/8571d251b4312c9a8303f12155f2626555b5560b/content/browser/android/java/gin_java_method_invocation_helper_unittest.cc [modify] https://crrev.com/8571d251b4312c9a8303f12155f2626555b5560b/content/browser/android/java/java_method.cc [delete] https://crrev.com/e983a4e5bb74dda566a30dc325a4380b56cbd8b7/content/browser/android/java/jni_helper.cc [delete] https://crrev.com/e983a4e5bb74dda566a30dc325a4380b56cbd8b7/content/browser/android/java/jni_helper.h [delete] https://crrev.com/e983a4e5bb74dda566a30dc325a4380b56cbd8b7/content/browser/android/java/jni_helper_unittest.cc [add] https://crrev.com/8571d251b4312c9a8303f12155f2626555b5560b/content/browser/android/java/jni_reflect.cc [add] https://crrev.com/8571d251b4312c9a8303f12155f2626555b5560b/content/browser/android/java/jni_reflect.h [modify] https://crrev.com/8571d251b4312c9a8303f12155f2626555b5560b/content/test/BUILD.gn
,
Feb 16 2018
The code has been simplified by using the JNI generator instead and these constants are all gone, so should cease being an issue for jumbo builds. If you see any other cases where there are constants with JNI-related strings in them then please let me know as this generally shouldn't be being done at all - thanks for pointing me at this one :) |
||
►
Sign in to add a comment |
||
Comment 1 by tedc...@chromium.org
, Feb 7 2018