[webkit-reviews] review granted: [Bug 175569] Add the PerformanceServerTiming Interface which makes Server-Timing header timing values available to JavaScript running in the browser. : [Attachment 337383] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Fri Apr 6 12:44:16 PDT 2018
youenn fablet <youennf at gmail.com> has granted cvazac at gmail.com's request for
review:
Bug 175569: Add the PerformanceServerTiming Interface which makes Server-Timing
header timing values available to JavaScript running in the browser.
https://bugs.webkit.org/show_bug.cgi?id=175569
Attachment 337383: Patch
https://bugs.webkit.org/attachment.cgi?id=337383&action=review
--- Comment #35 from youenn fablet <youennf at gmail.com> ---
Comment on attachment 337383
--> https://bugs.webkit.org/attachment.cgi?id=337383
Patch
I was going to say r+, patch is not applying though.
Some comments below.
View in context: https://bugs.webkit.org/attachment.cgi?id=337383&action=review
> Source/WebCore/loader/ServerTimingParser.cpp:41
> + if (!headerValue.isNull()) {
We should early return here.
> Source/WebCore/loader/ServerTimingParser.cpp:59
> + value = tokenizer.consumeTokenOrQuotedString();
If value is null here, is it fine skipping it/adding the entry or should we
raise some kind of error?
> Source/WebCore/loader/ServerTimingParser.h:36
> +Vector<ServerTiming> parseServerTiming(const String&);
It would probably be good to add some unit tests for this method.
See Tools/TestWebKitAPI/Tests/ for examples.
> Source/WebCore/page/mac/WheelEventDeltaFilterMac.h:31
> +#include <wtf/MonotonicTime.h>
Probably not needed/unrelated?
More information about the webkit-reviews
mailing list