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
Visible on the GTK as well, but (as on EFL) only in debug builds.
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.
Created attachment 183351 [details] Patch
Comment on attachment 183351 [details] Patch LGTM
(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.
(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.
(In reply to comment #3) > Created an attachment (id=183351) [details] > Patch Thanks very much for fixing this Arko.
Comment on attachment 183351 [details] Patch Rejecting attachment 183351 [details] 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
Committed r140182: <http://trac.webkit.org/changeset/140182>