Web Inspector: Network Graphs are missing minimum sizes, might have no graph at all
authordrousso@apple.com <drousso@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 14 Nov 2018 09:26:23 +0000 (09:26 +0000)
committerdrousso@apple.com <drousso@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 14 Nov 2018 09:26:23 +0000 (09:26 +0000)
https://bugs.webkit.org/show_bug.cgi?id=191208

Reviewed by Joseph Pecoraro.

`WI.NetworkTableContentView` used to rely on the `WI.timelineManager.persistentNetworkTimeline`
for events when a new resource is added. It also listened for `WI.Frame.Event.MainResourceDidChange`
on it's own, which was also listened to by `WI.timelineManager.persistentNetworkTimeline`.
Due to the order in which these listeners are added, the new main resource would get added
to the `WI.timelineManager.persistentNetworkTimeline` (and the `WI.NetworkTableContentView`
shortly after), and right after that the `WI.NetworkTableContentView` would reset the graph
in it's own listener for `WI.Frame.Event.MainResourceDidChange`.

This patch removes `WI.timelineManager.persistentNetworkTimeline` and instead makes it so
that `WI.NetworkTableContentView` listens for resource added and main frame changed events
on its own (similar to other views that follow this pattern), ensuring that there are no
event races.

Also relaxes the "requirement" that prevented 0 width blocks from being drawn, allowing
requests served from memory/disk to appear in the graph (their time is effectively 0).

* UserInterface/Views/NetworkTableContentView.js:
(WI.NetworkTableContentView):
(WI.NetworkTableContentView.prototype.closed):
(WI.NetworkTableContentView.prototype._populateWaterfallGraph):
(WI.NetworkTableContentView.prototype._populateWaterfallGraph.appendBlock):
(WI.NetworkTableContentView.prototype._updateWaterfallTimeRange): Added.
(WI.NetworkTableContentView.prototype._resourceLoadingDidFinish):
(WI.NetworkTableContentView.prototype._resourceLoadingDidFail):
(WI.NetworkTableContentView.prototype._handleResourceAdded): Added.
(WI.NetworkTableContentView.prototype._insertResourceAndReloadTable):
(WI.NetworkTableContentView.prototype._handleNodeDidFireEvent):
(WI.NetworkTableContentView.prototype._handleNodeLowPowerChanged):
* UserInterface/Views/NetworkTableContentView.css:
(.network-table .waterfall .block:matches(.mouse-tracking, .filler) + .block:not(.mouse-tracking, .filler)): Added.
(.network-table .waterfall .block.filler + .block): Deleted.

* UserInterface/Controllers/TimelineManager.js:
(WI.TimelineManager):
(WI.TimelineManager.prototype._mainResourceDidChange):
(WI.TimelineManager.prototype._resourceWasAdded):
(WI.TimelineManager.prototype.get persistentNetworkTimeline): Deleted.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@238168 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebInspectorUI/ChangeLog
Source/WebInspectorUI/UserInterface/Controllers/TimelineManager.js
Source/WebInspectorUI/UserInterface/Views/NetworkTableContentView.css
Source/WebInspectorUI/UserInterface/Views/NetworkTableContentView.js

