[webkit-reviews] review granted: [Bug 58354] [Navigation Timing] Use monotonicallyIncreasingTime() instead of currentTime() : [Attachment 104142] Patch

[bugzilla-daemon at webkit.org]

Pavel Feldman <pfeldman at chromium.org> has granted James Simonsen
<simonjam at chromium.org>'s request for review:
Bug 58354: [Navigation Timing] Use monotonicallyIncreasingTime() instead of
currentTime()
https://bugs.webkit.org/show_bug.cgi?id=58354

Attachment 104142: Patch
https://bugs.webkit.org/attachment.cgi?id=104142&action=review

------- Additional Comments from Pavel Feldman <pfeldman at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=104142&action=review


Looks good to me. Sorry guys, I was sure I posted my review two days ago saying
inspector part was fine by me and suggesting that Tony takes a look at the web
timing. Opened this review link now to find my comments pending submit...

>> Source/WebCore/CMakeLists.txt:891
>> +	loader/DocumentLoadTiming.cpp
> 
> I think we do ascii sorting (caps before lower)

We do.

> Source/WebCore/loader/DocumentLoadTiming.cpp:63
> +    Document* rootDocument = frame->tree()->top(true)->document();

Nit: I got used to the frame->page()->mainFrame()->document() form.

> Source/WebCore/loader/DocumentLoadTiming.cpp:64
> +    if (rootDocument && this != rootDocument->loader()->timing())

Why do you need the second check?

> Source/WebCore/loader/MainResourceLoader.cpp:587
> +    documentLoader()->timing()->setFetchStart();

Nit: markFetchStart?