Bug 147366 – [PerformanceTiming] Don't expose the restrictedKeyMap() HashMap

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

[Zan Dobersek]

restrictedKeyMap() in PerformanceUserTiming.cpp should return a const reference to the HashMap

[Zan Dobersek]

Created attachment 257670 [details]
Patch

[Darin Adler]

Comment on attachment 257670 [details]
Patch

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

> Source/WebCore/page/PerformanceUserTiming.cpp:68
> +    static NeverDestroyed<RestrictedKeyMap> map;
> +    if (map.get().isEmpty()) {
> +        map.get().add("navigationStart", &PerformanceTiming::navigationStart);
> +        map.get().add("unloadEventStart", &PerformanceTiming::unloadEventStart);
> +        map.get().add("unloadEventEnd", &PerformanceTiming::unloadEventEnd);
> +        map.get().add("redirectStart", &PerformanceTiming::redirectStart);
> +        map.get().add("redirectEnd", &PerformanceTiming::redirectEnd);
> +        map.get().add("fetchStart", &PerformanceTiming::fetchStart);
> +        map.get().add("domainLookupStart", &PerformanceTiming::domainLookupStart);
> +        map.get().add("domainLookupEnd", &PerformanceTiming::domainLookupEnd);
> +        map.get().add("connectStart", &PerformanceTiming::connectStart);
> +        map.get().add("connectEnd", &PerformanceTiming::connectEnd);
> +        map.get().add("secureConnectionStart", &PerformanceTiming::secureConnectionStart);
> +        map.get().add("requestStart", &PerformanceTiming::requestStart);
> +        map.get().add("responseStart", &PerformanceTiming::responseStart);
> +        map.get().add("responseEnd", &PerformanceTiming::responseEnd);
> +        map.get().add("domLoading", &PerformanceTiming::domLoading);
> +        map.get().add("domInteractive", &PerformanceTiming::domInteractive);
> +        map.get().add("domContentLoadedEventStart", &PerformanceTiming::domContentLoadedEventStart);
> +        map.get().add("domContentLoadedEventEnd", &PerformanceTiming::domContentLoadedEventEnd);
> +        map.get().add("domComplete", &PerformanceTiming::domComplete);
> +        map.get().add("loadEventStart", &PerformanceTiming::loadEventStart);
> +        map.get().add("loadEventEnd", &PerformanceTiming::loadEventEnd);
> +    };

This is not a good way to create the map. All the adds end up getting inlined. Instead we should use a loop to build the map. I think the best practice is to write a separate create function.

UserTiming::findExistingMarkStartTime also has double hash lookup mistakes in it.

I’m not even sure we should expose the map. I think that a function that takes a string and returns a possibly-null NavigationTimingFunction would be better.

[Zan Dobersek]

Created attachment 258904 [details]
Patch

[WebKit Commit Bot]

Attachment 258904 [details] did not pass style-queue:


