1756187 - [wpt-sync] Sync PR 32904 - [@container] Make UpdateStyleAndLayoutTreeForNode understand CQ

Status: RESOLVED FIXED

(Core :: Layout, task, P4)

Description

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

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

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

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

[@container] Make UpdateStyleAndLayoutTreeForNode understand CQ

We currently have pretty serious bugs in getComputedStyle (and similar
APIs that force style), since NeedsLayoutTreeUpdateForNodeIncluding-
DisplayLocked is completely unaware of container queries: if nothing
is style dirty in the ancestor chain, we will early-out from
UpdateStyleAndLayoutTreeForNode, and never actually update anything.
However, we container queries, we have to also check for layout-
dirtiness, since doing that layout can affect container-query-
dependent elements.

This CL tries to formalize the concept of a "layout upgrade", which
is a name for when UpdateStyleAndLayoutTree needs to also call
UpdateStyleAndLayout due layout dependencies.

• NeedsLayoutTreeUpdateForNodeIncludingDisplayLocked needs to return
  true whenever we may need to upgrade. Otherwise we'll early-out,
  and never even get the chance to upgrade. Hence, we look a the
  target node and the ancestor chain, and try to figure out if a
  layout upgrade will be needed. We can not know this for sure
  at this time, because the subsequent call to update style may
  (for example) remove all container-query containers, i.e.
  remove layout dependencies.
• UpdateStyleAndLayoutTree now accepts a LayoutUpgrade object, which
  decides how or if an upgrade should take place. NodeLayoutUpgrade
  being the most interesting of these objects.
• NodeLayoutUpgrade::ShouldUpgrade does a second traversal of the
  ancestor chain to understand if an upgrade is needed. It is not
  possible to answer this question in the first traversal
  definitively, since style (first pass) has not been updated yet
  at that time.

For forced updates which target nodes in display:none (or elements
without ComputedStyle in general), we can not tell ahead-of-time
whether or not something may require an upgrade, since the
DependsOnContainerQueries exists on ComputedStyle. Hence, for those
situations we defensively assume that we require an upgrade if we're
inside a interleaving root [1].

Pseudo-elements also needed adjustment in this CL (the change in
ElementRuleCollector). getComputedStyle will call NeedsLayoutTree-
UpdateForNodeIncludingDisplayLocked with the originating element,
so it is not enough to mark the pseudo-style with as
DependsOnContainerQueries, since we never actually observe that style
in NeedsLayoutTreeUpdateForNode(etc). Hence we mark the originating
element as well, although this will cause some over-invalidation in
some cases. (It's possible to iterate on this if needed).

[1] Interleaving root = basically a container-for-container-queries
that is not display:none.

Fixed: 1295717
Change-Id: I2c7d3a82dd513b618ad5245df9b3b6cd7e306d9a
Reviewed-on: https://chromium-review.googlesource.com/3472067
WPT-Export-Revision: 293503b3af57340cc65292ae5df2747c997636ef

Comment 1

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

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

Comment 2

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

CI Results

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

Total 2 tests and 4 subtests

Status Summary

Firefox

ERROR: 2

Chrome

ERROR: 2

Safari

OK : 2
PASS : 5
FAIL : 17

Links

Gecko CI (Treeherder)
GitHub PR Head
GitHub PR Base

Details

New Tests That Don't Pass

/css/css-contain/container-queries/display-none.html: ERROR (Chrome: ERROR, Safari: OK)
/css/css-contain/container-queries/sibling-layout-dependency.html: ERROR (Chrome: ERROR, Safari: OK)

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/714bf2083214
[wpt PR 32904] - [@container] Make UpdateStyleAndLayoutTreeForNode understand CQ, a=testonly
https://hg.mozilla.org/integration/autoland/rev/fd3aa6102761
[wpt PR 32904] - Update wpt metadata, a=testonly

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6468c2527fb2
[wpt PR 32904] - [@container] Make UpdateStyleAndLayoutTreeForNode understand CQ, a=testonly
https://hg.mozilla.org/integration/autoland/rev/b1bfe1bedc24
[wpt PR 32904] - Update wpt metadata, a=testonly

Comment 5

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/6468c2527fb2
https://hg.mozilla.org/mozilla-central/rev/b1bfe1bedc24