Bug 146568

Summary: Web Inspector: Add a dedicated Network tab that is always live
Product: WebKit Reporter: Timothy Hatcher <timothy>
Component: Web InspectorAssignee: Timothy Hatcher <timothy>
Status: RESOLVED FIXED    
Severity: Normal CC: agomez, bburg, commit-queue, graouts, joepeck, jonowells, mattbaker, nvasilyev, timothy, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
Patch
none
Network Screenshot 1
none
Network Screenshot 2
none
Patch joepeck: review+, timothy: commit-queue-

Description Timothy Hatcher 2015-07-02 17:47:46 PDT
The Network timeline only shows resources while recording, we should add a network tab that is always live.
Comment 1 Radar WebKit Bug Importer 2015-07-02 17:48:27 PDT
<rdar://problem/21663901>
Comment 2 Timothy Hatcher 2015-07-02 17:55:42 PDT
Created attachment 256058 [details]
Patch
Comment 3 Timothy Hatcher 2015-07-02 17:56:08 PDT
Created attachment 256059 [details]
Network Screenshot 1
Comment 4 Timothy Hatcher 2015-07-02 17:56:24 PDT
Created attachment 256060 [details]
Network Screenshot 2
Comment 5 Joseph Pecoraro 2015-07-02 19:13:33 PDT
Comment on attachment 256058 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=256058&action=review

Nice! I played with it. It looks like a great start. One issue I ran into was if the timeline was not recording, then all Network requests end up with a "start time" matching the last time in the Timeline. Maybe the backend's stopwatch is stopped and makes it appear as if all network requests are happening in that instant because it hasn't been advancing?!

I haven't reviewed in detail yet.

> Source/WebInspectorUI/UserInterface/Views/NetworkGridContentView.css:2
> + * Copyright (C) 2014 Apple Inc. All rights reserved.

2015?

> Source/WebInspectorUI/UserInterface/Views/NewTabContentView.js:39
> +        {image: "Images/Timeline.svg", title: WebInspector.UIString("Timelines"), type: WebInspector.TimelineTabContentView.Type}

Style: Trailing comma! Would make future diffs cleaner.
Comment 6 Timothy Hatcher 2015-07-03 16:24:07 PDT
Good catch! I didn't notice that.

I am thinking we need to just start the inspectorEnvironment().executionStopwatch() in InspectorResourceAgent::mainFrameNavigated and keep it on until another mainFrameNavigated.
Comment 7 Timothy Hatcher 2015-07-04 08:30:40 PDT
Created attachment 256148 [details]
Patch
Comment 8 Joseph Pecoraro 2015-07-05 22:30:52 PDT
Comment on attachment 256148 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=256148&action=review

r=me

> Source/WebCore/inspector/InspectorPageAgent.cpp:370
> +    m_instrumentingAgents->inspectorEnvironment().executionStopwatch()->start();

This might not play well if the page was paused and this agent was enabled (Debugger.pause, Runtime.evaluate, Page.enable), but that shouldn't be possible with our frontend.

> Source/WebCore/inspector/InspectorPageAgent.cpp:826
> +    auto stopwatch = m_instrumentingAgents->inspectorEnvironment().executionStopwatch();
> +    stopwatch->reset();
> +    stopwatch->start();

If/when we push timeline / profiling down to JSContext inspection this will need to be someplace lower.

Does this only get called fro the main frame?

> Source/WebInspectorUI/UserInterface/Models/Timeline.js:111
> +            this.dispatchEventToListeners(WebInspector.Timeline.Event.Reset);

I just removed this! how unfortunate, hah

> Source/WebInspectorUI/UserInterface/Views/NetworkGridContentView.css:2
> + * Copyright (C) 2014 Apple Inc. All rights reserved.

2015?

> Source/WebInspectorUI/UserInterface/Views/NetworkGridContentView.js:177
> +            var treeElement = this._contentTreeOutline.findTreeElement(resourceTimelineRecord.resource);

I wonder how expensive findTreeElement is for each new record. It seems it could be expensive.

> Source/WebInspectorUI/UserInterface/Views/NetworkSidebarPanel.js:56
> +        this._scopeBar = new WebInspector.ScopeBar("resource-sidebar-scope-bar", scopeBarItems, scopeBarItems[0], true);

Typo: "resource-sidebar-..." should be "network-sidebar-..." I think, unless you are intending them to share / overwrite each other.

> Source/WebInspectorUI/UserInterface/Views/NetworkTabContentView.js:29
> +    var detailsSidebarPanels = [WebInspector.resourceDetailsSidebarPanel, WebInspector.probeDetailsSidebarPanel];

I wonder why we have the probe details sidebar panel everywhere. Seems it could just be in debugger.

> Source/WebInspectorUI/UserInterface/Views/TabBar.js:734
>      _handleNewTabClick(event) {

Style: drive by fix this style!
Comment 9 Timothy Hatcher 2015-07-06 12:45:45 PDT
Comment on attachment 256148 [details]
Patch

https://trac.webkit.org/r186368