Fix or workaround several issues of slimmingPaintInvalidation on spv1

third_party/WebKit/Source/core/paint/PaintInvalidator.cpp

Index: third_party/WebKit/Source/core/paint/PaintInvalidator.cpp
diff --git a/third_party/WebKit/Source/core/paint/PaintInvalidator.cpp b/third_party/WebKit/Source/core/paint/PaintInvalidator.cpp
index 98c7ff587eb23bc5538ef96a84bd6831d0211b32..70cd7730a31821981e2102dbb79280262e1b0e56 100644
--- a/third_party/WebKit/Source/core/paint/PaintInvalidator.cpp
+++ b/third_party/WebKit/Source/core/paint/PaintInvalidator.cpp
@@ -182,7 +182,9 @@ void PaintInvalidator::updateContext(const LayoutObject& object, PaintInvalidato
         context.forcedSubtreeInvalidationFlags |= PaintInvalidatorContext::ForcedSubtreeInvalidationChecking;
 
     // TODO(crbug.com/637313): This is temporary before we support filters in paint property tree.
-    if (object.hasFilterInducingProperty())
+    // TODO(crbug.com/648274): This is a workaround for multi-column contents.
+    // TODO(crbug.com/648409): This is a workaround for inline contents in vertical-rl container.
+    if (object.hasFilterInducingProperty() || object.isLayoutFlowThread() || object.hasFlippedBlocksWritingMode())

[pdr., 2016/09/19 22:19:15]

Can the hasFlippedBlocksWritingMode check be made a little tougher by checking
for inline too? For example, (object.isInline() &&
object.hasFlippedBlocksWritingMode())

[Xianzhu, 2016/09/19 23:37:40]

This would be complex: we need to check for the flipped blocks writing mode of a
writing-mode container and inline status of a descendant. I will work on this in
this week and hopefully fix the bug soon.

[Xianzhu, 2016/09/19 23:39:45]

P.S. I'm using the workarounds to see how many tests are affected and how many
tests are still failing because of other bugs.

         context.forcedSubtreeInvalidationFlags |= PaintInvalidatorContext::ForcedSubtreeSlowPathRect;
 
     context.oldBounds = object.previousPaintInvalidationRect();
@@ -206,8 +208,17 @@ void PaintInvalidator::invalidatePaintIfNeeded(FrameView& frameView, PaintInvali
     context.paintInvalidationContainer = context.paintInvalidationContainerForStackedContents = &layoutView->containerForPaintInvalidation();
     context.paintingLayer = layoutView->layer();
 
-    if (!RuntimeEnabledFeatures::rootLayerScrollingEnabled())
+    if (!RuntimeEnabledFeatures::rootLayerScrollingEnabled()) {
+        // Undo content clip and scroll before invalidating FrameView scrollbars.

[pdr., 2016/09/19 22:19:15]

If there is a scroll translation (because the frameview is scrolled), we will
incorrectly use the scrolled transform node for invalidating the scrollbars.
WDYT of splitting out the self and non-self invalidations like we do for
LayoutObjects?

Something like:
void PrePaintTreeWalk::walk(FrameView& frameView, const PrePaintTreeWalkContext&
context)
{
...
    PrePaintTreeWalkContext localContext(context);
    m_propertyTreeBuilder.buildTreeNodesForSelf(frameView,
localContext.treeBuilderContext);
    if (RuntimeEnabledFeatures::slimmingPaintInvalidationEnabled())
        m_paintInvalidator.invalidatePaintForSelfIfNeeded(frameView,
localContext.paintInvalidatorContext);
    m_propertyTreeBuilder.buildTreeNodesForChildren(frameView,
localContext.treeBuilderContext);
...
}

[Xianzhu, 2016/09/19 23:37:40]

Thought of this but chose the current method because
1. it will need less code change when we remove non-root-layer-scrolls code;
2. using the above method we must also split invalidatePaintIfNeeded() into two,
one for scrollbars, one for caret.

+        PaintPropertyTreeBuilderContext& treeBuilderContext = const_cast<PaintPropertyTreeBuilderContext&>(context.treeBuilderContext);
+        const ClipPaintPropertyNode* clip = treeBuilderContext.current.clip;
+        const ScrollPaintPropertyNode* scroll = treeBuilderContext.current.scroll;
+        treeBuilderContext.current.clip = clip->parent();
+        treeBuilderContext.current.scroll = scroll->parent();

[pdr., 2016/09/19 22:19:15]

This will be incorrect if the frameview does not scroll (and, therefore, does
not create a scroll node). Do any tests hit this case?

I think we need something like:
treeBuilderContext.current.scroll = frameView.scroll() ? scroll->parent() :
scroll;

[Xianzhu, 2016/09/19 23:37:40]

Fixed.

No test hit the bug because if the frameView is not scrollable, there will be no
scrollbars to calculate the paint invalidation rect using the paint properties.

         frameView.invalidatePaintOfScrollControlsIfNeeded(context);
+        treeBuilderContext.current.clip = clip;
+        treeBuilderContext.current.scroll = scroll;
+    }
 
     if (frameView.frame().selection().isCaretBoundsDirty())
         frameView.frame().selection().invalidateCaretRect();