Fix font-size shaking issue in some pages

third_party/WebKit/Source/core/layout/TextAutosizer.cpp

Index: third_party/WebKit/Source/core/layout/TextAutosizer.cpp
diff --git a/third_party/WebKit/Source/core/layout/TextAutosizer.cpp b/third_party/WebKit/Source/core/layout/TextAutosizer.cpp
index 4af1f5c63a330a978ddfd12803140a7120f740e2..a0b98b2e73dab12fa4cc2cd0cb449dc8424d6744 100644
--- a/third_party/WebKit/Source/core/layout/TextAutosizer.cpp
+++ b/third_party/WebKit/Source/core/layout/TextAutosizer.cpp
@@ -226,6 +226,10 @@ static LayoutObject* GetParent(const LayoutObject* object) {
   return parent;
 }
 
+static bool needMakeSelfAsWidthProvider(const LayoutBlock* block) {
+  return block->IsTable() || block->IsTableCell() || block->IsLayoutView();

[pdr., 2017/04/20 06:25:46]

I don't think it'll be correct to move the IsTable and IsTableCell changes too.
Aren't we just fixing an issue at the root of the document? If not, can you add
a test of the table changes?

[cathiechentx, 2017/04/27 10:51:42]

yes, table and td have the same inherit issues. I could add test cases.

Case 1:
<table><tr><td style="width:800px">
  <div style="width: 400px">
      This text should be autosized to just 20px computed font size, i.e. scaled
up by 1.25x, since although this block is in an 800px wide iframe, it is in a
400px block and 400 / 320 = 1.25. More text, long enough to autosize...
    </div>
</td></tr></table>
1. <div> will inherit <td>'s multiplier. In IsWiderOrNarrowerDescendant(), we
use DeepestBlockContainingAllText(not ClusterWidthProvider) of <td> to determine
if <div> should inherit from <td>.
2. <td>'s multiplier is 800 / 320 = 2.5. So the multiplier of <td> isn't as
expected.
3. Approach: Move needMakeSelfAsWidthProvider() from
TextAutosizer::ClusterWidthProvider() to
TextAutosizer::DeepestBlockContainingAllText().


After we move needMakeSelfAsWidthProvider to DeepestBlockContainingAllText.
Case2 still failed.

Case 2:
<table><tr><td style="width:800px">
  <div id="outer" style="width:800px;">
    <div id="inner" style="width: 400px">
      This text should be autosized to just 20px computed font size, i.e. scaled
up by 1.25x, since although this block is in an 800px wide iframe, it is in a
400px block and 400 / 320 = 1.25. More text, long enough to autosize...
    </div>
  </div>
</td></tr></table>
1. "inner" <div> inherit "outer" <div> which inherit <td>
2. "inner" <div>'s multiplier is still 2.5.
3. Approach: <td> could not be inherited

+}
+
 TextAutosizer::TextAutosizer(const Document* document)
     : document_(document),
       first_block_to_begin_layout_(nullptr),
@@ -911,9 +915,6 @@ float TextAutosizer::SuperclusterMultiplier(Cluster* cluster) {
 
 const LayoutBlock* TextAutosizer::ClusterWidthProvider(
     const LayoutBlock* root) const {
-  if (root->IsTable() || root->IsTableCell())
-    return root;
-
   return DeepestBlockContainingAllText(root);
 }
 
@@ -1009,6 +1010,9 @@ const LayoutBlock* TextAutosizer::DeepestBlockContainingAllText(
 // FIXME: Refactor this to look more like TextAutosizer::deepestCommonAncestor.
 const LayoutBlock* TextAutosizer::DeepestBlockContainingAllText(
     const LayoutBlock* root) const {
+  if (needMakeSelfAsWidthProvider(root))
+    return root;
+
   size_t first_depth = 0;
   const LayoutObject* first_text_leaf = FindTextLeaf(root, first_depth, kFirst);
   if (!first_text_leaf)
@@ -1149,6 +1153,10 @@ bool TextAutosizer::IsWiderOrNarrowerDescendant(Cluster* cluster) {
 
   const LayoutBlock* parent_deepest_block_containing_all_text =
       DeepestBlockContainingAllText(cluster->parent_);
+  // Block that needMakeSelfAsWidthProvider shouldn't be inherited.
+  if (needMakeSelfAsWidthProvider(parent_deepest_block_containing_all_text))

[pdr., 2017/04/20 06:25:46]

I think this change is just making child clusters of the layout view's cluster
return true. That will basically special-case the children of the layout view's
cluster to be a widerOrNarrowerDescendant. I think that'll fix the bug on
wikipedia but fail to fix the more general bug. I think it's okay to
special-case the root (layout view) but special-casing only direct descendants
of the layout view seems like it's too local of a fix.

Fix idea: in your previous response you said "Looks like this isn't a normal
scenario in general case(block with specific width). However it happens to
layoutView.". I wonder if we could remove layout view special cases so that the
layout view works more like a normal block with a specific width?

[cathiechentx, 2017/04/27 10:51:42]

If we choose to fix the general case, here is the problem we need to solve:

The font-size of a subCluster inside a specific width cluster(parentCluster)
shouldn't change when the DeepestBlockContainingAllText of parentCluster
changed.

Here is a specific example:

<div id="parentCluster" style="width:800px">
  <div id="subCluster" style="width:700px">
    ...
    text is long enough to autosize
    ...
  </div>
  <div id="EmptyAtFirst"></div>
</div>

1. First layout pass: Multiplier of subCluster is 700/320. SubCluster inherits
multiplier from parentCluster. The widthProvider is subCluster. So multiplier of
parentCluster is 700/320.

2. Insert text into EmptyAtFirst: The widthProvider of parentCluster is changed
to parentCluster itself. Multiplier of parentCluster is 800/320.

3. Trigger layout of subCluster: SubCluster inherits multiplier from
parentCluster. The new multiplier is 800/320. The old multiplier is 700/320. So
the font-size is changed.

SubCluster inherit mutable multiplier from parentCluster is the fundamental
problem. In order to reduce font-size change of subCluster. We could make
parentCluster using itself as widthProvider, like table and td. But
DeepestBlockContainingAllText is a basic design, there are risks to apply this
change to all clusters with specific width. Consider of the risk, this patch
just make LayoutView using itself as widthProvider, and others remain the same
as before. And it will cause two inherit issues as I mentioned before. (Table
and td have the same issues too). 

Or we could try another way. Any suggestions?

[cathiechentx, 2017/05/25 11:36:38]

We had a misunderstanding here. Sorry for my bad English expressions...

Actually, I mean "a normal block with a specific width" have font-size shaking
issue too. LayoutView is treat as "a normal block with a specific width", so
LayoutView has issus too.  LayoutView have lots of these scenarios which could
produce this issue: DeepestBlockContainingAllText of Layoutview grows wider as
its foot section loaded.

With this special treat of LayoutView, this could fix LayoutView scenarios. But
the normal block case couldn't be fixed.

+    return true;
+
 #if DCHECK_IS_ON()
   DCHECK(blocks_that_have_begun_layout_.Contains(cluster->root_));
   DCHECK(blocks_that_have_begun_layout_.Contains(