Fix traverseNonCompositingDescendantsInPaintOrder for float-under-inline cases

third_party/WebKit/Source/core/paint/ObjectPaintInvalidator.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/ObjectPaintInvalidator.h"
 
 #include "core/frame/FrameView.h"
 #include "core/frame/LocalFrame.h"
 #include "core/layout/LayoutView.h"
 #include "core/layout/api/LayoutPartItem.h"
@@ skipping 39 matching lines @@
          selectionVisualRectMap().contains(&object));
   if (object.hasPreviousSelectionVisualRect())
     selectionVisualRectMap().remove(&object);
 
   DCHECK(object.hasPreviousLocationInBacking() ==
          locationInBackingMap().contains(&object));
   if (object.hasPreviousLocationInBacking())
     locationInBackingMap().remove(&object);
 }
 
-// TODO(trchen): Use std::function<void, LayoutObject&> when available.
-template <typename LayoutObjectTraversalFunctor>
-void traverseNonCompositingDescendantsInPaintOrder(
+using LayoutObjectTraversalFunctor = std::function<void(const LayoutObject&)>;
+
+static void traverseNonCompositingDescendantsInPaintOrder(
     const LayoutObject&,
     const LayoutObjectTraversalFunctor&);
 
-template <typename LayoutObjectTraversalFunctor>
-void traverseNonCompositingDescendantsBelongingToAncestorPaintInvalidationContainer(
+static void
+traverseNonCompositingDescendantsBelongingToAncestorPaintInvalidationContainer(
     const LayoutObject& object,
     const LayoutObjectTraversalFunctor& functor) {
-  // |object| is a paint invalidation container but is not a stacking context,
-  // so the paint invalidation container of stacked descendants don't belong to
-  // |object| but belong to an ancestor. This function traverses all such
-  // descendants.
-  DCHECK(object.isPaintInvalidationContainer() &&
-         !object.styleRef().isStackingContext());
+  // |object| is a paint invalidation container, but is not a stacking context
+  // or is a non-block-flow, so the paint invalidation container of stacked
+  // descendants may not belong to |object| but belong to an ancestor. This
+  // function traverses all such descendants.
+  DCHECK(
+      object.isPaintInvalidationContainer() &&
+      (!object.styleRef().isStackingContext() || !object.isLayoutBlockFlow()));
 
   LayoutObject* descendant = object.nextInPreOrder(&object);
   while (descendant) {
-    if (!descendant->hasLayer() || !descendant->styleRef().isStacked()) {
-      // Case 1: The descendant is not stacked (or is stacked but has not been
+    if (!object.isLayoutBlockFlow() && descendant->isFloating()) {

[chrishtr, 2017/01/19 21:22:26]

&& !isStacked() ? See https://codereview.chromium.org/2640053004

[Xianzhu, 2017/01/20 17:38:12]

Yes. Moved the case under the condition at new line 86.

+      // Case 1 (rare): The descendant is a floating object below a composited
+      // non-block-flow object. The floating object subtree belongs to an
+      // ancestor in paint order, thus recur into the subtree.
+      traverseNonCompositingDescendantsInPaintOrder(*descendant, functor);
+      descendant = descendant->nextInPreOrderAfterChildren(&object);
+    } else if (!descendant->hasLayer() || !descendant->styleRef().isStacked()) {
+      // Case 2: The descendant is not stacked (or is stacked but has not been
       // allocated a layer yet during style change), so either it's a paint
       // invalidation container in the same situation as |object|, or its paint
       // invalidation container is in such situation. Keep searching until a
       // stacked layer is found.
       descendant = descendant->nextInPreOrder(&object);
     } else if (!descendant->isPaintInvalidationContainer()) {
-      // Case 2: The descendant is stacked and is not composited.
+      // Case 3: The descendant is stacked and is not composited.
       // The invalidation container of its subtree is our ancestor,
       // thus recur into the subtree.
       traverseNonCompositingDescendantsInPaintOrder(*descendant, functor);
       descendant = descendant->nextInPreOrderAfterChildren(&object);
-    } else if (descendant->styleRef().isStackingContext()) {
-      // Case 3: The descendant is an invalidation container and is a stacking
+    } else if (descendant->styleRef().isStackingContext() &&
+               descendant->isLayoutBlockFlow()) {
+      // Case 4: The descendant is an invalidation container and is a stacking
       // context.  No objects in the subtree can have invalidation container
       // outside of it, thus skip the whole subtree.
+      // This excludes non-block-flow because there might be floating objects
+      // under the descendant belonging to some ancestor in paint order.
       descendant = descendant->nextInPreOrderAfterChildren(&object);
     } else {
-      // Case 4: The descendant is an invalidation container but not a stacking
-      // context.  This is the same situation as |object|, thus keep searching.
+      // Case 5: The descendant is an invalidation container but not a stacking
+      // context, or the descendant is a non-block-flow stacking context.
+      // This is the same situation as |object|, thus keep searching.
       descendant = descendant->nextInPreOrder(&object);
     }
   }
 }
 
-template <typename LayoutObjectTraversalFunctor>
-void traverseNonCompositingDescendantsInPaintOrder(
+static void traverseNonCompositingDescendantsInPaintOrder(
     const LayoutObject& object,
     const LayoutObjectTraversalFunctor& functor) {
   functor(object);
   LayoutObject* descendant = object.nextInPreOrder(&object);
   while (descendant) {
     if (!descendant->isPaintInvalidationContainer()) {
       functor(*descendant);
       descendant = descendant->nextInPreOrder(&object);
-    } else if (descendant->styleRef().isStackingContext()) {
+    } else if (descendant->styleRef().isStackingContext() &&
+               descendant->isLayoutBlockFlow()) {
       // The descendant is an invalidation container and is a stacking context.
       // No objects in the subtree can have invalidation container outside of
       // it, thus skip the whole subtree.
+      // This excludes non-block-flow because there might be floating objects
+      // under the descendant belonging to some ancestor in paint order.
       descendant = descendant->nextInPreOrderAfterChildren(&object);
     } else {
-      // If a paint invalidation container is not a stacking context,
-      // some of its descendants may belong to the parent container.
+      // If a paint invalidation container is not a stacking context, or the
+      // descendant is a non-block-flow stacking context, some of its
+      // descendants may belong to the parent container.
       traverseNonCompositingDescendantsBelongingToAncestorPaintInvalidationContainer(
           *descendant, functor);
       descendant = descendant->nextInPreOrderAfterChildren(&object);
     }
   }
 }
 
+static void setPaintingLayerNeedsRepaintDuringTraverse(
+    const LayoutObject& object) {
+  if (object.hasLayer() &&
+      toLayoutBoxModelObject(object).hasSelfPaintingLayer()) {
+    toLayoutBoxModelObject(object).layer()->setNeedsRepaint();
+  } else if (object.isFloating() && object.parent() &&

[chrishtr, 2017/01/19 21:22:26]

Same here: skip isStacked() objects?

[Xianzhu, 2017/01/20 17:38:12]

Now rebased on https://codereview.chromium.org/2642763009/ and use
canContainFloatingObject().

+             !object.parent()->isLayoutBlockFlow()) {
+    object.paintingLayer()->setNeedsRepaint();
+  }
+}
+
 void ObjectPaintInvalidator::
     invalidateDisplayItemClientsIncludingNonCompositingDescendants(
         PaintInvalidationReason reason) {
   // This is valid because we want to invalidate the client in the display item
   // list of the current backing.
   DisableCompositingQueryAsserts disabler;
 
   slowSetPaintingLayerNeedsRepaint();
   traverseNonCompositingDescendantsInPaintOrder(
       m_object, [reason](const LayoutObject& object) {
-        if (object.hasLayer() &&
-            toLayoutBoxModelObject(object).hasSelfPaintingLayer())
-          toLayoutBoxModelObject(object).layer()->setNeedsRepaint();
+        setPaintingLayerNeedsRepaintDuringTraverse(object);
         object.invalidateDisplayItemClients(reason);
       });
 }
 
 DISABLE_CFI_PERF
 void ObjectPaintInvalidator::invalidatePaintOfPreviousVisualRect(
     const LayoutBoxModelObject& paintInvalidationContainer,
     PaintInvalidationReason reason) {
   // It's caller's responsibility to ensure enclosingSelfPaintingLayer's
   // needsRepaint is set.  Don't set the flag here because getting
@@ skipping 17 matching lines @@
   // the new paint invalidation container happens to be the same as the old one.
   m_object.getMutableForPainting().clearPreviousVisualRects();
 }
 
 void ObjectPaintInvalidator::
     invalidatePaintIncludingNonCompositingDescendants() {
   // Since we're only painting non-composited layers, we know that they all
   // share the same paintInvalidationContainer.
   const LayoutBoxModelObject& paintInvalidationContainer =
       m_object.containerForPaintInvalidation();
+  slowSetPaintingLayerNeedsRepaint();
   traverseNonCompositingDescendantsInPaintOrder(
       m_object, [&paintInvalidationContainer](const LayoutObject& object) {
-        if (object.hasLayer())
-          toLayoutBoxModelObject(object).layer()->setNeedsRepaint();
+        setPaintingLayerNeedsRepaintDuringTraverse(object);
         ObjectPaintInvalidator(object).invalidatePaintOfPreviousVisualRect(
             paintInvalidationContainer, PaintInvalidationSubtree);
       });
 }
 
 void ObjectPaintInvalidator::
     invalidatePaintIncludingNonSelfPaintingLayerDescendantsInternal(
         const LayoutBoxModelObject& paintInvalidationContainer) {
   invalidatePaintOfPreviousVisualRect(paintInvalidationContainer,
                                       PaintInvalidationSubtree);
@@ skipping 379 matching lines @@
 
   m_context.paintingLayer->setNeedsRepaint();
   m_object.invalidateDisplayItemClients(reason);
   return reason;
 }
 
 DisablePaintInvalidationStateAsserts::DisablePaintInvalidationStateAsserts()
     : m_disabler(&gDisablePaintInvalidationStateAsserts, true) {}
 
 }  // namespace blink