Bug 189694 – Implement CSS Custom Properties and Values Skeleton

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[Justin Michaud]

Implement a feature flag and DOM bindings for the CSS Custom Properties and Values API

[Justin Michaud]

Created attachment 350009 [details]
Patch

[Justin Michaud]

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.

[EWS Watchlist]

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

[EWS Watchlist]

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

[EWS Watchlist]

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

[Sam Weinig]

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.

[Justin Michaud]

Created attachment 350085 [details]
Patch

[Justin Michaud]

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.

[Justin Michaud]

Created attachment 350089 [details]
Patch

[Justin Michaud]

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

[Justin Michaud]

Created attachment 350118 [details]
Patch

[Justin Michaud]

Created attachment 350120 [details]
Patch

[Simon Fraser (smfr)]

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

[Simon Fraser (smfr)]

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.

[Justin Michaud]

Created attachment 350141 [details]
Patch

[Justin Michaud]

(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

[Justin Michaud]

Created attachment 350153 [details]
Patch

[Justin Michaud]

Created attachment 350231 [details]
Patch

[WebKit Commit Bot]

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 [e-mail]...
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

[WebKit Commit Bot]

Comment on attachment 350231 [details]
Patch

Clearing flags on attachment: 350231

Committed r236273: <https://trac.webkit.org/changeset/236273>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/44649888>