ERROR: Source/WebCore/page/PerformanceUserTiming.cpp:46:  Missing space before {  [whitespace/braces] [5]
ERROR: Source/WebCore/page/PerformanceUserTiming.cpp:47:  Missing space before {  [whitespace/braces] [5]
ERROR: Source/WebCore/page/PerformanceUserTiming.cpp:48:  Missing space before {  [whitespace/braces] [5]
ERROR: Source/WebCore/page/PerformanceUserTiming.cpp:49:  Missing space before {  [whitespace/braces] [5]
ERROR: Source/WebCore/page/PerformanceUserTiming.cpp:50:  Missing space before {  [whitespace/braces] [5]
ERROR: Source/WebCore/page/PerformanceUserTiming.cpp:51:  Missing space before {  [whitespace/braces] [5]
ERROR: Source/WebCore/page/PerformanceUserTiming.cpp:52:  Missing space before {  [whitespace/braces] [5]
ERROR: Source/WebCore/page/PerformanceUserTiming.cpp:53:  Missing space before {  [whitespace/braces] [5]
ERROR: Source/WebCore/page/PerformanceUserTiming.cpp:54:  Missing space before {  [whitespace/braces] [5]
ERROR: Source/WebCore/page/PerformanceUserTiming.cpp:55:  Missing space before {  [whitespace/braces] [5]
ERROR: Source/WebCore/page/PerformanceUserTiming.cpp:56:  Missing space before {  [whitespace/braces] [5]
ERROR: Source/WebCore/page/PerformanceUserTiming.cpp:57:  Missing space before {  [whitespace/braces] [5]
ERROR: Source/WebCore/page/PerformanceUserTiming.cpp:58:  Missing space before {  [whitespace/braces] [5]
ERROR: Source/WebCore/page/PerformanceUserTiming.cpp:59:  Missing space before {  [whitespace/braces] [5]
ERROR: Source/WebCore/page/PerformanceUserTiming.cpp:60:  Missing space before {  [whitespace/braces] [5]
ERROR: Source/WebCore/page/PerformanceUserTiming.cpp:61:  Missing space before {  [whitespace/braces] [5]
ERROR: Source/WebCore/page/PerformanceUserTiming.cpp:62:  Missing space before {  [whitespace/braces] [5]
ERROR: Source/WebCore/page/PerformanceUserTiming.cpp:63:  Missing space before {  [whitespace/braces] [5]
ERROR: Source/WebCore/page/PerformanceUserTiming.cpp:64:  Missing space before {  [whitespace/braces] [5]
ERROR: Source/WebCore/page/PerformanceUserTiming.cpp:65:  Missing space before {  [whitespace/braces] [5]
ERROR: Source/WebCore/page/PerformanceUserTiming.cpp:66:  Missing space before {  [whitespace/braces] [5]
Total errors found: 21 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Zan Dobersek]

Created attachment 268136 [details]
Patch

[WebKit Commit Bot]

Attachment 268136 [details] did not pass style-queue:


ERROR: Source/WebCore/page/PerformanceUserTiming.cpp:46:  Missing space before {  [whitespace/braces] [5]
ERROR: Source/WebCore/page/PerformanceUserTiming.cpp:47:  Missing space before {  [whitespace/braces] [5]
ERROR: Source/WebCore/page/PerformanceUserTiming.cpp:48:  Missing space before {  [whitespace/braces] [5]
ERROR: Source/WebCore/page/PerformanceUserTiming.cpp:49:  Missing space before {  [whitespace/braces] [5]
ERROR: Source/WebCore/page/PerformanceUserTiming.cpp:50:  Missing space before {  [whitespace/braces] [5]
ERROR: Source/WebCore/page/PerformanceUserTiming.cpp:51:  Missing space before {  [whitespace/braces] [5]
ERROR: Source/WebCore/page/PerformanceUserTiming.cpp:52:  Missing space before {  [whitespace/braces] [5]
ERROR: Source/WebCore/page/PerformanceUserTiming.cpp:53:  Missing space before {  [whitespace/braces] [5]
ERROR: Source/WebCore/page/PerformanceUserTiming.cpp:54:  Missing space before {  [whitespace/braces] [5]
ERROR: Source/WebCore/page/PerformanceUserTiming.cpp:55:  Missing space before {  [whitespace/braces] [5]
ERROR: Source/WebCore/page/PerformanceUserTiming.cpp:56:  Missing space before {  [whitespace/braces] [5]
ERROR: Source/WebCore/page/PerformanceUserTiming.cpp:57:  Missing space before {  [whitespace/braces] [5]
ERROR: Source/WebCore/page/PerformanceUserTiming.cpp:58:  Missing space before {  [whitespace/braces] [5]
ERROR: Source/WebCore/page/PerformanceUserTiming.cpp:59:  Missing space before {  [whitespace/braces] [5]
ERROR: Source/WebCore/page/PerformanceUserTiming.cpp:60:  Missing space before {  [whitespace/braces] [5]
ERROR: Source/WebCore/page/PerformanceUserTiming.cpp:61:  Missing space before {  [whitespace/braces] [5]
ERROR: Source/WebCore/page/PerformanceUserTiming.cpp:62:  Missing space before {  [whitespace/braces] [5]
ERROR: Source/WebCore/page/PerformanceUserTiming.cpp:63:  Missing space before {  [whitespace/braces] [5]
ERROR: Source/WebCore/page/PerformanceUserTiming.cpp:64:  Missing space before {  [whitespace/braces] [5]
ERROR: Source/WebCore/page/PerformanceUserTiming.cpp:65:  Missing space before {  [whitespace/braces] [5]
ERROR: Source/WebCore/page/PerformanceUserTiming.cpp:66:  Missing space before {  [whitespace/braces] [5]
Total errors found: 21 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Zan Dobersek]

Comment on attachment 268136 [details]
Patch

Clearing flags on attachment: 268136

Committed r194578: <http://trac.webkit.org/changeset/194578>

[Zan Dobersek]

All reviewed patches have been landed.  Closing bug.