Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2726103002:
[Devtools] Moved network waterfall to draw all text as a layer (Closed)
|
Created:
3 years, 9 months ago by allada Modified:
3 years, 9 months ago Reviewers:
caseq CC:
chromium-reviews, caseq+blink_chromium.org, lushnikov+blink_chromium.org, pfeldman+blink_chromium.org, apavlov+blink_chromium.org, devtools-reviews_chromium.org, blink-reviews, pfeldman, kozyatinskiy+blink_chromium.org Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
Description[Devtools] Moved network waterfall to draw all text as a layer
This patch is a series of patches that moves network waterfall to draw in
layers. This is the patch specifically to move text to be caluclated then
drawn all at once.
R=caseq
BUG=697668, 664704
Review-Url: https://codereview.chromium.org/2726103002
Cr-Commit-Position: refs/heads/master@{#455839}
Committed: https://chromium.googlesource.com/chromium/src/+/3e8277dbaf900a53ab0ec812a030ac4a95f7323a
Patch Set 1 #
Total comments: 7
Patch Set 2 : changes #Patch Set 3 : changes #
Depends on Patchset: Dependent Patchsets: Messages
Total messages: 15 (7 generated)
PTL https://codereview.chromium.org/2726103002/diff/1/third_party/WebKit/Source/d... File third_party/WebKit/Source/devtools/front_end/network/NetworkWaterfallColumn.js (right): https://codereview.chromium.org/2726103002/diff/1/third_party/WebKit/Source/d... third_party/WebKit/Source/devtools/front_end/network/NetworkWaterfallColumn.js:518: _drawSimplifiedBarDetails(context, leftText, rightText, startX, midX, endX, currentY) { This entire function will be inlined in an upcoming patch, in the mean time I am passing currentY, but it will know about it when it's inlined.
https://codereview.chromium.org/2726103002/diff/1/third_party/WebKit/Source/d... File third_party/WebKit/Source/devtools/front_end/network/NetworkWaterfallColumn.js (right): https://codereview.chromium.org/2726103002/diff/1/third_party/WebKit/Source/d... third_party/WebKit/Source/devtools/front_end/network/NetworkWaterfallColumn.js:365: '#888', /** @type {!UI.ThemeSupport.ColorUsage} */ (colorUsage.Background | colorUsage.Selection)); Why? It seems neither background nor selection. https://codereview.chromium.org/2726103002/diff/1/third_party/WebKit/Source/d... third_party/WebKit/Source/devtools/front_end/network/NetworkWaterfallColumn.js:529: const midBarX = startX + (midX - startX) / 2 - leftLabelWidth / 2; please keep it a var, we decided on not using consts for the time being.
PTaL https://codereview.chromium.org/2726103002/diff/1/third_party/WebKit/Source/d... File third_party/WebKit/Source/devtools/front_end/network/NetworkWaterfallColumn.js (right): https://codereview.chromium.org/2726103002/diff/1/third_party/WebKit/Source/d... third_party/WebKit/Source/devtools/front_end/network/NetworkWaterfallColumn.js:365: '#888', /** @type {!UI.ThemeSupport.ColorUsage} */ (colorUsage.Background | colorUsage.Selection)); On 2017/03/02 23:15:20, caseq wrote: > Why? It seems neither background nor selection. It only appears when a user hovers over it. It's a complement to line 55 above. This is also going to be moving to a style map toward the end of the cl chain. https://codereview.chromium.org/2726103002/diff/1/third_party/WebKit/Source/d... third_party/WebKit/Source/devtools/front_end/network/NetworkWaterfallColumn.js:529: const midBarX = startX + (midX - startX) / 2 - leftLabelWidth / 2; On 2017/03/02 23:15:20, caseq wrote: > please keep it a var, we decided on not using consts for the time being. Done.
https://codereview.chromium.org/2726103002/diff/1/third_party/WebKit/Source/d... File third_party/WebKit/Source/devtools/front_end/network/NetworkWaterfallColumn.js (right): https://codereview.chromium.org/2726103002/diff/1/third_party/WebKit/Source/d... third_party/WebKit/Source/devtools/front_end/network/NetworkWaterfallColumn.js:365: '#888', /** @type {!UI.ThemeSupport.ColorUsage} */ (colorUsage.Background | colorUsage.Selection)); On 2017/03/03 03:49:05, allada wrote: > On 2017/03/02 23:15:20, caseq wrote: > > Why? It seems neither background nor selection. > > It only appears when a user hovers over it. It's a complement to line 55 above. > This is also going to be moving to a style map toward the end of the cl chain. Right, but this is still a foreground color, not background or selection background.
PTaL https://codereview.chromium.org/2726103002/diff/1/third_party/WebKit/Source/d... File third_party/WebKit/Source/devtools/front_end/network/NetworkWaterfallColumn.js (right): https://codereview.chromium.org/2726103002/diff/1/third_party/WebKit/Source/d... third_party/WebKit/Source/devtools/front_end/network/NetworkWaterfallColumn.js:365: '#888', /** @type {!UI.ThemeSupport.ColorUsage} */ (colorUsage.Background | colorUsage.Selection)); On 2017/03/03 17:23:24, caseq wrote: > On 2017/03/03 03:49:05, allada wrote: > > On 2017/03/02 23:15:20, caseq wrote: > > > Why? It seems neither background nor selection. > > > > It only appears when a user hovers over it. It's a complement to line 55 > above. > > This is also going to be moving to a style map toward the end of the cl chain. > > Right, but this is still a foreground color, not background or selection > background. Done.
lgtm
The CQ bit was checked by allada@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by allada@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 40001, "attempt_start_ts": 1489088974556180,
"parent_rev": "d07b3613852b36e794f4794f1a86b696ca4562fa", "commit_rev":
"3e8277dbaf900a53ab0ec812a030ac4a95f7323a"}
Message was sent while issue was closed.
Description was changed from ========== [Devtools] Moved network waterfall to draw all text as a layer This patch is a series of patches that moves network waterfall to draw in layers. This is the patch specifically to move text to be caluclated then drawn all at once. R=caseq BUG=697668,664704 ========== to ========== [Devtools] Moved network waterfall to draw all text as a layer This patch is a series of patches that moves network waterfall to draw in layers. This is the patch specifically to move text to be caluclated then drawn all at once. R=caseq BUG=697668,664704 Review-Url: https://codereview.chromium.org/2726103002 Cr-Commit-Position: refs/heads/master@{#455839} Committed: https://chromium.googlesource.com/chromium/src/+/3e8277dbaf900a53ab0ec812a030... ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as https://chromium.googlesource.com/chromium/src/+/3e8277dbaf900a53ab0ec812a030... |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
