Skip to content

merge_pr_50907

Compare
Choose a tag to compare
@github-actions github-actions released this 24 Feb 14:15

[functions] Let parameter defaults reference other parameters

Per "evaluate a custom function" [1], the default value of a parameter
may "see" other parameters, e.g. @function --f(--x, --y:var(--x)).
Before this CL, that var(--x) would resolve in the outer scope,
but now it's supposed to resolve against the actual argument value
passed for --x. At the same time, default values should not have
access to the actual locals of the function body, meaning that
they resolve in some gray area between the outer scope and inner
scope.

In this CL, that "gray area" is implemented by using a disposable
FunctionContext with the unresolved defaults set as the unresolved
locals. This is convenient, because we get all the needed "dependency
handling" (including cycle detection) for free.

Since various things now need to parse against a type (arguments,
defaults, and the 'result' descriptor), and because these various
things differ in how they are represented (some CSSVariableData,
some StringView), this CL aligns all of those things to use
CSSVariableData, and adjusts ResolveFunctionExpression accordingly.
This would be needed anyway, to actually transport various tainting
information, though that is not explicit handled/tested in this CL.

Finally, note that there is a drive-by fix in this CL: the "already
applied" branch of ApplyLocalVariables now actually skips the work
if it's already applied. This should ideally have a performance test,
which is tracked as a follow-up task (Issue 397164440).

[1] https://drafts.csswg.org/css-mixins-1/#evaluate-a-custom-function

Fixed: 397459622
Bug: 325504770, 397164440
Change-Id: Ic705209d25f4bee6149f2397bda78e1224b16901
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6276516
Commit-Queue: Anders Hartvoll Ruud andruud@chromium.org
Reviewed-by: Steinar H Gunderson sesse@chromium.org
Cr-Commit-Position: refs/heads/main@{#1423855}