From e108cf70d31ac1233326a13ecd1cf6b1ad74d39a Mon Sep 17 00:00:00 2001 From: Kean Mariotti Date: Tue, 20 Dec 2022 13:44:16 +0000 Subject: [PATCH 1/2] Show progress bar while downloading from ABT chrome extension Test: npm run build:all && npm run test:all Change-Id: I47e6dc888a09519fb39ecbdac75930288899f21f --- .../abt_chrome_extension_protocol.ts | 11 ++- ...extension_protocol_dependency_inversion.ts | 2 + .../abt_chrome_extension_protocol_stub.ts | 11 ++- .../src/app/components/app.component.ts | 6 +- .../app/components/load_progress.component.ts | 8 ++- .../parser_error_snack_bar_component.ts | 24 ++++--- .../app/components/upload_traces.component.ts | 68 +++++++++++-------- ...d_traces_component_dependency_inversion.ts | 1 + .../upload_traces_component_stub.ts | 27 ++++++++ tools/winscope-ng/src/app/mediator.spec.ts | 22 ++++++ tools/winscope-ng/src/app/mediator.ts | 10 ++- .../winscope-ng/src/parsers/parser_factory.ts | 17 +++-- 12 files changed, 158 insertions(+), 49 deletions(-) create mode 100644 tools/winscope-ng/src/app/components/upload_traces_component_stub.ts diff --git a/tools/winscope-ng/src/abt_chrome_extension/abt_chrome_extension_protocol.ts b/tools/winscope-ng/src/abt_chrome_extension/abt_chrome_extension_protocol.ts index d33e21518..05ab9454d 100644 --- a/tools/winscope-ng/src/abt_chrome_extension/abt_chrome_extension_protocol.ts +++ b/tools/winscope-ng/src/abt_chrome_extension/abt_chrome_extension_protocol.ts @@ -16,6 +16,7 @@ import { AbtChromeExtensionProtocolDependencyInversion, + OnBugAttachmentsDownloadStart, OnBugAttachmentsReceived } from "./abt_chrome_extension_protocol_dependency_inversion"; import { @@ -27,7 +28,12 @@ import {FunctionUtils} from "common/utils/function_utils"; export class AbtChromeExtensionProtocol implements AbtChromeExtensionProtocolDependencyInversion { static readonly ABT_EXTENSION_ID = "mbbaofdfoekifkfpgehgffcpagbbjkmj"; - onBugAttachmentsReceived: OnBugAttachmentsReceived = FunctionUtils.DO_NOTHING_ASYNC; + private onBugAttachmentsDownloadStart: OnBugAttachmentsDownloadStart = FunctionUtils.DO_NOTHING; + private onBugAttachmentsReceived: OnBugAttachmentsReceived = FunctionUtils.DO_NOTHING_ASYNC; + + setOnBugAttachmentsDownloadStart(callback: OnBugAttachmentsDownloadStart) { + this.onBugAttachmentsDownloadStart = callback; + } setOnBugAttachmentsReceived(callback: OnBugAttachmentsReceived) { this.onBugAttachmentsReceived = callback; @@ -39,6 +45,8 @@ export class AbtChromeExtensionProtocol implements AbtChromeExtensionProtocolDep return; } + this.onBugAttachmentsDownloadStart(); + const openRequestMessage: OpenRequest = { action: MessageType.OPEN_REQUEST }; @@ -65,7 +73,6 @@ export class AbtChromeExtensionProtocol implements AbtChromeExtensionProtocolDep if (message.attachments.length === 0) { console.warn("ABT chrome extension protocol received no attachments"); - return; } const filesBlobPromises = message.attachments.map(async attachment => { diff --git a/tools/winscope-ng/src/abt_chrome_extension/abt_chrome_extension_protocol_dependency_inversion.ts b/tools/winscope-ng/src/abt_chrome_extension/abt_chrome_extension_protocol_dependency_inversion.ts index 4d1d363b3..790479d7c 100644 --- a/tools/winscope-ng/src/abt_chrome_extension/abt_chrome_extension_protocol_dependency_inversion.ts +++ b/tools/winscope-ng/src/abt_chrome_extension/abt_chrome_extension_protocol_dependency_inversion.ts @@ -14,9 +14,11 @@ * limitations under the License. */ +export type OnBugAttachmentsDownloadStart = () => void; export type OnBugAttachmentsReceived = (attachments: File[]) => Promise; export interface AbtChromeExtensionProtocolDependencyInversion { + setOnBugAttachmentsDownloadStart(callback: OnBugAttachmentsDownloadStart): void; setOnBugAttachmentsReceived(callback: OnBugAttachmentsReceived): void; run(): void; } diff --git a/tools/winscope-ng/src/abt_chrome_extension/abt_chrome_extension_protocol_stub.ts b/tools/winscope-ng/src/abt_chrome_extension/abt_chrome_extension_protocol_stub.ts index 9f60b7f50..390752ecd 100644 --- a/tools/winscope-ng/src/abt_chrome_extension/abt_chrome_extension_protocol_stub.ts +++ b/tools/winscope-ng/src/abt_chrome_extension/abt_chrome_extension_protocol_stub.ts @@ -15,13 +15,22 @@ */ import { + OnBugAttachmentsDownloadStart, OnBugAttachmentsReceived, AbtChromeExtensionProtocolDependencyInversion } from "./abt_chrome_extension_protocol_dependency_inversion"; +import {FunctionUtils} from "../common/utils/function_utils"; export class AbtChromeExtensionProtocolStub implements AbtChromeExtensionProtocolDependencyInversion { + onBugAttachmentsDownloadStart: OnBugAttachmentsDownloadStart = FunctionUtils.DO_NOTHING; + onBugAttachmentsReceived: OnBugAttachmentsReceived = FunctionUtils.DO_NOTHING_ASYNC; + + setOnBugAttachmentsDownloadStart(callback: OnBugAttachmentsDownloadStart) { + this.onBugAttachmentsDownloadStart = callback; + } + setOnBugAttachmentsReceived(callback: OnBugAttachmentsReceived) { - // do nothing + this.onBugAttachmentsReceived = callback; } run() { diff --git a/tools/winscope-ng/src/app/components/app.component.ts b/tools/winscope-ng/src/app/components/app.component.ts index a0809a209..870147f3b 100644 --- a/tools/winscope-ng/src/app/components/app.component.ts +++ b/tools/winscope-ng/src/app/components/app.component.ts @@ -251,8 +251,12 @@ export class AppComponent implements AppComponentDependencyInversion { TracingConfig.getInstance().initialize(localStorage); } - ngAfterViewChecked() { + ngAfterViewInit() { this.mediator.setUploadTracesComponent(this.uploadTracesComponent); + this.mediator.onWinscopeInitialized(); + } + + ngAfterViewChecked() { this.mediator.setTimelineComponent(this.timelineComponent); } diff --git a/tools/winscope-ng/src/app/components/load_progress.component.ts b/tools/winscope-ng/src/app/components/load_progress.component.ts index f4909eb7d..befc787d9 100644 --- a/tools/winscope-ng/src/app/components/load_progress.component.ts +++ b/tools/winscope-ng/src/app/components/load_progress.component.ts @@ -25,7 +25,11 @@ import {Component, Input} from "@angular/core";

- + + @@ -60,6 +64,6 @@ import {Component, Input} from "@angular/core"; ] }) export class LoadProgressComponent { - @Input() progressPercentage = 0; + @Input() progressPercentage?: number; @Input() message = "Loading..."; } diff --git a/tools/winscope-ng/src/app/components/parser_error_snack_bar_component.ts b/tools/winscope-ng/src/app/components/parser_error_snack_bar_component.ts index e1a1e3cfd..7b3f1b606 100644 --- a/tools/winscope-ng/src/app/components/parser_error_snack_bar_component.ts +++ b/tools/winscope-ng/src/app/components/parser_error_snack_bar_component.ts @@ -15,8 +15,8 @@ */ import {Component, Inject, NgZone} from "@angular/core"; import {MAT_SNACK_BAR_DATA, MatSnackBar, MatSnackBarRef} from "@angular/material/snack-bar"; -import {TRACE_INFO} from "app/trace_info"; import {ParserError, ParserErrorType} from "parsers/parser_factory"; +import {TRACE_INFO} from "app/trace_info"; @Component({ selector: "upload-snack-bar", @@ -89,14 +89,22 @@ export class ParserErrorSnackBarComponent { } private static convertErrorToMessage(error: ParserError): string { - let message = `${error.trace.name}: unknown error occurred`; - if (error.type === ParserErrorType.OVERRIDE && error.traceType) { - message = `${error.trace.name}:` + - ` overridden by another trace of type ${TRACE_INFO[error.traceType].name}`; - } else if (error.type === ParserErrorType.UNSUPPORTED_FORMAT) { - message = `${error.trace.name}: unsupported file format`; + const fileName = error.trace !== undefined ? + error.trace.name : ""; + const traceTypeName = error.traceType !== undefined ? + TRACE_INFO[error.traceType].name : ""; + + switch (error.type) { + case ParserErrorType.NO_INPUT_FILES: + return "No input files"; + case ParserErrorType.UNSUPPORTED_FORMAT: + return `${fileName}: unsupported file format`; + case ParserErrorType.OVERRIDE: { + return `${fileName}: overridden by another trace of type ${traceTypeName}`; + } + default: + return `${fileName}: unknown error occurred`; } - return message; } private static makeCroppedMessage(type: ParserErrorType, count: number): string { diff --git a/tools/winscope-ng/src/app/components/upload_traces.component.ts b/tools/winscope-ng/src/app/components/upload_traces.component.ts index 2d1b8b8cb..55e195dc2 100644 --- a/tools/winscope-ng/src/app/components/upload_traces.component.ts +++ b/tools/winscope-ng/src/app/components/upload_traces.component.ts @@ -37,29 +37,30 @@ import {ParserErrorSnackBarComponent} from "./parser_error_snack_bar_component"; Upload Traces + [progressPercentage]="progresPercentage" + [message]="progressMessage"> - + {{TRACE_INFO[trace.type].icon}} @@ -69,13 +70,15 @@ import {ParserErrorSnackBarComponent} from "./parser_error_snack_bar_component"; {{trace.file.name}} ({{TRACE_INFO[trace.type].name}})

- -
+

@@ -85,16 +88,20 @@ import {ParserErrorSnackBarComponent} from "./parser_error_snack_bar_component";
-
- - -
@@ -170,8 +177,8 @@ import {ParserErrorSnackBarComponent} from "./parser_error_snack_bar_component"; export class UploadTracesComponent implements UploadTracesComponentDependencyInversion { TRACE_INFO = TRACE_INFO; isLoadingFiles = false; - loadFilesMessage = "Unzipping files..."; - loadFilesProgress = 0; + progressMessage = ""; + progresPercentage?: number; @Input() traceData!: TraceData; @Output() traceDataLoaded = new EventEmitter(); @@ -187,6 +194,13 @@ export class UploadTracesComponent implements UploadTracesComponentDependencyInv this.traceData.clear(); } + public onFilesDownloadStart() { + this.isLoadingFiles = true; + this.progressMessage = "Downloading files..."; + this.progresPercentage = undefined; + this.changeDetectorRef.detectChanges(); + } + public async onInputFiles(event: Event) { const files = this.getInputFiles(event); await this.processFiles(files); @@ -205,16 +219,16 @@ export class UploadTracesComponent implements UploadTracesComponentDependencyInv } lastUiProgressUpdate = now; - this.loadFilesProgress = progress; + this.progresPercentage = progress; this.changeDetectorRef.detectChanges(); }; this.isLoadingFiles = true; - this.loadFilesMessage = "Unzipping files..."; + this.progressMessage = "Unzipping files..."; this.changeDetectorRef.detectChanges(); const unzippedFiles = await FileUtils.unzipFilesIfNeeded(files, onProgressUpdate); - this.loadFilesMessage = "Parsing files..."; + this.progressMessage = "Parsing files..."; this.changeDetectorRef.detectChanges(); const parserErrors = await this.traceData.loadTraces(unzippedFiles, onProgressUpdate); diff --git a/tools/winscope-ng/src/app/components/upload_traces_component_dependency_inversion.ts b/tools/winscope-ng/src/app/components/upload_traces_component_dependency_inversion.ts index 17ff24576..d4f824908 100644 --- a/tools/winscope-ng/src/app/components/upload_traces_component_dependency_inversion.ts +++ b/tools/winscope-ng/src/app/components/upload_traces_component_dependency_inversion.ts @@ -15,5 +15,6 @@ */ export interface UploadTracesComponentDependencyInversion { + onFilesDownloadStart(): void; processFiles(files: File[]): Promise; } diff --git a/tools/winscope-ng/src/app/components/upload_traces_component_stub.ts b/tools/winscope-ng/src/app/components/upload_traces_component_stub.ts new file mode 100644 index 000000000..cea484dc4 --- /dev/null +++ b/tools/winscope-ng/src/app/components/upload_traces_component_stub.ts @@ -0,0 +1,27 @@ +/* + * Copyright (C) 2022 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import {UploadTracesComponentDependencyInversion} from "./upload_traces_component_dependency_inversion"; + +export class UploadTracesComponentStub implements UploadTracesComponentDependencyInversion { + onFilesDownloadStart() { + // do nothing + } + + async processFiles(files: File[]) { + // do nothing + } +} diff --git a/tools/winscope-ng/src/app/mediator.spec.ts b/tools/winscope-ng/src/app/mediator.spec.ts index d530fc8d5..979d772b6 100644 --- a/tools/winscope-ng/src/app/mediator.spec.ts +++ b/tools/winscope-ng/src/app/mediator.spec.ts @@ -16,6 +16,7 @@ import {AppComponentStub} from "./components/app_component_stub"; import {TimelineComponentStub} from "./components/timeline/timeline_component_stub"; +import {UploadTracesComponentStub} from "./components/upload_traces_component_stub"; import {Mediator} from "./mediator"; import {AbtChromeExtensionProtocolStub} from "abt_chrome_extension/abt_chrome_extension_protocol_stub"; import {CrossToolProtocolStub} from "cross_tool/cross_tool_protocol_stub"; @@ -35,6 +36,7 @@ describe("Mediator", () => { let crossToolProtocol: CrossToolProtocolStub; let appComponent: AppComponentStub; let timelineComponent: TimelineComponentStub; + let uploadTracesComponent: UploadTracesComponentStub; let mediator: Mediator; const TIMESTAMP_10 = new RealTimestamp(10n); @@ -48,6 +50,7 @@ describe("Mediator", () => { crossToolProtocol = new CrossToolProtocolStub(); appComponent = new AppComponentStub(); timelineComponent = new TimelineComponentStub(); + uploadTracesComponent = new UploadTracesComponentStub(); mediator = new Mediator( traceData, timelineData, @@ -57,6 +60,7 @@ describe("Mediator", () => { new MockStorage() ); mediator.setTimelineComponent(timelineComponent); + mediator.setUploadTracesComponent(uploadTracesComponent); spyOn(ViewerFactory.prototype, "createViewers").and.returnValue([viewerStub]); }); @@ -84,6 +88,24 @@ describe("Mediator", () => { //TODO: test "data from ABT chrome extension" when FileUtils is fully compatible with Node.js // (b/262269229). + it("handles start download event from ABT chrome extension", () => { + spyOn(uploadTracesComponent, "onFilesDownloadStart"); + expect(uploadTracesComponent.onFilesDownloadStart).toHaveBeenCalledTimes(0); + + abtChromeExtensionProtocol.onBugAttachmentsDownloadStart(); + expect(uploadTracesComponent.onFilesDownloadStart).toHaveBeenCalledTimes(1); + }); + + it("handles empty downloaded files from ABT chrome extension", async () => { + spyOn(uploadTracesComponent, "processFiles"); + expect(uploadTracesComponent.processFiles).toHaveBeenCalledTimes(0); + + // Pass files even if empty so that the upload component will update the progress bar + // and display error messages + await abtChromeExtensionProtocol.onBugAttachmentsReceived([]); + expect(uploadTracesComponent.processFiles).toHaveBeenCalledTimes(1); + }); + it("propagates current timestamp changed through timeline", async () => { await loadTraces(); mediator.onWinscopeTraceDataLoaded(); diff --git a/tools/winscope-ng/src/app/mediator.ts b/tools/winscope-ng/src/app/mediator.ts index a66a37655..19a575de8 100644 --- a/tools/winscope-ng/src/app/mediator.ts +++ b/tools/winscope-ng/src/app/mediator.ts @@ -74,7 +74,11 @@ export class Mediator { this.abtChromeExtensionProtocol.setOnBugAttachmentsReceived(async (attachments: File[]) => { await this.onAbtChromeExtensionBugAttachmentsReceived(attachments); }); - this.abtChromeExtensionProtocol.run(); + + this.abtChromeExtensionProtocol.setOnBugAttachmentsDownloadStart(() => { + console.log("Mediator notifying onFilesDownloadStart()"); + this.uploadTracesComponent?.onFilesDownloadStart(); + }); } public setUploadTracesComponent( @@ -87,6 +91,10 @@ export class Mediator { this.timelineComponent = timelineComponent; } + public onWinscopeInitialized() { + this.abtChromeExtensionProtocol.run(); + } + public onWinscopeTraceDataLoaded() { this.processTraceData(); } diff --git a/tools/winscope-ng/src/parsers/parser_factory.ts b/tools/winscope-ng/src/parsers/parser_factory.ts index c615c35f6..e824e38de 100644 --- a/tools/winscope-ng/src/parsers/parser_factory.ts +++ b/tools/winscope-ng/src/parsers/parser_factory.ts @@ -29,7 +29,7 @@ import {ParserTransactions} from "./parser_transactions"; import {ParserWindowManager} from "./parser_window_manager"; import {ParserWindowManagerDump} from "./parser_window_manager_dump"; -class ParserFactory { +export class ParserFactory { static readonly PARSERS = [ ParserAccessibility, ParserInputMethodClients, @@ -52,6 +52,10 @@ class ParserFactory { Promise<[Parser[], ParserError[]]> { const errors: ParserError[] = []; + if (traces.length === 0) { + errors.push(new ParserError(ParserErrorType.NO_INPUT_FILES)); + } + for (const [index, trace] of traces.entries()) { let hasFoundParser = false; @@ -72,7 +76,7 @@ class ParserFactory { if (!hasFoundParser) { console.log(`Failed to load trace ${trace.name}`); - errors.push(new ParserError(trace, ParserErrorType.UNSUPPORTED_FORMAT)); + errors.push(new ParserError(ParserErrorType.UNSUPPORTED_FORMAT, trace)); } onProgressUpdate(100 * (index + 1) / traces.length); @@ -95,7 +99,7 @@ class ParserFactory { ); errors.push( new ParserError( - oldParser.getTrace().file, ParserErrorType.OVERRIDE, oldParser.getTraceType() + ParserErrorType.OVERRIDE, oldParser.getTrace().file, oldParser.getTraceType() ) ); return true; @@ -107,7 +111,7 @@ class ParserFactory { ); errors.push( new ParserError( - newParser.getTrace().file, ParserErrorType.OVERRIDE, newParser.getTraceType() + ParserErrorType.OVERRIDE, newParser.getTrace().file, newParser.getTraceType() ) ); return false; @@ -115,16 +119,15 @@ class ParserFactory { } export enum ParserErrorType { + NO_INPUT_FILES, UNSUPPORTED_FORMAT, OVERRIDE } export class ParserError { constructor( - public trace: File, public type: ParserErrorType, + public trace: File|undefined = undefined, public traceType: TraceType|undefined = undefined) { } } - -export {ParserFactory}; From b31d38d1812d803ef3fd3076e96217da58b13ace Mon Sep 17 00:00:00 2001 From: Kean Mariotti Date: Tue, 20 Dec 2022 17:07:16 +0000 Subject: [PATCH 2/2] Add link to legacy Winscope Test: open winscope, click on "Open legacy Wiscope", check that legacy winscope is opened Change-Id: I7dddbb107b20d07309a5033a7bf94d82f952afbd --- tools/winscope-ng/src/app/components/app.component.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tools/winscope-ng/src/app/components/app.component.ts b/tools/winscope-ng/src/app/components/app.component.ts index 870147f3b..d765aebb6 100644 --- a/tools/winscope-ng/src/app/components/app.component.ts +++ b/tools/winscope-ng/src/app/components/app.component.ts @@ -52,6 +52,12 @@ import {UploadTracesComponent} from "./upload_traces.component"; Winscope + + + +