Fix traverseNonCompositingDescendantsInPaintOrder for float-under-inline cases

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

Index: third_party/WebKit/Source/core/paint/ObjectPaintInvalidator.cpp
diff --git a/third_party/WebKit/Source/core/paint/ObjectPaintInvalidator.cpp b/third_party/WebKit/Source/core/paint/ObjectPaintInvalidator.cpp
index 74eecda3b676d49e0ce6ad2dd77791adb1aebe6d..88d0414f2d38b0ed6a79ee3997c25f16d735f1a4 100644
--- a/third_party/WebKit/Source/core/paint/ObjectPaintInvalidator.cpp
+++ b/third_party/WebKit/Source/core/paint/ObjectPaintInvalidator.cpp
@@ -57,53 +57,63 @@ void ObjectPaintInvalidator::objectWillBeDestroyed(const LayoutObject& object) {
     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);
@@ -112,14 +122,18 @@ void traverseNonCompositingDescendantsInPaintOrder(
     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);
@@ -127,6 +141,17 @@ void traverseNonCompositingDescendantsInPaintOrder(
   }
 }
 
+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) {
@@ -137,9 +162,7 @@ void ObjectPaintInvalidator::
   slowSetPaintingLayerNeedsRepaint();
   traverseNonCompositingDescendantsInPaintOrder(
       m_object, [reason](const LayoutObject& object) {
-        if (object.hasLayer() &&
-            toLayoutBoxModelObject(object).hasSelfPaintingLayer())
-          toLayoutBoxModelObject(object).layer()->setNeedsRepaint();
+        setPaintingLayerNeedsRepaintDuringTraverse(object);
         object.invalidateDisplayItemClients(reason);
       });
 }
@@ -177,10 +200,10 @@ void ObjectPaintInvalidator::
   // 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);
       });