[webkit-reviews] review denied: [Bug 79125] CachedScriptSourceProvider::getRange can unnecessarily make a full copy of the source : [Attachment 140039] new patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has denied Yong Li <yong.li.webkit at gmail.com>'s
request for review:
Bug 79125: CachedScriptSourceProvider::getRange can unnecessarily make a full
copy of the source
https://bugs.webkit.org/show_bug.cgi?id=79125

Attachment 140039: new patch
https://bugs.webkit.org/attachment.cgi?id=140039&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=140039&action=review


> Source/WebCore/bindings/js/CachedScriptSourceProvider.h:59
>	       ASSERT(start + length <= this->length());
>  
>	       String script = m_cachedScript->script();
> +
> +	       if (!start && length == script.length())
> +		   return stringToUString(script);
> +
>	       return JSC::UString(StringImpl::create(script.impl(), start,
length));

This whole thing can be done as a one-liner, without all those assertions,
using the substring function, which already correctly optimizes the case of a
range covering the entire string.

    return stringToUString(m_cachedScript->script().substring(start, end -
start));

> Source/WebCore/bindings/js/StringSourceProvider.h:54
>	       ASSERT(length >= 0);
>	       ASSERT(start + length <= this->length());
>  
> +	       if (!start && length == m_source.length())
> +		   return stringToUString(m_source);
> +
>	       return JSC::UString(StringImpl::create(m_source.impl(), start,
length));

Same here:

    return stringToUString(m_source.substring(start, end - start));