[webkit-reviews] review denied: [Bug 184182] Fix the lifetime of the TextRun : [Attachment 337246] Patch

[bugzilla-daemon at webkit.org]

Daniel Bates <dbates at webkit.org> has denied Said Abou-Hallawa
<sabouhallawa at apple.com>'s request for review:
Bug 184182: Fix the lifetime of the TextRun
https://bugs.webkit.org/show_bug.cgi?id=184182

Attachment 337246: Patch

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




--- Comment #2 from Daniel Bates <dbates at webkit.org> ---
Comment on attachment 337246
  --> https://bugs.webkit.org/attachment.cgi?id=337246
Patch

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

> Source/WebCore/platform/graphics/FontCascade.cpp:377
> +float FontCascade::widthForSimpleText(const StringView& text) const

This is not necessary. StringView is a small object and should be passed by
value.

> Source/WebCore/platform/graphics/TextRun.h:103
> +	   m_string = emptyString();

I do not understand the need for TextRun to hold both a non-owning and owning
String. It should either hold a non-owned string, StringView, or hold a String,
not both.