Attachment 360782 Details for Bug 194110 – [PATCH] Proposed Fix

[patch] [PATCH] Proposed Fix

discont-1.patch (text/plain), 8.37 KB, created by Joseph Pecoraro on 2019-01-31 14:27:40 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Joseph Pecoraro

Created: 2019-01-31 14:27:40 PST

Size: 8.37 KB

patch

obsolete

>diff --git a/Source/WebInspectorUI/ChangeLog b/Source/WebInspectorUI/ChangeLog
>index c80fbcd0021..a738c312c28 100644
>--- a/Source/WebInspectorUI/ChangeLog
>+++ b/Source/WebInspectorUI/ChangeLog
>@@ -1,3 +1,23 @@
>+2019-01-31  Joseph Pecoraro  <pecoraro@apple.com>
>+
>+        Web Inspector: Timeline graphs have drawing issues with multiple discontinuities
>+        https://bugs.webkit.org/show_bug.cgi?id=194110
>+        <rdar://problem/47714356>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * UserInterface/Views/CPUTimelineView.js:
>+        (WI.CPUTimelineView):
>+        (WI.CPUTimelineView.prototype.layout.xScale): Deleted.
>+        * UserInterface/Views/MemoryTimelineOverviewGraph.js:
>+        (WI.MemoryTimelineOverviewGraph.prototype.layout.insertDiscontinuity):
>+        (WI.MemoryTimelineOverviewGraph.prototype.layout):
>+        * UserInterface/Views/MemoryTimelineView.js:
>+        (WI.MemoryTimelineView.prototype.layout.xScale):
>+        (WI.MemoryTimelineView.prototype.layout.yScale):
>+        (WI.MemoryTimelineView.prototype.layout):
>+        Handle if multiple discontinuities exist between records.
>+
> 2019-01-31  Joseph Pecoraro  <pecoraro@apple.com>
> 
>         Web Inspector: Timeline time range selection should show duration alongside start and end
>diff --git a/Source/WebInspectorUI/UserInterface/Views/CPUTimelineView.js b/Source/WebInspectorUI/UserInterface/Views/CPUTimelineView.js
>index 2187b222505..741caf406ba 100644
>--- a/Source/WebInspectorUI/UserInterface/Views/CPUTimelineView.js
>+++ b/Source/WebInspectorUI/UserInterface/Views/CPUTimelineView.js
>@@ -133,10 +133,13 @@ WI.CPUTimelineView = class CPUTimelineView extends WI.TimelineView
>             let usage = record.usage;
> 
>             if (discontinuities.length && discontinuities[0].endTime < time) {
>-                let discontinuity = discontinuities.shift();
>-                dataPoints.push({time: discontinuity.startTime, size: 0});
>-                dataPoints.push({time: discontinuity.endTime, size: 0});
>-                dataPoints.push({time: discontinuity.endTime, size: usage});
>+                let startDiscontinuity = discontinuities.shift();
>+                let endDiscontinuity = startDiscontinuity;
>+                while (discontinuities.length && discontinuities[0].endTime < time)
>+                    endDiscontinuity = discontinuities.shift();
>+                dataPoints.push({time: startDiscontinuity.startTime, mainThreadUsage: 0, nonMainThreadUsage: 0, usage: 0});
>+                dataPoints.push({time: endDiscontinuity.endTime, mainThreadUsage: 0, nonMainThreadUsage: 0, usage: 0});
>+                dataPoints.push({time: endDiscontinuity.endTime, mainThreadUsage, nonMainThreadUsage, usage});
>             }
> 
>             dataPoints.push({time, size: usage});
>diff --git a/Source/WebInspectorUI/UserInterface/Views/MemoryTimelineOverviewGraph.js b/Source/WebInspectorUI/UserInterface/Views/MemoryTimelineOverviewGraph.js
>index 1fa6401b5c0..35452496532 100644
>--- a/Source/WebInspectorUI/UserInterface/Views/MemoryTimelineOverviewGraph.js
>+++ b/Source/WebInspectorUI/UserInterface/Views/MemoryTimelineOverviewGraph.js
>@@ -156,14 +156,14 @@ WI.MemoryTimelineOverviewGraph = class MemoryTimelineOverviewGraph extends WI.Ti
>         if (visibleRecords[0] === this._memoryTimeline.records[0] && (!discontinuities.length || discontinuities[0].startTime > visibleRecords[0].startTime))
>             this._chart.addPointSet(0, pointSetForRecord(visibleRecords[0]));
> 
>-        function insertDiscontinuity(previousRecord, discontinuity, nextRecord)
>+        function insertDiscontinuity(previousRecord, startDiscontinuity, endDiscontinuity, nextRecord)
>         {
>             console.assert(previousRecord || nextRecord);
>             if (!(previousRecord || nextRecord))
>                 return;
> 
>-            let xStart = xScale(discontinuity.startTime);
>-            let xEnd = xScale(discontinuity.endTime);
>+            let xStart = xScale(startDiscontinuity.startTime);
>+            let xEnd = xScale(endDiscontinuity.endTime);
> 
>             // Extend the previous record to the start of the discontinuity.
>             if (previousRecord)
>@@ -186,8 +186,11 @@ WI.MemoryTimelineOverviewGraph = class MemoryTimelineOverviewGraph extends WI.Ti
>         let previousRecord = null;
>         for (let record of visibleRecords) {
>             if (discontinuities.length && discontinuities[0].endTime < record.startTime) {
>-                let discontinuity = discontinuities.shift();
>-                insertDiscontinuity.call(this, previousRecord, discontinuity, record);
>+                let startDiscontinuity = discontinuities.shift();
>+                let endDiscontinuity = startDiscontinuity;
>+                while (discontinuities.length && discontinuities[0].endTime < record.startTime)
>+                    endDiscontinuity = discontinuities.shift();
>+                insertDiscontinuity.call(this, previousRecord, startDiscontinuity, endDiscontinuity, record);
>             }
> 
>             let x = xScale(record.startTime);
>@@ -197,7 +200,7 @@ WI.MemoryTimelineOverviewGraph = class MemoryTimelineOverviewGraph extends WI.Ti
>         }
> 
>         if (discontinuities.length)
>-            insertDiscontinuity.call(this, previousRecord, discontinuities[0], null);
>+            insertDiscontinuity.call(this, previousRecord, discontinuities[0], discontinuities[0], null);
>         else {
>             // Extend the last value to current / end time.
>             let lastRecord = visibleRecords.lastValue;
>diff --git a/Source/WebInspectorUI/UserInterface/Views/MemoryTimelineView.js b/Source/WebInspectorUI/UserInterface/Views/MemoryTimelineView.js
>index 66abb2c08c5..c692e0beb2c 100644
>--- a/Source/WebInspectorUI/UserInterface/Views/MemoryTimelineView.js
>+++ b/Source/WebInspectorUI/UserInterface/Views/MemoryTimelineView.js
>@@ -206,22 +206,27 @@ WI.MemoryTimelineView = class MemoryTimelineView extends WI.TimelineView
> 
>         for (let record of visibleRecords) {
>             let time = record.startTime;
>-            let discontinuity = null;
>-            if (discontinuities.length && discontinuities[0].endTime < time)
>-                discontinuity = discontinuities.shift();
>+            let startDiscontinuity = null;
>+            let endDiscontinuity = null;
>+            if (discontinuities.length && discontinuities[0].endTime < time) {
>+                startDiscontinuity = discontinuities.shift();
>+                endDiscontinuity = startDiscontinuity;
>+                while (discontinuities.length && discontinuities[0].endTime < time)
>+                    endDiscontinuity = discontinuities.shift();
>+            }
> 
>             for (let category of record.categories) {
>                 let categoryData = categoryDataMap[category.type];
> 
>-                if (discontinuity) {
>+                if (startDiscontinuity) {
>                     if (categoryData.dataPoints.length) {
>                         let previousDataPoint = categoryData.dataPoints.lastValue;
>-                        categoryData.dataPoints.push({time: discontinuity.startTime, size: previousDataPoint.size});
>+                        categoryData.dataPoints.push({time: startDiscontinuity.startTime, size: previousDataPoint.size});
>                     }
> 
>-                    categoryData.dataPoints.push({time: discontinuity.startTime, size: 0});
>-                    categoryData.dataPoints.push({time: discontinuity.endTime, size: 0});
>-                    categoryData.dataPoints.push({time: discontinuity.endTime, size: category.size});
>+                    categoryData.dataPoints.push({time: startDiscontinuity.startTime, size: 0});
>+                    categoryData.dataPoints.push({time: endDiscontinuity.endTime, size: 0});
>+                    categoryData.dataPoints.push({time: endDiscontinuity.endTime, size: category.size});
>                 }
> 
>                 categoryData.dataPoints.push({time, size: category.size});
>@@ -249,12 +254,12 @@ WI.MemoryTimelineView = class MemoryTimelineView extends WI.TimelineView
>                 return (time - graphStartTime) / secondsPerPixel;
>             }
> 
>-            let size = new WI.Size(xScale(graphEndTime), memoryCategoryViewHeight);
>-
>             function yScale(value) {
>                 return size.height - (((value - graphMin) / graphMax) * size.height);
>             }
> 
>+            let size = new WI.Size(xScale(graphEndTime), memoryCategoryViewHeight);
>+
>             categoryView.updateChart(dataPoints, size, visibleEndTime, min, max, xScale, yScale);
>         }
>

Flags:

hi: review+
hi: commit-queue-

Actions: View | Formatted Diff | Diff

Attachments on bug 194110: 360767 | 360782