[RPP] remove SDK dep of TimelineFrameModel It only needed it to get the full snapshot; this can easily be implemented in the UI layer outside of the trace engine. It's not enough of a useful feature to warrant SDK as a dep. Bug: 1499182 Change-Id: I351d85e7454640332ddd557de21fc3e4ec983a4a Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/5049233 Auto-Submit: Jack Franklin <jacktfranklin@chromium.org> Reviewed-by: Andres Olivares <andoli@chromium.org> Commit-Queue: Jack Franklin <jacktfranklin@chromium.org>
diff --git a/front_end/models/trace/frames/BUILD.gn b/front_end/models/trace/frames/BUILD.gn index 1de47db..300eebe 100644 --- a/front_end/models/trace/frames/BUILD.gn +++ b/front_end/models/trace/frames/BUILD.gn
@@ -11,7 +11,6 @@ deps = [ "../../../core/platform:bundle", - "../../../core/sdk:bundle", "../../../generated", "../handlers:bundle", "../helpers:bundle",
diff --git a/front_end/models/trace/frames/TimelineFrameModel.ts b/front_end/models/trace/frames/TimelineFrameModel.ts index 082deba..7df0fda 100644 --- a/front_end/models/trace/frames/TimelineFrameModel.ts +++ b/front_end/models/trace/frames/TimelineFrameModel.ts
@@ -31,7 +31,6 @@ /* eslint-disable @typescript-eslint/naming-convention */ import * as Platform from '../../../core/platform/platform.js'; -import * as SDK from '../../../core/sdk/sdk.js'; import * as Handlers from '../handlers/handlers.js'; import * as Helpers from '../helpers/helpers.js'; import * as Types from '../types/types.js'; @@ -85,7 +84,6 @@ private mainFrameRequested!: boolean; private lastLayerTree!: FrameLayerTreeData|null; private framePendingActivation!: PendingFrame|null; - private target!: SDK.Target.Target|null; private framePendingCommit?: PendingFrame|null; private lastBeginFrame?: number|null; private lastNeedsBeginFrame?: number|null; @@ -96,9 +94,7 @@ #traceParseData: Handlers.Types.TraceParseData; - constructor( - target: SDK.Target.Target|null, allEvents: readonly Types.TraceEvents.TraceEventData[], - traceParseData: Handlers.Types.TraceParseData) { + constructor(allEvents: readonly Types.TraceEvents.TraceEventData[], traceParseData: Handlers.Types.TraceParseData) { this.reset(); this.#traceParseData = traceParseData; const mainThreads = Handlers.Threads.threadsInTrace(traceParseData).filter(thread => { @@ -112,7 +108,7 @@ }; }); - this.addTraceEvents(target, allEvents, threadData); + this.addTraceEvents(allEvents, threadData); } getFrames(): TimelineFrame[] { @@ -141,7 +137,6 @@ this.lastNeedsBeginFrame = null; this.framePendingActivation = null; this.lastTaskBeginTime = null; - this.target = null; this.layerTreeId = null; } @@ -279,12 +274,11 @@ this.framePendingActivation = null; } - addTraceEvents(target: SDK.Target.Target|null, events: readonly Types.TraceEvents.TraceEventData[], threadData: { + addTraceEvents(events: readonly Types.TraceEvents.TraceEventData[], threadData: { pid: Types.TraceEvents.ProcessID, tid: Types.TraceEvents.ThreadID, startTime: Types.Timing.MicroSeconds, }[]): void { - this.target = target; let j = 0; this.#activeThreadId = threadData.length && threadData[0].tid || null; this.#activeProcessId = threadData.length && threadData[0].pid || null; @@ -310,7 +304,7 @@ this.layerTreeId = event.args.data.layerTreeId; } else if ( entryId && Types.TraceEvents.isTraceEventLayerTreeHostImplSnapshot(event) && - Number(entryId) === this.layerTreeId && this.target) { + Number(entryId) === this.layerTreeId) { this.handleLayerTreeSnapshot({ entry: event, paints: [], @@ -366,7 +360,7 @@ if (Types.TraceEvents.isTraceEventPaint(entry)) { const snapshot = this.#traceParseData.LayerTreeHandler.paintsToSnapshots.get(entry); if (snapshot) { - this.framePendingCommit.paints.push(new LayerPaintEvent(entry, snapshot, this.target)); + this.framePendingCommit.paints.push(new LayerPaintEvent(entry, snapshot)); } } // Commit will be replacing CompositeLayers but CompositeLayers is kept @@ -432,46 +426,27 @@ serializedPicture: string; } export class LayerPaintEvent { - private readonly eventInternal: Types.TraceEvents.TraceEventPaint; - private readonly target: SDK.Target.Target|null; + readonly #event: Types.TraceEvents.TraceEventPaint; #snapshot: Types.TraceEvents.TraceEventDisplayItemListSnapshot; - constructor( - event: Types.TraceEvents.TraceEventPaint, snapshot: Types.TraceEvents.TraceEventDisplayItemListSnapshot, - target: SDK.Target.Target|null) { - this.eventInternal = event; + constructor(event: Types.TraceEvents.TraceEventPaint, snapshot: Types.TraceEvents.TraceEventDisplayItemListSnapshot) { + this.#event = event; this.#snapshot = snapshot; - this.target = target; } - layerId(): string { - // TODO: could make this function return a number? - return String(this.eventInternal.args['data']['layerId']); + layerId(): number { + return this.#event.args.data.layerId; } event(): Types.TraceEvents.TraceEventPaint { - return this.eventInternal; + return this.#event; } picture(): LayerPaintEventPicture|null { - // TODO(crbug.com/1453234): this function does not need to be async now const rect = this.#snapshot.args.snapshot.params?.layer_rect; const pictureData = this.#snapshot.args.snapshot.skp64; return rect && pictureData ? {rect: rect, serializedPicture: pictureData} : null; } - - async snapshotPromise(): Promise<{ - rect: Array<number>, - snapshot: SDK.PaintProfiler.PaintProfilerSnapshot, - }|null> { - const paintProfilerModel = this.target && this.target.model(SDK.PaintProfiler.PaintProfilerModel); - const picture = this.picture(); - if (!picture || !paintProfilerModel) { - return null; - } - const snapshot = await paintProfilerModel.loadSnapshot(picture.serializedPicture); - return snapshot ? {rect: picture.rect, snapshot: snapshot} : null; - } } export class PendingFrame {
diff --git a/test/unittests/front_end/models/trace/frames/BUILD.gn b/test/unittests/front_end/models/trace/frames/BUILD.gn index 46a082d..6e38fda 100644 --- a/test/unittests/front_end/models/trace/frames/BUILD.gn +++ b/test/unittests/front_end/models/trace/frames/BUILD.gn
@@ -10,7 +10,6 @@ deps = [ "../../../../../../front_end/core/platform:bundle", - "../../../../../../front_end/core/sdk:bundle", "../../../../../../front_end/generated", "../../../../../../front_end/models/trace:bundle", "../../../../../../front_end/panels/timeline:bundle",
diff --git a/test/unittests/front_end/models/trace/frames/TimelineFrameModel_test.ts b/test/unittests/front_end/models/trace/frames/TimelineFrameModel_test.ts index cd50a46..6d1e3ab 100644 --- a/test/unittests/front_end/models/trace/frames/TimelineFrameModel_test.ts +++ b/test/unittests/front_end/models/trace/frames/TimelineFrameModel_test.ts
@@ -4,17 +4,14 @@ const {assert} = chai; import * as TraceEngine from '../../../../../../front_end/models/trace/trace.js'; -import {describeWithMockConnection, setMockConnectionResponseHandler} from '../../../helpers/MockConnection.js'; -import * as SDK from '../../../../../../front_end/core/sdk/sdk.js'; -import {createTarget} from '../../../helpers/EnvironmentHelpers.js'; +import {describeWithMockConnection} from '../../../helpers/MockConnection.js'; import {TraceLoader} from '../../../helpers/TraceLoader.js'; describeWithMockConnection('new-TimelineFrameModel', () => { it('can parse out a trace and return the frames', async function() { - const target = createTarget(); const rawEvents = await TraceLoader.rawEvents(this, 'web-dev-with-commit.json.gz'); const traceParsedData = await TraceLoader.traceEngine(this, 'web-dev-with-commit.json.gz'); - const frameModel = new TraceEngine.Frames.TimelineFrameModel.TimelineFrameModel(target, rawEvents, traceParsedData); + const frameModel = new TraceEngine.Frames.TimelineFrameModel.TimelineFrameModel(rawEvents, traceParsedData); const parsedFrames = frameModel.getFrames(); assert.lengthOf(frameModel.getFrames(), 18); @@ -36,10 +33,9 @@ // to process it. this.timeout(20_000); - const target = createTarget(); const rawEvents = await TraceLoader.rawEvents(this, 'web-dev-with-advanced-instrumentation.json.gz'); const traceParsedData = await TraceLoader.traceEngine(this, 'web-dev-with-advanced-instrumentation.json.gz'); - const frameModel = new TraceEngine.Frames.TimelineFrameModel.TimelineFrameModel(target, rawEvents, traceParsedData); + const frameModel = new TraceEngine.Frames.TimelineFrameModel.TimelineFrameModel(rawEvents, traceParsedData); assert.lengthOf(frameModel.getFrames(), 25); const frameWithPaints = frameModel.getFrames().at(2); @@ -50,30 +46,4 @@ assert.strictEqual(frameWithPaints.seqId, 1127448); assert.lengthOf(frameWithPaints.paints, 7); }); - - it('can load the full snapshot from CDP', async function() { - // Advanced instrumentation trace file is large: allow the bots more time - // to process it. - this.timeout(20_000); - // https://chromedevtools.github.io/devtools-protocol/tot/LayerTree/#method-loadSnapshot - setMockConnectionResponseHandler('LayerTree.loadSnapshot', async () => { - return { - snapshotId: 'fake-snapshot-123', - }; - }); - - const target = createTarget(); - const rawEvents = await TraceLoader.rawEvents(this, 'web-dev-with-advanced-instrumentation.json.gz'); - const traceParsedData = await TraceLoader.traceEngine(this, 'web-dev-with-advanced-instrumentation.json.gz'); - const frameModel = new TraceEngine.Frames.TimelineFrameModel.TimelineFrameModel(target, rawEvents, traceParsedData); - - assert.lengthOf(frameModel.getFrames(), 25); - const frameWithPaints = frameModel.getFrames().at(2); - const paintEvent = frameWithPaints?.paints.at(0); - if (!paintEvent) { - throw new Error('Could not find paint event.'); - } - const result = await paintEvent.snapshotPromise(); - assert.instanceOf(result?.snapshot, SDK.PaintProfiler.PaintProfilerSnapshot); - }); });