[webkit-reviews] review granted: [Bug 205623] [LFC][IFC] Add LineBreaker::Result::Revert to indicate an earlier line wrap opportunity : [Attachment 386480] Patch

[bugzilla-daemon at webkit.org]

Antti Koivisto <koivisto at iki.fi> has granted zalan <zalan at apple.com>'s request
for review:
Bug 205623: [LFC][IFC] Add LineBreaker::Result::Revert to indicate an earlier
line wrap opportunity
https://bugs.webkit.org/show_bug.cgi?id=205623

Attachment 386480: Patch

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




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

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

> Source/WebCore/layout/inlineformatting/InlineLineBreaker.cpp:140
> -	   return { Result::Action::Keep, IsEndOfLine::No, { } };
> +	   return Result { Result::Action::Keep };

Why name the type here and elsewhere?

> Source/WebCore/layout/inlineformatting/InlineLineBreaker.h:66
> +	   Result(Action, IsEndOfLine = IsEndOfLine::No,
Optional<PartialTrailingContent> = WTF::nullopt, const InlineItem* revertTo =
nullptr);
> +
> +	   Action action { Action::Keep };
> +	   IsEndOfLine isEndOfLine { IsEndOfLine::No };
>	   Optional<PartialTrailingContent> partialTrailingContent;
> +	   const InlineItem* revertTo { nullptr };
>      };

I don't think you need a constructor if you make it

Optional<PartialTrailingContent> partialTrailingContent { };