[webkit-reviews] review denied: [Bug 213319] WIP: We should resurrect the older patch that collects some statistics of web API calls : [Attachment 402477] Patch

[bugzilla-daemon at webkit.org]

John Wilander <wilander at apple.com> has denied Umar Iqbal <uiqbal at apple.com>'s
request for review:
Bug 213319: WIP: We should resurrect the older patch that collects some
statistics of web API calls
https://bugs.webkit.org/show_bug.cgi?id=213319

Attachment 402477: Patch

https://bugs.webkit.org/attachment.cgi?id=402477&action=review




--- Comment #12 from John Wilander <wilander at apple.com> ---
Comment on attachment 402477
  --> https://bugs.webkit.org/attachment.cgi?id=402477
Patch

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

The code looks good but there's a JSC test failure and change log entries are
missing for Layout Tests, Tools, JSC, WTF, and PAL (I think, even though it's
under WebCore). The prepare-Changelog script should have added them.

> Source/WebCore/ChangeLog:3
> +	   WIP: We should resurrect the older patch that collects some
statistics of web API calls

Is this patch a work-in-progress or ready for landing?

> Source/WebCore/ChangeLog:8
> +	   No new tests. Enabled existing tests. Enabled
http/tests/webAPIStatistics that test the functionality behind
WEB_API_STATISTICS flag.

Excellent to see these tests re-enabled! Look at previous change log entries
for the approximate line width. These long lines make it hard to read.

> Source/WebCore/ChangeLog:9
> +	   + Brought back encodeHashSet(KeyedEncoder& encoder, const String&
label,	const String& key, const HashSet<String>& hashSet) because it was
needed by encodeFontHashSet(KeyedEncoder& encoder, const String& label, const
HashSet<String>& hashSet)

When referring to functions in WebCore or WebKit, please include the name
space.

> Source/WebCore/loader/ResourceLoadStatistics.h:105
> +    enum class NavigatorAPI : uint8_t {

Do we have a trace of why these were 64 before?