Changeset 238168 in webkit


Ignore:
Timestamp:
Nov 14, 2018 1:26:23 AM (5 years ago)
Author:
Devin Rousso
Message:

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.

Location:
trunk/Source/WebInspectorUI
Files:
4 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebInspectorUI/ChangeLog

    r238140 r238168  
     12018-11-14  Devin Rousso  <drousso@apple.com>
     2
     3        Web Inspector: Network Graphs are missing minimum sizes, might have no graph at all
     4        https://bugs.webkit.org/show_bug.cgi?id=191208
     5
     6        Reviewed by Joseph Pecoraro.
     7
     8        `WI.NetworkTableContentView` used to rely on the `WI.timelineManager.persistentNetworkTimeline`
     9        for events when a new resource is added. It also listened for `WI.Frame.Event.MainResourceDidChange`
     10        on it's own, which was also listened to by `WI.timelineManager.persistentNetworkTimeline`.
     11        Due to the order in which these listeners are added, the new main resource would get added
     12        to the `WI.timelineManager.persistentNetworkTimeline` (and the `WI.NetworkTableContentView`
     13        shortly after), and right after that the `WI.NetworkTableContentView` would reset the graph
     14        in it's own listener for `WI.Frame.Event.MainResourceDidChange`.
     15
     16        This patch removes `WI.timelineManager.persistentNetworkTimeline` and instead makes it so
     17        that `WI.NetworkTableContentView` listens for resource added and main frame changed events
     18        on its own (similar to other views that follow this pattern), ensuring that there are no
     19        event races.
     20
     21        Also relaxes the "requirement" that prevented 0 width blocks from being drawn, allowing
     22        requests served from memory/disk to appear in the graph (their time is effectively 0).
     23
     24        * UserInterface/Views/NetworkTableContentView.js:
     25        (WI.NetworkTableContentView):
     26        (WI.NetworkTableContentView.prototype.closed):
     27        (WI.NetworkTableContentView.prototype._populateWaterfallGraph):
     28        (WI.NetworkTableContentView.prototype._populateWaterfallGraph.appendBlock):
     29        (WI.NetworkTableContentView.prototype._updateWaterfallTimeRange): Added.
     30        (WI.NetworkTableContentView.prototype._resourceLoadingDidFinish):
     31        (WI.NetworkTableContentView.prototype._resourceLoadingDidFail):
     32        (WI.NetworkTableContentView.prototype._handleResourceAdded): Added.
     33        (WI.NetworkTableContentView.prototype._insertResourceAndReloadTable):
     34        (WI.NetworkTableContentView.prototype._handleNodeDidFireEvent):
     35        (WI.NetworkTableContentView.prototype._handleNodeLowPowerChanged):
     36        * UserInterface/Views/NetworkTableContentView.css:
     37        (.network-table .waterfall .block:matches(.mouse-tracking, .filler) + .block:not(.mouse-tracking, .filler)): Added.
     38        (.network-table .waterfall .block.filler + .block): Deleted.
     39
     40        * UserInterface/Controllers/TimelineManager.js:
     41        (WI.TimelineManager):
     42        (WI.TimelineManager.prototype._mainResourceDidChange):
     43        (WI.TimelineManager.prototype._resourceWasAdded):
     44        (WI.TimelineManager.prototype.get persistentNetworkTimeline): Deleted.
     45
    1462018-11-13  Matt Baker  <mattbaker@apple.com>
    247

  • trunk/Source/WebInspectorUI/UserInterface/Controllers/TimelineManager.js

    r238048 r238168  
    4242        this._enabledTimelineTypesSetting = new WI.Setting("enabled-instrument-types", WI.TimelineManager.defaultTimelineTypes());
    4343
    44         this._persistentNetworkTimeline = new WI.NetworkTimeline;
    45 
    4644        this._isCapturing = false;
    4745        this._initiatedByBackendStart = false;
    … …  
    136134        console.assert(this._activeRecording || !this._isCapturing);
    137135        return this._activeRecording;
    138     }
    139 
    140     get persistentNetworkTimeline()
    141     {
    142         return this._persistentNetworkTimeline;
    143136    }
    144137
    … …  
    822815    _mainResourceDidChange(event)
    823816    {
    824         let frame = event.target;
    825         if (frame.isMainFrame() && WI.settings.clearNetworkOnNavigate.value)
    826             this._persistentNetworkTimeline.reset();
    827 
    828         let mainResource = frame.mainResource;
    829         let record = new WI.ResourceTimelineRecord(mainResource);
    830         if (!isNaN(record.startTime))
    831             this._persistentNetworkTimeline.addRecord(record);
    832 
    833817        // Ignore resource events when there isn't a main frame yet. Those events are triggered by
    834818        // loading the cached resources when the inspector opens, and they do not have timing information.
    … …  
    836820            return;
    837821
     822        let frame = event.target;
    838823        if (this._attemptAutoCapturingForFrame(frame))
    839824            return;
    … …  
    842827            return;
    843828
     829        let mainResource = frame.mainResource;
    844830        if (mainResource === this._mainResourceForAutoCapturing)
    845831            return;
    846832
    847         this._addRecord(record);
     833        this._addRecord(new WI.ResourceTimelineRecord(mainResource));
    848834    }
    849835
    850836    _resourceWasAdded(event)
    851837    {
    852         var record = new WI.ResourceTimelineRecord(event.data.resource);
    853         if (!isNaN(record.startTime))
    854             this._persistentNetworkTimeline.addRecord(record);
    855838
    856839        // Ignore resource events when there isn't a main frame yet. Those events are triggered by
    … …  
    862845            return;
    863846
    864         this._addRecord(record);
     847        this._addRecord(new WI.ResourceTimelineRecord(event.data.resource));
    865848    }
    866849

  • trunk/Source/WebInspectorUI/UserInterface/Views/NetworkTableContentView.css

    r237708 r238168  
    250250}
    251251
    252 .network-table .waterfall .block.filler + .block,
     252.network-table .waterfall .block:matches(.mouse-tracking, .filler) + .block:not(.mouse-tracking, .filler),
    253253.network-table .waterfall .block:not(.request, .response) + :matches(.request, .response) {
    254254    --start-radius: 2px;

  • trunk/Source/WebInspectorUI/UserInterface/Views/NetworkTableContentView.js

    r237720 r238168  
    131131        this._clearNetworkItemsNavigationItem.addEventListener(WI.ButtonNavigationItem.Event.Clicked, () => { this.reset(); });
    132132
     133        WI.Target.addEventListener(WI.Target.Event.ResourceAdded, this._handleResourceAdded, this);
    133134        WI.Frame.addEventListener(WI.Frame.Event.MainResourceDidChange, this._mainResourceDidChange, this);
     135        WI.Frame.addEventListener(WI.Frame.Event.ResourceWasAdded, this._handleResourceAdded, this);
    134136        WI.Resource.addEventListener(WI.Resource.Event.LoadingDidFinish, this._resourceLoadingDidFinish, this);
    135137        WI.Resource.addEventListener(WI.Resource.Event.LoadingDidFail, this._resourceLoadingDidFail, this);
    136138        WI.Resource.addEventListener(WI.Resource.Event.TransferSizeDidChange, this._resourceTransferSizeDidChange, this);
    137139        WI.networkManager.addEventListener(WI.NetworkManager.Event.MainFrameDidChange, this._mainFrameDidChange, this);
    138         WI.timelineManager.persistentNetworkTimeline.addEventListener(WI.Timeline.Event.RecordAdded, this._networkTimelineRecordAdded, this);
    139140
    140141        this._needsInitialPopulate = true;
    … …  
    251252        this._hideDetailView();
    252253
     254        WI.Target.removeEventListener(null, null, this);
    253255        WI.Frame.removeEventListener(null, null, this);
    254256        WI.Resource.removeEventListener(null, null, this);
    … …  
    256258        WI.settings.clearNetworkOnNavigate.removeEventListener(null, null, this);
    257259        WI.networkManager.removeEventListener(WI.NetworkManager.Event.MainFrameDidChange, this._mainFrameDidChange, this);
    258         WI.timelineManager.persistentNetworkTimeline.removeEventListener(WI.Timeline.Event.RecordAdded, this._networkTimelineRecordAdded, this);
    259260
    260261        super.closed();
    … …  
    764765
    765766        let {startTime, redirectStart, redirectEnd, fetchStart, domainLookupStart, domainLookupEnd, connectStart, connectEnd, secureConnectionStart, requestStart, responseStart, responseEnd} = resource.timingData;
    766         if (isNaN(startTime) || isNaN(responseEnd) || startTime >= responseEnd) {
     767        if (isNaN(startTime) || isNaN(responseEnd) || startTime > responseEnd) {
    767768            cell.textContent = zeroWidthSpace;
    768769            return;
    … …  
    782783        let lastEndTimestamp = NaN;
    783784        function appendBlock(startTimestamp, endTimestamp, className) {
    784             if (isNaN(startTimestamp) || isNaN(endTimestamp) || endTimestamp - startTimestamp <= 0)
     785            if (isNaN(startTimestamp) || isNaN(endTimestamp))
    785786                return null;
    786787
    … …  
    808809        let totalWidth = (responseEnd - startTime) / secondsPerPixel;
    809810        if (totalWidth <= 3) {
    810             appendBlock(startTime, requestStart, "queue");
    811             appendBlock(startTime, responseEnd, "response");
     811            let twoPixels = secondsPerPixel * 2;
     812            appendBlock(startTime, startTime + twoPixels, "queue");
     813            appendBlock(startTime + twoPixels, startTime + (2 * twoPixels), "response");
    812814            return;
    813815        }
    … …  
    11121114    // Private
    11131115
     1116    _updateWaterfallTimeRange(startTimestamp, endTimestamp)
     1117    {
     1118        if (isNaN(this._waterfallStartTime) || startTimestamp < this._waterfallStartTime)
     1119            this._waterfallStartTime = startTimestamp;
     1120
     1121        if (isNaN(this._waterfallEndTime) || endTimestamp > this._waterfallEndTime)
     1122            this._waterfallEndTime = endTimestamp;
     1123    }
     1124
    11141125    _updateWaterfallTimelineRuler()
    11151126    {
    … …  
    14151426        this._pendingUpdates.push(resource);
    14161427
    1417         if (resource.firstTimestamp < this._waterfallStartTime)
    1418             this._waterfallStartTime = resource.firstTimestamp;
    1419         if (resource.timingData.responseEnd > this._waterfallEndTime)
    1420             this._waterfallEndTime = resource.timingData.responseEnd;
     1428        this._updateWaterfallTimeRange(resource.firstTimestamp, resource.timingData.responseEnd);
    14211429
    14221430        if (this._hasURLFilter())
    … …  
    14311439        this._pendingUpdates.push(resource);
    14321440
    1433         if (resource.firstTimestamp < this._waterfallStartTime)
    1434             this._waterfallStartTime = resource.firstTimestamp;
    1435         if (resource.timingData.responseEnd > this._waterfallEndTime)
    1436             this._waterfallEndTime = resource.timingData.responseEnd;
     1441        this._updateWaterfallTimeRange(resource.firstTimestamp, resource.timingData.responseEnd);
    14371442
    14381443        if (this._hasURLFilter())
    … …  
    14701475    }
    14711476
    1472     _networkTimelineRecordAdded(event)
    1473     {
    1474         let resourceTimelineRecord = event.data.record;
    1475         console.assert(resourceTimelineRecord instanceof WI.ResourceTimelineRecord);
    1476 
    1477         let resource = resourceTimelineRecord.resource;
    1478         if (isNaN(this._waterfallStartTime))
    1479             this._waterfallStartTime = this._waterfallEndTime = resource.firstTimestamp;
    1480 
    1481         this._insertResourceAndReloadTable(resource);
     1477    _handleResourceAdded(event)
     1478    {
     1479        this._insertResourceAndReloadTable(event.data.resource);
    14821480    }
    14831481
    … …  
    14891487    _insertResourceAndReloadTable(resource)
    14901488    {
     1489        this._updateWaterfallTimeRange(resource.firstTimestamp, resource.timingData.responseEnd);
     1490
    14911491        if (!this._table || !(WI.tabBrowser.selectedTabContentView instanceof WI.NetworkTabContentView)) {
    14921492            this._pendingInsertions.push(resource);
    … …  
    16101610        this._pendingUpdates.push(domNode);
    16111611
    1612         if (domEvent.timestamp > this._waterfallEndTime)
    1613             this._waterfallEndTime = domEvent.timestamp + (this._waterfallTimelineRuler.secondsPerPixel * 10);
     1612        this._updateWaterfallTimeRange(NaN, domEvent.timestamp + (this._waterfallTimelineRuler.secondsPerPixel * 10));
    16141613
    16151614        this.needsLayout();
    … …  
    16231622        this._pendingUpdates.push(domNode);
    16241623
    1625         if (timestamp > this._waterfallEndTime)
    1626             this._waterfallEndTime = timestamp + (this._waterfallTimelineRuler.secondsPerPixel * 10);
     1624        this._updateWaterfallTimeRange(NaN, timestamp + (this._waterfallTimelineRuler.secondsPerPixel * 10));
    16271625
    16281626        this.needsLayout();
Note: See TracChangeset for help on using the changeset viewer.