Bug 113831

<rdar://problem/12035186>

Created attachment 196243 [details] Patch

Comment on attachment 196243 [details] Patch No testcase. You should also mention the actual site that this fixes.

Comment on attachment 196243 [details] Patch Have you done perf tests on this? Huge numbers of dom objects have static properties - it might be good to run dromaeo

It should be possible to include a test case here. I believe the bug report includes one.

> Have you done perf tests on this? Huge numbers of dom objects have static properties - it might be good to run dromaeo Since DOM objects only ever got the get_by_pname optimization for their custom properties, which are rare, we expect this change to have no performance effect. I agree that we should confirm using Dromaeo.

Dromaeo results: Without patch: http://dromaeo.com/?id=192872 With patch: http://dromaeo.com/?id=192876

Created attachment 196366 [details] Patch

Comment on attachment 196366 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=196366&action=review r=me > LayoutTests/fast/js/dom-static-property-for-in-iteration-expected.txt:163 > +PASS true is true This is a bit opaque. > LayoutTests/fast/js/dom-static-property-for-in-iteration.html:18 > + var actual = a[i]; > + var expected = a["" + i]; > + shouldBeTrue(String(actual === expected)); You'll get better test output if you do something like this: shouldBe('a[i]', 'a["" + i]').

> > LayoutTests/fast/js/dom-static-property-for-in-iteration-expected.txt:163 > > +PASS true is true > > This is a bit opaque. > > > LayoutTests/fast/js/dom-static-property-for-in-iteration.html:18 > > + var actual = a[i]; > > + var expected = a["" + i]; > > + shouldBeTrue(String(actual === expected)); > > You'll get better test output if you do something like this: shouldBe('a[i]', 'a["" + i]'). I originally did that, but I get the following error message in the test output: CONSOLE MESSAGE: line 1: ReferenceError: Can't find variable: a

> I originally did that, but I get the following error message in the test output: > > CONSOLE MESSAGE: line 1: ReferenceError: Can't find variable: a Nevermind, there were some nested double quotes inside the property value that were throwing things off. I got it working with shouldBe() now.

Committed r147570: <http://trac.webkit.org/changeset/147570>

Reopening to make the test better.

Created attachment 196389 [details] Patch

The test dom-static-property-for-in-iteration.html started failing after r147570 landed.

(In reply to comment #15) > The test dom-static-property-for-in-iteration.html started failing after r147570 landed. Ahh, there was a system-specific path in there. I should change the test to avoid those properties.

Created attachment 196410 [details] Patch

Committed r147585: <http://trac.webkit.org/changeset/147585>

On Qt port there is some difference between actual and expected results. Is it acceptable? http://build.webkit.org/results/Qt%20Linux%20Release/r147616%20%2859023%29/fast/js/dom-static-property-for-in-iteration-pretty-diff.html --- /ramdisk/qt-linux-release/build/layout-test-results/fast/js/dom-static-property-for-in-iteration-expected.txt +++ /ramdisk/qt-linux-release/build/layout-test-results/fast/js/dom-static-property-for-in-iteration-actual.txt @@ -45,7 +45,7 @@ PASS a["scrollWidth"] is 0 PASS a["attributes"] is [object NamedNodeMap] PASS a["webkitRegionOverset"] is undefined -PASS a["offsetWidth"] is 39 +PASS a["offsetWidth"] is 37 PASS a["classList"] is PASS a["offsetLeft"] is 8 PASS a["className"] is @@ -59,9 +59,9 @@ PASS a["scrollLeft"] is 0 PASS a["firstElementChild"] is null PASS a["clientLeft"] is 0 -PASS a["offsetHeight"] is 18 +PASS a["offsetHeight"] is 19 PASS a["clientHeight"] is 0 -PASS a["offsetTop"] is 954 +PASS a["offsetTop"] is 956 PASS a["scrollTop"] is 0 PASS a["scrollHeight"] is 0 PASS a["previousSibling"] is [object Text] @@ -85,7 +85,6 @@ PASS a["hick"] is 4 PASS a["hock"] is 5 PASS a["snood"] is 6 -PASS a["ALLOW_KEYBOARD_INPUT"] is 1 PASS a["NOTATION_NODE"] is 12 PASS a["CDATA_SECTION_NODE"] is 4 PASS a["ELEMENT_NODE"] is 1

Some addition information on Qt. On Qt WK2 only offset is different. Qt WK1 misses "PASS a["ALLOW_KEYBOARD_INPUT"] is 1". http://build.webkit.sed.hu/results/x86-32%20Linux%20Qt%20Release%20WebKit2/r147616%20%2834444%29/fast/js/dom-static-property-for-in-iteration-pretty-diff.html

*** Bug 114036 has been marked as a duplicate of this bug. ***