| Summary: | Move many miscellaneous classes from ExceptionCode to Exception | ||||||
|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Darin Adler <darin> | ||||
| Component: | Bindings | Assignee: | Darin Adler <darin> | ||||
| Status: | RESOLVED FIXED | ||||||
| Severity: | Normal | CC: | andersca, cdumez, commit-queue, dbates, esprehn+autocc, japhet, kondapallykalyan, rniwa, sam, youennf | ||||
| Priority: | P2 | ||||||
| Version: | WebKit Nightly Build | ||||||
| Hardware: | Unspecified | ||||||
| OS: | Unspecified | ||||||
| Attachments: |
|
||||||
|
Description
Darin Adler
2016-10-18 20:02:39 PDT
Created attachment 292039 [details]
Patch
Comment on attachment 292039 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=292039&action=review > Source/WebCore/page/Performance.h:98 > - unsigned m_resourceTimingBufferSize; > + unsigned m_resourceTimingBufferSize { 150 }; We might wanna add a hyperlink to https://w3c.github.io/resource-timing/#extensions-performance-interface indicating where the source of the number 150. > Source/WebCore/page/PerformanceUserTiming.cpp:50 > + { ASCIILiteral { "navigationStart" }, &PerformanceTiming::navigationStart }, I think ASCIILiteral(~) was more readable than ASCIILiteral { ~ } especially because we're nesting curly brackets here. > Source/WebCore/page/PerformanceUserTiming.cpp:129 > + double value = static_cast<double>((m_performance.timing().*(function))()); Jeez, this is some crazy member function call. I'm sure we can do better with lambdas & std::function but I guess that's for another time. > Source/WebCore/page/PerformanceUserTiming.cpp:175 > + Vector<RefPtr<PerformanceEntry>> entries; > for (auto& entry : performanceEntryMap.values()) > entries.appendVector(entry); Can't we just call copyValuesToVector instead? In fact, this entire function seems to be redundant given copyValuesToVector. Comment on attachment 292039 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=292039&action=review >> Source/WebCore/page/Performance.h:98 >> + unsigned m_resourceTimingBufferSize { 150 }; > > We might wanna add a hyperlink to https://w3c.github.io/resource-timing/#extensions-performance-interface indicating where the source of the number 150. OK, will do. >> Source/WebCore/page/PerformanceUserTiming.cpp:50 >> + { ASCIILiteral { "navigationStart" }, &PerformanceTiming::navigationStart }, > > I think ASCIILiteral(~) was more readable than ASCIILiteral { ~ } especially because we're nesting curly brackets here. Not sure what to say. I prefer it the new way, but really don’t care too much about this strange source file so I guess I will do it your way. >> Source/WebCore/page/PerformanceUserTiming.cpp:175 >> entries.appendVector(entry); > > Can't we just call copyValuesToVector instead? > In fact, this entire function seems to be redundant given copyValuesToVector. No, we can’t. The values in this case are all vectors, and the result is the a single vector with all their contents appended. Committed r207522: <http://trac.webkit.org/changeset/207522> |