106997 2013-01-16 02:36:15 -0800 PropertyCollection test fails on JavaScriptCore 2013-01-18 11:01:09 -0800 1 1 1 Unclassified WebKit WebCore JavaScript 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 d-r arko abarth arko fpizlo ggaren gyuyoung.kim haraken japhet rakuco sam webkit.review.bot zan oldest_to_newest 808204 0 d-r 2013-01-16 02:36:15 -0800 r139756 introduced the new properties-collection-namedgetter-with-invalid-name.html test which seems to change behavior only for V8. We probably need a similar behavior change for JavaScriptCore. This makes fast/dom/MicroData/propertiescollection-crash.html fails as well, at least on EFL. @@ -3,7 +3,7 @@ On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE". -PASS testDiv.properties['bar'] == '[object PropertyNodeList]' is true +FAIL testDiv.properties['bar'] == '[object PropertyNodeList]' should be true. Was false. PASS testDiv.properties['bar'].length is 1 PASS successfullyParsed is true 808294 1 zan 2013-01-16 05:13:13 -0800 Visible on the GTK as well, but (as on EFL) only in debug builds. 809192 2 zan 2013-01-17 01:48:32 -0800 It turns out testDiv.properties['bar'] returns a NodeList object in debug builds. Basically, both release and debug builds follow the same code path, but in release build `toJS(JSC::ExecState*, JSDOMGlobalObject*, PropertyNodeList*)` is used to wrap the WebCore::PropertyNodeList object while `toJS(JSC::ExecState*, JSDOMGlobalObject*, NodeList*)` is used in the debug build. 810109 3 183351 arko 2013-01-17 19:21:24 -0800 Created attachment 183351 Patch 810308 4 183351 haraken 2013-01-18 00:16:42 -0800 Comment on attachment 183351 Patch LGTM 810309 5 haraken 2013-01-18 00:18:06 -0800 (In reply to comment #2) > It turns out testDiv.properties['bar'] returns a NodeList object in debug builds. > Basically, both release and debug builds follow the same code path, but in release build `toJS(JSC::ExecState*, JSDOMGlobalObject*, PropertyNodeList*)` is used to wrap the WebCore::PropertyNodeList object while `toJS(JSC::ExecState*, JSDOMGlobalObject*, NodeList*)` is used in the debug build. BTW, do you know where this difference came from? I'm wondering if we might have a bug around it. 810455 6 zan 2013-01-18 04:05:54 -0800 (In reply to comment #5) > (In reply to comment #2) > > It turns out testDiv.properties['bar'] returns a NodeList object in debug builds. > > Basically, both release and debug builds follow the same code path, but in release build `toJS(JSC::ExecState*, JSDOMGlobalObject*, PropertyNodeList*)` is used to wrap the WebCore::PropertyNodeList object while `toJS(JSC::ExecState*, JSDOMGlobalObject*, NodeList*)` is used in the debug build. > > BTW, do you know where this difference came from? I'm wondering if we might have a bug around it. I have no idea. The PropertyNodeList code was generated and compiled into the library in the debug build as well. The only difference in code path visible from the backtraces is the additional call to the inline toJS function in debug builds, with the method taking the PassRefPtr parameter that contains the PropertyNodeList object. https://trac.webkit.org/browser/trunk/Source/WebCore/bindings/js/JSDOMBinding.h#L334 The patch basically bypasses that. 810641 7 d-r 2013-01-18 08:40:49 -0800 (In reply to comment #3) > Created an attachment (id=183351) [details] > Patch Thanks very much for fixing this Arko. 810775 8 183351 webkit.review.bot 2013-01-18 10:19:13 -0800 Comment on attachment 183351 Patch Rejecting attachment 183351 from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2 cwd: /mnt/git/webkit-commit-queue Last 500 characters of output: sts/ChangeLog Hunk #1 succeeded at 1 with fuzz 3. patching file LayoutTests/platform/efl/TestExpectations Hunk #1 succeeded at 1818 with fuzz 2 (offset 4 lines). patching file LayoutTests/platform/gtk/TestExpectations Hunk #1 FAILED at 1344. 1 out of 1 hunk FAILED -- saving rejects to file LayoutTests/platform/gtk/TestExpectations.rej Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', '--force', '--reviewer', 'Kentaro Hara']" exit_code: 1 cwd: /mnt/git/webkit-commit-queue Full output: http://queues.webkit.org/results/15831697 810842 9 arko 2013-01-18 11:01:09 -0800 Committed r140182: <http://trac.webkit.org/changeset/140182> 183351 2013-01-17 19:21:24 -0800 2013-01-18 10:19:13 -0800 Patch bug-106997-20130117191824.patch text/plain 4594 arko SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDE0MDA4OCkKKysrIFNvdXJjZS9XZWJDb3JlL0NoYW5n ZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDIwIEBACisyMDEzLTAxLTE3ICBBcmtvIFNh aGEgIDxhcmtvQG1vdG9yb2xhLmNvbT4KKworICAgICAgICBQcm9wZXJ0eUNvbGxlY3Rpb24gdGVz dCBmYWlscyBvbiBKYXZhU2NyaXB0Q29yZQorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9y Zy9zaG93X2J1Zy5jZ2k/aWQ9MTA2OTk3CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChP T1BTISkuCisKKyAgICAgICAgTWFkZSBjaGFuZ2VzIGluIENvZGVHZW5lcmF0b3JKUy5wbSBzbyB0 aGF0IGl0IGNhbiB1c2UKKyAgICAgICAgdG9KUyhKU0M6OkV4ZWNTdGF0ZSosIEpTRE9NR2xvYmFs T2JqZWN0KiwgUHJvcGVydHlOb2RlTGlzdCopCisgICAgICAgIHRvIHdyYXAgdGhlIG9iamVjdCBh bmQgcmV0dXJuIFByb3BlcnR5Tm9kZUxpc3QgaW5zdGVhZCBvZiBOb2RlTGlzdC4KKworICAgICAg ICBUZXN0czogZmFzdC9kb20vTWljcm9EYXRhL3Byb3BlcnRpZXNjb2xsZWN0aW9uLWNyYXNoLmh0 bWwKKyAgICAgICAgICAgICAgIGZhc3QvZG9tL01pY3JvRGF0YS9wcm9wZXJ0aWVzLWNvbGxlY3Rp b24tbmFtZWRnZXR0ZXItd2l0aC1pbnZhbGlkLW5hbWUuaHRtbAorCisgICAgICAgICogYmluZGlu Z3Mvc2NyaXB0cy9Db2RlR2VuZXJhdG9ySlMucG06CisgICAgICAgIChHZW5lcmF0ZUltcGxlbWVu dGF0aW9uKToKKwogMjAxMy0wMS0xNyAgVGltIEhvcnRvbiAgPHRpbW90aHlfaG9ydG9uQGFwcGxl LmNvbT4KIAogICAgICAgICBbV0syXSBNaW5pbXVtIGxheW91dCB3aWR0aCBhdXRvLXNpemluZyBz aG91bGQgdXNlIEZyYW1lVmlldzo6ZW5hYmxlQXV0b1NpemVNb2RlIHNvIHRoYXQgaXQgY2FuIHNo cmluayB0aGUgdmlld3BvcnQKSW5kZXg6IFNvdXJjZS9XZWJDb3JlL2JpbmRpbmdzL3NjcmlwdHMv Q29kZUdlbmVyYXRvckpTLnBtCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFNvdXJjZS9XZWJDb3JlL2JpbmRpbmdz L3NjcmlwdHMvQ29kZUdlbmVyYXRvckpTLnBtCShyZXZpc2lvbiAxNDAwODgpCisrKyBTb3VyY2Uv V2ViQ29yZS9iaW5kaW5ncy9zY3JpcHRzL0NvZGVHZW5lcmF0b3JKUy5wbQkod29ya2luZyBjb3B5 KQpAQCAtMjQ1OCw3ICsyNDU4LDcgQEAgc3ViIEdlbmVyYXRlSW1wbGVtZW50YXRpb24KICAgICAg ICAgICAgIHB1c2goQGltcGxDb250ZW50LCAie1xuIik7CiAgICAgICAgICAgICBwdXNoKEBpbXBs Q29udGVudCwgIiAgICAke2NsYXNzTmFtZX0qIHRoaXNPYmogPSBqc0Nhc3Q8JGNsYXNzTmFtZSo+ KGFzT2JqZWN0KHNsb3RCYXNlKSk7XG4iKTsKICAgICAgICAgICAgIGlmICgkaW50ZXJmYWNlTmFt ZSBlcSAiSFRNTFByb3BlcnRpZXNDb2xsZWN0aW9uIikgewotICAgICAgICAgICAgICAgIHB1c2go QGltcGxDb250ZW50LCAiICAgIHJldHVybiB0b0pTKGV4ZWMsIHRoaXNPYmotPmdsb2JhbE9iamVj dCgpLCBzdGF0aWNfY2FzdDwkaW50ZXJmYWNlTmFtZSo+KHRoaXNPYmotPmltcGwoKSktPnByb3Bl cnR5Tm9kZUxpc3QocHJvcGVydHlOYW1lVG9BdG9taWNTdHJpbmcocHJvcGVydHlOYW1lKSkpO1xu Iik7CisgICAgICAgICAgICAgICAgcHVzaChAaW1wbENvbnRlbnQsICIgICAgcmV0dXJuIHRvSlMo ZXhlYywgdGhpc09iai0+Z2xvYmFsT2JqZWN0KCksIFdURjo6Z2V0UHRyKHN0YXRpY19jYXN0PCRp bnRlcmZhY2VOYW1lKj4odGhpc09iai0+aW1wbCgpKS0+cHJvcGVydHlOb2RlTGlzdChwcm9wZXJ0 eU5hbWVUb0F0b21pY1N0cmluZyhwcm9wZXJ0eU5hbWUpKSkpO1xuIik7CiAgICAgICAgICAgICB9 IGVsc2UgewogICAgICAgICAgICAgICAgIHB1c2goQGltcGxDb250ZW50LCAiICAgIHJldHVybiB0 b0pTKGV4ZWMsIHRoaXNPYmotPmdsb2JhbE9iamVjdCgpLCBzdGF0aWNfY2FzdDwkaW50ZXJmYWNl TmFtZSo+KHRoaXNPYmotPmltcGwoKSktPm5hbWVkSXRlbShwcm9wZXJ0eU5hbWVUb0F0b21pY1N0 cmluZyhwcm9wZXJ0eU5hbWUpKSk7XG4iKTsKICAgICAgICAgICAgIH0KSW5kZXg6IExheW91dFRl c3RzL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBMYXlvdXRUZXN0cy9DaGFuZ2VMb2cJKHJldmlz aW9uIDE0MDA4OCkKKysrIExheW91dFRlc3RzL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAt MSwzICsxLDE1IEBACisyMDEzLTAxLTE3ICBBcmtvIFNhaGEgIDxhcmtvQG1vdG9yb2xhLmNvbT4K KworICAgICAgICBQcm9wZXJ0eUNvbGxlY3Rpb24gdGVzdCBmYWlscyBvbiBKYXZhU2NyaXB0Q29y ZQorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTA2OTk3 CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgVW5za2lw cGluZyByZWxhdGVkIHRlc3RjYXNlcyBmcm9tIFRlc3RFeHBlY3RhdGlvbnMuCisKKyAgICAgICAg KiBwbGF0Zm9ybS9lZmwvVGVzdEV4cGVjdGF0aW9uczoKKyAgICAgICAgKiBwbGF0Zm9ybS9ndGsv VGVzdEV4cGVjdGF0aW9uczoKKwogMjAxMy0wMS0xNyAgQWxwaGEgTGFtICA8aGNsYW1AY2hyb21p dW0ub3JnPgogCiAgICAgICAgIFtjaHJvbWl1bV0gVW5yZXZpZXdlZCBnYXJkZW5pbmcuCkluZGV4 OiBMYXlvdXRUZXN0cy9wbGF0Zm9ybS9lZmwvVGVzdEV4cGVjdGF0aW9ucwo9PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0t LSBMYXlvdXRUZXN0cy9wbGF0Zm9ybS9lZmwvVGVzdEV4cGVjdGF0aW9ucwkocmV2aXNpb24gMTQw MDg4KQorKysgTGF5b3V0VGVzdHMvcGxhdGZvcm0vZWZsL1Rlc3RFeHBlY3RhdGlvbnMJKHdvcmtp bmcgY29weSkKQEAgLTE4MTQsNyArMTgxNCw0IEBAIHdlYmtpdC5vcmcvYi8xMDY5OTIgc2Nyb2xs aW5nY29vcmRpbmF0b3IKIHdlYmtpdC5vcmcvYi8xMDY5OTIgc3ZnL3pvb20vcGFnZS96b29tLXJl cGxhY2VkLWludHJpbnNpYy1yYXRpby0wMDEuaHRtIFsgRmFpbHVyZSBdCiB3ZWJraXQub3JnL2Iv MTA2OTkyIGh0dHAvdGVzdHMvbWlzYy9vYmplY3QtZW1iZWRkaW5nLXN2Zy1kZWxheWVkLXNpemUt bmVnb3RpYXRpb24tMi5odG0gWyBGYWlsdXJlIF0KIAotd2Via2l0Lm9yZy9iLzEwNjk5NyBmYXN0 L2RvbS9NaWNyb0RhdGEvcHJvcGVydGllc2NvbGxlY3Rpb24tY3Jhc2guaHRtbCBbIEZhaWx1cmUg XQotd2Via2l0Lm9yZy9iLzEwNjk5NyBmYXN0L2RvbS9NaWNyb0RhdGEvcHJvcGVydGllcy1jb2xs ZWN0aW9uLW5hbWVkZ2V0dGVyLXdpdGgtaW52YWxpZC1uYW1lLmh0bWwgWyBGYWlsdXJlIF0KLQog d2Via2l0Lm9yZy9iLzEwNzAwMSBjb21wb3NpdGluZy9sYXllci1jcmVhdGlvbi9vdmVybGFwLWFu aW1hdGlvbi1jb250YWluZXIuaHRtbCBbIEZhaWx1cmUgXQpJbmRleDogTGF5b3V0VGVzdHMvcGxh dGZvcm0vZ3RrL1Rlc3RFeHBlY3RhdGlvbnMKPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gTGF5b3V0VGVzdHMvcGxh dGZvcm0vZ3RrL1Rlc3RFeHBlY3RhdGlvbnMJKHJldmlzaW9uIDE0MDA4OCkKKysrIExheW91dFRl c3RzL3BsYXRmb3JtL2d0ay9UZXN0RXhwZWN0YXRpb25zCSh3b3JraW5nIGNvcHkpCkBAIC0xMzQ0 LDkgKzEzNDQsNiBAQCB3ZWJraXQub3JnL2IvMTAzOTI3IGh0dHAvdGVzdHMvdzNjL3dlYnBlCiB3 ZWJraXQub3JnL2IvMTAzOTI3IGh0dHAvdGVzdHMvdzNjL3dlYnBlcmYvc3VibWlzc2lvbi9yZXNv dXJjZS10aW1pbmcvaHRtbC90ZXN0X3Jlc291cmNlX2luaXRpYXRvcl90eXBlcy5odG1sIFsgRmFp bHVyZSBdCiB3ZWJraXQub3JnL2IvMTAzOTI3IGh0dHAvdGVzdHMvdzNjL3dlYnBlcmYvc3VibWlz c2lvbi9yZXNvdXJjZS10aW1pbmcvaHRtbC90ZXN0X3Jlc291cmNlX3JlZGlyZWN0cy5odG1sIFsg RmFpbHVyZSBdCiAKLXdlYmtpdC5vcmcvYi8xMDY5OTcgWyBEZWJ1ZyBdIGZhc3QvZG9tL01pY3Jv RGF0YS9wcm9wZXJ0aWVzLWNvbGxlY3Rpb24tbmFtZWRnZXR0ZXItd2l0aC1pbnZhbGlkLW5hbWUu aHRtbCBbIEZhaWx1cmUgXQotd2Via2l0Lm9yZy9iLzEwNjk5NyBbIERlYnVnIF0gZmFzdC9kb20v TWljcm9EYXRhL3Byb3BlcnRpZXNjb2xsZWN0aW9uLWNyYXNoLmh0bWwgWyBGYWlsdXJlIF0KLQog Iy8vLy8vLy8vLy8vLy8vLy8vLy8vLy8vLy8vLy8vLy8vLy8vLy8vLy8vLy8vLy8vLy8vLy8vLy8v Ly8vLy8vLy8vLy8vLy8vLy8vLy8vLy8vLy8vLy8vLy8KICMgRW5kIG9mIFRlc3RzIGZhaWxpbmcK ICMvLy8vLy8vLy8vLy8vLy8vLy8vLy8vLy8vLy8vLy8vLy8vLy8vLy8vLy8vLy8vLy8vLy8vLy8v Ly8vLy8vLy8vLy8vLy8vLy8vLy8vLy8vLy8vLy8vLy8vCg==