[webkit-reviews] review denied: [Bug 109694] Add support for convenient conversion from JSStringRef to QString : [Attachment 188081] Patch

[bugzilla-daemon at webkit.org]

Benjamin Poulain <benjamin at webkit.org> has denied Simon Hausmann
<hausmann at webkit.org>'s request for review:
Bug 109694: Add support for convenient conversion from JSStringRef to QString
https://bugs.webkit.org/show_bug.cgi?id=109694

Attachment 188081: Patch
https://bugs.webkit.org/attachment.cgi?id=188081&action=review

------- Additional Comments from Benjamin Poulain <benjamin at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=188081&action=review


Honestly, that looks like an early optimization.


> Source/JavaScriptCore/API/OpaqueJSString.h:87
> +#if PLATFORM(QT)
> +	   // If we're going to make a deep copy for use in the API layer,
> +	   // then we might as well store it in a format particularly
> +	   // suitable for "export" later.
> +	   m_string = QString(string);
> +#else
>	   // Make a copy of the source string.
>	   if (string.is8Bit())
>	       m_string = String(string.characters8(), string.length());
>	   else
>	       m_string = String(string.characters16(), string.length());
> +#endif
>      }

This should probably be better replaced by
    m_string = string.isolatedCopy()

You can then make a special case for BufferAdoptedQString.