Optimize bugreport loading
Fix: b/269342535
Test: npm run test:all
Change-Id: I89e03fff08fe1c7e7169a6eeb08efeda04d098d0
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 a8aba83..787cc2e 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 @@
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 @@
});
const files = await Promise.all(filesBlobPromises);
- await this.onttachmentsDownloaded(files);
+ await this.onAttachmentsDownloaded(files);
}
private isOpenBuganizerResponseMessage(
diff --git a/tools/winscope/src/app/mediator.ts b/tools/winscope/src/app/mediator.ts
index 3544a2c..60ee7ea 100644
--- a/tools/winscope/src/app/mediator.ts
+++ b/tools/winscope/src/app/mediator.ts
@@ -83,7 +83,7 @@
}
);
- this.crossToolProtocol.setOnTimestampReceived(async (timestamp: Timestamp) => {
+ this.crossToolProtocol.setOnTimestampReceived((timestamp: Timestamp) => {
this.onRemoteTimestampReceived(timestamp);
});
@@ -203,7 +203,7 @@
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 8ab16b7..cad4c61 100644
--- a/tools/winscope/src/app/trace_pipeline.ts
+++ b/tools/winscope/src/app/trace_pipeline.ts
@@ -37,6 +37,7 @@
traceFiles: TraceFile[],
onLoadProgressUpdate: OnProgressUpdateType = FunctionUtils.DO_NOTHING
): Promise<ParserError[]> {
+ traceFiles = await this.filterBugreportFilesIfNeeded(traceFiles);
const [parsers, parserErrors] = await this.parserFactory.createParsers(
traceFiles,
onLoadProgressUpdate
@@ -111,6 +112,36 @@
this.files = new Map<TraceType, TraceFile>();
}
+ private async filterBugreportFilesIfNeeded(files: TraceFile[]): Promise<TraceFile[]> {
+ 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 20e01a0..46c6701 100644
--- a/tools/winscope/src/app/trace_pipeline_test.ts
+++ b/tools/winscope/src/app/trace_pipeline_test.ts
@@ -39,6 +39,74 @@
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 447e043..ac2b688 100644
--- a/tools/winscope/src/common/file_utils.ts
+++ b/tools/winscope/src/common/file_utils.ts
@@ -61,9 +61,8 @@
// 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 876c3c1..42d6cf1 100644
--- a/tools/winscope/src/common/time_utils.ts
+++ b/tools/winscope/src/common/time_utils.ts
@@ -128,7 +128,6 @@
}
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 8ad842b..315bbe1 100644
--- a/tools/winscope/src/test/common/file_impl.ts
+++ b/tools/winscope/src/test/common/file_impl.ts
@@ -46,7 +46,11 @@
}
text(): Promise<string> {
- throw new Error('Not implemented!');
+ const utf8Decoder = new TextDecoder();
+ const text = utf8Decoder.decode(this.buffer);
+ return new Promise<string>((resolve) => {
+ resolve(text);
+ });
}
}
diff --git a/tools/winscope/src/test/common/utils.ts b/tools/winscope/src/test/common/utils.ts
index d7f1e88..8c81a63 100644
--- a/tools/winscope/src/test/common/utils.ts
+++ b/tools/winscope/src/test/common/utils.ts
@@ -18,9 +18,12 @@
import {FileImpl} from './file_impl';
class CommonTestUtils {
- static async getFixtureFile(filename: string): Promise<File> {
- const buffer = CommonTestUtils.loadFixture(filename);
- return new FileImpl(buffer, filename) as unknown as File;
+ static async getFixtureFile(
+ srcFilename: string,
+ dstFilename: string = srcFilename
+ ): Promise<File> {
+ 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 0000000..c02add5
--- /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 36a2e73..c91a595 100644
--- a/tools/winscope/src/test/fixtures/bugreports/bugreport_stripped.zip
+++ b/tools/winscope/src/test/fixtures/bugreports/bugreport_stripped.zip
Binary files 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 0000000..3399205
--- /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 4f2f6da..0ae7a98 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})`;
}
