diff --git a/tools/winscope/package.json b/tools/winscope/package.json index 5eaa4e6b5..ef27d793a 100644 --- a/tools/winscope/package.json +++ b/tools/winscope/package.json @@ -19,6 +19,7 @@ "test:unit": "webpack --config webpack.config.unit_test.js && jasmine dist/unit_test/bundle.js", "test:component": "npx karma start", "test:e2e": "rm -rf dist/e2e_test && npx tsc -p ./src/test/e2e && npx protractor protractor.config.js", + "test:presubmit": "npm run test:unit && npm run test:component && npm run format:check && npm run tslint:check && npm run eslint:check && npm run deps_graph:check_cycles", "test:all": "npm run test:unit && npm run test:component && npm run test:e2e && npm run format:check && npm run tslint:check && npm run eslint:check && npm run deps_graph:check_cycles" }, "private": true, diff --git a/tools/winscope/src/abt_chrome_extension/abt_chrome_extension_protocol.ts b/tools/winscope/src/abt_chrome_extension/abt_chrome_extension_protocol.ts index a8aba832f..787cc2eab 100644 --- a/tools/winscope/src/abt_chrome_extension/abt_chrome_extension_protocol.ts +++ b/tools/winscope/src/abt_chrome_extension/abt_chrome_extension_protocol.ts @@ -26,14 +26,15 @@ export class AbtChromeExtensionProtocol implements BuganizerAttachmentsDownloadE static readonly ABT_EXTENSION_ID = 'mbbaofdfoekifkfpgehgffcpagbbjkmj'; private onAttachmentsDownloadStart: OnBuganizerAttachmentsDownloadStart = FunctionUtils.DO_NOTHING; - private onttachmentsDownloaded: OnBuganizerAttachmentsDownloaded = FunctionUtils.DO_NOTHING_ASYNC; + private onAttachmentsDownloaded: OnBuganizerAttachmentsDownloaded = + FunctionUtils.DO_NOTHING_ASYNC; setOnBuganizerAttachmentsDownloadStart(callback: OnBuganizerAttachmentsDownloadStart) { this.onAttachmentsDownloadStart = callback; } setOnBuganizerAttachmentsDownloaded(callback: OnBuganizerAttachmentsDownloaded) { - this.onttachmentsDownloaded = callback; + this.onAttachmentsDownloaded = callback; } run() { @@ -83,7 +84,7 @@ export class AbtChromeExtensionProtocol implements BuganizerAttachmentsDownloadE }); const files = await Promise.all(filesBlobPromises); - await this.onttachmentsDownloaded(files); + await this.onAttachmentsDownloaded(files); } private isOpenBuganizerResponseMessage( diff --git a/tools/winscope/src/app/components/app_component.ts b/tools/winscope/src/app/components/app_component.ts index 512b19815..f249c0c06 100644 --- a/tools/winscope/src/app/components/app_component.ts +++ b/tools/winscope/src/app/components/app_component.ts @@ -356,7 +356,7 @@ export class AppComponent implements TraceDataListener { return ''; } - return `${TRACE_INFO[trace.type].name} (${trace.descriptors.join(', ')})`; + return `${trace.descriptors.join(', ')}`; } private getActiveTrace(view: View): LoadedTrace | undefined { diff --git a/tools/winscope/src/app/mediator.ts b/tools/winscope/src/app/mediator.ts index 3544a2cd6..60ee7eab8 100644 --- a/tools/winscope/src/app/mediator.ts +++ b/tools/winscope/src/app/mediator.ts @@ -83,7 +83,7 @@ export class Mediator { } ); - this.crossToolProtocol.setOnTimestampReceived(async (timestamp: Timestamp) => { + this.crossToolProtocol.setOnTimestampReceived((timestamp: Timestamp) => { this.onRemoteTimestampReceived(timestamp); }); @@ -203,7 +203,7 @@ export class Mediator { private async processRemoteFilesReceived(files: File[]) { this.resetAppToInitialState(); - this.processFiles(files); + await this.processFiles(files); } private async processFiles(files: File[]) { diff --git a/tools/winscope/src/app/trace_pipeline.ts b/tools/winscope/src/app/trace_pipeline.ts index 8a9a472c0..e1432c860 100644 --- a/tools/winscope/src/app/trace_pipeline.ts +++ b/tools/winscope/src/app/trace_pipeline.ts @@ -38,6 +38,7 @@ class TracePipeline { traceFiles: TraceFile[], onLoadProgressUpdate: OnProgressUpdateType = FunctionUtils.DO_NOTHING ): Promise { + traceFiles = await this.filterBugreportFilesIfNeeded(traceFiles); const [parsers, parserErrors] = await this.parserFactory.createParsers( traceFiles, onLoadProgressUpdate @@ -117,6 +118,36 @@ class TracePipeline { this.files = new Map(); } + private async filterBugreportFilesIfNeeded(files: TraceFile[]): Promise { + const bugreportMainEntry = files.find((file) => file.file.name === 'main_entry.txt'); + if (!bugreportMainEntry) { + return files; + } + + const bugreportName = (await bugreportMainEntry.file.text()).trim(); + const isBugreport = files.find((file) => file.file.name === bugreportName) !== undefined; + if (!isBugreport) { + return files; + } + + const BUGREPORT_TRACE_DIRS = ['FS/data/misc/wmtrace/', 'FS/data/misc/perfetto-traces/']; + const isFileWithinBugreportTraceDir = (file: TraceFile) => { + for (const traceDir of BUGREPORT_TRACE_DIRS) { + if (file.file.name.startsWith(traceDir)) { + return true; + } + } + return false; + }; + + const fileBelongsToBugreport = (file: TraceFile) => + file.parentArchive === bugreportMainEntry.parentArchive; + + return files.filter((file) => { + return isFileWithinBugreportTraceDir(file) || !fileBelongsToBugreport(file); + }); + } + private getCommonTimestampType(): TimestampType { if (this.commonTimestampType !== undefined) { return this.commonTimestampType; diff --git a/tools/winscope/src/app/trace_pipeline_test.ts b/tools/winscope/src/app/trace_pipeline_test.ts index 20e01a07c..46c670177 100644 --- a/tools/winscope/src/app/trace_pipeline_test.ts +++ b/tools/winscope/src/app/trace_pipeline_test.ts @@ -39,6 +39,74 @@ describe('TracePipeline', () => { expect(traceEntries.get(TraceType.SURFACE_FLINGER)?.length).toBeGreaterThan(0); }); + it('can load bugreport and ignores non-trace dirs', async () => { + expect(tracePipeline.getLoadedTraces().length).toEqual(0); + + // Could be any file, we just need an instance of File to be used as a fake bugreport archive + const bugreportArchive = await UnitTestUtils.getFixtureFile( + 'bugreports/bugreport_stripped.zip' + ); + + const bugreportFiles = [ + new TraceFile( + await UnitTestUtils.getFixtureFile('bugreports/main_entry.txt', 'main_entry.txt'), + bugreportArchive + ), + new TraceFile( + await UnitTestUtils.getFixtureFile( + 'bugreports/bugreport-codename_beta-UPB2.230407.019-2023-05-30-14-33-48.txt', + 'bugreport-codename_beta-UPB2.230407.019-2023-05-30-14-33-48.txt' + ), + bugreportArchive + ), + new TraceFile( + await UnitTestUtils.getFixtureFile( + 'traces/elapsed_and_real_timestamp/SurfaceFlinger.pb', + 'FS/data/misc/wmtrace/surface_flinger.bp' + ), + bugreportArchive + ), + new TraceFile( + await UnitTestUtils.getFixtureFile( + 'traces/elapsed_and_real_timestamp/Transactions.pb', + 'FS/data/misc/wmtrace/transactions.bp' + ), + bugreportArchive + ), + new TraceFile( + await UnitTestUtils.getFixtureFile( + 'traces/elapsed_and_real_timestamp/WindowManager.pb', + 'FS/data/misc/ignored-dir/window_manager.bp' + ), + bugreportArchive + ), + ]; + + // Corner case: + // A plain trace file is loaded along the bugreport -> trace file must not be ignored + // + // Note: + // The even weirder corner case where two bugreports are loaded at the same time is + // currently not properly handled. + const plainTraceFile = new TraceFile( + await UnitTestUtils.getFixtureFile( + 'traces/elapsed_and_real_timestamp/InputMethodClients.pb', + 'would-be-ignored-if-was-part-of-bugreport/input_method_clients.pb' + ) + ); + + const mergedFiles = bugreportFiles.concat([plainTraceFile]); + const errors = await tracePipeline.loadTraceFiles(mergedFiles); + expect(errors.length).toEqual(0); + tracePipeline.buildTraces(); + const traces = tracePipeline.getTraces(); + + expect(traces.getTrace(TraceType.SURFACE_FLINGER)).toBeDefined(); + expect(traces.getTrace(TraceType.TRANSACTIONS)).toBeDefined(); + expect(traces.getTrace(TraceType.WINDOW_MANAGER)).toBeUndefined(); // ignored + expect(traces.getTrace(TraceType.INPUT_METHOD_CLIENTS)).toBeDefined(); + }); + it('is robust to invalid trace files', async () => { const invalidTraceFiles = [ new TraceFile(await UnitTestUtils.getFixtureFile('winscope_homepage.png')), diff --git a/tools/winscope/src/common/file_utils.ts b/tools/winscope/src/common/file_utils.ts index 447e043ad..ac2b688a7 100644 --- a/tools/winscope/src/common/file_utils.ts +++ b/tools/winscope/src/common/file_utils.ts @@ -61,9 +61,8 @@ class FileUtils { // Ignore directories continue; } else { - const name = FileUtils.removeDirFromFileName(filename); const fileBlob = await file.async('blob'); - const unzippedFile = new File([fileBlob], name); + const unzippedFile = new File([fileBlob], filename); unzippedFiles.push(unzippedFile); } diff --git a/tools/winscope/src/common/time_utils.ts b/tools/winscope/src/common/time_utils.ts index 876c3c1e8..42d6cf109 100644 --- a/tools/winscope/src/common/time_utils.ts +++ b/tools/winscope/src/common/time_utils.ts @@ -128,7 +128,6 @@ export class TimeUtils { } static formattedKotlinTimestamp(time: any, timestampType: TimestampType): string { - console.log('time', time); if (timestampType === TimestampType.ELAPSED) { return TimeUtils.format(new ElapsedTimestamp(BigInt(time.elapsedNanos.toString()))); } diff --git a/tools/winscope/src/test/common/file_impl.ts b/tools/winscope/src/test/common/file_impl.ts index 8ad842bcf..315bbe1b4 100644 --- a/tools/winscope/src/test/common/file_impl.ts +++ b/tools/winscope/src/test/common/file_impl.ts @@ -46,7 +46,11 @@ class FileImpl { } text(): Promise { - throw new Error('Not implemented!'); + const utf8Decoder = new TextDecoder(); + const text = utf8Decoder.decode(this.buffer); + return new Promise((resolve) => { + resolve(text); + }); } } diff --git a/tools/winscope/src/test/common/utils.ts b/tools/winscope/src/test/common/utils.ts index d7f1e8853..8c81a6394 100644 --- a/tools/winscope/src/test/common/utils.ts +++ b/tools/winscope/src/test/common/utils.ts @@ -18,9 +18,12 @@ import * as path from 'path'; import {FileImpl} from './file_impl'; class CommonTestUtils { - static async getFixtureFile(filename: string): Promise { - const buffer = CommonTestUtils.loadFixture(filename); - return new FileImpl(buffer, filename) as unknown as File; + static async getFixtureFile( + srcFilename: string, + dstFilename: string = srcFilename + ): Promise { + const buffer = CommonTestUtils.loadFixture(srcFilename); + return new FileImpl(buffer, dstFilename) as unknown as File; } static loadFixture(filename: string): ArrayBuffer { diff --git a/tools/winscope/src/test/fixtures/bugreports/bugreport-codename_beta-UPB2.230407.019-2023-05-30-14-33-48.txt b/tools/winscope/src/test/fixtures/bugreports/bugreport-codename_beta-UPB2.230407.019-2023-05-30-14-33-48.txt new file mode 100644 index 000000000..c02add504 --- /dev/null +++ b/tools/winscope/src/test/fixtures/bugreports/bugreport-codename_beta-UPB2.230407.019-2023-05-30-14-33-48.txt @@ -0,0 +1,6 @@ +======================================================== +== dumpstate: 2023-05-30 14:33:48 +======================================================== + +# ORIGINAL CONTENTS REMOVED TO AVOID INFORMATION LEAKS + diff --git a/tools/winscope/src/test/fixtures/bugreports/bugreport_stripped.zip b/tools/winscope/src/test/fixtures/bugreports/bugreport_stripped.zip index 36a2e7300..c91a59532 100644 Binary files a/tools/winscope/src/test/fixtures/bugreports/bugreport_stripped.zip and b/tools/winscope/src/test/fixtures/bugreports/bugreport_stripped.zip differ diff --git a/tools/winscope/src/test/fixtures/bugreports/main_entry.txt b/tools/winscope/src/test/fixtures/bugreports/main_entry.txt new file mode 100644 index 000000000..33992053a --- /dev/null +++ b/tools/winscope/src/test/fixtures/bugreports/main_entry.txt @@ -0,0 +1 @@ +bugreport-codename_beta-UPB2.230407.019-2023-05-30-14-33-48.txt diff --git a/tools/winscope/src/trace/trace_file.ts b/tools/winscope/src/trace/trace_file.ts index 4f2f6da16..0ae7a9801 100644 --- a/tools/winscope/src/trace/trace_file.ts +++ b/tools/winscope/src/trace/trace_file.ts @@ -14,11 +14,13 @@ * limitations under the License. */ +import {FileUtils} from 'common/file_utils'; + export class TraceFile { constructor(public file: File, public parentArchive?: File) {} getDescriptor(): string { - let descriptor = this.file.name; + let descriptor = FileUtils.removeDirFromFileName(this.file.name); if (this.parentArchive?.name !== undefined) { descriptor += ` (${this.parentArchive?.name})`; }