index 2b02d5f..ee5db56 100644 (file)
@@ -1,3 +1,48 @@
+2018-11-14  Devin Rousso  <drousso@apple.com>
+
+        Web Inspector: Network Graphs are missing minimum sizes, might have no graph at all
+        https://bugs.webkit.org/show_bug.cgi?id=191208
+
+        Reviewed by Joseph Pecoraro.
+
+        `WI.NetworkTableContentView` used to rely on the `WI.timelineManager.persistentNetworkTimeline`
+        for events when a new resource is added. It also listened for `WI.Frame.Event.MainResourceDidChange`
+        on it's own, which was also listened to by `WI.timelineManager.persistentNetworkTimeline`.
+        Due to the order in which these listeners are added, the new main resource would get added
+        to the `WI.timelineManager.persistentNetworkTimeline` (and the `WI.NetworkTableContentView`
+        shortly after), and right after that the `WI.NetworkTableContentView` would reset the graph
+        in it's own listener for `WI.Frame.Event.MainResourceDidChange`.
+
+        This patch removes `WI.timelineManager.persistentNetworkTimeline` and instead makes it so
+        that `WI.NetworkTableContentView` listens for resource added and main frame changed events
+        on its own (similar to other views that follow this pattern), ensuring that there are no
+        event races.
+
+        Also relaxes the "requirement" that prevented 0 width blocks from being drawn, allowing
+        requests served from memory/disk to appear in the graph (their time is effectively 0).
+
+        * UserInterface/Views/NetworkTableContentView.js:
+        (WI.NetworkTableContentView):
+        (WI.NetworkTableContentView.prototype.closed):
+        (WI.NetworkTableContentView.prototype._populateWaterfallGraph):
+        (WI.NetworkTableContentView.prototype._populateWaterfallGraph.appendBlock):
+        (WI.NetworkTableContentView.prototype._updateWaterfallTimeRange): Added.
+        (WI.NetworkTableContentView.prototype._resourceLoadingDidFinish):
+        (WI.NetworkTableContentView.prototype._resourceLoadingDidFail):
+        (WI.NetworkTableContentView.prototype._handleResourceAdded): Added.
+        (WI.NetworkTableContentView.prototype._insertResourceAndReloadTable):
+        (WI.NetworkTableContentView.prototype._handleNodeDidFireEvent):
+        (WI.NetworkTableContentView.prototype._handleNodeLowPowerChanged):
+        * UserInterface/Views/NetworkTableContentView.css:
+        (.network-table .waterfall .block:matches(.mouse-tracking, .filler) + .block:not(.mouse-tracking, .filler)): Added.
+        (.network-table .waterfall .block.filler + .block): Deleted.
+
+        * UserInterface/Controllers/TimelineManager.js:
+        (WI.TimelineManager):
+        (WI.TimelineManager.prototype._mainResourceDidChange):
+        (WI.TimelineManager.prototype._resourceWasAdded):
+        (WI.TimelineManager.prototype.get persistentNetworkTimeline): Deleted.
+
 2018-11-13  Matt Baker  <mattbaker@apple.com>
 
         Web Inspector: Table should support select all (Cmd-A)
index 328cd3e..bf22adb 100644 (file)
@@ -41,8 +41,6 @@ WI.TimelineManager = class TimelineManager extends WI.Object
 
         this._enabledTimelineTypesSetting = new WI.Setting("enabled-instrument-types", WI.TimelineManager.defaultTimelineTypes());
 
-        this._persistentNetworkTimeline = new WI.NetworkTimeline;
-
         this._isCapturing = false;
         this._initiatedByBackendStart = false;
         this._initiatedByBackendStop = false;
@@ -137,11 +135,6 @@ WI.TimelineManager = class TimelineManager extends WI.Object
         return this._activeRecording;
     }
 
-    get persistentNetworkTimeline()
-    {
-        return this._persistentNetworkTimeline;
-    }
-
     get recordings()
     {
         return this._recordings.slice();
@@ -821,37 +814,27 @@ WI.TimelineManager = class TimelineManager extends WI.Object
 
     _mainResourceDidChange(event)
     {
-        let frame = event.target;
-        if (frame.isMainFrame() && WI.settings.clearNetworkOnNavigate.value)
-            this._persistentNetworkTimeline.reset();
-
-        let mainResource = frame.mainResource;
-        let record = new WI.ResourceTimelineRecord(mainResource);
-        if (!isNaN(record.startTime))
-            this._persistentNetworkTimeline.addRecord(record);
-
         // Ignore resource events when there isn't a main frame yet. Those events are triggered by
         // loading the cached resources when the inspector opens, and they do not have timing information.
         if (!WI.networkManager.mainFrame)
             return;
 
+        let frame = event.target;
         if (this._attemptAutoCapturingForFrame(frame))
             return;
 
         if (!this._isCapturing)
             return;
 
+        let mainResource = frame.mainResource;
         if (mainResource === this._mainResourceForAutoCapturing)
             return;
 
-        this._addRecord(record);
+        this._addRecord(new WI.ResourceTimelineRecord(mainResource));
     }
 
     _resourceWasAdded(event)
     {
-        var record = new WI.ResourceTimelineRecord(event.data.resource);
-        if (!isNaN(record.startTime))
-            this._persistentNetworkTimeline.addRecord(record);
 
         // Ignore resource events when there isn't a main frame yet. Those events are triggered by
         // loading the cached resources when the inspector opens, and they do not have timing information.
@@ -861,7 +844,7 @@ WI.TimelineManager = class TimelineManager extends WI.Object
         if (!this._isCapturing)
             return;
 
-        this._addRecord(record);
+        this._addRecord(new WI.ResourceTimelineRecord(event.data.resource));
     }
 
     _garbageCollected(event)
index 784070d..38f7603 100644 (file)
@@ -249,7 +249,7 @@ body[dir=rtl] .waterfall .block {
     --end-radius: 0;
 }
 
-.network-table .waterfall .block.filler + .block,
+.network-table .waterfall .block:matches(.mouse-tracking, .filler) + .block:not(.mouse-tracking, .filler),
 .network-table .waterfall .block:not(.request, .response) + :matches(.request, .response) {
     --start-radius: 2px;
 }
index 8d9dcbe..2e9c14d 100644 (file)
@@ -130,12 +130,13 @@ WI.NetworkTableContentView = class NetworkTableContentView extends WI.ContentVie
         this._clearNetworkItemsNavigationItem = new WI.ButtonNavigationItem("clear-network-items", WI.UIString("Clear Network Items (%s)").format(WI.clearKeyboardShortcut.displayName), "Images/NavigationItemTrash.svg", 15, 15);
         this._clearNetworkItemsNavigationItem.addEventListener(WI.ButtonNavigationItem.Event.Clicked, () => { this.reset(); });
 
+        WI.Target.addEventListener(WI.Target.Event.ResourceAdded, this._handleResourceAdded, this);
         WI.Frame.addEventListener(WI.Frame.Event.MainResourceDidChange, this._mainResourceDidChange, this);
+        WI.Frame.addEventListener(WI.Frame.Event.ResourceWasAdded, this._handleResourceAdded, this);
         WI.Resource.addEventListener(WI.Resource.Event.LoadingDidFinish, this._resourceLoadingDidFinish, this);
         WI.Resource.addEventListener(WI.Resource.Event.LoadingDidFail, this._resourceLoadingDidFail, this);
         WI.Resource.addEventListener(WI.Resource.Event.TransferSizeDidChange, this._resourceTransferSizeDidChange, this);
         WI.networkManager.addEventListener(WI.NetworkManager.Event.MainFrameDidChange, this._mainFrameDidChange, this);
-        WI.timelineManager.persistentNetworkTimeline.addEventListener(WI.Timeline.Event.RecordAdded, this._networkTimelineRecordAdded, this);
 
         this._needsInitialPopulate = true;
     }
