Bug 229915

Created attachment 437344 [details] Patch

*** Bug 229899 has been marked as a duplicate of this bug. ***

Comment on attachment 437344 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=437344&action=review > Source/WebCore/css/makeprop.pl:1307 > - $setterContent .= $indent . "static void applyValue" . $nameToId{$name} . "(BuilderState& builderState, CSSValue& value)\n"; > + $setterContent .= $indent . "static void applyValue" . $nameToId{$name} . "(CSSPropertyID property, BuilderState& builderState, CSSValue& value)\n"; > $setterContent .= $indent . "{\n"; > + > + if (!exists $propertiesWithStyleBuilderOptions{$name}{"fill-layer-property"}) { > + $setterContent .= $indent . " UNUSED_PARAM(property);\n"; > + } If I understand correctly this adds the property argument to every one of the 647 generated applyValueFoo functions while it can be statically determined in all cases except one. This doesn't seem ideal. The code generation could be check for 'synonym' concept and only generate the property parameter for it. > Source/WebCore/css/makeprop.pl:1385 > + next if (exists $synonyms{$name}); I suppose an alternative would be to just generate a separate function for the synonym and do the synonym name mapping when generating the function. This would avoid having to pass the propertyid dynamically and might be generally simpler.

Created attachment 437364 [details] Patch

(In reply to Antti Koivisto from comment #3) > > Source/WebCore/css/makeprop.pl:1385 > > + next if (exists $synonyms{$name}); > > I suppose an alternative would be to just generate a separate function for > the synonym and do the synonym name mapping when generating the function. > This would avoid having to pass the propertyid dynamically and might be > generally simpler. I decided for now it was simplest just to pass the CSSPropertyID as an argument to all the fill layer value applier functions, since their implementations already had a hardcoded propertyID that can now just use the argument.

Created attachment 437367 [details] Patch

Comment on attachment 437367 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=437367&action=review > Source/WebCore/css/makeprop.pl:306 > $nameToAliases{$name} = $codegenProperties->{"aliases"}; > + } elsif ($codegenOptionName eq "synonym") { > + $synonyms{$name} = 1; > + push @{$namesToSynonyms{$codegenProperties->{"synonym"}}}, $name; > } elsif ($styleBuilderOptions{$codegenOptionName}) { weird indentation

<rdar://problem/82795719>

(In reply to Simon Fraser (smfr) from comment #8) > https://trac.webkit.org/changeset/282058/webkit This seems to have broken two layout tests on iOS: - imported/w3c/web-platform-tests/css/cssom/cssstyledeclaration-csstext.html - imported/w3c/web-platform-tests/css/cssom/getComputedStyle-detached-subtree.html History: https://results.webkit.org/?suite=layout-tests&suite=layout-tests&test=imported%2Fw3c%2Fweb-platform-tests%2Fcss%2Fcssom%2Fcssstyledeclaration-csstext.html&test=imported%2Fw3c%2Fweb-platform-tests%2Fcss%2Fcssom%2FgetComputedStyle-detached-subtree.html EWS indicated these failures on previous patch in https://ews-build.webkit.org/#/builders/51/builds/20605 Patch was landed manually and might have been different from the one which passed EWS.

Separately, please try to use commit-queue. We are trying to discourage manual commits. If you need to land a patch quickly, you can use fast-cq mode (See https://lists.webkit.org/pipermail/webkit-dev/2021-April/031782.html).

Re-opened since this is blocked by bug 229978

Created attachment 437447 [details] Patch

Created attachment 437509 [details] Patch

Committed r282143 (241440@main): <https://commits.webkit.org/241440@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 437509 [details].