New issue
Advanced search Search tips

Issue 918276 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jan 1
Cc:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Mac
Pri: 2
Type: Bug-Regression



Sign in to add a comment

SVG continuously repainting

Reported by dens.sas...@gmail.com, Dec 30

Issue description

UserAgent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_14_2) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/73.0.3655.0 Safari/537.36

Steps to reproduce the problem:
1. Go to https://codepen.io/SaschaDens/pen/roYBPL
2. Enable in devtools rendering: Paint flashing

What is the expected behavior?
Similar as Chrome Version 71.0.3578.98 where the same code doesn't trigger paints without any animation or browser resizing.

What went wrong?
When creating two image tags pointing to the same resource URL and applying on one of the SVG's a height: <x>vh; the browser is continuously repainting all the SVG's (See attachement).

Did this work before? Yes 71.0.3578.98

Chrome version: 73.0.3655.0  Channel: canary
OS Version: OS X 10.14.2
Flash Version:
 
SVG_continuously_drawing.mov
8.4 MB View Download
TE@, the expected behavior is "both rectangles are gray"

Bisected to: 618425 (good) - 618429 (bad)
https://chromium.googlesource.com/chromium/src/+log/c08ae9e1..ba6fc418?pretty=fuller
Suspecting r618428 = a465e8196b0d3b34cdc2e3acb7b8e79a1d0e4591 = crrev.com/c/1387465 by wangxianzhu@chromium.org
"[PE] Fix SVGImage performance regression"
Landed in 73.0.3647.0
Labels: Needs-Triage-M73 Needs-Bisect
Cc: santhoshkumar@chromium.org
Labels: -Needs-Bisect Release-Block-Stable RegressedIn-73 Triaged-ET Target-73 M-73 FoundIn-73 hasbisect OS-Linux OS-Windows
Owner: wangxianzhu@chromium.org
Status: Assigned (was: Unconfirmed)
Able to reproduce the issue on reported chrome version #73.0.3655.0, and latest chrome #73.0.3656.0 using Mac 10.12 ,windows and Linux by following steps as per comment#0. 

Bisect Information:
----------------------------
Good Build: 73.0.3646.0
Bad Build: 73.0.3647.0

From comment #1 Confirming Suspect: r618428 = a465e8196b0d3b34cdc2e3acb7b8e79a1d0e4591

@Xianzhu :Please help us in re-assigning if this is not related to your change.
Adding Release-Block-Stable as this is recent regression. Please feel free to remove if not applicable.
Thanks
Cc: schenney@chromium.org
Though the issue occurred after r618428, I believe the actual cause is r618316 which may invalidate the container document of an SVGImage if the image document doesn't finish a whole document lifecycle update (to paint). Will revert r618316.
Project Member

Comment 5 by bugdroid1@chromium.org, Jan 1

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0401c8beeec988a40346839c6386c510e6728047

commit 0401c8beeec988a40346839c6386c510e6728047
Author: Xianzhu Wang <wangxianzhu@chromium.org>
Date: Tue Jan 01 02:15:29 2019

Revert "[CompositeAfterPaint] Invalidate chrome client when needed"

This reverts commit 29021ede403d79c59ed4016b5f5fa9ef765b62b6.

Reason for revert: Suspect performance regression

Bug:  918276 

Original change's description:
> [CompositeAfterPaint] Invalidate chrome client when needed
> 
> For pre-CompositeAfterPaint, we call InvalidateChromeClient()
> when an object is invalidated in a view referenced from a plugin or
> an svg-image.
> 
> Now let the path also work for CompositeAfterPaint.
> 
> This fixes several web plugin tests for CompositeAfterPaint.
> 
> Bug: 524134
> Change-Id: Ia4c3ccba4632ddcdfdfdfec299fb7a68440a1419
> Reviewed-on: https://chromium-review.googlesource.com/c/1385112
> Reviewed-by: Philip Rogers <pdr@chromium.org>
> Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#618316}

