[webkit-reviews] review granted: [Bug 187688] [LFC] Introduce simple line breaker. : [Attachment 345345] Patch

[bugzilla-daemon at webkit.org]

Antti Koivisto <koivisto at iki.fi> has granted  review:
Bug 187688: [LFC] Introduce simple line breaker.
https://bugs.webkit.org/show_bug.cgi?id=187688

Attachment 345345: Patch

https://bugs.webkit.org/attachment.cgi?id=345345&action=review




--- Comment #8 from Antti Koivisto <koivisto at iki.fi> ---
Comment on attachment 345345
  --> https://bugs.webkit.org/attachment.cgi?id=345345
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=345345&action=review

> Source/WebCore/Sources.txt:1237
> +layout/inlineformatting/textlayout/ContentProvider.cpp
> +layout/inlineformatting/textlayout/simple/SimpleContentProvider.cpp
> +layout/inlineformatting/textlayout/simple/SimpleLineBreaker.cpp

Do we need such deep paths at this point? Also I suppose ContentProvide won't
be explicitly about 'text layout' in the future as it will cover non-text
inline layout too?

> Source/WebCore/layout/inlineformatting/textlayout/ContentProvider.h:42
> +class ContentProvider {

Layout::ContentProvider is bit generic. Maybe the name could include word
'inline'? InlineProvider or something.

>
Source/WebCore/layout/inlineformatting/textlayout/simple/SimpleContentProvider.
h:40
> +class SimpleContentProvider {

Naming, similarly.