[webkit-reviews] review granted: [Bug 100927] Improve ContentTypeParser, so that it could be used to validate mime type according to RFC : [Attachment 172365] Patch 6

[bugzilla-daemon at webkit.org]

Alexey Proskuryakov <ap at webkit.org> has granted Alexander Shalamov
<alexander.shalamov at gmail.com>'s request for review:
Bug 100927: Improve ContentTypeParser, so that it could be used to validate
mime type according to RFC
https://bugs.webkit.org/show_bug.cgi?id=100927

Attachment 172365: Patch 6
https://bugs.webkit.org/attachment.cgi?id=172365&action=review

------- Additional Comments from Alexey Proskuryakov <ap at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=172365&action=review


Looks good to me. A few nits below.

> Source/WebCore/platform/network/ParsedContentType.cpp:2
> + * Copyright (C) 2011 Google Inc. All rights reserved.

Is it intentional that you did not add your copyright, or update the date if
you work for Google?

> Source/WebCore/platform/network/ParsedContentType.cpp:39
> +class EmptyParsedContentType {

We normally use "dummy" for unused variables. Perhaps it would be more
understandable here, too.

> Source/WebCore/platform/network/ParsedContentType.cpp:59
> +    const unsigned inputLength = input.length();
> +    const unsigned index = startIndex;

As a matter of coding style, we don't use const with local variables.

> Source/WebCore/platform/network/ParsedContentType.cpp:77
> +    const unsigned inputLength = input.length();
> +    const unsigned index = startIndex + 1;

Ditto.

"index" is not a good name for a variable other than a loop one.

> Source/WebCore/platform/network/ParsedContentType.cpp:153
> +bool parseContentType(const String& contentType, T& parserType)

parserTyoe does not seem to be accurate. I think that this should be
"ReceiverType& receiver".

> Source/WebCore/platform/network/ParsedContentType.h:36
> +#include <wtf/text/WTFString.h>

This include should not be needed.