Update list markers in notify change.

Source/core/layout/TextAutosizer.cpp

 /*
  * Copyright (C) 2013 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
  * met:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
  *     * Redistributions in binary form must reproduce the above
@@ skipping 354 matching lines @@
 
     if (Cluster* cluster = maybeCreateCluster(block))
         m_clusterStack.append(adoptPtr(cluster));
 
     // Cells in auto-layout tables are handled separately by inflateAutoTable.
     bool isAutoTableCell = block->isTableCell() && !toLayoutTableCell(block)->table()->style()->isFixedTableLayout();
     if (!isAutoTableCell && !m_clusterStack.isEmpty())
         inflate(block);
 }
 
-void TextAutosizer::inflateListItem(LayoutListItem* listItem, LayoutListMarker* listItemMarker)
-{
-    if (!shouldHandleLayout())
-        return;
-    ASSERT(listItem && listItemMarker);
-
-    if (prepareForLayout(listItem) == StopLayout)
-        return;
-
-    // Force the LI to be inside the DBCAT when computing the multiplier.
-    // This guarantees that the DBCAT has entered layout, so we can ask for its width.
-    // It also makes sense because the list marker is autosized like a text node.
-    float multiplier = clusterMultiplier(currentCluster());
-
-    applyMultiplier(listItem, multiplier);
-    applyMultiplier(listItemMarker, multiplier);
-}
-
 void TextAutosizer::inflateAutoTable(LayoutTable* table)
 {
     ASSERT(table);
     ASSERT(!table->style()->isFixedTableLayout());
     ASSERT(table->containingBlock());
 
     Cluster* cluster = currentCluster();
     if (cluster->m_root != table)
         return;
 
@@ skipping 46 matching lines @@
         child = toLayoutInline(parent)->firstChild();
 
     while (child) {
         if (child->isText()) {
             hasTextChild = true;
             // We only calculate this multiplier on-demand to ensure the parent block of this text
             // has entered layout.
             if (!multiplier)
                 multiplier = cluster->m_flags & SUPPRESSING ? 1.0f : clusterMultiplier(cluster);
             applyMultiplier(child, multiplier);
+
             // FIXME: Investigate why MarkOnlyThis is sufficient.
             if (parent->isLayoutInline())
                 child->setPreferredLogicalWidthsDirty(MarkOnlyThis);
         } else if (child->isLayoutInline()) {
             multiplier = inflate(child, behavior, multiplier);
         } else if (child->isLayoutBlock() && behavior == DescendToInnerBlocks
             && !classifyBlock(child, INDEPENDENT | EXPLICIT_WIDTH | SUPPRESSING)) {
             multiplier = inflate(child, behavior, multiplier);
         }
         child = child->nextSibling();
     }
 
     if (hasTextChild) {
         applyMultiplier(parent, multiplier); // Parent handles line spacing.
     } else if (!parent->isListItem()) {
         // For consistency, a block with no immediate text child should always have a
-        // multiplier of 1 (except for list items which are handled in inflateListItem).
+        // multiplier of 1.
         applyMultiplier(parent, 1);
     }
+
+    if (parent->isListItem()) {
+        float multiplier = clusterMultiplier(cluster);

[brucedawson, 2015/05/11 18:02:28]

This variable shadows the same named parameter to this function. This creates
some reader ambiguity in the code because it is not clear whether multiplier on
line 472 is supposed to refer to this multiplier or the function parameter.

Similarly, it is not clear to a reader whether this line is supposed to affect
the return value of the function.

In other words, the code is not necessarily buggy, but it is also not clearly
correct. Probably worth renaming or fixing to avoid possible confusion.

+        applyMultiplier(parent, multiplier);
+
+        // The list item has to be treated special because we can have a tree such that you have
+        // a list item for a form inside it. The list marker then ends up inside the form and when
+        // we try to get the clusterMultiplier we have the wrong cluster root to work from and get
+        // the wrong value.
+        LayoutListItem* item = toLayoutListItem(parent);
+        if (LayoutListMarker* marker = item->marker()) {
+            applyMultiplier(marker, multiplier);
+            marker->setPreferredLogicalWidthsDirty(MarkOnlyThis);
+        }
+    }
+
     return multiplier;
 }
 
 bool TextAutosizer::shouldHandleLayout() const
 {
     return m_pageInfo.m_settingEnabled && m_pageInfo.m_pageNeedsAutosizing && !m_updatePageInfoDeferred;
 }
 
 void TextAutosizer::updatePageInfoInAllFrames()
 {
@@ skipping 445 matching lines @@
     if (!containingBlock)
         return root;
 
     ASSERT(containingBlock->isDescendantOf(root));
     return containingBlock;
 }
 
 const LayoutObject* TextAutosizer::findTextLeaf(const LayoutObject* parent, size_t& depth, TextLeafSearch firstOrLast) const
 {
     // List items are treated as text due to the marker.
-    // The actual layoutObject for the marker (LayoutListMarker) may not be in the tree yet since it is added during layout.
     if (parent->isListItem())
         return parent;
 
     if (parent->isText())
         return parent;
 
     ++depth;
     const LayoutObject* child = (firstOrLast == First) ? parent->slowFirstChild() : parent->slowLastChild();
     while (child) {
         // Note: At this point clusters may not have been created for these blocks so we cannot rely
@@ skipping 217 matching lines @@
     }
     return computedSize;
 }
 
 DEFINE_TRACE(TextAutosizer)
 {
     visitor->trace(m_document);
 }
 
 } // namespace blink