Let RenderTable reach table cells needing overflow recalc

Source/core/rendering/RenderObject.cpp

 /*
  * Copyright (C) 1999 Lars Knoll (knoll@kde.org)
  *           (C) 1999 Antti Koivisto (koivisto@kde.org)
  *           (C) 2000 Dirk Mueller (mueller@kde.org)
  *           (C) 2004 Allan Sandfeld Jensen (kde@carewolf.com)
  * Copyright (C) 2004, 2005, 2006, 2007, 2008, 2011 Apple Inc. All rights reserved.
  * Copyright (C) 2009 Google Inc. All rights reserved.
  * Copyright (C) 2009 Torch Mobile Inc. All rights reserved. (http://www.torchmobile.com/)
  *
  * This library is free software; you can redistribute it and/or
@@ skipping 1596 matching lines @@
     if (isImage() || isQuote()) {
         RefPtr<RenderStyle> style = RenderStyle::create();
         style->inheritFrom(pseudoStyle.get());
         setStyle(style.release());
         return;
     }
 
     setStyle(pseudoStyle);
 }
 
-void RenderObject::markContainingBlocksForOverflowRecalc()
+static RenderObject* containerForOverflowRecalc(const RenderObject* object)
 {
-    for (RenderBlock* container = containingBlock(); container && !container->childNeedsOverflowRecalcAfterStyleChange(); container = container->containingBlock())
-        container->setChildNeedsOverflowRecalcAfterStyleChange(true);
+    // For table parts, we let the parent (e.g. RenderTableRow for RenderTableCell)
+    // handle the childNeedsOverflowRecalcAfterStyleChange flag, because
+    // RenderBlock::recalcChildOverflowAfterStyleChange() may not reach the table parts.
+    return object->isTablePart() ? object->parent() : object->containingBlock();
+}

[Julien - ping for review, 2014/11/25 18:33:10]

:(

Wouldn't container() work without needing this logic?

Also what you really need to do is propagate the needs-overflow-computation flag
from RenderTableCell::styleDidChange. That would require a lot less
modifications.

[Xianzhu, 2014/11/25 20:17:12]

This seems not the best because we'll set the flag on unnecessary objects which
won't handle the flag, and leave the flag uncleared.

How about renaming the function to markContainersForOverflowRecalcIfNeeded()
which reflect the fact that we are not marking all containers.
styleDidChange doesn't work for the test case because
RenderTableCell::styleDidChange won't be called when its descendant needs
overflow recalc.

+
+void RenderObject::markContainersForOverflowRecalc()

[Julien - ping for review, 2014/11/25 18:33:10]

If you put container in the name, I would have expected you to use container().

+{
+    for (RenderObject* container = containerForOverflowRecalc(this); container && !container->childNeedsOverflowRecalcAfterStyleChange(); container = containerForOverflowRecalc(container))
+        container->setChildNeedsOverflowRecalcAfterStyleChange();
 }
 
 void RenderObject::setNeedsOverflowRecalcAfterStyleChange()
 {
     bool neededRecalc = needsOverflowRecalcAfterStyleChange();
-    setSelfNeedsOverflowRecalcAfterStyleChange(true);
+    setSelfNeedsOverflowRecalcAfterStyleChange();
     if (!neededRecalc)
-        markContainingBlocksForOverflowRecalc();
+        markContainersForOverflowRecalc();
 }
 
 void RenderObject::setStyle(PassRefPtr<RenderStyle> style)
 {
     ASSERT(style);
 
     if (m_style == style) {
         // We need to run through adjustStyleDifference() for iframes, plugins, and canvas so
         // style sharing is disabled for them. That should ensure that we never hit this code path.
         ASSERT(!isRenderIFrame() && !isEmbeddedObject() && !isCanvas());
@@ skipping 181 matching lines @@
         // If the object already needs layout, then setNeedsLayout won't do
         // any work. But if the containing block has changed, then we may need
         // to mark the new containing blocks for layout. The change that can
         // directly affect the containing block of this object is a change to
         // the position style.
         if (needsLayout() && oldStyle->position() != m_style->position())
             markContainingBlocksForLayout();
 
         // Ditto.
         if (needsOverflowRecalcAfterStyleChange() && oldStyle->position() != m_style->position())
-            markContainingBlocksForOverflowRecalc();
+            markContainersForOverflowRecalc();
 
         if (diff.needsFullLayout())
             setNeedsLayoutAndPrefWidthsRecalc();
     } else if (diff.needsPositionedMovementLayout())
         setNeedsPositionedMovementLayout();
 
     // Don't check for paint invalidation here; we need to wait until the layer has been
     // updated by subclasses before we know if we have to invalidate paints (in setStyle()).
 
     if (oldStyle && !areCursorsEqual(oldStyle, style())) {
@@ skipping 1299 matching lines @@
 {
     if (object1) {
         const blink::RenderObject* root = object1;
         while (root->parent())
             root = root->parent();
         root->showRenderTreeAndMark(object1, "*", object2, "-", 0);
     }
 }
 
 #endif