LayoutObject::paintInvalidationParent() to fix multicol spanner invalidation

third_party/WebKit/Source/core/layout/LayoutObject.cpp

Index: third_party/WebKit/Source/core/layout/LayoutObject.cpp
diff --git a/third_party/WebKit/Source/core/layout/LayoutObject.cpp b/third_party/WebKit/Source/core/layout/LayoutObject.cpp
index 7b7996ca542c27b32613429569ccd690bd5d2d9f..7204d62640705d2b2e67fd960b83aebcbf2bb4e2 100644
--- a/third_party/WebKit/Source/core/layout/LayoutObject.cpp
+++ b/third_party/WebKit/Source/core/layout/LayoutObject.cpp
@@ -604,17 +604,19 @@ PaintLayer* LayoutObject::enclosingLayer() const
         if (current->hasLayer())
             return toLayoutBoxModelObject(current)->layer();
     }
-    // FIXME: we should get rid of detached layout subtrees, at which point this code should
-    // not be reached. crbug.com/411429
+    // TODO(crbug.com/365897): we should get rid of detached layout subtrees, at which point this code should
+    // not be reached.
     return nullptr;
 }
 
 PaintLayer* LayoutObject::paintingLayer() const
 {
-    for (const LayoutObject* current = this; current; current = current->isColumnSpanAll() ? current->containingBlock() : current->parent()) {
+    for (const LayoutObject* current = this; current; current = current->paintInvalidationParent()) {
         if (current->hasLayer() && toLayoutBoxModelObject(current)->layer()->isSelfPaintingLayer())
             return toLayoutBoxModelObject(current)->layer();
     }
+    // TODO(crbug.com/365897): we should get rid of detached layout subtrees, at which point this code should
+    // not be reached.
     return nullptr;
 }
 
@@ -2615,9 +2617,13 @@ LayoutObject* LayoutObject::container(const LayoutBoxModelObject* ancestor, bool
     return o;
 }
 
-LayoutObject* LayoutObject::parentCrossingFrameBoundaries() const
+LayoutObject* LayoutObject::paintInvalidationParent() const
 {
-    return isLayoutView() ? frame()->ownerLayoutObject() : parent();
+    if (isLayoutView())

[chrishtr, 2016/06/13 19:49:39]

Is it important for this method to not be virtual, and hence have
type-introspection in it?

[Xianzhu, 2016/06/13 19:58:01]

Not important, but just want to make the fact that the function is crossing
frame boundary more explicit here. Also the added logic about isColumnSpanAll()
does belong here because we don't have a class for columnSpanAll.

[chrishtr, 2016/06/13 20:09:33]

It feels a little cleaner to go with virtual like various other methods, but
centralizing logic
is also nice. Up to you.

+        return frame()->ownerLayoutObject();
+    if (isColumnSpanAll())
+        return spannerPlaceholder();
+    return parent();
 }
 
 bool LayoutObject::isSelectionBorder() const
@@ -3424,7 +3430,7 @@ static PaintInvalidationReason documentLifecycleBasedPaintInvalidationReason(con
 
 inline void LayoutObject::markAncestorsForPaintInvalidation()
 {
-    for (LayoutObject* parent = this->parentCrossingFrameBoundaries(); parent && !parent->shouldCheckForPaintInvalidationRegardlessOfPaintInvalidationState(); parent = parent->parentCrossingFrameBoundaries())
+    for (LayoutObject* parent = this->paintInvalidationParent(); parent && !parent->shouldCheckForPaintInvalidationRegardlessOfPaintInvalidationState(); parent = parent->paintInvalidationParent())
         parent->m_bitfields.setChildShouldCheckForPaintInvalidation(true);
 }