Fix TextAutosizer not to scheduleRelayout() by MarkContainerChainInLayout

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 b50666e149cafa8f7a238acd015a60bf75c3d3ca..bee3de489613f90e3875eda357662fe173885ee8 100644
--- a/third_party/WebKit/Source/core/layout/LayoutObject.cpp
+++ b/third_party/WebKit/Source/core/layout/LayoutObject.cpp
@@ -748,23 +748,13 @@ static inline bool objectIsRelayoutBoundary(const LayoutObject* object)
     return true;
 }
 
-void LayoutObject::markContainerChainForLayout(bool scheduleRelayout)
-{
-    markContainerChainForLayout(scheduleRelayout, nullptr);
-}
-
-void LayoutObject::markContainerChainForLayout(SubtreeLayoutScope* layouter)
-{
-    // When we have a layouter, it means that we're in layout and we're marking
-    // a descendant as needing layout with the intention of visiting it during
-    // this layout. We shouldn't be scheduling it to be laid out later.
-    markContainerChainForLayout(!layouter, layouter);
-}
-
 void LayoutObject::markContainerChainForLayout(bool scheduleRelayout, SubtreeLayoutScope* layouter)
 {
     ASSERT(!isSetNeedsLayoutForbidden());
     ASSERT(!layouter || this != layouter->root());
+    // When we have a layouter, it means that we're in layout and we're marking
+    // a descendant as needing layout with the intention of visiting it during
+    // this layout. We shouldn't be scheduling it to be laid out later.
     ASSERT(!scheduleRelayout || !layouter);

[kojii, 2016/04/18 06:32:28]

Should we make this RELEASE_ASSERT()?

[cbiesinger, 2016/04/18 15:53:46]

I don't really see value in that. The more interesting case, IMO, would be the
one where we have scheduleRelayout=true but should be false, when we did not
pass a layouter when we should have.

Actually, maybe we could do that -- RELEASE_ASSERT(!scheduleRelayout ||
!frameView->isInPerformLayout())?

Based on bug 600998, it seems that we'd usually crash in such a case anyway, and
making this one a release assert would help in tracking this down, so I would
support adding this.

[kojii, 2016/04/19 06:39:08]

It doesn't always crash, bug 600998 was unable to minimize because it crashes
only 2 out of 10 tries, so having proper RELEASE_ASSERT() is good.

And your suggestion looks far better, thank you.

However, by doing that, over 100 tests fail. By looking at some failures
randomly, we can probably narrow down to 4-5 code paths to fix.

So I reverted the RELEASE_ASSERT() for now. I can use it locally to find what to
fix, and we can make the RELEASE_ASSERT when all of them are fixed.

 
     LayoutObject* object = container();