Split list marker updating into two steps

Source/core/rendering/RenderListItem.cpp

 /**
  * Copyright (C) 1999 Lars Knoll (knoll@kde.org)
  *           (C) 1999 Antti Koivisto (koivisto@kde.org)
  * Copyright (C) 2003, 2004, 2005, 2006, 2010 Apple Inc. All rights reserved.
  * Copyright (C) 2006 Andrew Wellington (proton@wiretapped.net)
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Library General Public
  * License as published by the Free Software Foundation; either
  * version 2 of the License, or (at your option) any later version.
@@ skipping 249 matching lines @@
 }
 
 static RenderObject* firstNonMarkerChild(RenderObject* parent)
 {
     RenderObject* result = parent->slowFirstChild();
     while (result && result->isListMarker())
         result = result->nextSibling();
     return result;
 }
 
-void RenderListItem::updateMarkerLocation()
+void RenderListItem::updateMarkerAndInvalidateWidth()
 {
-    // Sanity check the location of our marker.
-    if (m_marker) {
-        RenderObject* markerParent = m_marker->parent();
-        RenderObject* lineBoxParent = getParentOfFirstLineBox(this, m_marker);
-        if (!lineBoxParent) {
-            // If the marker is currently contained inside an anonymous box,
-            // then we are the only item in that anonymous box (since no line box
-            // parent was found).  It's ok to just leave the marker where it is
-            // in this case.
-            if (markerParent && markerParent->isAnonymousBlock())
-                lineBoxParent = markerParent;
-            else
-                lineBoxParent = this;
-        }
+    ASSERT(m_marker);
 
-        if (markerParent != lineBoxParent || m_marker->preferredLogicalWidthsDirty()) {
-            // FIXME: We should not modify the structure of the render tree
-            // during layout. crbug.com/370461
-            DeprecatedDisableModifyRenderTreeStructureAsserts disabler;
+    // FIXME: We should not modify the structure of the render tree
+    // during layout. crbug.com/370461
+    DeprecatedDisableModifyRenderTreeStructureAsserts disabler;
+    // Removing and adding the marker can trigger repainting in
+    // containers other than ourselves, so we need to disable LayoutState.
+    ForceHorriblySlowRectMapping slowRectMapping(*this);
+    if (updateMarker()) {
+        // If the marker is inside we need to redo the preferred width calculations
+        // as the size of the item now includes the size of the list marker.
+        if (m_marker->isInside())
+            containingBlock()->updateLogicalWidth();
+    }
+}
 
-            // Removing and adding the marker can trigger repainting in
-            // containers other than ourselves, so we need to disable LayoutState.
-            ForceHorriblySlowRectMapping slowRectMapping(*this);
-            updateFirstLetter();
-            m_marker->remove();
-            if (markerParent)
-                markerParent->dirtyLinesFromChangedChild(m_marker);

[leviw_travelin_and_unemployed, 2014/07/02 18:07:14]

I'm suspicious of this line being removed entirely. Perhaps do this in a
different change?

[pdr., 2014/07/02 18:39:30]

I agree, lets do this as a followup. Added a fixme and filed
http://crbug.com/391009

-            if (!lineBoxParent)
-                lineBoxParent = this;
-            lineBoxParent->addChild(m_marker, firstNonMarkerChild(lineBoxParent));
-            m_marker->updateMarginsAndContent();
-            // If markerParent is an anonymous block that has lost all its children, destroy it.
-            if (markerParent && markerParent->isAnonymousBlock() && !toRenderBlock(markerParent)->firstChild() && !toRenderBlock(markerParent)->continuation())
-                markerParent->destroy();
+bool RenderListItem::updateMarker()
+{
+    ASSERT(m_marker);
+    RenderObject* markerParent = m_marker->parent();
+    RenderObject* lineBoxParent = getParentOfFirstLineBox(this, m_marker);
+    if (!lineBoxParent) {
+        // If the marker is currently contained inside an anonymous box, then we
+        // are the only item in that anonymous box (since no line box parent was
+        // found). It's ok to just leave the marker where it is in this case.
+        if (markerParent && markerParent->isAnonymousBlock())
+            lineBoxParent = markerParent;
+        else
+            lineBoxParent = this;
+    }
 
-            // If the marker is inside we need to redo the preferred width calculations
-            // as the size of the item now includes the size of the list marker.
-            if (m_marker->isInside())
-                containingBlock()->updateLogicalWidth();
-        }
+    if (markerParent != lineBoxParent) {
+        updateFirstLetter();
+        m_marker->remove();
+        lineBoxParent->addChild(m_marker, firstNonMarkerChild(lineBoxParent));
+        m_marker->updateMarginsAndContent();
+        // If markerParent is an anonymous block with no children, destroy it.
+        if (markerParent && markerParent->isAnonymousBlock() && !toRenderBlock(markerParent)->firstChild() && !toRenderBlock(markerParent)->continuation())
+            markerParent->destroy();
+        return true;
     }
+
+    return false;
 }
 
 void RenderListItem::layout()
 {
     ASSERT(needsLayout());
 
-    // The marker must be autosized before calling updateMarkerLocation.
-    // It cannot be done in the parent's beginLayout because it is not yet in the render tree.
     if (m_marker) {
-        FastTextAutosizer* textAutosizer = document().fastTextAutosizer();
-        if (textAutosizer)
+        // The marker must be autosized before calling
+        // updateMarkerAndInvalidateWidth. It cannot be done in the parent's
+        // beginLayout because it is not yet in the render tree.
+        if (FastTextAutosizer* textAutosizer = document().fastTextAutosizer())
             textAutosizer->inflateListItem(this, m_marker);
+
+        updateMarkerAndInvalidateWidth();
     }
 
-    updateMarkerLocation();
     RenderBlockFlow::layout();
 }
 
 void RenderListItem::addOverflowFromChildren()
 {
     RenderBlockFlow::addOverflowFromChildren();
     positionListMarker();
 }
 
 void RenderListItem::positionListMarker()
@@ skipping 130 matching lines @@
 {
     ASSERT(node());
 
     if (!m_hasExplicitValue)
         return;
     m_hasExplicitValue = false;
     m_isValueUpToDate = false;
     explicitValueChanged();
 }
 
+void RenderListItem::setNotInList(bool notInList)
+{
+    m_notInList = notInList;
+    if (m_marker)
+        updateMarker();
+}
+
 static RenderListItem* previousOrNextItem(bool isListReversed, Node* list, RenderListItem* item)
 {
     return isListReversed ? previousListItem(list, item) : nextListItem(list, item);
 }
 
 void RenderListItem::updateListMarkerNumbers()
 {
     // If distribution recalc is needed, updateListMarkerNumber will be re-invoked
     // after distribution is calculated.
     if (node()->document().childNeedsDistributionRecalc())
@@ skipping 22 matching lines @@
             // assume that all the following ones have too.
             // This gives us the opportunity to stop here and avoid
             // marking the same nodes again.
             break;
         }
         item->updateValue();
     }
 }
 
 } // namespace WebCore