Bug 90319 – Make TextCodecLatin1 handle 8 bit data without converting to UChar's

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

[Michael Saboff]

Most of the Latin-1 tagged resources on the web can be processed as 8-bit data using 8-bit strings.  The task is to modify TextCodecLatin1 and related code to return strings appropriate for the source data.

[Michael Saboff]

Created attachment 151833 [details]
Patch for Review

[Oliver Hunt]

Comment on attachment 151833 [details]
Patch for Review

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

> Source/WebCore/platform/text/TextCodecASCIIFastPath.h:43
> +    static void copy(LChar* destination, const uint8_t* source)
> +    {
> +        destination[0] = source[0];
> +        destination[1] = source[1];
> +        destination[2] = source[2];
> +        destination[3] = source[3];
> +    }
> +    

What does the assembly for this look like?

> Source/WebCore/platform/text/TextCodecASCIIFastPath.h:62
> +        destination[0] = source[0];
> +        destination[1] = source[1];
> +        destination[2] = source[2];
> +        destination[3] = source[3];
> +        destination[4] = source[4];
> +        destination[5] = source[5];
> +        destination[6] = source[6];
> +        destination[7] = source[7];

ditto

> Source/WebCore/platform/text/TextCodecLatin1.cpp:172
> +    while (characters < destination)

This condition is really confusing to me, what on earth is destination?

[Michael Saboff]

Created attachment 153039 [details]
Patch with updates from review

(In reply to comment #2)
> (From update of attachment 151833 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=151833&action=review
> 
> > Source/WebCore/platform/text/TextCodecASCIIFastPath.h:43
> > +    static void copy(LChar* destination, const uint8_t* source)
> > +    {
> > +        destination[0] = source[0];
> > +        destination[1] = source[1];
> > +        destination[2] = source[2];
> > +        destination[3] = source[3];
> > +    }
> > +    
> 
> What does the assembly for this look like?
> 
> > Source/WebCore/platform/text/TextCodecASCIIFastPath.h:62
> > +        destination[0] = source[0];
> > +        destination[1] = source[1];
> > +        destination[2] = source[2];
> > +        destination[3] = source[3];
> > +        destination[4] = source[4];
> > +        destination[5] = source[5];
> > +        destination[6] = source[6];
> > +        destination[7] = source[7];
> 
> ditto

Changed both of these explicit copies to memcpy(destination, source, <4 or 8>)

For Mac 64 bit with these changes, the compiler generated:
    mov    (%r12),%rdx
    ...
    mov    %rdx,(%r15)

> 
> > Source/WebCore/platform/text/TextCodecLatin1.cpp:172
> > +    while (characters < destination)
> 
> This condition is really confusing to me, what on earth is destination?

Changed this to:

    // Zero extend and copy already processed 8 bit data
    LChar* ptr8 = characters;
    LChar* endPtr8 = destination;

    while (ptr8 < endPtr8)
        *destination16++ = *ptr8++;

[Michael Saboff]

Committed r123008: <http://trac.webkit.org/changeset/123008>