[webkit-reviews] review denied: [Bug 107770] Add form-related instrumentations, and support 33+ features in FeatureObserver : [Attachment 184371] Patch

[bugzilla-daemon at webkit.org]

Kentaro Hara <haraken at chromium.org> has denied Kent Tamura
<tkent at chromium.org>'s request for review:
Bug 107770: Add form-related instrumentations, and support 33+ features in
FeatureObserver
https://bugs.webkit.org/show_bug.cgi?id=107770

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

------- Additional Comments from Kentaro Hara <haraken at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=184371&action=review


> Source/WebCore/html/TelephoneInputType.cpp:43
> +    FeatureObserver::observe(element->document()->domWindow(),
FeatureObserver::InputTypeTel);

InputTypeTel => InputTypeTelephone ?

> Source/WebCore/page/FeatureObserver.cpp:48
> +    if (!m_hasNonZeroFeatureBits)

'if (!m_featureBits.size())' is slower than '!m_hasNonZeroFeatureBits' ?

If you need to use m_hasNonZeroFeatureBits for performance, let's rename it to
m_hasFeatureBits (and flip true/false).

> Source/WebCore/page/FeatureObserver.h:94
> +	   AutoFocusAttribute,
> +	   AutoSaveAttribute,
> +	   FormAttribute,
> +	   IncrementalAttribute,
> +	   ListAttribute,
> +	   MinAttribute,
> +	   MaxAttribute,
> +	   PatternAttribute,
> +	   PlaceholderAttribute,
> +	   PrecisionAttribute,
> +	   PrefixedDirectoryAttribute,
> +	   PrefixedSpeechAttribute,
> +	   RequiredAttribute,
> +	   ResultsAttribute,
> +	   StepAttribute,
> +	   DataListElement,
> +	   InputTypeColor,
> +	   InputTypeDate,
> +	   InputTypeDateTime,
> +	   InputTypeDateTimeFallback,
> +	   InputTypeDateTimeLocal,
> +	   InputTypeEmail,
> +	   InputTypeMonth,
> +	   InputTypeNumber,
> +	   InputTypeRange,
> +	   InputTypeSearch,
> +	   InputTypeTel,
> +	   InputTypeTime,
> +	   InputTypeURL,
> +	   InputTypeWeek,
> +	   InputTypeWeekFallback,

Shall we sort them in the alphabetical order?