Bug 193274 – Implement TextEncoder's encodeInto()

• Status: RESOLVED
• Resolution: FIXED
• Component: DOM

[Anne van Kesteren]

This API allows for encoding a string into UTF-8 bytes using a preexisting target buffer.

See https://github.com/whatwg/encoding/pull/166 for the standard change and https://github.com/web-platform-tests/wpt/pull/14505 for tests.

[Alex Christensen]

Created attachment 407853 [details]
Patch

[Darin Adler]

Comment on attachment 407853 [details]
Patch

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

> Source/WebCore/dom/TextEncoder.cpp:64
> +        uint8_t buffer[U8_MAX_LENGTH];
> +        unsigned offset = 0;
> +        U8_APPEND(buffer, offset, sizeof(buffer), token, sawError);
> +        if (sawError)
> +            break;
> +        if (written + offset > capacity)
> +            break;
> +        memcpy(destinationBytes + written, buffer, offset);
> +        written += offset;

Since U8_APPEND has bounds checking built in we don’t need to keep using a buffer every time. We could do something more like this:

    auto offset = written;
    U8_APPEND(destinationBytes, offset, capacity, token, sawError);
    if (sawError)
        break;
    written = offset;

> Source/WebCore/dom/TextEncoder.cpp:68
> +        if (U_IS_BMP(token))
> +            read++;
> +        else
> +            read += 2;

This could be:

    read += U16_LENGTH(token);

[Darin Adler]

Comment on attachment 407853 [details]
Patch

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

>> Source/WebCore/dom/TextEncoder.cpp:64
>> +        written += offset;
> 
> Since U8_APPEND has bounds checking built in we don’t need to keep using a buffer every time. We could do something more like this:
> 
>     auto offset = written;
>     U8_APPEND(destinationBytes, offset, capacity, token, sawError);
>     if (sawError)
>         break;
>     written = offset;

Reviewed the U8_APPEND implementation and we can do better than that:

    U8_APPEND(destinationBytes, written, capacity, token, sawError);
    if (sawError)
        break;

[youenn fablet]

Comment on attachment 407853 [details]
Patch

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

> Source/WebCore/dom/TextEncoder.idl:31
> +};

Seems like there is an issue with the binding generator here in identifying that TextEncoder should be exposed to worker.

Just move the dictionary declaration in another idl file or at the end of this file to circumvent the issue.
We should probably file a bug for this issue.

[Alex Christensen]

Adding Exposed=(Window,Worker) to the dictionary works, too.

[youenn fablet]

(In reply to Alex Christensen from comment #5)
> Adding Exposed=(Window,Worker) to the dictionary works, too.

Sure but I do not think dictionaries have Exposed in WebIDL.

[Alex Christensen]

(In reply to Darin Adler from comment #3)
>     U8_APPEND(destinationBytes, written, capacity, token, sawError);
>     if (sawError)
>         break;

This works, but I first have to check if written == capacity and break.  It is a precondition of U8_APPEND that written is strictly less than capacity.

[Alex Christensen]

http://trac.webkit.org/r266533

[Radar WebKit Bug Importer]

<rdar://problem/68288806>

[Alex Christensen]

http://trac.webkit.org/r266621