 Chromium Code Reviews
 Chromium Code Reviews Issue 1110233003:
  Update list markers in notify change.  (Closed) 
  Base URL: svn://svn.chromium.org/blink/trunk
    
  
    Issue 1110233003:
  Update list markers in notify change.  (Closed) 
  Base URL: svn://svn.chromium.org/blink/trunk| Index: Source/core/layout/LayoutListItem.cpp | 
| diff --git a/Source/core/layout/LayoutListItem.cpp b/Source/core/layout/LayoutListItem.cpp | 
| index 35a1e778fc5c5e961fb42caae3ae943d0c0ff85c..8d7f80bc34ab2a19c4510758f739899069a4a88a 100644 | 
| --- a/Source/core/layout/LayoutListItem.cpp | 
| +++ b/Source/core/layout/LayoutListItem.cpp | 
| @@ -29,7 +29,6 @@ | 
| #include "core/html/HTMLOListElement.h" | 
| #include "core/layout/LayoutListMarker.h" | 
| #include "core/layout/LayoutView.h" | 
| -#include "core/layout/TextAutosizer.h" | 
| #include "wtf/StdLibExtras.h" | 
| #include "wtf/text/StringBuilder.h" | 
| @@ -45,6 +44,10 @@ LayoutListItem::LayoutListItem(Element* element) | 
| , m_notInList(false) | 
| { | 
| setInline(false); | 
| + | 
| + setConsumesSubtreeChangeNotification(); | 
| + registerSubtreeChangeListenerOnDescendants(true); | 
| + notifyOfSubtreeChange(); | 
| 
esprehn
2015/05/06 16:10:28
This isn't right though, it's like calling setNeed
 | 
| } | 
| void LayoutListItem::styleDidChange(StyleDifference diff, const ComputedStyle* oldStyle) | 
| @@ -56,6 +59,7 @@ void LayoutListItem::styleDidChange(StyleDifference diff, const ComputedStyle* o | 
| if (!m_marker) | 
| m_marker = LayoutListMarker::createAnonymous(this); | 
| m_marker->listItemStyleDidChange(); | 
| + notifyOfSubtreeChange(); | 
| } else if (m_marker) { | 
| m_marker->destroy(); | 
| m_marker = nullptr; | 
| @@ -85,6 +89,20 @@ void LayoutListItem::willBeRemovedFromTree() | 
| updateListMarkerNumbers(); | 
| } | 
| +void LayoutListItem::subtreeDidChange() | 
| +{ | 
| + if (!m_marker) | 
| + return; | 
| + | 
| + if (!updateMarkerLocation()) | 
| + return; | 
| + | 
| + // 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()) | 
| + setPreferredLogicalWidthsDirty(); | 
| +} | 
| + | 
| static bool isList(const Node& node) | 
| { | 
| return isHTMLUListElement(node) || isHTMLOListElement(node); | 
| @@ -265,38 +283,10 @@ static LayoutObject* firstNonMarkerChild(LayoutObject* parent) | 
| return result; | 
| } | 
| -void LayoutListItem::updateMarkerLocationAndInvalidateWidth() | 
| -{ | 
| - ASSERT(m_marker); | 
| - | 
| - // FIXME: We should not modify the structure of the layout tree | 
| - // during layout. crbug.com/370461 | 
| - DeprecatedDisableModifyLayoutTreeStructureAsserts disabler; | 
| - LayoutState* layoutState = view()->layoutState(); | 
| - LayoutFlowThread* currentFlowThread = nullptr; | 
| - if (layoutState) { | 
| - // We're about to modify the layout tree structure (during layout!), and any code using | 
| - // LayoutState might get utterly confused by that. There's no evidence that anything other | 
| - // than the flow thread code will suffer, though, so just reset the current flow thread | 
| - // temporarily. | 
| - // FIXME: get rid of this hack, including the flow thread setter in LayoutState, as part of | 
| - // fixing crbug.com/370461 | 
| - currentFlowThread = layoutState->flowThread(); | 
| - layoutState->setFlowThread(nullptr); | 
| - } | 
| - if (updateMarkerLocation()) { | 
| - // 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 (layoutState) | 
| - layoutState->setFlowThread(currentFlowThread); | 
| -} | 
| - | 
| bool LayoutListItem::updateMarkerLocation() | 
| { | 
| ASSERT(m_marker); | 
| + | 
| LayoutObject* markerParent = m_marker->parent(); | 
| LayoutObject* lineBoxParent = getParentOfFirstLineBox(this, m_marker); | 
| if (!lineBoxParent) { | 
| @@ -322,23 +312,6 @@ bool LayoutListItem::updateMarkerLocation() | 
| return false; | 
| } | 
| -void LayoutListItem::layout() | 
| -{ | 
| - ASSERT(needsLayout()); | 
| - | 
| - if (m_marker) { | 
| - // The marker must be autosized before calling | 
| - // updateMarkerLocationAndInvalidateWidth. It cannot be done in the | 
| - // parent's beginLayout because it is not yet in the layout tree. | 
| - if (TextAutosizer* textAutosizer = document().textAutosizer()) | 
| - textAutosizer->inflateListItem(this, m_marker); | 
| - | 
| - updateMarkerLocationAndInvalidateWidth(); | 
| - } | 
| - | 
| - LayoutBlockFlow::layout(); | 
| -} | 
| - | 
| void LayoutListItem::addOverflowFromChildren() | 
| { | 
| LayoutBlockFlow::addOverflowFromChildren(); | 
| @@ -347,7 +320,7 @@ void LayoutListItem::addOverflowFromChildren() | 
| void LayoutListItem::positionListMarker() | 
| { | 
| - if (m_marker && m_marker->parent()->isBox() && !m_marker->isInside() && m_marker->inlineBoxWrapper()) { | 
| + if (m_marker && m_marker->parent() && m_marker->parent()->isBox() && !m_marker->isInside() && m_marker->inlineBoxWrapper()) { | 
| LayoutUnit markerOldLogicalLeft = m_marker->logicalLeft(); | 
| LayoutUnit blockOffset = 0; | 
| LayoutUnit lineOffset = 0; | 
| @@ -489,8 +462,6 @@ void LayoutListItem::clearExplicitValue() | 
| void LayoutListItem::setNotInList(bool notInList) | 
| { | 
| m_notInList = notInList; | 
| - if (m_marker) | 
| - updateMarkerLocation(); | 
| } | 
| static LayoutListItem* previousOrNextItem(bool isListReversed, Node* list, LayoutListItem* item) |