Bug 127969 – Push DOM attributes into the prototype chain

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Oliver Hunt]

Push DOM attributes into the prototype chain

[Oliver Hunt]

Created attachment 222763 [details]
Patch

just the constructor attribute at the moment

[Oliver Hunt]

holy crap it's building on other platforms :-O

[Geoffrey Garen]

Comment on attachment 222763 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=222763&action=review

r=me

> Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:595
> +    my $hasNumericIndexedGetter = $indexedGetterFunction ? $codeGenerator->IsNumericType($indexedGetterFunction->signature->type) : 0;

This variable seems to be unused. Is that a mistake?

> Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:621
> +    return 1;

This needs a comment to explain that it's temporary.

> Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:1266
> +

Stray newline.

[Oliver Hunt]

(In reply to comment #3)
> (From update of attachment 222763 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=222763&action=review
> 
> r=me
> 
> > Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:595
> > +    my $hasNumericIndexedGetter = $indexedGetterFunction ? $codeGenerator->IsNumericType($indexedGetterFunction->signature->type) : 0;
> 
> This variable seems to be unused. Is that a mistake?
> 
> > Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:621
> > +    return 1;
> 
> This needs a comment to explain that it's temporary.

I was pushing the start of my work so there was a backup and to make sure it would actually build elsewhere.

Good to have initial work reviewed though, thanks! (The bulk of the rest is done, but i'm stepping through enabling pieces one at a time to find individual failures and regressions)

> 
> > Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:1266
> > +
> 
> Stray newline.

[Oliver Hunt]

Created attachment 222912 [details]
Patch

[Build Bot]

Comment on attachment 222912 [details]
Patch

Attachment 222912 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/6687066310049792

New failing tests:
js/dom/dfg-custom-getter-throw-inlined.html
svg/hixie/dynamic/005.xml
inspector-protocol/debugger/setPauseOnExceptions-uncaught.html
js/dom/dom-static-property-for-in-iteration.html
inspector-protocol/debugger/setPauseOnExceptions-none.html
svg/custom/class-baseValue.svg
svg/W3C-SVG-1.1-SE/types-dom-04-b.svg
svg/W3C-SVG-1.1-SE/svgdom-over-01-f.svg
inspector-protocol/debugger/setPauseOnExceptions-all.html
fast/dom/SelectorAPI/resig-SelectorsAPI-test.xhtml
fast/dom/wrapper-classes.html
js/dom/dfg-custom-getter-throw.html

[Build Bot]

Created attachment 222919 [details]
Archive of layout-test-results from webkit-ews-09 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-09  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5

[Build Bot]

Comment on attachment 222912 [details]
Patch

Attachment 222912 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/5797458323963904

New failing tests:
js/dom/dfg-custom-getter-throw-inlined.html
svg/hixie/dynamic/005.xml
inspector-protocol/debugger/setPauseOnExceptions-uncaught.html
js/dom/dom-static-property-for-in-iteration.html
inspector-protocol/debugger/setPauseOnExceptions-none.html
svg/custom/class-baseValue.svg
svg/W3C-SVG-1.1-SE/types-dom-04-b.svg
http/tests/security/cross-frame-access-enumeration.html
svg/W3C-SVG-1.1-SE/svgdom-over-01-f.svg
inspector-protocol/debugger/setPauseOnExceptions-all.html
fast/dom/SelectorAPI/resig-SelectorsAPI-test.xhtml
fast/dom/wrapper-classes.html
js/dom/dfg-custom-getter-throw.html

[Build Bot]

Created attachment 222921 [details]
Archive of layout-test-results from webkit-ews-07 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-07  Port: mac-mountainlion  Platform: Mac OS X 10.8.5

[Build Bot]

Comment on attachment 222912 [details]
Patch

Attachment 222912 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/6074071397695488

New failing tests:
js/dom/dfg-custom-getter-throw-inlined.html
svg/hixie/dynamic/005.xml
inspector-protocol/debugger/setPauseOnExceptions-uncaught.html
js/dom/dom-static-property-for-in-iteration.html
inspector-protocol/debugger/setPauseOnExceptions-none.html
svg/custom/class-baseValue.svg
svg/W3C-SVG-1.1-SE/types-dom-04-b.svg
http/tests/security/cross-frame-access-enumeration.html
svg/W3C-SVG-1.1-SE/svgdom-over-01-f.svg
inspector-protocol/debugger/setPauseOnExceptions-all.html
fast/dom/SelectorAPI/resig-SelectorsAPI-test.xhtml
fast/dom/wrapper-classes.html
js/dom/dfg-custom-getter-throw.html

[Build Bot]

Created attachment 222922 [details]
Archive of layout-test-results from webkit-ews-08 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-08  Port: mac-mountainlion  Platform: Mac OS X 10.8.5

[Build Bot]

Comment on attachment 222912 [details]
Patch

Attachment 222912 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/6403426669821952

New failing tests:
js/dom/dfg-custom-getter-throw-inlined.html
svg/hixie/dynamic/005.xml
inspector-protocol/debugger/setPauseOnExceptions-uncaught.html
js/dom/dom-static-property-for-in-iteration.html
inspector-protocol/debugger/setPauseOnExceptions-none.html
svg/custom/class-baseValue.svg
svg/W3C-SVG-1.1-SE/types-dom-04-b.svg
svg/W3C-SVG-1.1-SE/svgdom-over-01-f.svg
inspector-protocol/debugger/setPauseOnExceptions-all.html
fast/dom/SelectorAPI/resig-SelectorsAPI-test.xhtml
fast/dom/wrapper-classes.html
js/dom/dfg-custom-getter-throw.html

[Build Bot]

Created attachment 222923 [details]
Archive of layout-test-results from webkit-ews-15 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-15  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5

[Oliver Hunt]

Created attachment 222939 [details]
Patch

[Build Bot]

Comment on attachment 222939 [details]
Patch

Attachment 222939 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/6366197893300224

New failing tests:
js/dom/dfg-custom-getter-throw-inlined.html
js/dom/dfg-custom-getter-throw.html

[Build Bot]

Created attachment 222940 [details]
Archive of layout-test-results from webkit-ews-13 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-13  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5

[Build Bot]

Comment on attachment 222939 [details]
Patch

Attachment 222939 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/4584905304440832

New failing tests:
http/tests/security/cross-frame-access-enumeration.html
js/dom/dfg-custom-getter-throw-inlined.html
js/dom/dfg-custom-getter-throw.html

[Build Bot]

Created attachment 222942 [details]
Archive of layout-test-results from webkit-ews-01 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-01  Port: mac-mountainlion  Platform: Mac OS X 10.8.5

[Oliver Hunt]

Created attachment 222945 [details]
Patch

[Oliver Hunt]

Created attachment 222948 [details]
Patch

[Mark Lam]

Comment on attachment 222948 [details]
Patch

r=me

[Oliver Hunt]

Committed r163280: <http://trac.webkit.org/changeset/163280>

[Geoffrey Garen]

Reopening to attach new patch.

[Geoffrey Garen]

Created attachment 223171 [details]
Patch

[Geoffrey Garen]

Comment on attachment 223171 [details]
Patch

Roll-out patch.

[WebKit Commit Bot]

Attachment 223171 [details] did not pass style-queue:


ERROR: Source/WebCore/bindings/js/JSStorageCustom.cpp:52:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Total errors found: 1 in 40 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Geoffrey Garen]

Created attachment 223172 [details]
Patch

[WebKit Commit Bot]

Attachment 223172 [details] did not pass style-queue:


ERROR: Source/WebCore/bindings/js/JSStorageCustom.cpp:52:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Total errors found: 1 in 40 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Geoffrey Garen]

Committed r163413: <http://trac.webkit.org/changeset/163413>

[Oliver Hunt]

Committed r163562: <http://trac.webkit.org/changeset/163562>

[Ryosuke Niwa]

Looks like this was still 7~8% regression on Dromaeo/dom-traverse:

https://perf.webkit.org/#mode=charts&chartList=%5B%5B%22mac-mountainlion%22%2C%22Dromaeo%2Fdom-traverse%3ARuns%22%5D%2C%5B%22mac-mavericks%22%2C%22Dromaeo%2Fdom-traverse%3ARuns%22%5D%5D&days=63&zoom=%5B1391618803263.1353%2C1391828625577.3062%5D

[Ryosuke Niwa]

This patch also appears to have regressed Chromium's dom_perf benchmark by 30-40%:

https://perf.webkit.org/#mode=charts&chartList=%5B%5B%22mac-mavericks%22%2C%22DOM%2FModifyAttribute%3ATime%22%5D%2C%5B%22mac-mountainlion%22%2C%22DOM%2FModifyAttribute%3ATime%22%5D%5D&days=365&zoom=%5B1389742499618.0234%2C1392139963925.9648%5D

[Oliver Hunt]

(In reply to comment #32)
> This patch also appears to have regressed Chromium's dom_perf benchmark by 30-40%:
> 
> https://perf.webkit.org/#mode=charts&chartList=%5B%5B%22mac-mavericks%22%2C%22DOM%2FModifyAttribute%3ATime%22%5D%2C%5B%22mac-mountainlion%22%2C%22DOM%2FModifyAttribute%3ATime%22%5D%5D&days=365&zoom=%5B1389742499618.0234%2C1392139963925.9648%5D

Wow, that is vastly more concerning to me than the dromaeo regression :( Can we get a separate bug for that?