This type is required to allow us to make the JavaInstance API independent of JNI. See 57019.
This bug will add the JavaType class and use it for the conversions to and from JavaNPObject that are used by the V8 Java bridge. Bug 57023 tracks using JavaType in the corresponding conversions for JSC.
Created attachment 86787 [details] Patch
Created attachment 86916 [details] Patch
View in context: https://bugs.webkit.org/attachment.cgi?id=86916&action=review > Source/WebCore/bridge/jni/JavaType.h:62 > + // special JavaTypeString which will be converted to a Java String when we I think you could make this comment a bit clearer by saying something like: "We cannot make the assumption that, at conversion time, the type to convert to is a JNI type. This is because there may be some mechanism other than JNI for reaching back to Java. Instead, we use..." > Source/WebCore/bridge/jni/v8/JNIUtilityPrivate.cpp:68 > + if (type == NPVariantType_String) should this be on the line above, next to the else? > Source/WebCore/bridge/jni/v8/JNIUtilityPrivate.cpp:298 > + result.l = getJNIEnv()->NewString(value.m_stringValue.characters(), value.m_stringValue.length()); Add a comment saying this is a local ref so it'll be released once the the executions returns back to Java from native? Also mention that probably this may leak if the call does not originate from java (e.g. from a worker, which may run its own native message loop).
> I think you could make this comment a bit clearer by saying something like: Done > should this be on the line above, next to the else? No, there's an '#else' in between > Add a comment saying this is a local ref so it'll be released once the the executions returns back to Java from native? Done
Created attachment 86935 [details] Patch
LGTM
Comment on attachment 86935 [details] Patch rubber stamp since Andrei said it LGTM and style wise it seems fine (There's a good reason for the #ifs)
The commit-queue encountered the following flaky tests while processing attachment 86935 [details]: fast/workers/storage/use-same-database-in-page-and-workers.html bug 50995 (author: dumi@chromium.org) The commit-queue is continuing to process your patch.
Comment on attachment 86935 [details] Patch Clearing flags on attachment: 86935 Committed r82194: <http://trac.webkit.org/changeset/82194>
All reviewed patches have been landed. Closing bug.