1705517 - [wpt-sync] Sync PR 28525 - Revert "Include custom properties on computed CSSStyleDeclaration"

Status: RESOLVED FIXED

(Core :: DOM: CSS Object Model, task, P4)

Description

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

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

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

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

Revert "Include custom properties on computed CSSStyleDeclaration"

This reverts commit 3b8999adcdc905f13b22bbde6b198f158cb9d6c7.

Reason for revert: Huge performance regressions.

Bug: 949807
Fixed: 1199142

Original change's description:

Include custom properties on computed CSSStyleDeclaration

Since we need a "stable" list of custom properties for length()
and item(), and we also need not-abysmal performance when calling
those functions, this CL caches a vector with the variables names
on ComputedStyle itself.

In CSSComputedStyleDeclaration::item(), we check if the incoming
index is in the range of the standard properties, and if so return
the appropriate one. Otherwise, we're in the variable range (which
is defined per spec to appear after the standard properties), and
we'll fetch the list of variable names.

Unfortunately the inclusion of custom properties requires a clean
style, which means length()/item() now updates style/layout as needed.
This might cause performance regressions, but I don't see a way around
this.

Note: ComputedStyle::StyleInheritedVariables/NonInheritedVariables
were changed to return const pointers, as to not "leak mutability",
which would have made it hard to invalidate the cache. (Nothing
requires those return values to be non-const after StyleCascade
anyway).

Note: ComputedStyle::GetVariableNames() has pretty good test coverage
in ComputedStyleTest already. Added a couple of new ones that target
cache invalidation.

Fixed: 949807
Change-Id: I6b181af5c4f025c4fd433de0a6ca221055c16693
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2822260
Commit-Queue: Anders Hartvoll Ruud \<[e-mail]>
Reviewed-by: Rune Lillesveen \<[e-mail]>
Cr-Commit-Position: refs/heads/master@{#872446}

Change-Id: I071cc2e00838a17407fc0d1e5745db9894d59ceb

Reviewed-on: https://chromium-review.googlesource.com/2829151
WPT-Export-Revision: a8a8c3666cfbe889081ebca5638b0c2a8f897289

Comment 1

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

PR 28525 applied with additional changes from upstream: 72e8d5c2463fb0109f53a3a4357a998cb66d391b

Comment 2

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

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

Comment 3

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

CI Results

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

Total 1 tests and 7 subtests

Status Summary

Firefox

ERROR: 1

Chrome

OK : 1
PASS : 7

Safari

ERROR: 1

Links

Gecko CI (Treeherder)
GitHub PR Head
GitHub PR Base

Details

New Tests That Don't Pass

/css/css-typed-om/the-stylepropertymap/computed/computed.tentative.html: ERROR [GitHub], SKIP [Gecko-android-em-7.0-x86_64-debug-geckoview, Gecko-android-em-7.0-x86_64-opt-geckoview, Gecko-android-em-7.0-x86_64-qr-debug-geckoview, Gecko-android-em-7.0-x86_64-qr-opt-geckoview, Gecko-linux1804-64-debug, Gecko-linux1804-64-opt, Gecko-linux1804-64-qr-debug, Gecko-linux1804-64-qr-opt, Gecko-linux1804-64-tsan-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, Safari: ERROR)

Tests Disabled in Gecko Infrastructure

/css/css-typed-om/the-stylepropertymap/computed/computed.tentative.html: ERROR [GitHub], SKIP [Gecko-android-em-7.0-x86_64-debug-geckoview, Gecko-android-em-7.0-x86_64-opt-geckoview, Gecko-android-em-7.0-x86_64-qr-debug-geckoview, Gecko-android-em-7.0-x86_64-qr-opt-geckoview, Gecko-linux1804-64-debug, Gecko-linux1804-64-opt, Gecko-linux1804-64-qr-debug, Gecko-linux1804-64-qr-opt, Gecko-linux1804-64-tsan-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, Safari: ERROR)

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/128bb9ccf9c9
[wpt PR 28525] - Revert "Include custom properties on computed CSSStyleDeclaration", a=testonly

Comment 5

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/128bb9ccf9c9