Fix or workaround several issues of slimmingPaintInvalidation on spv1

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

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "core/paint/PaintInvalidator.h"
 
 #include "core/editing/FrameSelection.h"
 #include "core/frame/FrameView.h"
 #include "core/frame/LocalFrame.h"
 #include "core/frame/Settings.h"
@@ skipping 164 matching lines @@
         }
     }
 
     DCHECK(context.paintInvalidationContainer == object.containerForPaintInvalidation());
     DCHECK(context.paintingLayer == object.paintingLayer());
 
     if (object.mayNeedPaintInvalidationSubtree())
         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();
     context.oldLocation = object.previousPositionFromPaintInvalidationBacking();
     context.newBounds = computePaintInvalidationRectInBacking(object, context);
     context.newLocation = computeLocationFromPaintInvalidationBacking(object, context);
 
     IntSize adjustment = object.scrollAdjustmentForPaintInvalidation(*context.paintInvalidationContainer);
     context.newLocation.move(adjustment);
     context.newBounds.move(adjustment);
 
     object.getMutableForPainting().setPreviousPaintInvalidationRect(context.newBounds);
     object.getMutableForPainting().setPreviousPositionFromPaintInvalidationBacking(context.newLocation);
 }
 
 void PaintInvalidator::invalidatePaintIfNeeded(FrameView& frameView, PaintInvalidatorContext& context)
 {
     LayoutView* layoutView = frameView.layoutView();
     CHECK(layoutView);
 
     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();
 
     // Temporary callback for crbug.com/487345,402044
     // TODO(ojan): Make this more general to be used by PositionObserver
     // and rAF throttling.
     IntRect visibleRect = frameView.rootFrameToContents(frameView.computeVisibleArea());
     layoutView->sendMediaPositionChangeNotifications(visibleRect);
 }
@@ skipping 64 matching lines @@
     object.getMutableForPainting().clearPaintInvalidationFlags();
 }
 
 void PaintInvalidator::processPendingDelayedPaintInvalidations()
 {
     for (auto target : m_pendingDelayedPaintInvalidations)
         target->getMutableForPainting().setShouldDoFullPaintInvalidation(PaintInvalidationDelayedFull);
 }
 
 } // namespace blink