Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add an iterator for the StyleProperties family of classes #6981

Conversation

weinig
Copy link
Contributor

@weinig weinig commented Nov 30, 2022

bd076ef

Add an iterator for the StyleProperties family of classes
https://bugs.webkit.org/show_bug.cgi?id=248546
rdar://102823936

Reviewed by Antoine Quint.

Add iterator support for StyleProperties, ImmutableStyleProperties
and MutableStyleProperties to allow iterating through the stored
properties using for-in as well generic algorithms.

* Source/WTF/wtf/Vector.h:
(WTF::Mapper::map):
(WTF::CompactMapper::compactMap):
(WTF::copyToVectorSpecialization):
Use a universal reference to allow iterators that return temporaries
(like the one for StyleProperties) to work with the mapping code.

* Source/WebCore/animation/KeyframeEffect.cpp:
(WebCore::KeyframeEffect::computeHasImplicitKeyframeForAcceleratedProperty):
(WebCore::KeyframeEffect::computeHasKeyframeComposingAcceleratedProperty):
Adopt range-based for loop using the new iterator.

* Source/WebCore/css/CSSValueList.cpp:
(WebCore::CSSValueList::removeAll):
(WebCore::CSSValueList::hasValue const):
* Source/WebCore/css/CSSValueList.h:
Remove versions of removeAll() and hasValue() taking a nullable CSSValue*,
and replace them with overloads that take either a CSSValue& or a CSSValueID.
The CSSValueID allows us to check use without allocation or pool lookup.

* Source/WebCore/css/ComputedStyleExtractor.cpp:
(WebCore::ComputedStyleExtractor::fontVariantShorthandValue):
(WebCore::ComputedStyleExtractor::getCSSPropertyValuesForShorthandProperties):
(WebCore::ComputedStyleExtractor::getCSSPropertyValuesForGridShorthand):
Adopt range-based for loop using existing shorthand iterator support. Use
isValueID helper to simplify a condition.

* Source/WebCore/css/PropertySetCSSStyleDeclaration.cpp:
(WebCore::PropertySetCSSStyleDeclaration::length const):
Adopt range-based for loop using the new iterator.

* Source/WebCore/css/StyleProperties.cpp:
(WebCore::MutableStyleProperties::MutableStyleProperties):
(WebCore::StyleProperties::commonShorthandChecks const):
(WebCore::StyleProperties::asTextInternal const):
(WebCore::MutableStyleProperties::mergeAndOverrideOnConflict):
(WebCore::StyleProperties::traverseSubresources const):
* Source/WebCore/css/StyleProperties.h:
(WebCore::StyleProperties::Iterator::Iterator):
(WebCore::StyleProperties::Iterator::operator* const):
(WebCore::StyleProperties::Iterator::operator++):
(WebCore::StyleProperties::Iterator::operator== const):
(WebCore::StyleProperties::Iterator::operator!= const):
(WebCore::StyleProperties::begin const):
(WebCore::StyleProperties::end):
(WebCore::StyleProperties::size const):
Adds and adopts iterator support. The two subclasses of StyleProperties
overload begin() to allow specialization when the concrete type is known.

* Source/WebCore/css/parser/CSSPropertyParserHelpers.cpp:
(WebCore::CSSPropertyParserHelpers::consumeTouchAction):
(WebCore::CSSPropertyParserHelpers::consumeTextDecorationLine):
Adopt template consumeIdent and update for new reference taking hasValue().

* Source/WebCore/css/typedom/DeclaredStylePropertyMap.cpp:
(WebCore::DeclaredStylePropertyMap::entries const):
Adopt WTF::map() now that it can be used due to iterator for StyleProperties.

* Source/WebCore/editing/EditingStyle.cpp:
(WebCore::applyTextDecorationChangeToValueList):
Update for new hasValue/removeValue interfaces.

(WebCore::EditingStyle::removeStyleAddedByNode):
Use auto (which is going to infer Ref now rather than RefPtr) and update
uses to pass non-null references.

(WebCore::EditingStyle::removeStyleConflictingWithStyleOfNode):
Adopt range-based for loop using the new iterator.

(WebCore::EditingStyle::conflictsWithInlineStyleOfElement const):
Adopt range-based for loop using the new iterator.

* Source/WebCore/editing/Editor.cpp:
(WebCore::Editor::fontAttributesAtSelectionStart):
Adopt new CSSValueID overloads of hasValue.

* Source/WebCore/page/PageSerializer.cpp:
(WebCore::PageSerializer::retrieveResourcesForProperties):
Adopt range-based for loop using the new iterator.

* Source/WebCore/style/ElementRuleCollector.cpp:
(WebCore::Style::ElementRuleCollector::addMatchedProperties):
Adopt range-based for loop using the new iterator.

* Source/WebCore/style/PropertyCascade.cpp:
(WebCore::Style::PropertyCascade::addMatch):
(WebCore::Style::hasImportantProperties):
Adopt range-based for loop using the new iterator.

* Source/WebCore/style/StyleResolver.cpp:
(WebCore::Style::Resolver::styleForKeyframe):
Adopt range-based for loop using the new iterator.

