[webkit-reviews] review granted: [Bug 191801] [LFC][IFC] InlineFormattingState::addDetachingRule should accumulate rules. : [Attachment 355180] Patch

[bugzilla-daemon at webkit.org]

Antti Koivisto <koivisto at iki.fi> has granted zalan <zalan at apple.com>'s request
for review:
Bug 191801: [LFC][IFC] InlineFormattingState::addDetachingRule should
accumulate rules.
https://bugs.webkit.org/show_bug.cgi?id=191801

Attachment 355180: Patch

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




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

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

> Source/WebCore/layout/inlineformatting/InlineFormattingState.cpp:58
> +    auto result = m_detachingRules.add(&layoutBox, detachingRule);
> +    if (result.isNewEntry)
> +	   return;
> +    result.iterator->value.add(detachingRule);

m_detachingRules.add(&layoutBox, { }).iterator->value.add(detachingRule);

is probably equivalent and bit more compact?

> Source/WebCore/layout/inlineformatting/InlineFormattingState.h:77
> +    void setDetachingRules(const Box& layoutBox, DetachingRules
detachingRules) { m_detachingRules.set(&layoutBox, detachingRules); }

Not from this patch but most OptionSet sites don't have type alias but simply
write out the whole thing. 'OptionSet<DetachingRule>' is a less mysterious type
than 'DetachingRules'.