[Blink] Reset finished SVG image animations on reload [chromium/src : main]

0 views
Skip to first unread message

zhenbang Jiang (Gerrit)

unread,
Apr 16, 2026, 10:40:26 PM (5 days ago) Apr 16
to Kevin Ellis, android-bu...@system.gserviceaccount.com, Chromium LUCI CQ, Menard, Alexis, chromium...@chromium.org, Dirk Schulze, Fredrik Söderquist, Olga Gerchikov, Nate Chapin, Stephen Chenney, blink-rev...@chromium.org, blink-revie...@chromium.org, blink-...@chromium.org, fmalit...@chromium.org, gavinp...@chromium.org, kouhe...@chromium.org, loading...@chromium.org, pdr+svgw...@chromium.org
Attention needed from Kevin Ellis

zhenbang Jiang added 1 comment

Patchset-level comments
File-level comment, Patchset 5 (Latest):
zhenbang Jiang . resolved

Hi, could you take a look at this? thanks!

Open in Gerrit

Related details

Attention is currently required from:
  • Kevin Ellis
Submit Requirements:
  • requirement satisfiedCode-Coverage
  • requirement is not satisfiedCode-Owners
  • requirement is not satisfiedCode-Review
  • requirement is not satisfiedReview-Enforcement
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
Gerrit-MessageType: comment
Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: I662712aeac3f523dc9c39dbe4711a318cc6156d0
Gerrit-Change-Number: 7760909
Gerrit-PatchSet: 5
Gerrit-Owner: zhenbang Jiang <edom...@gmail.com>
Gerrit-Reviewer: Kevin Ellis <kev...@chromium.org>
Gerrit-Reviewer: zhenbang Jiang <edom...@gmail.com>
Gerrit-CC: Dirk Schulze <dsch...@chromium.org>
Gerrit-CC: Fredrik Söderquist <f...@opera.com>
Gerrit-CC: Menard, Alexis <alexis...@intel.com>
Gerrit-CC: Nate Chapin <jap...@chromium.org>
Gerrit-CC: Olga Gerchikov <gerc...@microsoft.com>
Gerrit-CC: Stephen Chenney <sche...@chromium.org>
Gerrit-Attention: Kevin Ellis <kev...@chromium.org>
Gerrit-Comment-Date: Fri, 17 Apr 2026 02:39:51 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
satisfied_requirement
unsatisfied_requirement
open
diffy

Fredrik Söderquist (Gerrit)

unread,
Apr 17, 2026, 7:40:27 AM (5 days ago) Apr 17
to zhenbang Jiang, Kevin Ellis, android-bu...@system.gserviceaccount.com, Chromium LUCI CQ, Menard, Alexis, chromium...@chromium.org, Dirk Schulze, Olga Gerchikov, Nate Chapin, Stephen Chenney, blink-rev...@chromium.org, blink-revie...@chromium.org, blink-...@chromium.org, fmalit...@chromium.org, gavinp...@chromium.org, kouhe...@chromium.org, loading...@chromium.org, pdr+svgw...@chromium.org
Attention needed from Kevin Ellis and zhenbang Jiang

Fredrik Söderquist added 1 comment

File third_party/blink/renderer/core/loader/image_loader.cc
Line 907, Patchset 5 (Latest):void ImageLoader::ResetAnimationOrDeferUntilAttach() {
Fredrik Söderquist . unresolved

I don't think there's supposed to be any connection between resetting an animation and whether the animation is visible at all, so I suspect what you want to do here is rather to separate resetting the animation and potentially triggering invalidation (if needed).

Open in Gerrit

Related details

Attention is currently required from:
  • Kevin Ellis
  • zhenbang Jiang
Submit Requirements:
    • requirement satisfiedCode-Coverage
    • requirement is not satisfiedCode-Owners
    • requirement is not satisfiedCode-Review
    • requirement is not satisfiedNo-Unresolved-Comments
    • requirement is not satisfiedReview-Enforcement
    Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
    Gerrit-MessageType: comment
    Gerrit-Project: chromium/src
    Gerrit-Branch: main
    Gerrit-Change-Id: I662712aeac3f523dc9c39dbe4711a318cc6156d0
    Gerrit-Change-Number: 7760909
    Gerrit-PatchSet: 5
    Gerrit-Owner: zhenbang Jiang <edom...@gmail.com>
    Gerrit-Reviewer: Kevin Ellis <kev...@chromium.org>
    Gerrit-Reviewer: zhenbang Jiang <edom...@gmail.com>
    Gerrit-CC: Dirk Schulze <dsch...@chromium.org>
    Gerrit-CC: Fredrik Söderquist <f...@opera.com>
    Gerrit-CC: Menard, Alexis <alexis...@intel.com>
    Gerrit-CC: Nate Chapin <jap...@chromium.org>
    Gerrit-CC: Olga Gerchikov <gerc...@microsoft.com>
    Gerrit-CC: Stephen Chenney <sche...@chromium.org>
    Gerrit-Attention: Kevin Ellis <kev...@chromium.org>
    Gerrit-Attention: zhenbang Jiang <edom...@gmail.com>
    Gerrit-Comment-Date: Fri, 17 Apr 2026 11:40:20 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: No
    satisfied_requirement
    unsatisfied_requirement
    open
    diffy

    Kevin Ellis (Gerrit)

    unread,
    Apr 21, 2026, 10:18:18 AM (22 hours ago) Apr 21
    to zhenbang Jiang, android-bu...@system.gserviceaccount.com, Chromium LUCI CQ, Menard, Alexis, chromium...@chromium.org, Dirk Schulze, Fredrik Söderquist, Olga Gerchikov, Nate Chapin, Stephen Chenney, blink-rev...@chromium.org, blink-revie...@chromium.org, blink-...@chromium.org, fmalit...@chromium.org, gavinp...@chromium.org, kouhe...@chromium.org, loading...@chromium.org, pdr+svgw...@chromium.org
    Attention needed from zhenbang Jiang

    Kevin Ellis added 2 comments

    File third_party/blink/renderer/core/animation/document_animations.cc
    Line 274, Patchset 5 (Latest): if (timeline->HasAnimations()) {
    Kevin Ellis . unresolved

    Not sure we need another HasAnimations method. We already have Element::HasAnimations(), Document::getAnimations(), Element::getAnimations(), and Element::GetElementAnimations().

    This method would return true if any element in the document has an animation, or even if it recently had an animation that simply hasn't been garbage collected yet. Methinks Element::GetElementAnimations() provides what you need.

    File third_party/blink/renderer/core/svg/graphics/svg_image.cc
    Line 574, Patchset 5 (Latest): document.GetDocumentAnimations().HasAnimations();
    Kevin Ellis . unresolved

    Element already has a HasAnimations method.

    root_element->HasAnimations()

    Open in Gerrit

    Related details

    Attention is currently required from:
    • zhenbang Jiang
    Gerrit-Attention: zhenbang Jiang <edom...@gmail.com>
    Gerrit-Comment-Date: Tue, 21 Apr 2026 14:18:12 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: No
    satisfied_requirement
    unsatisfied_requirement
    open
    diffy
    Reply all
    Reply to author
    Forward
    0 new messages