@@ -250,12 +251,12 @@ WI.NetworkTableContentView = class NetworkTableContentView extends WI.ContentVie
         this._hidePopover();
         this._hideDetailView();
 
+        WI.Target.removeEventListener(null, null, this);
         WI.Frame.removeEventListener(null, null, this);
         WI.Resource.removeEventListener(null, null, this);
         WI.settings.resourceCachingDisabled.removeEventListener(null, null, this);
         WI.settings.clearNetworkOnNavigate.removeEventListener(null, null, this);
         WI.networkManager.removeEventListener(WI.NetworkManager.Event.MainFrameDidChange, this._mainFrameDidChange, this);
-        WI.timelineManager.persistentNetworkTimeline.removeEventListener(WI.Timeline.Event.RecordAdded, this._networkTimelineRecordAdded, this);
 
         super.closed();
     }
@@ -763,7 +764,7 @@ WI.NetworkTableContentView = class NetworkTableContentView extends WI.ContentVie
         }
 
         let {startTime, redirectStart, redirectEnd, fetchStart, domainLookupStart, domainLookupEnd, connectStart, connectEnd, secureConnectionStart, requestStart, responseStart, responseEnd} = resource.timingData;
-        if (isNaN(startTime) || isNaN(responseEnd) || startTime >= responseEnd) {
+        if (isNaN(startTime) || isNaN(responseEnd) || startTime > responseEnd) {
             cell.textContent = zeroWidthSpace;
             return;
         }
@@ -781,7 +782,7 @@ WI.NetworkTableContentView = class NetworkTableContentView extends WI.ContentVie
 
         let lastEndTimestamp = NaN;
         function appendBlock(startTimestamp, endTimestamp, className) {
-            if (isNaN(startTimestamp) || isNaN(endTimestamp) || endTimestamp - startTimestamp <= 0)
+            if (isNaN(startTimestamp) || isNaN(endTimestamp))
                 return null;
 
             if (Math.abs(startTimestamp - lastEndTimestamp) < secondsPerPixel * 2)
@@ -807,8 +808,9 @@ WI.NetworkTableContentView = class NetworkTableContentView extends WI.ContentVie
         // Super small visualization.
         let totalWidth = (responseEnd - startTime) / secondsPerPixel;
         if (totalWidth <= 3) {
-            appendBlock(startTime, requestStart, "queue");
-            appendBlock(startTime, responseEnd, "response");
+            let twoPixels = secondsPerPixel * 2;
+            appendBlock(startTime, startTime + twoPixels, "queue");
+            appendBlock(startTime + twoPixels, startTime + (2 * twoPixels), "response");
             return;
         }
 
@@ -1111,6 +1113,15 @@ WI.NetworkTableContentView = class NetworkTableContentView extends WI.ContentVie
 
     // Private
 
+    _updateWaterfallTimeRange(startTimestamp, endTimestamp)
+    {
+        if (isNaN(this._waterfallStartTime) || startTimestamp < this._waterfallStartTime)
+            this._waterfallStartTime = startTimestamp;
+
+        if (isNaN(this._waterfallEndTime) || endTimestamp > this._waterfallEndTime)
+            this._waterfallEndTime = endTimestamp;
+    }
+
     _updateWaterfallTimelineRuler()
     {
         if (!this._waterfallTimelineRuler)
@@ -1414,10 +1425,7 @@ WI.NetworkTableContentView = class NetworkTableContentView extends WI.ContentVie
         let resource = event.target;
         this._pendingUpdates.push(resource);
 
-        if (resource.firstTimestamp < this._waterfallStartTime)
-            this._waterfallStartTime = resource.firstTimestamp;
-        if (resource.timingData.responseEnd > this._waterfallEndTime)
-            this._waterfallEndTime = resource.timingData.responseEnd;
+        this._updateWaterfallTimeRange(resource.firstTimestamp, resource.timingData.responseEnd);
 
         if (this._hasURLFilter())
             this._checkURLFilterAgainstResource(resource);
@@ -1430,10 +1438,7 @@ WI.NetworkTableContentView = class NetworkTableContentView extends WI.ContentVie
         let resource = event.target;
         this._pendingUpdates.push(resource);
 
-        if (resource.firstTimestamp < this._waterfallStartTime)
-            this._waterfallStartTime = resource.firstTimestamp;
-        if (resource.timingData.responseEnd > this._waterfallEndTime)
-            this._waterfallEndTime = resource.timingData.responseEnd;
+        this._updateWaterfallTimeRange(resource.firstTimestamp, resource.timingData.responseEnd);
 
         if (this._hasURLFilter())
             this._checkURLFilterAgainstResource(resource);
@@ -1469,16 +1474,9 @@ WI.NetworkTableContentView = class NetworkTableContentView extends WI.ContentVie
         this._table.reloadCell(rowIndex, "transferSize");
     }
 
-    _networkTimelineRecordAdded(event)
+    _handleResourceAdded(event)
     {
-        let resourceTimelineRecord = event.data.record;
-        console.assert(resourceTimelineRecord instanceof WI.ResourceTimelineRecord);
-
-        let resource = resourceTimelineRecord.resource;
-        if (isNaN(this._waterfallStartTime))
-            this._waterfallStartTime = this._waterfallEndTime = resource.firstTimestamp;
-
-        this._insertResourceAndReloadTable(resource);
+        this._insertResourceAndReloadTable(event.data.resource);
     }
 
     _isDefaultSort()
@@ -1488,6 +1486,8 @@ WI.NetworkTableContentView = class NetworkTableContentView extends WI.ContentVie
 
     _insertResourceAndReloadTable(resource)
     {
+        this._updateWaterfallTimeRange(resource.firstTimestamp, resource.timingData.responseEnd);
+
         if (!this._table || !(WI.tabBrowser.selectedTabContentView instanceof WI.NetworkTabContentView)) {
             this._pendingInsertions.push(resource);
             this.needsLayout();
@@ -1609,8 +1609,7 @@ WI.NetworkTableContentView = class NetworkTableContentView extends WI.ContentVie
 
         this._pendingUpdates.push(domNode);
 
-        if (domEvent.timestamp > this._waterfallEndTime)
-            this._waterfallEndTime = domEvent.timestamp + (this._waterfallTimelineRuler.secondsPerPixel * 10);
+        this._updateWaterfallTimeRange(NaN, domEvent.timestamp + (this._waterfallTimelineRuler.secondsPerPixel * 10));
 
         this.needsLayout();
     }
@@ -1622,8 +1621,7 @@ WI.NetworkTableContentView = class NetworkTableContentView extends WI.ContentVie
 
         this._pendingUpdates.push(domNode);
 
-        if (timestamp > this._waterfallEndTime)
-            this._waterfallEndTime = timestamp + (this._waterfallTimelineRuler.secondsPerPixel * 10);
+        this._updateWaterfallTimeRange(NaN, timestamp + (this._waterfallTimelineRuler.secondsPerPixel * 10));
 
         this.needsLayout();
     }