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 457 matching lines @@
         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).
         applyMultiplier(parent, 1);
     }
+
+    if (parent->isListItem()) {

[ojan, 2015/04/30 02:56:45]

I think it'd be better to keep this code in LayoutListItem::layout instead of in
the generic TextAutosizer code. It avoids a needless branch for non list-items
that way.

pdr, mind taking a look over this since you understand the auto-sizing code? I
believe the code inside this if-statement can just be wholesale moved back into
LayoutListItem::layout.

[dsinclair, 2015/04/30 17:52:30]

From reading the comment in the old LayoutListItem::layout it sounds like that
was also the wrong place for it:

        // The marker must be autosized before calling
        // updateMarkerLocationAndInvalidateWidth. It cannot be done in the
        // parent's beginLayout because it is not yet in the render tree.

We should now have the marker in the tree before ::layout is kicked off. It
looks like this is called through the beginLayout part of the autosizer?

[ojan, 2015/05/01 01:09:55]

Yeah. I don't really understand this comment. pdr or skobes should take a look
to see what the right thing to do here is.

[skobes, 2015/05/01 17:58:16]

TextAutosizer::inflateListItem was introduced because of the strange way the
list item marker was added to the tree during layout.  We had to inflate the
marker before the call to LayoutListItem::updateMarkerLocation, so we added a
new hook, since by the time we call inflate() on the list item it is too late to
autosize.

The change that added it is https://codereview.chromium.org/145003009/.

If we no longer add the marker during layout, perhaps inflateListItem can go
away.  In any case it doesn't make much sense to call inflateListItem from
wihtin inflate.

You can run the tests in fast/text-autosizing to verify that you are not
regressing the behavior.

+        LayoutListItem* item = toLayoutListItem(parent);
+        if (item->marker()) {
+            inflateListItem(item, item->marker());
+            item->marker()->setPreferredLogicalWidthsDirty(MarkOnlyThis);
+        }
+    }
+
     return multiplier;
 }
 
 bool TextAutosizer::shouldHandleLayout() const
 {
     return m_pageInfo.m_settingEnabled && m_pageInfo.m_pageNeedsAutosizing && !m_updatePageInfoDeferred;
 }
 
 void TextAutosizer::updatePageInfoInAllFrames()
 {
@@ skipping 683 matching lines @@
     }
     return computedSize;
 }
 
 DEFINE_TRACE(TextAutosizer)
 {
     visitor->trace(m_document);
 }
 
 } // namespace blink