TBR=wangxianzhu@chromium.org,pdr@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 524134
Change-Id: I9b4f67a74732919ba23acc00fd501d45baa498e6
Reviewed-on: https://chromium-review.googlesource.com/c/1392439
Reviewed-by: Xianzhu Wang <wangxianzhu@chromium.org>
Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#619326}
[modify] https://crrev.com/0401c8beeec988a40346839c6386c510e6728047/third_party/blink/renderer/core/paint/paint_invalidator.cc
[modify] https://crrev.com/0401c8beeec988a40346839c6386c510e6728047/third_party/blink/renderer/core/paint/pre_paint_tree_walk.cc
[modify] https://crrev.com/0401c8beeec988a40346839c6386c510e6728047/third_party/blink/web_tests/FlagExpectations/enable-blink-features=CompositeAfterPaint

Status: Fixed (was: Assigned)
Project Member

Comment 7 by bugdroid1@chromium.org, Jan 3

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/eaaaf143f6fcedd0897b6d30e55bb5893a66c996

commit eaaaf143f6fcedd0897b6d30e55bb5893a66c996
Author: Xianzhu Wang <wangxianzhu@chromium.org>
Date: Thu Jan 03 20:16:45 2019

Reland "[CompositeAfterPaint] Invalidate chrome client when needed"

This is a reland of crrev.com/618316 which was reverted in
crrev.com/619326 because of performance regression. This patch
changed the condition in PrePaintTreeWalk::WalkTree() from
 if (root_frame_view.GetLayoutView()->Layer()->NeedsRepaint())
to
 if (needs_invalidate_chrome_client_).
needs_invalidate_chrome_client_ is set by PaintInvalidator.

The problem of previous code was that NeedsRepaint() is always
true if the document doesn't update lifecycle to paint which is
the case for the subdocument of SVGImage, causing infinite
invalidation of the chrome client.

Original change's description:
> Revert "[CompositeAfterPaint] Invalidate chrome client when needed"
>
> This reverts commit 29021ede403d79c59ed4016b5f5fa9ef765b62b6.
>
> Reason for revert: Suspect performance regression
>
> Bug:  918276 
>
> Original change's description:
> > [CompositeAfterPaint] Invalidate chrome client when needed
> >
> > For pre-CompositeAfterPaint, we call InvalidateChromeClient()
> > when an object is invalidated in a view referenced from a plugin or
> > an svg-image.
> >
> > Now let the path also work for CompositeAfterPaint.
> >
> > This fixes several web plugin tests for CompositeAfterPaint.
> >
> > Bug: 524134
> > Change-Id: Ia4c3ccba4632ddcdfdfdfec299fb7a68440a1419
> > Reviewed-on: https://chromium-review.googlesource.com/c/1385112
> > Reviewed-by: Philip Rogers <pdr@chromium.org>
> > Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#618316}
>
> TBR=wangxianzhu@chromium.org,pdr@chromium.org
>
> # Not skipping CQ checks because original CL landed > 1 day ago.
>
> Bug: 524134
> Change-Id: I9b4f67a74732919ba23acc00fd501d45baa498e6
> Reviewed-on: https://chromium-review.googlesource.com/c/1392439
> Reviewed-by: Xianzhu Wang <wangxianzhu@chromium.org>
> Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#619326}

Bug:  918276 , 524134
Change-Id: Ifad1c2eb8bdddcceea26a286c3309665f6d94c1a
Reviewed-on: https://chromium-review.googlesource.com/c/1393558
Reviewed-by: Philip Rogers <pdr@chromium.org>
Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#619733}
[modify] https://crrev.com/eaaaf143f6fcedd0897b6d30e55bb5893a66c996/third_party/blink/renderer/core/paint/paint_and_raster_invalidation_test.cc
[modify] https://crrev.com/eaaaf143f6fcedd0897b6d30e55bb5893a66c996/third_party/blink/renderer/core/paint/paint_invalidator.cc
[modify] https://crrev.com/eaaaf143f6fcedd0897b6d30e55bb5893a66c996/third_party/blink/renderer/core/paint/paint_invalidator.h
[modify] https://crrev.com/eaaaf143f6fcedd0897b6d30e55bb5893a66c996/third_party/blink/renderer/core/paint/pre_paint_tree_walk.cc
[modify] https://crrev.com/eaaaf143f6fcedd0897b6d30e55bb5893a66c996/third_party/blink/renderer/core/paint/pre_paint_tree_walk.h
[modify] https://crrev.com/eaaaf143f6fcedd0897b6d30e55bb5893a66c996/third_party/blink/web_tests/FlagExpectations/enable-blink-features=CompositeAfterPaint

Sign in to add a comment