[FastTextAutosizer] Refactor FTA::inflate() to be internal to FTA

[FastTextAutosizer] Refactor FTA::inflate() to be internal to FTA

This patch moves inflate() to be a private internal detail of
FastTextAutosizer and relies on beginLayout to call inflate. This
patch also has a minor cleanup removing an untrue FIXME in inflate().

No new tests as this is just a refactoring.

BUG=302005
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=165134

[skobes, 2014-01-15 01:12:20]

lgtm

https://codereview.chromium.org/138143004/diff/1/Source/core/rendering/FastTe...
File Source/core/rendering/FastTextAutosizer.cpp (right):

https://codereview.chromium.org/138143004/diff/1/Source/core/rendering/FastTe...
Source/core/rendering/FastTextAutosizer.cpp:87: if (block->childrenInline())
Can a block return true for childrenInline() but have RenderBlock objects
beneath it (maybe with display:inline-block or something like that)?

If so we should make sure inflate doesn't descend into RenderBlocks.

[pdr., 2014-01-15 07:08:54]

https://codereview.chromium.org/138143004/diff/1/Source/core/rendering/FastTe...
File Source/core/rendering/FastTextAutosizer.cpp (right):

https://codereview.chromium.org/138143004/diff/1/Source/core/rendering/FastTe...
Source/core/rendering/FastTextAutosizer.cpp:87: if (block->childrenInline())
On 2014/01/15 01:12:20, skobes wrote:
> Can a block return true for childrenInline() but have RenderBlock objects
> beneath it (maybe with display:inline-block or something like that)?
> 
> If so we should make sure inflate doesn't descend into RenderBlocks.

Excellent catch, I found childrenInline() does not imply that the block has no
block children!

I've filed crbug.com/334549 to fix this.

[pdr., 2014-01-15 07:28:04]

Pasting this chatlog as you may find it helpful as well:

10:59 PM <pdr> esprehn, answering my own question: no, it doesn't imply that at
all.
11:00 PM  ⇐ tasak, sadrul, jyasskin, stromsund, c4milo and nick_46_2 quit  ↔
scottmg nipped out  
11:19 PM <pdr> dstockwell, Is the CQ down?
11:22 PM ⇐ lgombos quit (~gombos@210.94.41.89) Read error: Connection reset by
peer
11:22 PM <eseidel> pdr: it shouldn't have direct block children if
childrenInline() are set, no?
11:22 PM <eseidel> pdr: since blocks have either/or
11:22 PM <eseidel> either all block or all inline kids
11:23 PM <•dstockwell> pdr: I think it's barely alive :/
11:23 PM <pdr> dstockwell, ok, I'll be patient :)
https://codereview.chromium.org/137123004 has been in the hopper for 7.5 hours
New messages since you tabbed out
11:23 PM  → •lgombos (voiced), imjacobclark and YukiSeki_ joined  ⇐ scottmg quit
 
11:25 PM <pdr> eseidel, I found childrenInline can return true with inlineBlock
children.
New messages
11:25 PM <eseidel> pdr: they're still inline :)
11:25 PM <eseidel> pdr: display is really a split concept. how you are to your
parent and how you are to your kids
11:25 PM <eseidel> block = block for both
11:25 PM <eseidel> inline = inline for both
11:25 PM <eseidel> inline-block means inline for parent, block for kids
11:26 PM <eseidel> inlines can only have inline kids
11:26 PM <eseidel> blocks can have either all inlines or all blocks
11:26 PM <eseidel> if they have a mix, the inlines get wrapped in anonymous
blocks
11:26 PM <eseidel> if inlines have a block in them, then the inlines get split! 
and wrapped in anonymous blocks, and turned into continuations :(
11:26 PM <eseidel> pdr: make sense?
11:27 PM <pdr> eseidel, got it, thanks for the explanation!

[commit-bot: I haz the power, 2014-01-15 07:34:48]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/pdr@chromium.org/138143004/1

[commit-bot: I haz the power, 2014-01-15 15:28:34]

Change committed as 165134

[skobes, 2014-01-15 19:20:40]

Sounds like if childrenInline() is true, the RenderBlock won't have direct block
children, but it may have indirect block children, since an inline-block child
will be RenderInline but have RenderBlocks underneath it... so
http://crbug.com/334549 is still relevant?  Or does InlineWalker handle this
already?


On 2014/01/15 07:28:04, pdr wrote:
> Pasting this chatlog as you may find it helpful as well:
> 
> 10:59 PM <pdr> esprehn, answering my own question: no, it doesn't imply that
at
> all.
> 11:00 PM  ⇐ tasak, sadrul, jyasskin, stromsund, c4milo and nick_46_2 quit  ↔
> scottmg nipped out  
> 11:19 PM <pdr> dstockwell, Is the CQ down?
> 11:22 PM ⇐ lgombos quit (mailto:~gombos@210.94.41.89) Read error: Connection
reset by
> peer
> 11:22 PM <eseidel> pdr: it shouldn't have direct block children if
> childrenInline() are set, no?
> 11:22 PM <eseidel> pdr: since blocks have either/or
> 11:22 PM <eseidel> either all block or all inline kids
> 11:23 PM <•dstockwell> pdr: I think it's barely alive :/
> 11:23 PM <pdr> dstockwell, ok, I'll be patient :)
> https://codereview.chromium.org/137123004 has been in the hopper for 7.5 hours
> New messages since you tabbed out
> 11:23 PM  → •lgombos (voiced), imjacobclark and YukiSeki_ joined  ⇐ scottmg
quit
>  
> 11:25 PM <pdr> eseidel, I found childrenInline can return true with
inlineBlock
> children.
> New messages
> 11:25 PM <eseidel> pdr: they're still inline :)
> 11:25 PM <eseidel> pdr: display is really a split concept. how you are to your
> parent and how you are to your kids
> 11:25 PM <eseidel> block = block for both
> 11:25 PM <eseidel> inline = inline for both
> 11:25 PM <eseidel> inline-block means inline for parent, block for kids
> 11:26 PM <eseidel> inlines can only have inline kids
> 11:26 PM <eseidel> blocks can have either all inlines or all blocks
> 11:26 PM <eseidel> if they have a mix, the inlines get wrapped in anonymous
> blocks
> 11:26 PM <eseidel> if inlines have a block in them, then the inlines get
split! 
> and wrapped in anonymous blocks, and turned into continuations :(
> 11:26 PM <eseidel> pdr: make sense?
> 11:27 PM <pdr> eseidel, got it, thanks for the explanation!