Issue 721703002: DevTools: instant events should not overlap on Timeline flame chart when zooming in

Issue 721703002: DevTools: instant events should not overlap on Timeline flame chart when zooming in (Closed)

Created:
6 years, 1 month ago by yurys

Modified:
6 years, 1 month ago

Reviewers:
alph, loislo

CC:
blink-reviews, caseq+blink_chromium.org, loislo+blink_chromium.org, eustas+blink_chromium.org, malch+blink_chromium.org, yurys+blink_chromium.org, lushnikov+blink_chromium.org, vsevik+blink_chromium.org, pfeldman+blink_chromium.org, paulirish+reviews_chromium.org, apavlov+blink_chromium.org, devtools-reviews_chromium.org, sergeyv+blink_chromium.org, aandrey+blink_chromium.org

Base URL:
svn://svn.chromium.org/blink/trunk

Project:
blink

Visibility:
Public.

More Reviews

Description

DevTools: instant events should not overlap on Timeline flame chart when zooming in Instant events are now considered as having duration 1us rather than 10us which makes overlapping much less likely. BUG=398783 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=185217

Patch Set 1 #

Total comments: 6

Patch Set 2 : Addressed comments #

Patch Set 3 : Rebase #

Created: 6 years, 1 month ago

Download [raw] [tar.bz2]

		Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+4 lines, -4 lines)			Patch
	M	Source/devtools/front_end/timeline/TimelineFlameChart.js	View		1 chunk	+1 line, -1 line	0 comments	Download
	M	Source/devtools/front_end/ui/FlameChart.js	View	1 2	2 chunks	+3 lines, -3 lines	0 comments	Download

Messages

Total messages: 11 (4 generated)

Expand Messages | Collapse Messages | Show Generated Messages | Hide Generated Messages

alph

lgtm https://codereview.chromium.org/721703002/diff/1/Source/devtools/front_end/components/FlameChart.js File Source/devtools/front_end/components/FlameChart.js (right): https://codereview.chromium.org/721703002/diff/1/Source/devtools/front_end/components/FlameChart.js#newcode911 Source/devtools/front_end/components/FlameChart.js:911: var titleIndices = new Uint32Array(entryTotalTimes); could you please ...

6 years, 1 month ago (2014-11-12 13:42:58 UTC) #3

yurys

https://codereview.chromium.org/721703002/diff/1/Source/devtools/front_end/components/FlameChart.js File Source/devtools/front_end/components/FlameChart.js (right): https://codereview.chromium.org/721703002/diff/1/Source/devtools/front_end/components/FlameChart.js#newcode911 Source/devtools/front_end/components/FlameChart.js:911: var titleIndices = new Uint32Array(entryTotalTimes); On 2014/11/12 13:42:57, alph ...

6 years, 1 month ago (2014-11-12 13:46:50 UTC) #4

commit-bot: I haz the power

CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/721703002/20001

6 years, 1 month ago (2014-11-12 13:49:09 UTC) #6

commit-bot: I haz the power

Failed to apply patch for Source/devtools/front_end/components/FlameChart.js: While running patch -p1 --forward --force --no-backup-if-mismatch; can't find ...

6 years, 1 month ago (2014-11-12 15:05:59 UTC) #8

Failed to apply patch for Source/devtools/front_end/components/FlameChart.js:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  can't find file to patch at input line 6
  Perhaps you used the wrong -p or --strip option?
  The text leading up to this was:
  --------------------------
  |Index: Source/devtools/front_end/components/FlameChart.js
  |diff --git a/Source/devtools/front_end/components/FlameChart.js
b/Source/devtools/front_end/components/FlameChart.js
  |index
15cd67d576ee0de431f622e800e0ec4aa246bd05..8327bddd7e2e01db418ccd0dce52bd535198ec6e
100644
  |--- a/Source/devtools/front_end/components/FlameChart.js
  |+++ b/Source/devtools/front_end/components/FlameChart.js
  --------------------------
  No file to patch.  Skipping patch.
  2 out of 2 hunks ignored

Patch:       Source/devtools/front_end/components/FlameChart.js
Index: Source/devtools/front_end/components/FlameChart.js
diff --git a/Source/devtools/front_end/components/FlameChart.js
b/Source/devtools/front_end/components/FlameChart.js
index
15cd67d576ee0de431f622e800e0ec4aa246bd05..8327bddd7e2e01db418ccd0dce52bd535198ec6e
100644
--- a/Source/devtools/front_end/components/FlameChart.js
+++ b/Source/devtools/front_end/components/FlameChart.js
@@ -93,7 +93,7 @@ WebInspector.FlameChart = function(dataProvider,
flameChartDelegate, isTopDown)
     this._timeWindowRight = Infinity;
     this._barHeight = dataProvider.barHeight();
     this._barHeightDelta = this._isTopDown ? -this._barHeight :
this._barHeight;
-    this._minWidth = 1;
+    this._minWidth = 2;
     this._paddingLeft = this._dataProvider.paddingLeft();
     this._markerPadding = 2;
     this._markerRadius = this._barHeight / 2 - this._markerPadding;
@@ -908,9 +908,9 @@ WebInspector.FlameChart.prototype = {
         var entryStartTimes = timelineData.entryStartTimes;
         var entryLevels = timelineData.entryLevels;
 
-        var titleIndices = new Uint32Array(timelineData.entryTotalTimes);
+        var titleIndices = new Uint32Array(entryTotalTimes.length);
         var nextTitleIndex = 0;
-        var markerIndices = new Uint32Array(timelineData.entryTotalTimes);
+        var markerIndices = new Uint32Array(entryTotalTimes.length);
         var nextMarkerIndex = 0;
         var textPadding = this._dataProvider.textPadding();
         this._minTextWidth = 2 * textPadding + this._measureWidth(context,
"\u2026");

commit-bot: I haz the power

CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/721703002/40001

6 years, 1 month ago (2014-11-12 15:19:43 UTC) #10

commit-bot: I haz the power

6 years, 1 month ago (2014-11-12 16:29:31 UTC) #11

Message was sent while issue was closed.

Committed patchset #3 (id:40001) as 185217

Expand Messages | Collapse Messages | Show Generated Messages | Hide Generated Messages