Skip to content

Commit 282eff6

Browse files
paulirishDevtools-frontend LUCI CQ
authored andcommitted
RPP: Fix incorrect candystriping placement
If an interaction started exceeding 200ms after processingEnd, the candystripe decoration was incorrectly drawn. This simplifies our logic and correctly draws the decoration. Bug: 410531484 Change-Id: Ie499bfb7641a130db802c11f4c35db53335b20e8 Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6685592 Auto-Submit: Paul Irish <paulirish@chromium.org> Reviewed-by: Jack Franklin <jacktfranklin@chromium.org> Commit-Queue: Jack Franklin <jacktfranklin@chromium.org>
1 parent df716ad commit 282eff6

3 files changed

Lines changed: 8 additions & 15 deletions

File tree

front_end/panels/timeline/InteractionsTrackAppender.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -115,11 +115,11 @@ export class InteractionsTrackAppender implements TrackAppender {
115115
decorationsForEvent.push(
116116
{
117117
type: PerfUI.FlameChart.FlameChartDecorationType.CANDY,
118+
// Where the striping starts is hard. The problem is the whole interaction, isolating the part of it *responsible* for
119+
// making the interaction 200ms is hard and our decoration won't do it perfectly. To simplify we just flag all the overage.
120+
// AKA the first 200ms of the interaction aren't flagged. A downside is we often flag a lot of render delay.
121+
// It'd be fair to shift the candystriping segment earlier in the interaction... Let's see what the feedback is like.
118122
startAtTime: Trace.Handlers.ModelHandlers.UserInteractions.LONG_INTERACTION_THRESHOLD,
119-
// Interaction events have whiskers, so we do not want to candy stripe
120-
// the entire duration. The box represents processing duration, so we only
121-
// candystripe up to the end of processing.
122-
endAtTime: entry.processingEnd,
123123
},
124124
{
125125
type: PerfUI.FlameChart.FlameChartDecorationType.WARNING_TRIANGLE,

front_end/panels/timeline/track_appenders/InteractionsTrackAppender.test.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,6 @@ describeWithEnvironment('InteractionsTrackAppender', function() {
108108
{
109109
type: PerfUI.FlameChart.FlameChartDecorationType.CANDY,
110110
startAtTime: Trace.Types.Timing.Micro(200_000),
111-
endAtTime: longInteraction.processingEnd,
112111
},
113112
{
114113
type: PerfUI.FlameChart.FlameChartDecorationType.WARNING_TRIANGLE,

front_end/ui/legacy/components/perf_ui/FlameChart.ts

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2377,11 +2377,8 @@ export class FlameChart extends Common.ObjectWrapper.eventMixin<EventTypes, type
23772377
// Draw a rectangle over the event, starting at the X value of the
23782378
// event's start time + the startDuration of the candy striping.
23792379
const barXStart = this.timeToPositionClipped(entryStartTime + candyStripeStartTime);
2380-
2381-
// If a custom end time was passed in, that is when we stop striping, else we stripe until the very end of the entry.
2382-
const stripingEndTime = decoration.endAtTime ? Trace.Helpers.Timing.microToMilli(decoration.endAtTime) :
2383-
entryStartTime + duration;
2384-
const barXEnd = this.timeToPositionClipped(stripingEndTime);
2380+
// We stripe until the very end of the entry.
2381+
const barXEnd = this.timeToPositionClipped(entryStartTime + duration);
23852382
this.#drawEventRect(context, timelineData, entryIndex, {
23862383
startX: barXStart,
23872384
width: barXEnd - barXStart,
@@ -4103,12 +4100,9 @@ export const enum FlameChartDecorationType {
41034100
**/
41044101
export type FlameChartDecoration = {
41054102
type: FlameChartDecorationType.CANDY,
4106-
// We often only want to highlight problem parts of events, so this time sets
4107-
// the minimum time at which the candystriping will start. If you want to
4108-
// candystripe the entire event, set this to 0.
4103+
/** Relative to entry's ts. We often only want to highlight problem parts of events, so this time sets the minimum
4104+
* time at which the candystriping will start. If you want to candystripe the entire event, set this to 0. */
41094105
startAtTime: Trace.Types.Timing.Micro,
4110-
// Optionally set the end time for the striping. If this is not provided, the entire entry will be striped.
4111-
endAtTime?: Trace.Types.Timing.Micro,
41124106
}|{
41134107
type: FlameChartDecorationType.WARNING_TRIANGLE,
41144108
customStartTime?: Trace.Types.Timing.Micro,

0 commit comments

Comments
 (0)