DevTools: Slim down timeline overview height (issue 2644373004 by alph@chromium.org)

[al...@chromium.org]

Reviewers: caseq, pfeldman
CL: https://codereview.chromium.org/2644373004/

Description:
DevTools: Slim down timeline overview height

Affected files (+35, -42 lines):
M third_party/WebKit/Source/devtools/front_end/timeline/TimelineEventOverview.js
M third_party/WebKit/Source/devtools/front_end/timeline/timelinePanel.css


Index: third_party/WebKit/Source/devtools/front_end/timeline/TimelineEventOverview.js
diff --git a/third_party/WebKit/Source/devtools/front_end/timeline/TimelineEventOverview.js b/third_party/WebKit/Source/devtools/front_end/timeline/TimelineEventOverview.js
index 5e23e23fd5f0719e440dd0f6effa4571be6471f4..b966565d114334f2003dc0acb24b5f5910f50ec8 100644
--- a/third_party/WebKit/Source/devtools/front_end/timeline/TimelineEventOverview.js
+++ b/third_party/WebKit/Source/devtools/front_end/timeline/TimelineEventOverview.js
@@ -157,26 +157,24 @@ Timeline.TimelineEventOverviewNetwork = class extends Timeline.TimelineEventOver
*/
update() {
super.update();
- var height = this.height();
- var numBands = categoryBand(Timeline.TimelineUIUtils.NetworkCategory.Other) + 1;
- var bandHeight = Math.floor(height / numBands);
- var devicePixelRatio = window.devicePixelRatio;
- var timeOffset = this._model.minimumRecordTime();
- var timeSpan = this._model.maximumRecordTime() - timeOffset;
- var canvasWidth = this.width();
- var scale = canvasWidth / timeSpan;
- var ctx = this.context();
- var requests = this._model.networkRequests();
- /** @type {!Map<string,!{waiting:!Path2D,transfer:!Path2D}>} */
- var paths = new Map();
+ const height = this.height();
+ const numBands = categoryBand(Timeline.TimelineUIUtils.NetworkCategory.Other) + 1;
+ const bandHeight = height - numBands;
+ const timeOffset = this._model.minimumRecordTime();
+ const timeSpan = this._model.maximumRecordTime() - timeOffset;
+ const canvasWidth = this.width();
+ const scale = canvasWidth / timeSpan;
+ const ctx = this.context();
+ const requests = this._model.networkRequests();
+ const paths = new Array(numBands).fill(0).map(() => ({style: '', path: new Path2D()}));
requests.forEach(drawRequest);
- for (var path of paths) {
- ctx.fillStyle = path[0];
- ctx.globalAlpha = 0.3;
- ctx.fill(path[1]['waiting']);
- ctx.globalAlpha = 1;
- ctx.fill(path[1]['transfer']);
- }
+ ctx.globalAlpha = 0.7;
+ paths.reverse().forEach(path => {
+ ctx.fillStyle = 'white';
+ ctx.fill(path.path);
+ ctx.fillStyle = path.style;
+ ctx.fill(path.path);
+ });

/**
* @param {!Timeline.TimelineUIUtils.NetworkCategory} category
@@ -202,24 +200,15 @@ Timeline.TimelineEventOverviewNetwork = class extends Timeline.TimelineEventOver
* @param {!TimelineModel.TimelineModel.NetworkRequest} request
*/
function drawRequest(request) {
- var tickWidth = 2 * devicePixelRatio;
- var category = Timeline.TimelineUIUtils.networkRequestCategory(request);
- var style = Timeline.TimelineUIUtils.networkCategoryColor(category);
- var band = categoryBand(category);
- var y = band * bandHeight;
- var path = paths.get(style);
- if (!path) {
- path = {waiting: new Path2D(), transfer: new Path2D()};
- paths.set(style, path);
- }
- var s = Math.max(Math.floor((request.startTime - timeOffset) * scale), 0);
- var e = Math.min(Math.ceil((request.endTime - timeOffset) * scale), canvasWidth);
- path['waiting'].rect(s, y, e - s, bandHeight - 1);
- path['transfer'].rect(e - tickWidth / 2, y, tickWidth, bandHeight - 1);
- if (!request.responseTime)
- return;
- var r = Math.ceil((request.responseTime - timeOffset) * scale);
- path['transfer'].rect(r - tickWidth / 2, y, tickWidth, bandHeight - 1);
+ const category = Timeline.TimelineUIUtils.networkRequestCategory(request);
+ const style = Timeline.TimelineUIUtils.networkCategoryColor(category);
+ const band = categoryBand(category);
+ const path = paths[band];
+ const y = (numBands - band - 1);
+ const s = Math.max(Math.floor((request.startTime - timeOffset) * scale), 0);
+ const e = Math.min(Math.ceil((request.endTime - timeOffset) * scale), canvasWidth);
+ path.style = style;
+ path.path.rect(s, y, e - s, bandHeight - 1);
}
}
};
Index: third_party/WebKit/Source/devtools/front_end/timeline/timelinePanel.css
diff --git a/third_party/WebKit/Source/devtools/front_end/timeline/timelinePanel.css b/third_party/WebKit/Source/devtools/front_end/timeline/timelinePanel.css
index 9b0dfba68045aae107e5de8384bd10e4bc6acc63..d4357dacc994149d55978efde3618344924f1624 100644
--- a/third_party/WebKit/Source/devtools/front_end/timeline/timelinePanel.css
+++ b/third_party/WebKit/Source/devtools/front_end/timeline/timelinePanel.css
@@ -114,20 +114,24 @@
}

