Bug 189694

Created attachment 350009 [details] Patch

For reviewer: The bindings code is particularly scary/messy/likely to break something, I would appreciate some help with it. Also, is Document the right place to store the list of registered properties? I was planning on adding code to StyleResolver::applyProperty to handle inherits and initialValue after this patch is sorted out, which does have access to a Document, but I wonder if there is a cleaner way.

Comment on attachment 350009 [details] Patch Attachment 350009 [details] did not pass bindings-ews (mac): Output: https://webkit-queues.webkit.org/results/9253990 New failing tests: (JS) JSTestEnabledBySetting.cpp (JS) JSTestGenerateIsReachable.cpp (JS) JSTestInterface.cpp (JS) JSTestNode.cpp (JS) JSTestObj.cpp

Comment on attachment 350009 [details] Patch Attachment 350009 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: https://webkit-queues.webkit.org/results/9254260 New failing tests: storage/indexeddb/keyrange.html storage/indexeddb/keyrange-private.html css3/flexbox/align-absolute-child.html fast/dom/DOMURL/check-instanceof-domurl-functions.html

Created attachment 350011 [details] Archive of layout-test-results from ews122 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews122 Port: ios-simulator-wk2 Platform: Mac OS X 10.13.6

Comment on attachment 350009 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=350009&action=review > Source/WebCore/ChangeLog:19 > + * bindings/scripts/CodeGeneratorJS.pm: Please add bindings tests for this new functionality. > Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:3100 > + #next if ($operation->isStatic); Please don't check in commented out code. > Source/WebCore/css/DOMCSSNamespace.idl:37 > + [CallWith=Document,EnabledAtRuntime=CSSCustomPropertiesAndValues] static void registerProperty(DOMCSSPropertyDescriptor descriptor); This should probably be in it's own IDL file using a partial interface. > Source/WebCore/dom/Document.cpp:8224 > + if (m_CSSRegisteredPropertySet.contains(prop.name)) > + return false; > + m_CSSRegisteredPropertySet.add(prop.name, prop); > + return true; This should use HashMap::ensure to avoid the double hash lookup. Also, is the return value necessary? It doesn't seem like the caller uses it. > Source/WebCore/dom/Document.h:1496 > + bool registerCSSProperty(const CSSRegisteredProperty&); This could probably take an r-value CSSRegisteredProperty. > Source/WebKit/Shared/WebPreferences.yaml:1358 > + humanReadableName: "CSS Custom Properties and Values Api Level 1" > + humanReadableDescription: "Enable CSS Properties and Values Api Level 1" "Api" -> "API". Also, the "Level" 1 is probably not necessary.

Created attachment 350085 [details] Patch

I responded to the feedback about the patch. I also moved the code generator changes out to a new patch, which I will upload shortly. The feature flag will not work properly until the code generator changes land.

Created attachment 350089 [details] Patch

https://bugs.webkit.org/show_bug.cgi?id=189729 is the bug for the bindings

Created attachment 350118 [details] Patch

Created attachment 350120 [details] Patch

We should import https://github.com/web-platform-tests/wpt/tree/master/css/css-properties-values-api at some point.

Comment on attachment 350120 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=350120&action=review > Source/WebCore/css/DOMCSSCustomPropertyDescriptor.h:15 > + * 1. Redistributions of source code must retain the above copyright > + * notice, this list of conditions and the following disclaimer. > + * 2. Redistributions in binary form must reproduce the above > + * copyright notice, this list of conditions and the following disclaimer in > + * the documentation and/or other materials provided with the distribution. > + * 3. Neither the name of Motorola Mobility Inc. nor the names of its > + * contributors may be used to endorse or promote products derived from this > + * software without specific prior written permission. Wrong license. > Source/WebCore/css/DOMCSSCustomPropertyDescriptor.idl:15 > +* 1. Redistributions of source code must retain the above copyright > +* notice, this list of conditions and the following disclaimer. > +* 2. Redistributions in binary form must reproduce the above > +* copyright notice, this list of conditions and the following disclaimer in > +* the documentation and/or other materials provided with the distribution. > +* 3. Neither the name of Motorola Mobility Inc. nor the names of its > +* contributors may be used to endorse or promote products derived from this > +* software without specific prior written permission. Wrong license. > Source/WebCore/css/DOMCSSRegisterCustomProperty.cpp:15 > + * 1. Redistributions of source code must retain the above copyright > + * notice, this list of conditions and the following disclaimer. > + * 2. Redistributions in binary form must reproduce the above > + * copyright notice, this list of conditions and the following disclaimer in > + * the documentation and/or other materials provided with the distribution. > + * 3. Neither the name of Motorola Mobility Inc. nor the names of its > + * contributors may be used to endorse or promote products derived from this > + * software without specific prior written permission. Wrong license. > Source/WebCore/css/DOMCSSRegisterCustomProperty.h:2 > + * Copyright (C) 2012 Motorola Mobility Inc. All rights reserved. Apple > Source/WebCore/dom/Document.h:2029 > + HashMap<String, CSSRegisteredCustomProperty> m_CSSRegisteredPropertySet; Probably want a HashMap<String, std::unique_ptr<CSSRegisteredCustomProperty>> at some point.

