Bug 55645 – DatasetDOMStringMap::item and ::contains copies attribute name string

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

[Emil A Eklund]

DatasetDOMStringMap::item and DatasetDOMStringMap::contains both use propertyNameMatchesAttributeName to match dataset property names with attribute names. The current implementation does this matching by creating a new string that's converted from a attribute name to a property name. It also loops over the attribute string twice (first to check if it's valid and then to create the copy).

This is unnecessary and rather inefficient.

[Emil A Eklund]

Created attachment 84510 [details]
Patch

[Dimitri Glazkov (Google)]

Comment on attachment 84510 [details]
Patch

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

Ask abarth to take a gander, I am not very well-versed in character encodings and the challenges sorrounding them. I am just scared of them :)

> Source/WebCore/dom/DatasetDOMStringMap.cpp:-80
> -    String convertedName = convertAttributeNameToPropertyName(attributeName);

Do we still need this helper?

> Source/WebCore/dom/DatasetDOMStringMap.cpp:95
> +        if (attribute[a] == '-' && a + 1 < attributeLength && attribute[a + 1] != '-')
> +            wordBoundary = true;
> +        else {
> +            if ((wordBoundary ? toASCIIUpper(attribute[a]) : attribute[a]) != property[p])
> +                return false;
> +            p++;
> +            wordBoundary = false;
> +        }
> +        a++;

Will this still work with non-ASCII characters?

[Emil A Eklund]

Comment on attachment 84510 [details]
Patch

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

>> Source/WebCore/dom/DatasetDOMStringMap.cpp:-80
>> -    String convertedName = convertAttributeNameToPropertyName(attributeName);
> 
> Do we still need this helper?

Yeah, at least for now. It's used in one more place.

>> Source/WebCore/dom/DatasetDOMStringMap.cpp:95
>> +        a++;
> 
> Will this still work with non-ASCII characters?

No, but then again neither will convertAttributeNameToPropertyName.

[Dimitri Glazkov (Google)]

Comment on attachment 84510 [details]
Patch

alrighty.

[WebKit Commit Bot]

The commit-queue encountered the following flaky tests while processing attachment 84510 [details]:

http/tests/inspector/console-websocket-error.html bug 57392 (authors: [e-mail] and [e-mail])
The commit-queue is continuing to process your patch.

[WebKit Commit Bot]

Comment on attachment 84510 [details]
Patch

Clearing flags on attachment: 84510

Committed r82332: <http://trac.webkit.org/changeset/82332>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.