Bug 163645 – Move many miscellaneous classes from ExceptionCode to Exception

• Status: RESOLVED
• Resolution: FIXED
• Component: Bindings

[Darin Adler]

Move many miscellaneous classes from ExceptionCode to Exception

[Darin Adler]

Created attachment 292039 [details]
Patch

[Ryosuke Niwa]

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.

[Darin Adler]

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.

[Darin Adler]

Committed r207522: <http://trac.webkit.org/changeset/207522>