Canonical link: https://commits.webkit.org/257356@main

62c3aa9

Misc iOS, tvOS & watchOS macOS Linux Windows
❌ πŸ§ͺ style βœ… πŸ›  ios βœ… πŸ›  mac βœ… πŸ›  wpe βœ… πŸ›  πŸ§ͺ win
βœ… πŸ§ͺ bindings βœ… πŸ›  ios-sim βœ… πŸ›  mac-AS-debug βœ… πŸ›  gtk   πŸ›  wincairo
βœ… πŸ§ͺ webkitperl βœ… πŸ§ͺ ios-wk2 βœ… πŸ§ͺ api-mac ❌ πŸ§ͺ gtk-wk2
βœ… πŸ§ͺ api-ios ❌ πŸ§ͺ mac-wk1 βœ… πŸ§ͺ api-gtk
βœ… πŸ›  πŸ§ͺ jsc βœ… πŸ›  tv ❌ πŸ§ͺ mac-wk2 βœ… πŸ›  jsc-armv7
βœ… πŸ›  tv-sim ❌ πŸ§ͺ mac-AS-debug-wk2 βœ… πŸ§ͺ jsc-armv7-tests
  πŸ›  πŸ§ͺ merge βœ… πŸ›  watch βœ… πŸ§ͺ mac-wk2-stress βœ… πŸ›  jsc-mips
βœ… πŸ›  πŸ§ͺ unsafe-merge βœ… πŸ›  watch-sim βœ… πŸ§ͺ jsc-mips-tests

