[webkit-reviews] review denied: [Bug 41332] Implement Web Timing navigationStart, fetchStart, load and unload events : [Attachment 59985] Patch

[bugzilla-daemon at webkit.org]

Nate Chapin <japhet at chromium.org> has denied Tony Gentilcore
<tonyg at chromium.org>'s request for review:
Bug 41332: Implement Web Timing navigationStart, fetchStart, load and unload
events
https://bugs.webkit.org/show_bug.cgi?id=41332

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

------- Additional Comments from Nate Chapin <japhet at chromium.org>
> @@ -1496,6 +1498,9 @@ void
FrameLoader::loadWithDocumentLoader(DocumentLoader* loader, FrameLoadType t
>  
>      ASSERT(m_frame->view());
>  
> +    m_frameLoadTimeline = FrameLoadTimeline();
> +    m_frameLoadTimeline.navigationStart = currentTime();
> +
>      if (m_pageDismissalEventBeingDispatched)
>	   return;
>  
> @@ -2494,6 +2499,9 @@ void FrameLoader::continueLoadAfterWillSubmitForm()
>  
>      unsigned long identifier = 0;
>  
> +    ASSERT(!m_frameLoadTimeline.fetchStart);
> +    m_frameLoadTimeline.fetchStart = currentTime();
> +
>      if (Page* page = m_frame->page()) {
>	   identifier = page->progress()->createUniqueIdentifier();
>	   notifier()->assignIdentifierToInitialRequest(identifier,
m_provisionalDocumentLoader.get(),
m_provisionalDocumentLoader->originalRequest());

I don't know the precise sequence of this part of FrameLoader well enough to
know for certain that these are the correct places for navigationStart and
fetchStart. abarth might know better.

> +struct FrameLoadTimeline {
> +    FrameLoadTimeline()
> +	   : navigationStart(0)
> +	   , unloadEventStart(0)
> +	   , unloadEventEnd(0)
> +	   , fetchStart(0)
> +	   , loadEventStart(0)
> +	   , loadEventEnd(0)
> +    {
> +    }
> +
> +    double navigationStart;
> +    double unloadEventStart;
> +    double unloadEventEnd;
> +    double fetchStart;
> +    double loadEventStart;
> +    double loadEventEnd;
> +};
> +

This should be in FrameLoaderTypes.h