Created attachment 350141 [details] Patch

(In reply to Simon Fraser (smfr) from comment #13) > We should import > https://github.com/web-platform-tests/wpt/tree/master/css/css-properties- > values-api at some point. https://bugs.webkit.org/show_bug.cgi?id=189764

Created attachment 350153 [details] Patch

Created attachment 350231 [details] Patch

Comment on attachment 350231 [details] Patch Rejecting attachment 350231 [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-03', 'land-attachment', '--force-clean', '--non-interactive', '--parent-command=commit-queue', 350231, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit Logging in as commit-queue@webkit.org... Fetching: https://bugs.webkit.org/attachment.cgi?id=350231&action=edit Fetching: https://bugs.webkit.org/show_bug.cgi?id=189694&ctype=xml&excludefield=attachmentdata Processing 1 patch from 1 bug. Updating working directory Processing patch 350231 from bug 189694. Fetching: https://bugs.webkit.org/attachment.cgi?id=350231 Failed to run "['git', 'svn', 'dcommit', '--rmdir']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit Committing to http://svn.webkit.org/repository/webkit/trunk ... A LayoutTests/css-custom-properties-api/registerProperty-expected.txt A LayoutTests/css-custom-properties-api/registerProperty.html A Source/WebCore/css/CSSRegisteredCustomProperty.h A Source/WebCore/css/DOMCSSCustomPropertyDescriptor.h A Source/WebCore/css/DOMCSSCustomPropertyDescriptor.idl A Source/WebCore/css/DOMCSSRegisterCustomProperty.cpp A Source/WebCore/css/DOMCSSRegisterCustomProperty.h A Source/WebCore/css/DOMCSSRegisterCustomProperty.idl M LayoutTests/ChangeLog ERROR from SVN: Item is out of date: File '/trunk/LayoutTests/ChangeLog' is out of date W: b9d443dd00ccfedd6fcb922f344e2ff85415d675 and refs/remotes/origin/master differ, using rebase: :040000 040000 d046daf9cd393c65af5c4ae80c7db1d7c3c7650e 664ea62087e4092795ab709d5b041447814487fe M LayoutTests :040000 040000 d2c09978b4d1d5ccdd0f6ae18813226993bfde8f d64d3e018d269ac1c35ce279e79f5f52f5547f36 M Source Current branch master is up to date. ERROR: Not all changes have been committed into SVN, however the committed ones (if any) seem to be successfully integrated into the working tree. Please see the above messages for details. Failed to run "['git', 'svn', 'dcommit', '--rmdir']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit Committing to http://svn.webkit.org/repository/webkit/trunk ... A LayoutTests/css-custom-properties-api/registerProperty-expected.txt A LayoutTests/css-custom-properties-api/registerProperty.html A Source/WebCore/css/CSSRegisteredCustomProperty.h A Source/WebCore/css/DOMCSSCustomPropertyDescriptor.h A Source/WebCore/css/DOMCSSCustomPropertyDescriptor.idl A Source/WebCore/css/DOMCSSRegisterCustomProperty.cpp A Source/WebCore/css/DOMCSSRegisterCustomProperty.h A Source/WebCore/css/DOMCSSRegisterCustomProperty.idl M LayoutTests/ChangeLog ERROR from SVN: Item is out of date: File '/trunk/LayoutTests/ChangeLog' is out of date W: b9d443dd00ccfedd6fcb922f344e2ff85415d675 and refs/remotes/origin/master differ, using rebase: :040000 040000 d046daf9cd393c65af5c4ae80c7db1d7c3c7650e 664ea62087e4092795ab709d5b041447814487fe M LayoutTests :040000 040000 d2c09978b4d1d5ccdd0f6ae18813226993bfde8f d64d3e018d269ac1c35ce279e79f5f52f5547f36 M Source Current branch master is up to date. ERROR: Not all changes have been committed into SVN, however the committed ones (if any) seem to be successfully integrated into the working tree. Please see the above messages for details. Failed to run "['git', 'svn', 'dcommit', '--rmdir']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit Updating OpenSource From https://git.webkit.org/git/WebKit a92813138ca..db7de6d33eb master -> origin/master Partial-rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc ... Currently at 236266 = a92813138ca971a1efd5654eb374a1f51a74baa7 r236269 = 6a5b1c4cd780c104178671fc3cede538d570259b r236270 = ff1ca6c5e9acaba2eb6d522d1cb06a9acf0c46a0 r236271 = db7de6d33eb03a60876c1e45a5f0b3cac88b76b7 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. Total errors found: 0 in 2 files Full output: https://webkit-queues.webkit.org/results/9284354

Comment on attachment 350231 [details] Patch Clearing flags on attachment: 350231 Committed r236273: <https://trac.webkit.org/changeset/236273>

All reviewed patches have been landed. Closing bug.

<rdar://problem/44649888>