62389 2011-06-09 11:57:54 -0700 [V8] JavaMethodJobject::signature() incorrectly encodes arguments of type JavaTypeString 2011-07-20 08:58:46 -0700 1 1 1 Unclassified WebKit WebCore Misc. 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 steveblock steveblock andersca antonm dglazkov eric rniwa steveblock tonyg oldest_to_newest 418162 0 steveblock 2011-06-09 11:57:54 -0700 On V8, JavaTypeString is now distinct from JavaTypeObject, but both need to represented by a JNI class description when forming a JNI method signature. 418177 1 steveblock 2011-06-09 12:15:02 -0700 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. 418631 2 96728 steveblock 2011-06-10 04:01:32 -0700 Created attachment 96728 Patch 439786 3 96728 tonyg 2011-07-20 07:01:02 -0700 Comment on attachment 96728 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. 439798 4 101469 steveblock 2011-07-20 07:40:52 -0700 Created attachment 101469 Patch 439799 5 steveblock 2011-07-20 07:44:13 -0700 Committed r91362: <http://trac.webkit.org/changeset/91362> 439823 6 rniwa 2011-07-20 08:41:08 -0700 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 439828 7 steveblock 2011-07-20 08:49:47 -0700 I think that actually the break was due to http://trac.webkit.org/changeset/91355 439833 8 rniwa 2011-07-20 08:58:46 -0700 (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 :( 96728 2011-06-10 04:01:32 -0700 2011-07-20 07:40:46 -0700 Patch bug-62389-20110610120130.patch text/plain 2144 steveblock SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDg4NTM1KQorKysgU291cmNlL1dlYkNvcmUvQ2hhbmdl TG9nCSh3b3JraW5nIGNvcHkpCkBAIC0xLDMgKzEsMTUgQEAKKzIwMTEtMDYtMTAgIFN0ZXZlIEJs b2NrICA8c3RldmVibG9ja0Bnb29nbGUuY29tPgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9E WSAoT09QUyEpLgorCisgICAgICAgIFtWOF0gSmF2YU1ldGhvZEpvYmplY3Q6OnNpZ25hdHVyZSgp IGluY29ycmVjdGx5IGVuY29kZXMgYXJndW1lbnRzIG9mIHR5cGUgSmF2YVR5cGVTdHJpbmcKKyAg ICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTYyMzg5CisKKyAg ICAgICAgQ292ZXJlZCBieSBleGlzdGluZyB0ZXN0cy4KKworICAgICAgICAqIGJyaWRnZS9qbmkv SmF2YU1ldGhvZEpvYmplY3QuY3BwOgorICAgICAgICAoSmF2YU1ldGhvZEpvYmplY3Q6OnNpZ25h dHVyZSk6CisKIDIwMTEtMDYtMTEtMDYtMDkgIElseWEgVGlraG9ub3Zza3kgIDxsb2lzbG9AY2hy b21pdW0ub3JnPgogCiAgICAgICAgIFJldmlld2VkIGJ5IFl1cnkgU2VtaWtoYXRza3kuCkluZGV4 OiBTb3VyY2UvV2ViQ29yZS9icmlkZ2Uvam5pL0phdmFNZXRob2RKb2JqZWN0LmNwcAo9PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09Ci0tLSBTb3VyY2UvV2ViQ29yZS9icmlkZ2Uvam5pL0phdmFNZXRob2RKb2JqZWN0LmNwcAko cmV2aXNpb24gODg1MzMpCisrKyBTb3VyY2UvV2ViQ29yZS9icmlkZ2Uvam5pL0phdmFNZXRob2RK b2JqZWN0LmNwcAkod29ya2luZyBjb3B5KQpAQCAtMTMwLDcgKzEzMCwxMSBAQCBjb25zdCBjaGFy KiBKYXZhTWV0aG9kSm9iamVjdDo6c2lnbmF0dXJlCiAgICAgICAgICAgICAgICAgYXBwZW5kQ2xh c3NOYW1lKHNpZ25hdHVyZUJ1aWxkZXIsIGphdmFDbGFzc05hbWUuZGF0YSgpKTsKICAgICAgICAg ICAgIGVsc2UgewogICAgICAgICAgICAgICAgIHNpZ25hdHVyZUJ1aWxkZXIuYXBwZW5kKHNpZ25h dHVyZUZyb21KYXZhVHlwZSh0eXBlKSk7Ci0gICAgICAgICAgICAgICAgaWYgKHR5cGUgPT0gSmF2 YVR5cGVPYmplY3QpIHsKKyAgICAgICAgICAgICAgICBib29sIHJlcXVpcmVzQ2xhc3NOYW1lID0g KHR5cGUgPT0gSmF2YVR5cGVPYmplY3QpOworI2lmIFVTRShWOCkKKyAgICAgICAgICAgICAgICBy ZXF1aXJlc0NsYXNzTmFtZSB8PSAodHlwZSA9PSBKYXZhVHlwZVN0cmluZyk7CisjZW5kaWYKKyAg ICAgICAgICAgICAgICBpZiAocmVxdWlyZXNDbGFzc05hbWUpIHsKICAgICAgICAgICAgICAgICAg ICAgYXBwZW5kQ2xhc3NOYW1lKHNpZ25hdHVyZUJ1aWxkZXIsIGphdmFDbGFzc05hbWUuZGF0YSgp KTsKICAgICAgICAgICAgICAgICAgICAgc2lnbmF0dXJlQnVpbGRlci5hcHBlbmQoJzsnKTsKICAg ICAgICAgICAgICAgICB9CkBAIC0xNDMsNyArMTQ3LDExIEBAIGNvbnN0IGNoYXIqIEphdmFNZXRo b2RKb2JqZWN0OjpzaWduYXR1cmUKICAgICAgICAgICAgIGFwcGVuZENsYXNzTmFtZShzaWduYXR1 cmVCdWlsZGVyLCByZXR1cm5UeXBlKTsKICAgICAgICAgZWxzZSB7CiAgICAgICAgICAgICBzaWdu YXR1cmVCdWlsZGVyLmFwcGVuZChzaWduYXR1cmVGcm9tSmF2YVR5cGUobV9yZXR1cm5UeXBlKSk7 Ci0gICAgICAgICAgICBpZiAobV9yZXR1cm5UeXBlID09IEphdmFUeXBlT2JqZWN0KSB7CisgICAg ICAgICAgICBib29sIHJlcXVpcmVzQ2xhc3NOYW1lID0gKG1fcmV0dXJuVHlwZSA9PSBKYXZhVHlw ZU9iamVjdCk7CisjaWYgVVNFKFY4KQorICAgICAgICAgICAgcmVxdWlyZXNDbGFzc05hbWUgfD0g KG1fcmV0dXJuVHlwZSA9PSBKYXZhVHlwZVN0cmluZyk7CisjZW5kaWYKKyAgICAgICAgICAgIGlm IChyZXF1aXJlc0NsYXNzTmFtZSkgewogICAgICAgICAgICAgICAgIGFwcGVuZENsYXNzTmFtZShz aWduYXR1cmVCdWlsZGVyLCByZXR1cm5UeXBlKTsKICAgICAgICAgICAgICAgICBzaWduYXR1cmVC dWlsZGVyLmFwcGVuZCgnOycpOwogICAgICAgICAgICAgfQo= 101469 2011-07-20 07:40:52 -0700 2011-07-20 07:42:05 -0700 Patch bug-62389-20110720154051.patch text/plain 2178 steveblock U3VidmVyc2lvbiBSZXZpc2lvbjogOTEzNjEKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL0No YW5nZUxvZyBiL1NvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwppbmRleCBlZDgwNmQyNTJmNjQ3Mzli YTRjOGI3NjIwZGEzMjNmNzViZjFlZDhlLi40MzBlMDk4ODFjYzM4YzNiMzhlNWJkMzYyNDg3NDg0 Y2JlODRlZGE0IDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKKysrIGIvU291 cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTUgQEAKKzIwMTEtMDctMjAgIFN0ZXZl IEJsb2NrICA8c3RldmVibG9ja0Bnb29nbGUuY29tPgorCisgICAgICAgIFtWOF0gSmF2YU1ldGhv ZEpvYmplY3Q6OnNpZ25hdHVyZSgpIGluY29ycmVjdGx5IGVuY29kZXMgYXJndW1lbnRzIG9mIHR5 cGUgSmF2YVR5cGVTdHJpbmcKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19i dWcuY2dpP2lkPTYyMzg5CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisK KyAgICAgICAgQ292ZXJlZCBieSBleGlzdGluZyB0ZXN0cy4KKworICAgICAgICAqIGJyaWRnZS9q bmkvSmF2YU1ldGhvZEpvYmplY3QuY3BwOgorICAgICAgICAoSmF2YU1ldGhvZEpvYmplY3Q6OnNp Z25hdHVyZSk6CisKIDIwMTEtMDctMjAgIFl1cnkgU2VtaWtoYXRza3kgIDx5dXJ5c0BjaHJvbWl1 bS5vcmc+CiAKICAgICAgICAgV2ViIEluc3BlY3Rvcjogc2lkZWJhciB3aWR0aCBzb21ldGltZXMg aXMgbm90IHJlc3RvcmVkCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9icmlkZ2Uvam5pL0ph dmFNZXRob2RKb2JqZWN0LmNwcCBiL1NvdXJjZS9XZWJDb3JlL2JyaWRnZS9qbmkvSmF2YU1ldGhv ZEpvYmplY3QuY3BwCmluZGV4IDJmMjgwMmNhMGMyNDRmMDk5NTQzMzhjMjA2MTBkNTExMjUyMTZl NjUuLjczY2FlNTQ4MjliZWE2YTE0OGJmYzkwZjgwN2FhNWQ0ZDA4YjM1NzggMTAwNjQ0Ci0tLSBh L1NvdXJjZS9XZWJDb3JlL2JyaWRnZS9qbmkvSmF2YU1ldGhvZEpvYmplY3QuY3BwCisrKyBiL1Nv dXJjZS9XZWJDb3JlL2JyaWRnZS9qbmkvSmF2YU1ldGhvZEpvYmplY3QuY3BwCkBAIC0xMzAsNyAr MTMwLDExIEBAIGNvbnN0IGNoYXIqIEphdmFNZXRob2RKb2JqZWN0OjpzaWduYXR1cmUoKSBjb25z dAogICAgICAgICAgICAgICAgIGFwcGVuZENsYXNzTmFtZShzaWduYXR1cmVCdWlsZGVyLCBqYXZh Q2xhc3NOYW1lLmRhdGEoKSk7CiAgICAgICAgICAgICBlbHNlIHsKICAgICAgICAgICAgICAgICBz aWduYXR1cmVCdWlsZGVyLmFwcGVuZChzaWduYXR1cmVGcm9tSmF2YVR5cGUodHlwZSkpOwotICAg ICAgICAgICAgICAgIGlmICh0eXBlID09IEphdmFUeXBlT2JqZWN0KSB7CisgICAgICAgICAgICAg ICAgaWYgKHR5cGUgPT0gSmF2YVR5cGVPYmplY3QKKyNpZiBVU0UoVjgpCisgICAgICAgICAgICAg ICAgICAgIHx8IHR5cGUgPT0gSmF2YVR5cGVTdHJpbmcKKyNlbmRpZgorICAgICAgICAgICAgICAg ICAgICApIHsKICAgICAgICAgICAgICAgICAgICAgYXBwZW5kQ2xhc3NOYW1lKHNpZ25hdHVyZUJ1 aWxkZXIsIGphdmFDbGFzc05hbWUuZGF0YSgpKTsKICAgICAgICAgICAgICAgICAgICAgc2lnbmF0 dXJlQnVpbGRlci5hcHBlbmQoJzsnKTsKICAgICAgICAgICAgICAgICB9CkBAIC0xNDMsNyArMTQ3 LDExIEBAIGNvbnN0IGNoYXIqIEphdmFNZXRob2RKb2JqZWN0OjpzaWduYXR1cmUoKSBjb25zdAog ICAgICAgICAgICAgYXBwZW5kQ2xhc3NOYW1lKHNpZ25hdHVyZUJ1aWxkZXIsIHJldHVyblR5cGUp OwogICAgICAgICBlbHNlIHsKICAgICAgICAgICAgIHNpZ25hdHVyZUJ1aWxkZXIuYXBwZW5kKHNp Z25hdHVyZUZyb21KYXZhVHlwZShtX3JldHVyblR5cGUpKTsKLSAgICAgICAgICAgIGlmIChtX3Jl dHVyblR5cGUgPT0gSmF2YVR5cGVPYmplY3QpIHsKKyAgICAgICAgICAgIGlmIChtX3JldHVyblR5 cGUgPT0gSmF2YVR5cGVPYmplY3QKKyNpZiBVU0UoVjgpCisgICAgICAgICAgICAgICAgfHwgbV9y ZXR1cm5UeXBlID09IEphdmFUeXBlU3RyaW5nCisjZW5kaWYKKyAgICAgICAgICAgICAgICApIHsK ICAgICAgICAgICAgICAgICBhcHBlbmRDbGFzc05hbWUoc2lnbmF0dXJlQnVpbGRlciwgcmV0dXJu VHlwZSk7CiAgICAgICAgICAgICAgICAgc2lnbmF0dXJlQnVpbGRlci5hcHBlbmQoJzsnKTsKICAg ICAgICAgICAgIH0K