99977 2012-10-22 04:28:18 -0700 [WK2] Regression(r131990): plugins/npruntime/remove-property.html started failing 2012-10-22 12:33:56 -0700 1 1 1 Unclassified WebKit WebKit2 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 99865 0 cdumez andersca andersca ap kling ossy zan oldest_to_newest 747410 0 cdumez 2012-10-22 04:28:18 -0700 plugins/npruntime/remove-property.html started failing consistently after r131990 so it looks like a regression. I confirmed this on Mac, GTK and EFL ports. The diff looks like: --- /Volumes/Data/slave/mountainlion-debug-tests-wk2/build/layout-test-results/plugins/npruntime/remove-property-expected.txt +++ /Volumes/Data/slave/mountainlion-debug-tests-wk2/build/layout-test-results/plugins/npruntime/remove-property-actual.txt @@ -7,5 +7,5 @@ PASS 'property' in obj is true PASS 'property' in obj is false PASS array[1] is 2 -PASS array[1] is undefined +FAIL array[1] should be undefined (of type undefined). Was 2 (of type number). 747418 1 cdumez 2012-10-22 04:44:05 -0700 Skipped for EFL-WK2 in r132064. 747759 2 169956 andersca 2012-10-22 12:15:30 -0700 Created attachment 169956 Patch 747767 3 169956 ap 2012-10-22 12:19:55 -0700 Comment on attachment 169956 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=169956&action=review > Source/WebKit2/ChangeLog:9 > + a temporary to be created which the number was then decoded into. Is there a way to prevent this from happening in the future? 747768 4 andersca 2012-10-22 12:21:07 -0700 (In reply to comment #3) > (From update of attachment 169956 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=169956&action=review > > > Source/WebKit2/ChangeLog:9 > > + a temporary to be created which the number was then decoded into. > > Is there a way to prevent this from happening in the future? Yeah, I'm going to remove the overload. 747772 5 169956 darin 2012-10-22 12:21:21 -0700 Comment on attachment 169956 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=169956&action=review > Source/WebKit2/Shared/Plugins/NPIdentifierData.cpp:76 > - encoder->encode(static_cast<int32_t>(m_number)); > + encoder->encode(m_number); Why are all the other encode casts still a good idea and this one not? > Source/WebKit2/Shared/Plugins/NPIdentifierData.cpp:87 > - return decoder->decode(static_cast<int32_t>(result.m_number)); > + return decoder->decode(result.m_number); This cast is clearly wrong, but can’t we create an explicit temporary of the right type, instead? Was the whole idea of using these type casts a flawed one? Why didn’t the compiler give an error, since we were passing a temporary as a non-const reference argument? Is that a compiler bug? 747776 6 andersca 2012-10-22 12:24:02 -0700 Committed r132120: <http://trac.webkit.org/changeset/132120> 747777 7 andersca 2012-10-22 12:26:09 -0700 (In reply to comment #5) > (From update of attachment 169956 [details]) > Why didn’t the compiler give an error, since we were passing a temporary as a non-const reference argument? Is that a compiler bug? The ArgumentDecoder overload takes a const reference and casts away the constness. I'm going to get rid of that overload. 747779 8 andersca 2012-10-22 12:33:56 -0700 (In reply to comment #5) > (From update of attachment 169956 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=169956&action=review > > > Source/WebKit2/Shared/Plugins/NPIdentifierData.cpp:76 > > - encoder->encode(static_cast<int32_t>(m_number)); > > + encoder->encode(m_number); > > Why are all the other encode casts still a good idea and this one not? > I think that the ones that cast between int32_t and int are completely unnecessary. 169956 2012-10-22 12:15:30 -0700 2012-10-22 12:21:21 -0700 Patch bug-99977-20121022121408.patch text/plain 3632 andersca U3VidmVyc2lvbiBSZXZpc2lvbjogMTMyMTA2CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0Mi9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViS2l0Mi9DaGFuZ2VMb2cKaW5kZXggYzIwNDM4ZGQwM2MzN2Rm NmFkYWJjOTRlMjQyODlhMWQ1OTk4M2JmMi4uNjRjMjAwZmQ4OGY5MGEyNDM2NGZjZmY3NjU1Mjc1 N2JhZGMxYmFhMSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYktpdDIvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJLaXQyL0NoYW5nZUxvZwpAQCAtMSw1ICsxLDE5IEBACiAyMDEyLTEwLTIyICBBbmRl cnMgQ2FybHNzb24gIDxhbmRlcnNjYUBhcHBsZS5jb20+CiAKKyAgICAgICAgW1dLMl0gUmVncmVz c2lvbihyMTMxOTkwKTogcGx1Z2lucy9ucHJ1bnRpbWUvcmVtb3ZlLXByb3BlcnR5Lmh0bWwgc3Rh cnRlZCBmYWlsaW5nCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNn aT9pZD05OTk3NworCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAg ICAgIFJlbW92ZSBpbnQzMl90IGNhc3RzIC0gdGhlIEFyZ3VtZW50RGVjb2Rlcjo6ZGVjb2RlIG92 ZXJsb2FkIHRoYXQgdGFrZXMgYSBjb25zdCBpbnQgd291bGQgY2F1c2UKKyAgICAgICAgYSB0ZW1w b3JhcnkgdG8gYmUgY3JlYXRlZCB3aGljaCB0aGUgbnVtYmVyIHdhcyB0aGVuIGRlY29kZWQgaW50 by4KKworICAgICAgICAqIFNoYXJlZC9QbHVnaW5zL05QSWRlbnRpZmllckRhdGEuY3BwOgorICAg ICAgICAoV2ViS2l0OjpOUElkZW50aWZpZXJEYXRhOjplbmNvZGUpOgorICAgICAgICAoV2ViS2l0 OjpOUElkZW50aWZpZXJEYXRhOjpkZWNvZGUpOgorCisyMDEyLTEwLTIyICBBbmRlcnMgQ2FybHNz b24gIDxhbmRlcnNjYUBhcHBsZS5jb20+CisKICAgICAgICAgRG9uJ3QgdXNlIENTdHJpbmdzIGZv ciBtZXNzYWdlIHJlY2VpdmVyIG5hbWVzIGFuZCBtZXNzYWdlIG5hbWVzCiAgICAgICAgIGh0dHBz Oi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD05OTg1MwogCmRpZmYgLS1naXQgYS9T b3VyY2UvV2ViS2l0Mi9TaGFyZWQvUGx1Z2lucy9OUElkZW50aWZpZXJEYXRhLmNwcCBiL1NvdXJj ZS9XZWJLaXQyL1NoYXJlZC9QbHVnaW5zL05QSWRlbnRpZmllckRhdGEuY3BwCmluZGV4IDc0NzE3 MjUwNGE1ZjBjN2Y1NjUxMDVlZTI3YThiYTVhZjdjN2FlNDguLmZmMmJlZjE0MDAzYjkwNWE3ZjVh NDY0ZTcyNzhiMzJmZWE3ZmEwYzYgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJLaXQyL1NoYXJlZC9Q bHVnaW5zL05QSWRlbnRpZmllckRhdGEuY3BwCisrKyBiL1NvdXJjZS9XZWJLaXQyL1NoYXJlZC9Q bHVnaW5zL05QSWRlbnRpZmllckRhdGEuY3BwCkBAIC03Myw3ICs3Myw3IEBAIHZvaWQgTlBJZGVu dGlmaWVyRGF0YTo6ZW5jb2RlKENvcmVJUEM6OkFyZ3VtZW50RW5jb2RlciogZW5jb2RlcikgY29u c3QKICAgICBpZiAobV9pc1N0cmluZykKICAgICAgICAgZW5jb2Rlci0+ZW5jb2RlKG1fc3RyaW5n KTsKICAgICBlbHNlCi0gICAgICAgIGVuY29kZXItPmVuY29kZShzdGF0aWNfY2FzdDxpbnQzMl90 PihtX251bWJlcikpOworICAgICAgICBlbmNvZGVyLT5lbmNvZGUobV9udW1iZXIpOwogfQogCiBi b29sIE5QSWRlbnRpZmllckRhdGE6OmRlY29kZShDb3JlSVBDOjpBcmd1bWVudERlY29kZXIqIGRl Y29kZXIsIE5QSWRlbnRpZmllckRhdGEmIHJlc3VsdCkKQEAgLTg0LDcgKzg0LDcgQEAgYm9vbCBO UElkZW50aWZpZXJEYXRhOjpkZWNvZGUoQ29yZUlQQzo6QXJndW1lbnREZWNvZGVyKiBkZWNvZGVy LCBOUElkZW50aWZpZXJEYXQKICAgICBpZiAocmVzdWx0Lm1faXNTdHJpbmcpCiAgICAgICAgIHJl dHVybiBkZWNvZGVyLT5kZWNvZGUocmVzdWx0Lm1fc3RyaW5nKTsKIAotICAgIHJldHVybiBkZWNv ZGVyLT5kZWNvZGUoc3RhdGljX2Nhc3Q8aW50MzJfdD4ocmVzdWx0Lm1fbnVtYmVyKSk7CisgICAg cmV0dXJuIGRlY29kZXItPmRlY29kZShyZXN1bHQubV9udW1iZXIpOwogfQogCiB9IC8vIG5hbWVz cGFjZSBXZWJLaXQKZGlmZiAtLWdpdCBhL0xheW91dFRlc3RzL0NoYW5nZUxvZyBiL0xheW91dFRl c3RzL0NoYW5nZUxvZwppbmRleCA4Yzk3YjE2MWIyZmE3ZmQ3YTMzNTU0MThhYTllODI0NjM4YzA3 NzgzLi4xNWFkNjJiMzRmZDZlZDA2ZjNlMWYxYzVjMzFmOGUyODA1YTgwOWFiIDEwMDY0NAotLS0g YS9MYXlvdXRUZXN0cy9DaGFuZ2VMb2cKKysrIGIvTGF5b3V0VGVzdHMvQ2hhbmdlTG9nCkBAIC0x LDMgKzEsMTQgQEAKKzIwMTItMTAtMjIgIEFuZGVycyBDYXJsc3NvbiAgPGFuZGVyc2NhQGFwcGxl LmNvbT4KKworICAgICAgICBbV0syXSBSZWdyZXNzaW9uKHIxMzE5OTApOiBwbHVnaW5zL25wcnVu dGltZS9yZW1vdmUtcHJvcGVydHkuaHRtbCBzdGFydGVkIGZhaWxpbmcKKyAgICAgICAgaHR0cHM6 Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTk5OTc3CisKKyAgICAgICAgUmV2aWV3 ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgUmVtb3ZlIG5vdyBwYXNzaW5nIHRlc3Qu CisKKyAgICAgICAgKiBwbGF0Zm9ybS9lZmwtd2syL1Rlc3RFeHBlY3RhdGlvbnM6CisKIDIwMTIt MTAtMjIgIEVtaWwgQSBFa2x1bmQgIDxlYWVAY2hyb21pdW0ub3JnPgogCiAgICAgICAgIFVucmV2 aWV3ZWQgZ2FyZGVuaW5nLCBtYXJraW5nIGZhc3Qvc3ViLXBpeGVsL2ZpbGUtdXBsb2FkLWNvbnRy b2wtYXQtZnJhY3Rpb25hbC1vZmZzZXQuaHRtbCBhcyBmYWlsaW5nIG9uIExJT04gZm9yIG5vdy4K ZGlmZiAtLWdpdCBhL0xheW91dFRlc3RzL3BsYXRmb3JtL2VmbC13azIvVGVzdEV4cGVjdGF0aW9u cyBiL0xheW91dFRlc3RzL3BsYXRmb3JtL2VmbC13azIvVGVzdEV4cGVjdGF0aW9ucwppbmRleCA2 MmNhNDYxNjM3ZjhjNGQ2Mzc5OWVlODg5MDhiNTM2ZmU5OTZmYzIwLi4wNjA3NzI3NTQ3OTZiMDEx MmRjZGU4NmRlNjVkZWViN2I0MTM2MzlhIDEwMDY0NAotLS0gYS9MYXlvdXRUZXN0cy9wbGF0Zm9y bS9lZmwtd2syL1Rlc3RFeHBlY3RhdGlvbnMKKysrIGIvTGF5b3V0VGVzdHMvcGxhdGZvcm0vZWZs LXdrMi9UZXN0RXhwZWN0YXRpb25zCkBAIC0yNDcsOSArMjQ3LDYgQEAgd2Via2l0Lm9yZy9iLzk4 NzQxIHBsdWdpbnMvcmVmY291bnQtbGVha3MuaHRtbCBbIEZhaWx1cmUgXQogIyBOUFBfU2V0V2lu ZG93IGlzIG5vdCBjYWxsZWQgZHVyaW5nIHBsdWdpbiBkZXN0cnVjdGlvbiBvbiBYMTEuCiB3ZWJr aXQub3JnL2IvNjMyNjEgcGx1Z2lucy9ucHAtc2V0LXdpbmRvdy1jYWxsZWQtZHVyaW5nLWRlc3Ry dWN0aW9uLmh0bWwgWyBGYWlsdXJlIF0KIAotIyBUZXN0IHN0YXJ0ZWQgZmFpbGluZyBvbiBhbGwg V0syIHBvcnRzIGFmdGVyIHIxMzE5OTAuCi13ZWJraXQub3JnL2IvOTk5NzcgcGx1Z2lucy9ucHJ1 bnRpbWUvcmVtb3ZlLXByb3BlcnR5Lmh0bWwgWyBGYWlsdXJlIF0KLQogIyBEZWxldGUgcGx1Z2lu IGZhaWx1cmVzLgogd2Via2l0Lm9yZy9iLzk4NzQwIHBsdWdpbnMvbnBydW50aW1lL2RlbGV0ZS1w bHVnaW4td2l0aGluLWhhc1Byb3BlcnR5LXJldHVybi1mYWxzZS5odG1sIFsgRmFpbHVyZSBdCiB3 ZWJraXQub3JnL2IvOTg3NDAgcGx1Z2lucy9ucHJ1bnRpbWUvZGVsZXRlLXBsdWdpbi13aXRoaW4t aGFzUHJvcGVydHktcmV0dXJuLXRydWUuaHRtbCBbIEZhaWx1cmUgXQo=