New issue
Advanced search Search tips

Issue 597298 link

Starred by 1 user
Status: Fixed
Owner:
chrishtr@chromium.org
Closed: Mar 2016
Cc:
wangxianzhu@chromium.org
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Make sure subsequence will be repainted when a GraphicsLayer is fully invalidated

Project Member Reported by wangxianzhu@chromium.org, Mar 23 2016 
 Bug 593758  might be a case that we still used cached subsequence after a GraphicsLayer is fully invalidated. We have fixed the bug by disabling subsequence in pending style mode, but I'm afraid we might have problems in other cases that a GraphicsLayer is fully invalidated.

It's supposed to work like:
1. The GraphicsLayers is invalidated and the PaintController is also fully invalidated by removing all cached display items;
2. When checking for cached subsequence, because the PaintController doesn't contain cached subsequence, we should repaint the subsequence.

Comment 1 by chrishtr@chromium.org, Mar 23 2016 

I debugged a bit. Turns out that the PaintLayerCompositor for the iframe is not
in compositing mode, so LayoutView::invalidatePaintForViewAndCompositedLayers doesn't have the desired effect.

We may want to change Document::styleResolverMayHaveChanged to force-invalidate
on the root frame of the frame tree.

Comment 2 by chrishtr@chromium.org, Mar 23 2016 

Writing a CL to fix now.

Comment 3 by wangxianzhu@chromium.org, Mar 23 2016 

I'm still confused: LayoutView::invalidatePaintForViewAndCompositedLayers() also calls setShouldDoFullPaintInvalidation(), which is supposed to cause setNeedsRepaint() on the LayoutView's layer during the next paint invalidation. Any GraphicsLayers under the LayoutView should also be invalidated during LayoutView::invalidatePaintForViewAndCompositedLayers().

Comment 4 by chrishtr@chromium.org, Mar 23 2016 

Not sure either. Digging deeper..

Comment 5 by wangxianzhu@chromium.org, Mar 23 2016

Cc: -chrishtr@chromium.org
Owner: chrishtr@chromium.org

Comment 6 by chrishtr@chromium.org, Mar 23 2016

Cc: wangxianzhu@chromium.org
It's because the layer that we need to set needsRepaint on is a child of the
LayoutView. See BlockPainter::paint: it avoids skipping on the LayoutView (who
knows why - probably to paint the document background?)

Comment 7 by wangxianzhu@chromium.org, Mar 23 2016 

> It's because the layer that we need to set needsRepaint on is a child of the
> LayoutView.

Understood.

> See BlockPainter::paint: it avoids skipping on the LayoutView (who
> knows why - probably to paint the document background?)

Not understood. About which lines of code?
Project Member

Comment 9 by bugdroid1@chromium.org, Mar 23 2016 

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

commit 4cb074fdde6959fa8d7e9cc5d787262875740e53
Author: chrishtr <chrishtr@chromium.org>
Date: Wed Mar 23 23:40:15 2016

Invalidate the LayoutView and all non-composited descendants when doing a full invalidate.

This handles the case when didLayoutWithPendingStylesheets becomes false. In at
scenario, it is the layer just below the LayoutView that needs invalidation.

BUG= 597298 

Review URL: https://codereview.chromium.org/1832453002

Cr-Commit-Position: refs/heads/master@{#382973}

[modify] https://crrev.com/4cb074fdde6959fa8d7e9cc5d787262875740e53/third_party/WebKit/Source/core/layout/LayoutView.cpp

Comment 10 by chrishtr@chromium.org, Mar 23 2016

Status: Fixed (was: Assigned)
►

Sign in to add a comment