Summary: | [PerformanceTiming] Don't expose the restrictedKeyMap() HashMap | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Zan Dobersek <zan> | ||||||||
Component: | New Bugs | Assignee: | Zan Dobersek <zan> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | commit-queue, darin | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Attachments: |
|
Description
Zan Dobersek
2015-07-28 12:38:42 PDT
Created attachment 257670 [details]
Patch
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. Created attachment 258904 [details]
Patch
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.
Created attachment 268136 [details]
Patch
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.
Comment on attachment 268136 [details] Patch Clearing flags on attachment: 268136 Committed r194578: <http://trac.webkit.org/changeset/194578> All reviewed patches have been landed. Closing bug. |