Bug 62389

Summary:

[V8] JavaMethodJobject::signature() incorrectly encodes arguments of type JavaTypeString

Product:

WebKit

Reporter:

Steve Block <steveblock>

Component:

WebCore Misc.

Assignee:

Steve Block <steveblock>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

andersca, antonm, dglazkov, eric, rniwa, steveblock, tonyg

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Patch	none
Patch	tonyg: review+

Steve Block

Reported 2011-06-09 11:57:54 PDT

On V8, JavaTypeString is now distinct from JavaTypeObject, but both need to represented by a JNI class description when forming a JNI method signature.

Attachments
Patch (2.09 KB, patch) 2011-06-10 04:01 PDT, Steve Block	no flags	Details Formatted Diff Diff
Patch (2.13 KB, patch) 2011-07-20 07:40 PDT, Steve Block	tonyg: review+	Details Formatted Diff Diff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Steve Block

Comment 1 2011-06-09 12:15:02 PDT

It looks like the culprit is http://trac.webkit.org/changeset/82194, which introduced JavaTypeString, but didn't update JavaMethodJobject::signature() to account for this.

Steve Block

Comment 2 2011-06-10 04:01:32 PDT

Created attachment 96728 [details] Patch

Tony Gentilcore

Comment 3 2011-07-20 07:01:02 PDT

Comment on attachment 96728 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=96728&action=review > Source/WebCore/ChangeLog:8 > + Covered by existing tests. I don't understand. Then, should this patch be changing expectations to pass an existing failing test? > Source/WebCore/bridge/jni/JavaMethodJobject.cpp:135 > + requiresClassName |= (type == JavaTypeString); Is this code perf sensitive? Unless the compiler is smarter than I think it is, it seems like we lost a shortcut for JavaTypeObject.

Steve Block

Comment 4 2011-07-20 07:40:52 PDT

Created attachment 101469 [details] Patch

Steve Block

Comment 5 2011-07-20 07:44:13 PDT

Committed r91362: <http://trac.webkit.org/changeset/91362>

Ryosuke Niwa

Comment 6 2011-07-20 08:41:08 PDT

This patch broke Mac builds: http://build.webkit.org/builders/SnowLeopard%20Intel%20Debug%20%28Build%29/builds/1387 http://build.webkit.org/builders/Leopard%20Intel%20Debug%20%28Build%29/builds/37433 Speculative build fix landed: http://trac.webkit.org/changeset/91366

Steve Block

Comment 7 2011-07-20 08:49:47 PDT

I think that actually the break was due to http://trac.webkit.org/changeset/91355

Ryosuke Niwa

Comment 8 2011-07-20 08:58:46 PDT

(In reply to comment #7) > I think that actually the break was due to http://trac.webkit.org/changeset/91355 You're right. Commented on the right bug :(

Note You need to log in before you can comment on or make changes to this bug.