#timeline-overview-cpu-activity {
- flex-basis: 25px;
+ flex-basis: 20px;
+}
+
+#timeline-overview-network {
+ flex-basis: 12px;
}

-#timeline-overview-network,
#timeline-overview-framerate {
- flex-basis: 20px;
+ flex-basis: 16px;
+ margin-top: 0 !important;
}

#timeline-overview-filmstrip {
- flex-basis: 40px;
+ flex-basis: 30px;
}

#timeline-overview-memory {
- flex-basis: 22px;
+ flex-basis: 20px;
}

#timeline-overview-framerate::before,

[ca...@chromium.org]

can we have a screenshot for this one?


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

https://codereview.chromium.org/2644373004/diff/1/third_party/WebKit/Source/devtools/front_end/timeline/TimelineEventOverview.js#newcode170
third_party/WebKit/Source/devtools/front_end/timeline/TimelineEventOverview.js:170:
requests.forEach(drawRequest);
let's make changes to path more explicit?

https://codereview.chromium.org/2644373004/diff/1/third_party/WebKit/Source/devtools/front_end/timeline/TimelineEventOverview.js#newcode204
third_party/WebKit/Source/devtools/front_end/timeline/TimelineEventOverview.js:204:
const style = Timeline.TimelineUIUtils.networkCategoryColor(category);
inline?

https://codereview.chromium.org/2644373004/diff/1/third_party/WebKit/Source/devtools/front_end/timeline/TimelineEventOverview.js#newcode210
third_party/WebKit/Source/devtools/front_end/timeline/TimelineEventOverview.js:210:
path.style = style;
why do we change band path's style each time?

https://codereview.chromium.org/2644373004/

[al...@chromium.org]

all done. ptal

https://codereview.chromium.org/2644373004/

[al...@chromium.org]

ss:
https://drive.google.com/a/google.com/file/d/0ByLjXIa7NKi9M1RzemRMV045WjQ/view?usp=sharing

https://codereview.chromium.org/2644373004/

[pfel...@chromium.org]

lgtm



https://codereview.chromium.org/2644373004/

[commit-bot@chromium.org via codereview.chromium.org]

CQ is trying da patch. Follow status at

https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2644373004/20001


https://codereview.chromium.org/2644373004/

[commit-bot@chromium.org via codereview.chromium.org]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/799d1b122128aa137ee7c4a66f7e65eaf39b0485

https://codereview.chromium.org/2644373004/