Bug 106997 – PropertyCollection test fails on JavaScriptCore

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore JavaScript

[Dominik Röttsches (drott)]

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

[Zan Dobersek]

Visible on the GTK as well, but (as on EFL) only in debug builds.

[Zan Dobersek]

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.

[Arko Saha]

Created attachment 183351 [details]
Patch

[Kentaro Hara]

Comment on attachment 183351 [details]
Patch

LGTM

[Kentaro Hara]

(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.

[Zan Dobersek]

(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.

[Dominik Röttsches (drott)]

(In reply to comment #3)
> Created an attachment (id=183351) [details]
> Patch

Thanks very much for fixing this Arko.

[WebKit Review Bot]

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

[Arko Saha]

Committed r140182: <http://trac.webkit.org/changeset/140182>