Bug 1625894 [wpt PR 22527] - Let StyleCascade own MatchResult and interpolations...
commit736bbded6a7235c3a1c8e673738971b75b9b6bae
authorAnders Hartvoll Ruud <andruud@chromium.org>
Tue, 31 Mar 2020 12:22:37 +0000 (31 12:22 +0000)
committermoz-wptsync-bot <wptsync@mozilla.com>
Thu, 2 Apr 2020 10:49:40 +0000 (2 10:49 +0000)
tree8a58aeb11b02c52412de376d356f9a2e69904a4a
parentb1af6c3a0d5733516994318ee599076b4c348489
Bug 1625894 [wpt PR 22527] - Let StyleCascade own MatchResult and interpolations, a=testonly

Automatic update from web-platform-tests
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/c/chromium/src/+/2124649
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Commit-Queue: Anders Hartvoll Ruud <andruud@chromium.org>
Cr-Commit-Position: refs/heads/master@{#754457}

--

wpt-commits: 724eb3d3feb35cb1d762b9196842917aafaf0399
wpt-pr: 22527
testing/web-platform/tests/css/css-animations/animation-base-response-003.html [new file with mode: 0644]
testing/web-platform/tests/css/css-transitions/transition-base-response-003.html [new file with mode: 0644]