Strip anonymous blocks when change in style removes need for them

Source/core/rendering/RenderBlock.cpp

 /*
  * Copyright (C) 1999 Lars Knoll (knoll@kde.org)
  *           (C) 1999 Antti Koivisto (koivisto@kde.org)
  *           (C) 2007 David Smith (catfish.man@gmail.com)
  * Copyright (C) 2003, 2004, 2005, 2006, 2007, 2008, 2009, 2010, 2011 Apple Inc. All rights reserved.
  * Copyright (C) Research In Motion Limited 2010. All rights reserved.
  * Copyright (C) 2014 Samsung Electronics. All rights reserved.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Library General Public
@@ skipping 92 matching lines @@
 static TrackedContainerMap* gPercentHeightContainerMap = 0;
 
 typedef WTF::HashMap<RenderBlock*, OwnPtr<ListHashSet<RenderInline*> > > ContinuationOutlineTableMap;
 
 typedef WTF::HashSet<RenderBlock*> DelayedUpdateScrollInfoSet;
 static int gDelayUpdateScrollInfo = 0;
 static DelayedUpdateScrollInfoSet* gDelayedUpdateScrollInfoSet = 0;
 
 static bool gColumnFlowSplitEnabled = true;
 
+static bool s_noLongerAffectsParentBlock = false;

[esprehn, 2014/05/06 18:55:44]

This should be a member of RenderBlock like the other static bits.

+
 // This class helps dispatching the 'overflow' event on layout change. overflow can be set on RenderBoxes, yet the existing code
 // only works on RenderBlocks. If this changes, this class should be shared with other RenderBoxes.
 class OverflowEventDispatcher {
     WTF_MAKE_NONCOPYABLE(OverflowEventDispatcher);
 public:
     OverflowEventDispatcher(const RenderBlock* block)
         : m_block(block)
         , m_hadHorizontalLayoutOverflow(false)
         , m_hadVerticalLayoutOverflow(false)
     {
@@ skipping 178 matching lines @@
                     break;
                 }
                 cb = cb->parent();
             }
 
             if (cb->isRenderBlock())
                 toRenderBlock(cb)->removePositionedObjects(this, NewContainingBlock);
         }
     }
 
+    s_noLongerAffectsParentBlock = !isFloatingOrOutOfFlowPositioned()

[Julien - ping for review, 2014/05/07 01:38:46]

RenderObject has a static boolean called s_affectsParentBlock, are those
related?

+        && (newStyle.isFloating() || newStyle.hasOutOfFlowPosition())
+        && parent() && parent()->isRenderBlockFlow();
+
     RenderBox::styleWillChange(diff, newStyle);
 }
 
 static bool borderOrPaddingLogicalWidthChanged(const RenderStyle* oldStyle, const RenderStyle* newStyle)
 {
     if (newStyle->isHorizontalWritingMode())
         return oldStyle->borderLeftWidth() != newStyle->borderLeftWidth()
             || oldStyle->borderRightWidth() != newStyle->borderRightWidth()
             || oldStyle->paddingLeft() != newStyle->paddingLeft()
             || oldStyle->paddingRight() != newStyle->paddingRight();
 
     return oldStyle->borderTopWidth() != newStyle->borderTopWidth()
         || oldStyle->borderBottomWidth() != newStyle->borderBottomWidth()
         || oldStyle->paddingTop() != newStyle->paddingTop()
         || oldStyle->paddingBottom() != newStyle->paddingBottom();
 }
 
 void RenderBlock::styleDidChange(StyleDifference diff, const RenderStyle* oldStyle)
 {
     RenderBox::styleDidChange(diff, oldStyle);
 
+    if (s_noLongerAffectsParentBlock)

[ojan, 2014/05/06 18:43:02]

Why do you need the static? Can't you do the check in styleWillChange here
instead?

+        toRenderBlock(parent())->removeAnonymousWrappersIfRequired();
+
     RenderStyle* newStyle = style();
 
     if (!isAnonymousBlock()) {
         // Ensure that all of our continuation blocks pick up the new style.
         for (RenderBlock* currCont = blockElementContinuation(); currCont; currCont = currCont->blockElementContinuation()) {
             RenderBoxModelObject* nextCont = currCont->continuation();
             currCont->setContinuation(0);
             currCont->setStyle(newStyle);
             currCont->setContinuation(nextCont);
         }
@@ skipping 703 matching lines @@
         return false;
 
     if (!prev || !next)
         return true;
 
     // Make sure the types of the anonymous blocks match up.
     return prev->isAnonymousColumnsBlock() == next->isAnonymousColumnsBlock()
            && prev->isAnonymousColumnSpanBlock() == next->isAnonymousColumnSpanBlock();
 }
 
+bool RenderBlock::canRemoveAnonymousWrappers()
+{
+    for (RenderObject* child = firstChild(); child; child = child->nextSibling()) {

[Julien - ping for review, 2014/05/07 01:38:46]

You seem to care only about blocks so we should probably just walk our box
children.

Better even, don't have a children walk here.

+        if (!child->isAnonymousBlock() && !child->isFloatingOrOutOfFlowPositioned())
+            return false;
+        if (child->isRenderBlock() && toRenderBlock(child)->continuation())
+            return false;
+    }
+    return true;
+}
+
+void RenderBlock::removeAnonymousWrappersIfRequired()
+{
+    if (!canRemoveAnonymousWrappers())
+        return;
+
+    RenderObject* child = firstChild();
+    while (child) {

[Julien - ping for review, 2014/05/07 01:38:46]

Same comment about walking boxes (ideally blocks but we don't have them).

+        RenderObject* next = child->nextSibling();
+        if (child->isAnonymousBlock())
+            collapseAnonymousBlockChild(this, toRenderBlock(child));
+        child = next;

[Julien - ping for review, 2014/05/07 01:38:46]

I would put a comment here as this is important:

// WARNING: |child| is destroyed here.

+    }
+}
+
 void RenderBlock::collapseAnonymousBlockChild(RenderBlock* parent, RenderBlock* child)
 {
     // It's possible that this block's destruction may have been triggered by the
     // child's removal. Just bail if the anonymous child block is already being
     // destroyed. See crbug.com/282088
     if (child->beingDestroyed())
         return;
     parent->setNeedsLayoutAndPrefWidthsRecalc();
     parent->setChildrenInline(child->childrenInline());
     RenderObject* nextSibling = child->nextSibling();
@@ skipping 3955 matching lines @@
 void RenderBlock::showLineTreeAndMark(const InlineBox* markedBox1, const char* markedLabel1, const InlineBox* markedBox2, const char* markedLabel2, const RenderObject* obj) const
 {
     showRenderObject();
     for (const RootInlineBox* root = firstRootBox(); root; root = root->nextRootBox())
         root->showLineTreeAndMark(markedBox1, markedLabel1, markedBox2, markedLabel2, obj, 1);
 }
 
 #endif
 
 } // namespace WebCore