@weinig weinig self-assigned this Nov 30, 2022
@weinig weinig added the WebCore Misc. For miscellaneous bugs in the WebCore framework (and not JavaScriptCore or WebKit). label Nov 30, 2022
@weinig weinig marked this pull request as draft November 30, 2022 20:12
@@ -2271,8 +2271,8 @@ void KeyframeEffect::computeHasImplicitKeyframeForAcceleratedProperty()
HashSet<CSSPropertyID> explicitZeroProperties;
HashSet<CSSPropertyID> explicitOneProperties;
auto styleProperties = keyframe.style;
for (unsigned i = 0; i < styleProperties->propertyCount(); ++i) {
auto property = styleProperties->propertyAt(i).id();
for (auto propertyReference : styleProperties.get()) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

πŸŽ‰

for (unsigned i = 0; i < styleProperties->propertyCount(); ++i) {
auto property = styleProperties->propertyAt(i).id();
if (CSSPropertyAnimation::animationOfPropertyIsAccelerated(property))
for (auto property : styleProperties.get()) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🌭

bool hasRevert = false;
for (unsigned i = 0; i < propertyCount; ++i) {
auto propertyReference = keyframe.properties().propertyAt(i);
for (auto propertyReference : keyframe.properties()) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

πŸ’˜

@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Nov 30, 2022
@weinig weinig removed the merging-blocked Applied to prevent a change from being merged label Dec 1, 2022
@weinig weinig force-pushed the eng/Add-an-iterator-for-the-StyleProperties-family-of-classes branch from 9f61846 to 9a0837c Compare December 1, 2022 21:34
@weinig weinig force-pushed the eng/Add-an-iterator-for-the-StyleProperties-family-of-classes branch from 9a0837c to 2adf7eb Compare December 2, 2022 02:07
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Dec 2, 2022
@weinig weinig marked this pull request as ready for review December 2, 2022 17:26
@weinig weinig removed the merging-blocked Applied to prevent a change from being merged label Dec 3, 2022
@weinig weinig force-pushed the eng/Add-an-iterator-for-the-StyleProperties-family-of-classes branch from 2adf7eb to e734b74 Compare December 3, 2022 04:14
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Dec 3, 2022
@weinig weinig removed the merging-blocked Applied to prevent a change from being merged label Dec 3, 2022
@weinig weinig force-pushed the eng/Add-an-iterator-for-the-StyleProperties-family-of-classes branch from e734b74 to 62c3aa9 Compare December 3, 2022 22:25
@weinig weinig added the merge-queue Applied to send a pull request to merge-queue label Dec 4, 2022
@webkit-ews-buildbot webkit-ews-buildbot added merging-blocked Applied to prevent a change from being merged and removed merge-queue Applied to send a pull request to merge-queue labels Dec 4, 2022
@weinig weinig added unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing and removed merging-blocked Applied to prevent a change from being merged labels Dec 4, 2022
https://bugs.webkit.org/show_bug.cgi?id=248546
rdar://102823936

Reviewed by Antoine Quint.

Add iterator support for StyleProperties, ImmutableStyleProperties
and MutableStyleProperties to allow iterating through the stored
properties using for-in as well generic algorithms.

* Source/WTF/wtf/Vector.h:
(WTF::Mapper::map):
(WTF::CompactMapper::compactMap):
(WTF::copyToVectorSpecialization):
Use a universal reference to allow iterators that return temporaries
(like the one for StyleProperties) to work with the mapping code.

* Source/WebCore/animation/KeyframeEffect.cpp:
(WebCore::KeyframeEffect::computeHasImplicitKeyframeForAcceleratedProperty):
(WebCore::KeyframeEffect::computeHasKeyframeComposingAcceleratedProperty):
Adopt range-based for loop using the new iterator.

* Source/WebCore/css/CSSValueList.cpp:
(WebCore::CSSValueList::removeAll):
(WebCore::CSSValueList::hasValue const):
* Source/WebCore/css/CSSValueList.h:
Remove versions of removeAll() and hasValue() taking a nullable CSSValue*,
and replace them with overloads that take either a CSSValue& or a CSSValueID.
The CSSValueID allows us to check use without allocation or pool lookup.

* Source/WebCore/css/ComputedStyleExtractor.cpp:
(WebCore::ComputedStyleExtractor::fontVariantShorthandValue):
(WebCore::ComputedStyleExtractor::getCSSPropertyValuesForShorthandProperties):
(WebCore::ComputedStyleExtractor::getCSSPropertyValuesForGridShorthand):
Adopt range-based for loop using existing shorthand iterator support. Use
isValueID helper to simplify a condition.

* Source/WebCore/css/PropertySetCSSStyleDeclaration.cpp:
(WebCore::PropertySetCSSStyleDeclaration::length const):
Adopt range-based for loop using the new iterator.

* Source/WebCore/css/StyleProperties.cpp:
(WebCore::MutableStyleProperties::MutableStyleProperties):
(WebCore::StyleProperties::commonShorthandChecks const):
(WebCore::StyleProperties::asTextInternal const):
(WebCore::MutableStyleProperties::mergeAndOverrideOnConflict):
(WebCore::StyleProperties::traverseSubresources const):
* Source/WebCore/css/StyleProperties.h:
(WebCore::StyleProperties::Iterator::Iterator):
(WebCore::StyleProperties::Iterator::operator* const):
(WebCore::StyleProperties::Iterator::operator++):
(WebCore::StyleProperties::Iterator::operator== const):
(WebCore::StyleProperties::Iterator::operator!= const):
(WebCore::StyleProperties::begin const):
(WebCore::StyleProperties::end):
(WebCore::StyleProperties::size const):
Adds and adopts iterator support. The two subclasses of StyleProperties
overload begin() to allow specialization when the concrete type is known.

* Source/WebCore/css/parser/CSSPropertyParserHelpers.cpp:
(WebCore::CSSPropertyParserHelpers::consumeTouchAction):
(WebCore::CSSPropertyParserHelpers::consumeTextDecorationLine):
Adopt template consumeIdent and update for new reference taking hasValue().

* Source/WebCore/css/typedom/DeclaredStylePropertyMap.cpp:
(WebCore::DeclaredStylePropertyMap::entries const):
Adopt WTF::map() now that it can be used due to iterator for StyleProperties.

* Source/WebCore/editing/EditingStyle.cpp:
(WebCore::applyTextDecorationChangeToValueList):
Update for new hasValue/removeValue interfaces.

(WebCore::EditingStyle::removeStyleAddedByNode):
Use auto (which is going to infer Ref now rather than RefPtr) and update
uses to pass non-null references.

(WebCore::EditingStyle::removeStyleConflictingWithStyleOfNode):
Adopt range-based for loop using the new iterator.

(WebCore::EditingStyle::conflictsWithInlineStyleOfElement const):
Adopt range-based for loop using the new iterator.

* Source/WebCore/editing/Editor.cpp:
(WebCore::Editor::fontAttributesAtSelectionStart):
Adopt new CSSValueID overloads of hasValue.

* Source/WebCore/page/PageSerializer.cpp:
(WebCore::PageSerializer::retrieveResourcesForProperties):
Adopt range-based for loop using the new iterator.

* Source/WebCore/style/ElementRuleCollector.cpp:
(WebCore::Style::ElementRuleCollector::addMatchedProperties):
Adopt range-based for loop using the new iterator.

* Source/WebCore/style/PropertyCascade.cpp:
(WebCore::Style::PropertyCascade::addMatch):
(WebCore::Style::hasImportantProperties):
Adopt range-based for loop using the new iterator.

* Source/WebCore/style/StyleResolver.cpp:
(WebCore::Style::Resolver::styleForKeyframe):
Adopt range-based for loop using the new iterator.

Canonical link: https://commits.webkit.org/257356@main
@webkit-early-warning-system webkit-early-warning-system force-pushed the eng/Add-an-iterator-for-the-StyleProperties-family-of-classes branch from 62c3aa9 to bd076ef Compare December 4, 2022 16:46
@webkit-commit-queue
Copy link
Collaborator

Committed 257356@main (bd076ef): https://commits.webkit.org/257356@main

Reviewed commits have been landed. Closing PR #6981 and removing active labels.

@webkit-early-warning-system webkit-early-warning-system merged commit bd076ef into WebKit:main Dec 4, 2022
@webkit-commit-queue webkit-commit-queue removed the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label Dec 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
WebCore Misc. For miscellaneous bugs in the WebCore framework (and not JavaScriptCore or WebKit).
Projects
None yet
5 participants