Bug 228954 - REGRESSION (r275756): Accelerated animations freeze when invalidating layout with shadow dom
Summary: REGRESSION (r275756): Accelerated animations freeze when invalidating layout ...
Alias: None
Product: WebKit
Classification: Unclassified
Component: Animations (show other bugs)
Version: Safari Technology Preview
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
Keywords: InRadar
Depends on:
Reported: 2021-08-10 07:22 PDT by Liam DeBeasi
Modified: 2021-08-16 22:34 PDT (History)
15 users (show)

See Also:

Code reproduction (1.12 KB, text/html)
2021-08-10 07:22 PDT, Liam DeBeasi
no flags Details
patch (8.90 KB, patch)
2021-08-13 04:58 PDT, Antti Koivisto
no flags Details | Formatted Diff | Diff
patch (8.94 KB, patch)
2021-08-13 04:59 PDT, Antti Koivisto
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
patch (8.63 KB, patch)
2021-08-16 04:32 PDT, Antti Koivisto
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Liam DeBeasi 2021-08-10 07:22:25 PDT
Created attachment 435256 [details]
Code reproduction

When invalidating the layout on a component with the Shadow DOM, any accelerated animations running on the component temporarily freeze. This issue was introduced around iOS 14.6. This issue can happen in Ionic apps when a page transition is occurring and developers try to render content on the entering view while the animation is playing (see: https://github.com/ionic-team/ionic-framework/issues/23732).

Steps to reproduce:

1. Open code reproduction on an iOS device running iOS 14.6 or later.
2. Click the "Start Animation" button.
3. Content should translate from the right to the left but should freeze about 100ms in because the layout was invalidated.
4. Repeat steps 1-2 on an iOS device running iOS 14.5 or older. You should notice that the animation does not freeze.

Expected Behavior:

I would expect the animation not to freeze when invalidating the layout.

Actual Behavior:

The animation freezes when invalidating the layout.

Other Information:

- This only happens with accelerated animations.
- This does not happen on Chrome or Firefox.
- This does not reproduce on Safari for macOS on Version 14.1 (16611. However, the issue does reproduce in STP 128.
- Possibly related to an older bug that fixed a similar issue: https://bugs.webkit.org/show_bug.cgi?id=201048
Comment 1 Liam DeBeasi 2021-08-10 07:30:06 PDT
I should also mention that if you remove Shadow DOM from the component, then the issue no longer reproduces.
Comment 2 Simon Fraser (smfr) 2021-08-10 10:19:57 PDT
Bisected this to https://trac.webkit.org/changeset/275756/webkit
Comment 3 Radar WebKit Bug Importer 2021-08-10 10:29:22 PDT
Comment 4 Antti Koivisto 2021-08-11 07:34:12 PDT
The test replaces the children of the shadow host that has animation running. This triggers unconditional render tree rebuild, including the host, and interrupts the animation.

  * frame #0: 0x00000001436a7210 WebCore`WebCore::SlotAssignment::didChangeSlot(this=0x000000010ba20a68, slotAttrValue=0x0000000132a80008, shadowRoot=0x0000000104b6abc0) at SlotAssignment.cpp:311:5
    frame #1: 0x000000014356aa34 WebCore`WebCore::ShadowRoot::didChangeDefaultSlot(this=0x0000000104b6abc0) at SlotAssignment.h:131:27
    frame #2: 0x000000014356a704 WebCore`WebCore::Element::childrenChanged(this=0x0000000104b6aa60, change=0x000000016b4879d0) at Element.cpp:2700:25
    frame #3: 0x00000001439314bc WebCore`WebCore::HTMLElement::childrenChanged(this=0x0000000104b6aa60, change=0x000000016b4879d0) at HTMLElement.cpp:892:20
    frame #4: 0x00000001434333e0 WebCore`void WebCore::executeNodeInsertionWithScriptAssertion<WebCore::ContainerNode::appendChildWithoutPreInsertionValidityCheck(WebCore::Node&)::$_4>(containerNode=0x0000000104b6aa60, child=0x000000010597e490, source=API, replacedAllChildren=No, doNodeInsertion=(anonymous class) @ 0x000000016b487b10)::$_4) at ContainerNode.cpp:228:23
    frame #5: 0x0000000143430998 WebCore`WebCore::ContainerNode::appendChildWithoutPreInsertionValidityCheck(this=0x0000000104b6aa60, newChild=0x0000000105982360) at ContainerNode.cpp:766:9
    frame #6: 0x00000001434331b4 WebCore`WebCore::ContainerNode::appendChild(this=0x0000000104b6aa60, newChild=0x0000000105982360) at ContainerNode.cpp:732:12
    frame #7: 0x000000014383f1a0 WebCore`WebCore::replaceChildrenWithFragment(container=0x0000000104b6aa60, fragment=0x000000016b487e30) at markup.cpp:1397:34
    frame #8: 0x000000014356e6c4 WebCore`WebCore::Element::setInnerHTML(this=0x0000000104b6aa60, html=0x000000016b487f90) at Element.cpp:3305:12
    frame #9: 0x000000014137dce4 WebCore`WebCore::setJSElement_innerHTMLSetter(this=0x000000016b487f68)::'lambda'()::operator()() const at JSElement.cpp:2956:21
Comment 5 Antti Koivisto 2021-08-13 04:58:06 PDT
Created attachment 435475 [details]
Comment 6 Antti Koivisto 2021-08-13 04:59:22 PDT
Created attachment 435476 [details]
Comment 7 Antti Koivisto 2021-08-16 04:32:11 PDT
Created attachment 435588 [details]
Comment 8 Ryosuke Niwa 2021-08-16 11:43:38 PDT
Comment on attachment 435588 [details]

View in context: https://bugs.webkit.org/attachment.cgi?id=435588&action=review

> LayoutTests/animations/shadow-host-child-change.html:1
> +

Do we need these blank lines?
Comment 9 EWS 2021-08-16 22:34:11 PDT
Committed r281128 (240582@main): <https://commits.webkit.org/240582@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 435588 [details].