1625894 - [wpt-sync] Sync PR 22527 - Let StyleCascade own MatchResult and interpolations

Status: RESOLVED FIXED

(Core :: CSS Transitions and Animations, task, P4)

Description

[Web Platform Test Sync Bot (Matrix: #interop:mozilla.org)]

Sync web-platform-tests PR 22527 into mozilla-central (this bug is closed when the sync is complete).

PR: https://github.com/web-platform-tests/wpt/pull/22527
Details from upstream follow.

Anders Hartvoll Ruud <[e-mail]> wrote:

Let StyleCascade own MatchResult and interpolations

There is currently a bug where font-relative units in the base style
are not responsive to font-affecting animations (even with issue 437689
fixed) if the element hits the MatchedPropertiesCache. This can
easily happen if two identical elements (with the same MatchResult)
undergo the same CSS animation.

The reason this bug exists is in StyleResolver::
CascadeAndApplyMatchedProperties. Notice that, when
cache_success.IsFullCacheHit() is true in that function, we return
immediately, and never Analyze the MatchResult. In other words, the
MatchResult is there, fully populated (we're using it as the cache
key after all), but we never let the StyleCascade actually Analyze it,
which is the source of the bug. When we later apply animations
(see StyleResolver::ApplyAnimatedStandardProperties), we Analyze the
animations/transitions only. This was done on purpose, as too much
Analyzing is expensive. Hence, during ApplyAnimatedStandardProperties,
I assumed that either:

• We would hit the base computed style optimization, and hence the
  MatchResult would be empty, and there is no need to analyze it, or
• We would not hit the base computed style optimization, and the
  MatchResult object would be populated and Analyzed already.

But of course, I forgot about the third option: based computed style
optimization miss, but a hit on the matched properties cache. In that
case the we have a non-empty MatchResult that haven't been Analyzed.

To fix this, we could have simply Analyzed the MatchResult in
ApplyAnimatedStandardProperties (potentially a second time). But this
is inefficient. I don't want to do that.

Instead, this CL moves the MatchResult ownership (and
CascadeInterpolation ownership) to StyleCascade itself. It also adds
flags which tells us what needs to be analyzed. Using this, we can
now analyze-on-demand during Apply, which means we analyze exactly the
amount we need to analyze, also in the case where an animation frame
hits the MatchedProperties cache.

It also makes for a far more sensible (but still not perfect)
StyleCascade API, compared to "manually" analyzing.

Note that this CL also removes filtering Analyze-time. This means
that e.g. for ::marker, we add declarations to the CascadeMap that
we know aren't going to be applied (as they are filtered in the Apply
step). This is because analyze-on-demand is not easily compatible with
this kind of filtering. For example, when dealing with inherited-only
cache hits, we first try to apply the inherited properties only.
If the font or zoom was modified, we need to apply the non-inherited
properties after all. And for that second apply pass, we obviously need
all properties to have been analyzed before, not just the inherited
properties.

Bug: 1065387, 437689
Change-Id: I7e9b7f33fc12f055603e789d3ae700d5a7a3dbbf

Reviewed-on: https://chromium-review.googlesource.com/2124649
WPT-Export-Revision: 597e64b04da937037d989c9ed878dea263c4cd70

Comment 1

[Web Platform Test Sync Bot (Matrix: #interop:mozilla.org)]

Pushed to try (stability) https://treeherder.mozilla.org/#/jobs?repo=try&revision=269f0ab3ddba7bd562c588b2f10bc2dc7cb55159

Comment 2

[Web Platform Test Sync Bot (Matrix: #interop:mozilla.org)]

CI Results

Ran 13 Firefox configurations based on mozilla-central, and Firefox, Chrome, and Safari on GitHub CI

Total 49 tests

Status Summary

Firefox

OK : 2
PASS: 2[GitHub] 49[Gecko-android-em-7.0-x86_64-debug-geckoview, Gecko-android-em-7.0-x86_64-opt-geckoview, Gecko-linux1804-64-asan-opt, Gecko-linux1804-64-debug, Gecko-linux1804-64-opt, Gecko-linux1804-64-qr-debug, Gecko-linux1804-64-qr-opt, Gecko-windows10-64-debug, Gecko-windows10-64-opt, Gecko-windows10-64-qr-debug, Gecko-windows10-64-qr-opt, Gecko-windows7-32-debug, Gecko-windows7-32-opt]

Chrome

OK : 2
FAIL: 2

Safari

OK : 2
FAIL: 2

Links

Gecko CI (Treeherder)
GitHub PR Head
GitHub PR Base

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7e562a970b72
[wpt PR 22527] - Let StyleCascade own MatchResult and interpolations, a=testonly

Comment 4

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/7e562a970b72