98417 2012-10-04 09:16:03 -0700 After r130344, OpaqueJSString() creates an empty string which should be a null string 2012-10-08 11:46:13 -0700 1 1 1 Unclassified WebKit JavaScriptCore 528+ (Nightly build) All All RESOLVED FIXED P2 Normal --- 98300 1 msaboff msaboff cdumez ggaren webkit.review.bot oldest_to_newest 734746 0 msaboff 2012-10-04 09:16:03 -0700 The change for https://bugs.webkit.org/show_bug.cgi?id=98300 in changeset r130344: <http://trac.webkit.org/changeset/130344>, changes the default OpaqueJSString constructor to have an empty string. Before the change this was a null string. The new default constructor initializes m_string with m_string = emptyString(); this should be eliminated. 734783 1 167118 msaboff 2012-10-04 10:09:37 -0700 Created attachment 167118 Patch 734898 2 167118 webkit.review.bot 2012-10-04 11:49:32 -0700 Comment on attachment 167118 Patch Clearing flags on attachment: 167118 Committed r130413: <http://trac.webkit.org/changeset/130413> 734899 3 webkit.review.bot 2012-10-04 11:49:35 -0700 All reviewed patches have been landed. Closing bug. 735149 4 ap 2012-10-04 17:13:45 -0700 Just realized that this should have had an API test. 735185 5 msaboff 2012-10-04 17:41:50 -0700 There is a second case where we turn a null string into an empty string. That is when OpaqueJSString(String string) is called and string is null. If we fail the if (!string.isNull()) check, that is the string is null, we should return 0. 736384 6 ggaren 2012-10-07 11:32:41 -0700 > Just realized that this should have had an API test. Yes, please add an API test for both identified cases. 736765 7 167554 msaboff 2012-10-08 10:19:10 -0700 Created attachment 167554 Patch Working on API tests, although there isn't a direct way to test the API. Looking for an indirect method for both checks. 736863 8 msaboff 2012-10-08 11:46:13 -0700 Committed r130664: <http://trac.webkit.org/changeset/130664> 167118 2012-10-04 10:09:37 -0700 2012-10-04 11:49:32 -0700 Patch 98417.patch text/plain 1259 msaboff SW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKPT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291 cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkocmV2aXNpb24gMTMwNDA1KQorKysgU291cmNl L0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE3IEBA CisyMDEyLTEwLTA0ICBNaWNoYWVsIFNhYm9mZiAgPG1zYWJvZmZAYXBwbGUuY29tPgorCisgICAg ICAgIEFmdGVyIHIxMzAzNDQsIE9wYXF1ZUpTU3RyaW5nKCkgY3JlYXRlcyBhIGVtcHR5IHN0cmlu ZyB3aGljaCBzaG91bGQgYmUgYSBudWxsIHN0cmluZworICAgICAgICBodHRwczovL2J1Z3Mud2Vi a2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9OTg0MTcKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JP RFkgKE9PUFMhKS4KKworICAgICAgICBSZW1vdmVkIHRoZSBzZXR0aW5nIG9mIGVuY2xvc2VkIHN0 cmluZyB0byBhbiBlbXB0eSBzdHJpbmcgZnJvbSBkZWZhdWx0IGNvbnN0cnVjdG9yLgorICAgICAg ICBCZWZvcmUgY2hhbmdlc2V0IHIxMzAzNDQsIHRoZSBzZW1hbnRpYyB3YXMgdGhlIGRlZmF1bHQg Y29uc3RydWN0b3IgcHJvZHVjZWQgYSBudWxsCisgICAgICAgIHN0cmluZy4KKworICAgICAgICAq IEFQSS9PcGFxdWVKU1N0cmluZy5oOgorICAgICAgICAoT3BhcXVlSlNTdHJpbmc6Ok9wYXF1ZUpT U3RyaW5nKToKKwogMjAxMi0xMC0wNCAgQ3NhYmEgT3N6dHJvZ29uw6FjICA8b3NzeUB3ZWJraXQu b3JnPgogCiAgICAgICAgIFtRdF0gQWRkIG1pc3NpbmcgTExJbnQgZGVwZW5kZW5jaWVzIHRvIHRo ZSBidWlsZCBzeXN0ZW0KSW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9BUEkvT3BhcXVlSlNT dHJpbmcuaAo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvSmF2YVNjcmlwdENvcmUvQVBJL09wYXF1ZUpT U3RyaW5nLmgJKHJldmlzaW9uIDEzMDM5NSkKKysrIFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9BUEkv T3BhcXVlSlNTdHJpbmcuaAkod29ya2luZyBjb3B5KQpAQCAtNTksNyArNTksNiBAQCBwcml2YXRl OgogCiAgICAgT3BhcXVlSlNTdHJpbmcoKQogICAgIHsKLSAgICAgICAgbV9zdHJpbmcgPSBlbXB0 eVN0cmluZygpOwogICAgIH0KIAogICAgIE9wYXF1ZUpTU3RyaW5nKGNvbnN0IFN0cmluZyYgc3Ry aW5nKQo= 167554 2012-10-08 10:19:10 -0700 2012-10-08 10:30:25 -0700 Patch 98417-2.patch text/plain 1315 msaboff SW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKPT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291 cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkocmV2aXNpb24gMTMwNjQ0KQorKysgU291cmNl L0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE1IEBA CisyMDEyLTEwLTA4ICBNaWNoYWVsIFNhYm9mZiAgPG1zYWJvZmZAYXBwbGUuY29tPgorCisgICAg ICAgIEFmdGVyIHIxMzAzNDQsIE9wYXF1ZUpTU3RyaW5nKCkgY3JlYXRlcyBhbiBlbXB0eSBzdHJp bmcgd2hpY2ggc2hvdWxkIGJlIGEgbnVsbCBzdHJpbmcKKyAgICAgICAgaHR0cHM6Ly9idWdzLndl YmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTk4NDE3CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9C T0RZIChPT1BTISkuCisKKyAgICAgICAgQ2hhbmdlZCBjcmVhdGUoKSBvZiBhIG51bGwgc3RyaW5n IHRvIHJldHVybiAwLiBUaGlzIGlzIHRoZSBzYW1lIGJlaGF2aW9yIGFzIGJlZm9yZSByMTMwMzQ0 LgorCisgICAgICAgICogQVBJL09wYXF1ZUpTU3RyaW5nLmNwcDoKKyAgICAgICAgKE9wYXF1ZUpT U3RyaW5nOjpjcmVhdGUpOgorCiAyMDEyLTEwLTA3ICBDYWlvIE1hcmNlbG8gZGUgT2xpdmVpcmEg RmlsaG8gIDxjYWlvLm9saXZlaXJhQG9wZW5ib3NzYS5vcmc+CiAKICAgICAgICAgUmVuYW1lIGZp cnN0L3NlY29uZCB0byBrZXkvdmFsdWUgaW4gSGFzaE1hcCBpdGVyYXRvcnMKSW5kZXg6IFNvdXJj ZS9KYXZhU2NyaXB0Q29yZS9BUEkvT3BhcXVlSlNTdHJpbmcuY3BwCj09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFNv dXJjZS9KYXZhU2NyaXB0Q29yZS9BUEkvT3BhcXVlSlNTdHJpbmcuY3BwCShyZXZpc2lvbiAxMzA0 MjQpCisrKyBTb3VyY2UvSmF2YVNjcmlwdENvcmUvQVBJL09wYXF1ZUpTU3RyaW5nLmNwcAkod29y a2luZyBjb3B5KQpAQCAtMzYsNyArMzYsNyBAQCBQYXNzUmVmUHRyPE9wYXF1ZUpTU3RyaW5nPiBP cGFxdWVKU1N0cmluCiB7CiAgICAgaWYgKCFzdHJpbmcuaXNOdWxsKCkpCiAgICAgICAgIHJldHVy biBhZG9wdFJlZihuZXcgT3BhcXVlSlNTdHJpbmcoc3RyaW5nKSk7Ci0gICAgcmV0dXJuIGFkb3B0 UmVmKG5ldyBPcGFxdWVKU1N0cmluZygpKTsKKyAgICByZXR1cm4gMDsKIH0KIAogSWRlbnRpZmll ciBPcGFxdWVKU1N0cmluZzo6aWRlbnRpZmllcihKU0dsb2JhbERhdGEqIGdsb2JhbERhdGEpIGNv bnN0Cg==