Bug 162593 – Implement URLParser::syntaxViolation

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Alex Christensen]

Implement URLParser::syntaxViolation

[Alex Christensen]

Created attachment 289917 [details]
Patch

[Alex Christensen]

Created attachment 289920 [details]
Patch

[Alex Christensen]

Created attachment 289952 [details]
Patch

[Geoffrey Garen]

Comment on attachment 289952 [details]
Patch

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

r=me

> Source/WebCore/ChangeLog:15
> +        first time, we assume that everything in the input String up to that point is equal to what it would have been

assume => know

> Source/WebCore/ChangeLog:23
> +        This is about a 15% speed improvement on my URL parsing benchmark.

Are we fast enough now?

> Source/WebCore/platform/URLParser.cpp:413
> +void URLParser::incrementIteratorSkippingTabsAndNewlines(CodePointIterator<CharacterType>& iterator, const CodePointIterator<CharacterType>& iteratorForSyntaxViolationPosition)

I would call this "advanceSkippingTabsAndNewlines" or just "advance". The iterator thing is implied by argument type.

> Source/WebCore/platform/URLParser.cpp:950
> +    if (m_seenSyntaxViolation)

Let's call this "m_didSeeSyntaxViolation" because it's a boolean.

> Source/WebCore/platform/URLParser.cpp:985
> +    for (size_t i = 0; i < unicodeCodeUnitsToCopy; ++i)

I would write this as "(i = asciiCodeUnitsToCopy; i < asciiCodeUnitsToCopy + unicodeCodeUnitsToCopy; ++i)". I think that's a little clearer because then i always points to the current cursor.

> Source/WebCore/platform/URLParser.cpp:1345
> +                if (c.atEnd())
> +                    break;
>              }
> -            break;
> +            // Skip the check for tabs which might cause a syntaxViolation.
> +            // We want to handle the syntaxViolations while actually parsing the authority or host.
> +            goto CaseAuthorityOrHost;

Let's make this "do { ... } while (!c.atEnd());".

> Source/WebCore/platform/URLParser.cpp:1603
> +                m_seenUnicodeFragmentCodePoint = true;

m_didSeeUnicodeFragmentCodePoint.

> Source/WebCore/platform/URLParser.cpp:1621
> +            if (c.atEnd())
> +                break;
> +
> +            // Skip the check for tabs which might cause a syntaxViolation.
> +            // We need to handle them differently with fragmentSyntaxViolation.
> +            goto CaseFragment;

Let's make this "do { ... } while (!c.atEnd());".

[Alex Christensen]

http://trac.webkit.org/changeset/206457

[Alex Christensen]

(In reply to comment #4)
> > Source/WebCore/ChangeLog:23
> > +        This is about a 15% speed improvement on my URL parsing benchmark.
> 
> Are we fast enough now?
Almost

[Alex Christensen]

For future reference, this was prototyped in https://bugs.webkit.org/show_bug.cgi?id=162464