DevTools: Make network events continuous on Timeline

DevTools: Make network events continuous on Timeline

Network events were previously shown as instant events, while
in reality network actions take time. This is now fixed.

Committed: https://crrev.com/97c7563ac6cf52f9771815de739d9872c41949b5
Cr-Commit-Position: refs/heads/master@{#359609}

[alph, 2015-11-06 19:27:57]

alph@chromium.org changed reviewers:
+ caseq@chromium.org, pfeldman@chromium.org

[caseq, 2015-11-06 19:37:47]

https://codereview.chromium.org/1410153009/diff/1/content/child/resource_disp...
File content/child/resource_dispatcher.cc (right):

https://codereview.chromium.org/1410153009/diff/1/content/child/resource_disp...
content/child/resource_dispatcher.cc:154:
"ResourceDispatcher::OnReceivedResponse");
If we start exposing this in devtools trace, can we name this in a more stable
manner, i.e. independent on class/name method.

https://codereview.chromium.org/1410153009/diff/1/third_party/WebKit/Source/d...
File third_party/WebKit/Source/devtools/front_end/timeline/TimelineModel.js
(right):

https://codereview.chromium.org/1410153009/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/timeline/TimelineModel.js:1106: var
parent = eventStack.peekLast();
This looks fragile, we can't expect other events not to intervene.

[alph, 2015-11-06 21:04:51]

ptal

https://codereview.chromium.org/1410153009/diff/1/content/child/resource_disp...
File content/child/resource_dispatcher.cc (right):

https://codereview.chromium.org/1410153009/diff/1/content/child/resource_disp...
content/child/resource_dispatcher.cc:154:
"ResourceDispatcher::OnReceivedResponse");
On 2015/11/06 19:37:47, caseq wrote:
> If we start exposing this in devtools trace, can we name this in a more stable
> manner, i.e. independent on class/name method.

We can't change this name as it seems to be used elsewhere. And I don't think
it's feasible to introduce another event just for DevTools purposes.

https://codereview.chromium.org/1410153009/diff/1/third_party/WebKit/Source/d...
File third_party/WebKit/Source/devtools/front_end/timeline/TimelineModel.js
(right):

https://codereview.chromium.org/1410153009/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/timeline/TimelineModel.js:1106: var
parent = eventStack.peekLast();
On 2015/11/06 19:37:47, caseq wrote:
> This looks fragile, we can't expect other events not to intervene.

Done.

[caseq, 2015-11-06 21:38:38]

lgtm

https://codereview.chromium.org/1410153009/diff/20001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/inspector/tracing/category-filter.html
(right):

https://codereview.chromium.org/1410153009/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/inspector/tracing/category-filter.html:37:
{"name":"ResourceFetcher::requestResource","ts":1000015,"tid":mainThread,"pid":pid,"cat":"disabled-by-default-devtools.timeline","ph":"X","s":"g","args":{},
"dur": 10},
nit: use realistic category just in case?

https://codereview.chromium.org/1410153009/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/timeline/TimelineModel.js
(right):

https://codereview.chromium.org/1410153009/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/timeline/TimelineModel.js:1088:
function processNetworkEvent(event, parentType)
nit: I'd extract this to a member function.

[alph, 2015-11-06 22:16:31]

https://codereview.chromium.org/1410153009/diff/20001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/inspector/tracing/category-filter.html
(right):

https://codereview.chromium.org/1410153009/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/inspector/tracing/category-filter.html:37:
{"name":"ResourceFetcher::requestResource","ts":1000015,"tid":mainThread,"pid":pid,"cat":"disabled-by-default-devtools.timeline","ph":"X","s":"g","args":{},
"dur": 10},
On 2015/11/06 21:38:38, caseq wrote:
> nit: use realistic category just in case?

Done.

https://codereview.chromium.org/1410153009/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/timeline/TimelineModel.js
(right):

https://codereview.chromium.org/1410153009/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/timeline/TimelineModel.js:1088:
function processNetworkEvent(event, parentType)
On 2015/11/06 21:38:38, caseq wrote:
> nit: I'd extract this to a member function.

Done.

[alph, 2015-11-09 20:26:15]

ping

[pfeldman, 2015-11-09 22:53:45]

https://codereview.chromium.org/1410153009/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/fetch/ResourceFetcher.cpp (right):

https://codereview.chromium.org/1410153009/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/fetch/ResourceFetcher.cpp:329:
TRACE_EVENT0("devtools.timeline", "ResourceFetcher::requestResource");
How is this in both layers, resource dispatcher and fetch? Also, how is this
resource identified?

