RESOLVED FIXED 153735
Move properties that use custom bindings to the prototype
https://bugs.webkit.org/show_bug.cgi?id=153735
Summary Move properties that use custom bindings to the prototype
Chris Dumez
Reported 2016-01-31 21:19:40 PST
Move properties that use custom bindings to the prototype. Whether a property's bindings code is generated or custom-written should not impact where the location of the property.
Attachments
Patch (15.57 KB, patch)
2016-01-31 21:23 PST, Chris Dumez
no flags
Patch (15.57 KB, patch)
2016-01-31 21:24 PST, Chris Dumez
no flags
Archive of layout-test-results from ews101 for mac-yosemite (920.28 KB, application/zip)
2016-01-31 22:15 PST, Build Bot
no flags
Archive of layout-test-results from ews107 for mac-yosemite-wk2 (1.08 MB, application/zip)
2016-01-31 22:19 PST, Build Bot
no flags
Archive of layout-test-results from ews116 for mac-yosemite (880.54 KB, application/zip)
2016-01-31 22:27 PST, Build Bot
no flags
Patch (61.60 KB, patch)
2016-02-01 09:43 PST, Chris Dumez
no flags
Patch (61.64 KB, patch)
2016-02-01 11:41 PST, Chris Dumez
no flags
Chris Dumez
Comment 1 2016-01-31 21:23:42 PST
Chris Dumez
Comment 2 2016-01-31 21:24:56 PST
Build Bot
Comment 3 2016-01-31 22:15:56 PST
Comment on attachment 270371 [details] Patch Attachment 270371 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/766476 New failing tests: imported/w3c/web-platform-tests/html/dom/interfaces.html http/tests/security/cross-frame-access-enumeration.html imported/w3c/web-platform-tests/dom/interfaces.html
Build Bot
Comment 4 2016-01-31 22:15:59 PST
Created attachment 270373 [details] Archive of layout-test-results from ews101 for mac-yosemite The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews101 Port: mac-yosemite Platform: Mac OS X 10.10.5
Build Bot
Comment 5 2016-01-31 22:19:40 PST
Comment on attachment 270371 [details] Patch Attachment 270371 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/766479 New failing tests: imported/w3c/web-platform-tests/html/dom/interfaces.html imported/w3c/web-platform-tests/dom/interfaces.html
Build Bot
Comment 6 2016-01-31 22:19:43 PST
Created attachment 270374 [details] Archive of layout-test-results from ews107 for mac-yosemite-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews107 Port: mac-yosemite-wk2 Platform: Mac OS X 10.10.5
Build Bot
Comment 7 2016-01-31 22:26:58 PST
Comment on attachment 270371 [details] Patch Attachment 270371 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/766492 New failing tests: http/tests/security/cross-frame-access-enumeration.html
Build Bot
Comment 8 2016-01-31 22:27:02 PST
Created attachment 270375 [details] Archive of layout-test-results from ews116 for mac-yosemite The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews116 Port: mac-yosemite Platform: Mac OS X 10.10.5
Darin Adler
Comment 9 2016-02-01 08:53:24 PST
Comment on attachment 270371 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=270371&action=review > Source/WebCore/bindings/js/JSHTMLDocumentCustom.cpp:79 > + if (JSHTMLDocument::info()->staticPropHashTable) { > + if (const HashTableValue* entry = JSHTMLDocument::info()->staticPropHashTable->entry(propertyName)) { > + slot.setCacheableCustom(thisObject, entry->attributes(), entry->propertyGetter()); > + return true; > + } > } I’d write: if (auto* table = JSHTMLDocument::info()->staticPropHashTable) { if (auto* entry = table->entry(propertyName)) { slot.setCacheableCustom(thisObject, entry->attributes(), entry->propertyGetter()); return true; } } > Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:-708 > - return 1 if HasCustomGetter($attribute->signature->extendedAttributes); > - return 1 if HasCustomSetter($attribute->signature->extendedAttributes); Need to regenerate bindings tests expectations. > LayoutTests/imported/w3c/ChangeLog:10 > + * web-platform-tests/XMLHttpRequest/interfaces-expected.txt: Looks like there are progressions in a couple other tests too.
Chris Dumez
Comment 10 2016-02-01 09:03:57 PST
Comment on attachment 270371 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=270371&action=review >> Source/WebCore/bindings/js/JSHTMLDocumentCustom.cpp:79 >> } > > I’d write: > > if (auto* table = JSHTMLDocument::info()->staticPropHashTable) { > if (auto* entry = table->entry(propertyName)) { > slot.setCacheableCustom(thisObject, entry->attributes(), entry->propertyGetter()); > return true; > } > } Right, I hesitated. I'll make the change. >> Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:-708 >> - return 1 if HasCustomSetter($attribute->signature->extendedAttributes); > > Need to regenerate bindings tests expectations. I keep forgetting, will do. >> LayoutTests/imported/w3c/ChangeLog:10 >> + * web-platform-tests/XMLHttpRequest/interfaces-expected.txt: > > Looks like there are progressions in a couple other tests too. Yes. It surprised me that I did not see more progressions at the time. Now I realize I used a debug build to test and these interfaces.html tests are skipped in Debug builds because they are slow :( I'll rebase the others.
Chris Dumez
Comment 11 2016-02-01 09:43:46 PST
Radar WebKit Bug Importer
Comment 12 2016-02-01 09:44:28 PST
Radar WebKit Bug Importer
Comment 13 2016-02-01 09:44:29 PST
WebKit Commit Bot
Comment 14 2016-02-01 10:36:38 PST
Comment on attachment 270394 [details] Patch Rejecting attachment 270394 [details] from commit-queue. Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-02', 'land-attachment', '--force-clean', '--non-interactive', '--parent-command=commit-queue', 270394, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit Last 500 characters of output: /git.webkit.org/WebKit eac0aff..d73cdd5 master -> origin/master Partial-rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc ... Currently at 195966 = eac0afff8b3de43c1b8ca44cff8397b283c0b6f9 r195967 = d73cdd50547addd61f8cd309d5e4beff4222ae44 Done rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc First, rewinding head to replay your work on top of it... Fast-forwarded master to refs/remotes/origin/master. Full output: http://webkit-queues.webkit.org/results/768703
Chris Dumez
Comment 15 2016-02-01 11:41:31 PST
Chris Dumez
Comment 16 2016-02-01 11:42:19 PST
Comment on attachment 270408 [details] Patch Clearing flags on attachment: 270408 Committed r195969: <http://trac.webkit.org/changeset/195969>
Chris Dumez
Comment 17 2016-02-01 11:42:24 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.