[alph, 2015-11-11 02:20:01]

ptal

[pfeldman, 2015-11-11 04:52:10]

https://codereview.chromium.org/1410153009/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/fetch/ResourceFetcher.cpp (right):

https://codereview.chromium.org/1410153009/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/fetch/ResourceFetcher.cpp:38: #include
"core/inspector/InspectorTraceEvents.h"
I assume you don't want this dependency here as per DEPS comment above

[alph, 2015-11-11 20:09:38]

ptal

[caseq, 2015-11-13 00:23:23]

https://codereview.chromium.org/1410153009/diff/100001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/inspector/tracing/timeline-network-resource-expected.txt
(left):

https://codereview.chromium.org/1410153009/diff/100001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/inspector/tracing/timeline-network-resource-expected.txt:8:
frame : <string>
Can we get frame back?

https://codereview.chromium.org/1410153009/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/core.gypi (right):

https://codereview.chromium.org/1410153009/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/core.gypi:1516: 'fetch/TraceEventArguments.h',
DevToolsTraceEvents?

https://codereview.chromium.org/1410153009/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/fetch/TraceEventArguments.h (right):

https://codereview.chromium.org/1410153009/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/fetch/TraceEventArguments.h:8: #include
"core/CoreExport.h"
is this useD?

[alph, 2015-11-13 00:32:35]

https://codereview.chromium.org/1410153009/diff/100001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/inspector/tracing/timeline-network-resource-expected.txt
(left):

https://codereview.chromium.org/1410153009/diff/100001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/inspector/tracing/timeline-network-resource-expected.txt:8:
frame : <string>
On 2015/11/13 00:23:23, caseq wrote:
> Can we get frame back?

It's not used. Let's add it when there's a need.

https://codereview.chromium.org/1410153009/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/core.gypi (right):

https://codereview.chromium.org/1410153009/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/core.gypi:1516: 'fetch/TraceEventArguments.h',
On 2015/11/13 00:23:23, caseq wrote:
> DevToolsTraceEvents?

Done.

https://codereview.chromium.org/1410153009/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/fetch/TraceEventArguments.h (right):

https://codereview.chromium.org/1410153009/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/fetch/TraceEventArguments.h:8: #include
"core/CoreExport.h"
On 2015/11/13 00:23:23, caseq wrote:
> is this useD?

Done.

[pfeldman, 2015-11-13 05:25:46]

lgtm % nit

https://codereview.chromium.org/1410153009/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/fetch/DevToolsTraceEvents.h (right):

https://codereview.chromium.org/1410153009/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/fetch/DevToolsTraceEvents.h:5: #ifndef
DevToolsTraceEvents_h
InspectorTraceEvents ?

https://codereview.chromium.org/1410153009/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/fetch/ResourceFetcher.cpp (right):

https://codereview.chromium.org/1410153009/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/fetch/ResourceFetcher.cpp:71: } // namespace
double space if you are aligning w/ chromium style

[alph, 2015-11-13 18:46:13]

https://codereview.chromium.org/1410153009/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/fetch/DevToolsTraceEvents.h (right):

https://codereview.chromium.org/1410153009/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/fetch/DevToolsTraceEvents.h:5: #ifndef
DevToolsTraceEvents_h
On 2015/11/13 05:25:46, pfeldman wrote:
> InspectorTraceEvents ?

InspectorFetchTraceEvents to not clash with the one in core/inspector

https://codereview.chromium.org/1410153009/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/fetch/ResourceFetcher.cpp (right):

https://codereview.chromium.org/1410153009/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/fetch/ResourceFetcher.cpp:71: } // namespace
On 2015/11/13 05:25:46, pfeldman wrote:
> double space if you are aligning w/ chromium style

Two spaces don't pass Blink presubmit check.

[alph, 2015-11-13 18:47:34]

The CQ bit was checked by alph@chromium.org

[alph, 2015-11-13 18:47:35]

The patchset sent to the CQ was uploaded after l-g-t-m from caseq@chromium.org,
pfeldman@chromium.org
Link to the patchset: https://codereview.chromium.org/1410153009/#ps140001
(title: "4 landing")

[commit-bot: I haz the power, 2015-11-13 18:48:02]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1410153009/140001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1410153009/140001

[commit-bot: I haz the power, 2015-11-13 20:18:42]

Committed patchset #8 (id:140001)

[commit-bot: I haz the power, 2015-11-13 20:19:23]

Patchset 8 (id:??) landed as
https://crrev.com/97c7563ac6cf52f9771815de739d9872c41949b5
Cr-Commit-Position: refs/heads/master@{#359609}