Fix handling of traces with no entries

Test: npm run build:unit && npm run test:unit
Change-Id: Ic715711c0f9971b8b6d5dfc7219e6c459e1a421e

tools/winscope-ng/karma.conf.js

@@ -13,8 +13,6 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-const glob = require("glob");
-
 let webpackConfig = require("./webpack.config.common");
 delete webpackConfig.entry;
 delete webpackConfig.output;

tools/winscope-ng/src/app/components/trace_view.component.spec.ts

@@ -18,31 +18,7 @@ import {CUSTOM_ELEMENTS_SCHEMA, NO_ERRORS_SCHEMA} from "@angular/core";
 import {ComponentFixture, TestBed} from "@angular/core/testing";
 import {MatCardModule} from "@angular/material/card";
 import {TraceViewComponent} from "./trace_view.component";
-import {View, Viewer, ViewType} from "viewers/viewer";
-import {content} from "html2canvas/dist/types/css/property-descriptors/content";
-
-class FakeViewer implements Viewer {
-  constructor(title: string, content: string) {
-    this.title = title;
-    this.htmlElement = document.createElement("div");
-    this.htmlElement.innerText = content;
-  }
-
-  notifyCurrentTraceEntries(entries: any) {
-    // do nothing
-  }
-
-  getViews(): View[] {
-    return [new View(ViewType.TAB, this.htmlElement, this.title)];
-  }
-
-  getDependencies(): any[] {
-    return [];
-  }
-
-  private htmlElement: HTMLElement;
-  private title: string;
-}
+import {ViewerStub} from "viewers/viewer_stub";
 
 describe("TraceViewComponent", () => {
   let fixture: ComponentFixture<TraceViewComponent>;
@@ -62,8 +38,8 @@ describe("TraceViewComponent", () => {
     htmlElement = fixture.nativeElement;
     component = fixture.componentInstance;
     component.viewers = [
-      new FakeViewer("Title0", "Content0"),
-      new FakeViewer("Title1", "Content1")
+      new ViewerStub("Title0", "Content0"),
+      new ViewerStub("Title1", "Content1")
     ];
     component.ngOnChanges();
     fixture.detectChanges();

tools/winscope-ng/src/app/trace_coordinator.spec.ts

@@ -13,9 +13,12 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-import { TraceCoordinator } from "./trace_coordinator";
-import { UnitTestUtils } from "test/unit/utils";
-import { TraceType } from "common/trace/trace_type";
+import {Timestamp, TimestampType} from "common/trace/timestamp";
+import {TraceType} from "common/trace/trace_type";
+import {TraceCoordinator} from "./trace_coordinator";
+import {UnitTestUtils} from "test/unit/utils";
+import {ViewerFactory} from "viewers/viewer_factory";
+import {ViewerStub} from "viewers/viewer_stub";
 
 describe("TraceCoordinator", () => {
   let traceCoordinator: TraceCoordinator;
@@ -24,7 +27,7 @@ describe("TraceCoordinator", () => {
     traceCoordinator = new TraceCoordinator();
   });
 
-  it("adds parsers from recognised traces", async () => {
+  it("processes trace files", async () => {
     expect(traceCoordinator.getParsers().length).toEqual(0);
     const traces = [
       await UnitTestUtils.getFixtureFile("traces/elapsed_and_real_timestamp/dump_SurfaceFlinger.pb"),
@@ -35,7 +38,7 @@ describe("TraceCoordinator", () => {
     expect(errors.length).toEqual(0);
   });
 
-  it("handles unrecognised file types added", async () => {
+  it("it is robust to invalid trace files", async () => {
     expect(traceCoordinator.getParsers().length).toEqual(0);
     const traces = [
       await UnitTestUtils.getFixtureFile("winscope_homepage.png"),
@@ -45,7 +48,18 @@ describe("TraceCoordinator", () => {
     expect(errors.length).toEqual(1);
   });
 
-  it("handles both recognised and unrecognised file types added", async () => {
+  it("is robust to trace files with no entries", async () => {
+    const traces = [
+      await UnitTestUtils.getFixtureFile(
+        "traces/no_entries_InputMethodClients.pb")
+    ];
+    await traceCoordinator.addTraces(traces);
+
+    const timestamp = new Timestamp(TimestampType.ELAPSED, 0n);
+    traceCoordinator.notifyCurrentTimestamp(timestamp);
+  });
+
+  it("processes mixed valid and invalid trace files", async () => {
     expect(traceCoordinator.getParsers().length).toEqual(0);
     const traces = [
       await UnitTestUtils.getFixtureFile("winscope_homepage.png"),
@@ -77,6 +91,7 @@ describe("TraceCoordinator", () => {
 
     const parser = traceCoordinator.findParser(TraceType.SURFACE_FLINGER);
     expect(parser).toBeTruthy();
+    expect(parser!.getTraceType()).toEqual(TraceType.SURFACE_FLINGER);
   });
 
   it("cannot find parser that does not exist", async () => {
@@ -94,4 +109,40 @@ describe("TraceCoordinator", () => {
     const timestamps = traceCoordinator.getTimestamps();
     expect(timestamps.length).toEqual(48);
   });
+
+  it("can create viewers and notify current trace entries", async () => {
+    const viewerStub = new ViewerStub("Title");
+
+    spyOn(ViewerFactory.prototype, "createViewers").and.returnValue([viewerStub]);
+    spyOn(viewerStub, "notifyCurrentTraceEntries");
+
+    const traces = [
+      await UnitTestUtils.getFixtureFile(
+        "traces/elapsed_and_real_timestamp/SurfaceFlinger.pb"),
+      await UnitTestUtils.getFixtureFile(
+        "traces/elapsed_and_real_timestamp/WindowManager.pb"),
+      // trace file with no entries for some more robustness checks
+      await UnitTestUtils.getFixtureFile(
+        "traces/no_entries_InputMethodClients.pb")
+    ];
+    await traceCoordinator.addTraces(traces);
+
+    // create viewers (mocked factory)
+    expect(traceCoordinator.getViewers()).toEqual([]);
+    traceCoordinator.createViewers();
+    expect(traceCoordinator.getViewers()).toEqual([viewerStub]);
+
+    // notify invalid timestamp
+    traceCoordinator.notifyCurrentTimestamp(undefined);
+    expect(viewerStub.notifyCurrentTraceEntries).toHaveBeenCalledTimes(0);
+
+    // notify timestamp
+    const timestamp = new Timestamp(TimestampType.ELAPSED, 14500282843n);
+    traceCoordinator.notifyCurrentTimestamp(timestamp);
+    expect(viewerStub.notifyCurrentTraceEntries).toHaveBeenCalledTimes(1);
+
+    // notify timestamp again
+    traceCoordinator.notifyCurrentTimestamp(timestamp);
+    expect(viewerStub.notifyCurrentTraceEntries).toHaveBeenCalledTimes(2);
+  });
 });

tools/winscope-ng/src/app/trace_coordinator.ts

@@ -13,6 +13,7 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
+import {ArrayUtils} from "common/utils/array_utils";
 import {Timestamp, TimestampType} from "common/trace/timestamp";
 import {TraceType} from "common/trace/trace_type";
 import {Parser} from "parsers/parser";
@@ -21,7 +22,6 @@ import { setTraces } from "trace_collection/set_traces";
 import { Viewer } from "viewers/viewer";
 import { ViewerFactory } from "viewers/viewer_factory";
 import { LoadedTrace } from "app/loaded_trace";
-import { TimestampUtils } from "common/trace/timestamp_utils";
 import { FileUtils } from "common/utils/file_utils";
 import { TRACE_INFO } from "app/trace_info";
 
@@ -98,7 +98,11 @@ class TraceCoordinator {
     throw new Error("Failed to create aggregated timestamps (any type)");
   }
 
-  public notifyCurrentTimestamp(timestamp: Timestamp) {
+  public notifyCurrentTimestamp(timestamp: Timestamp|undefined) {
+    if (!timestamp) {
+      return;
+    }
+
     const traceEntries: Map<TraceType, any> = new Map<TraceType, any>();
 
     this.parsers.forEach(parser => {
@@ -107,17 +111,18 @@ class TraceCoordinator {
       let prevEntry = null;
 
       const parserTimestamps = parser.getTimestamps(timestamp.getType());
-      if (parserTimestamps) {
-        const closestIndex = TimestampUtils.getClosestIndex(targetTimestamp, parserTimestamps);
-        if (closestIndex) {
-          prevEntry = parser.getTraceEntry(parserTimestamps[closestIndex-1]) ?? null;
-        }
+      if (parserTimestamps === undefined) {
+        throw new Error(`Unexpected timestamp type ${timestamp.getType()}.`
+          + ` Not supported by parser for trace type: ${parser.getTraceType()}`);
       }
+
+      const index = ArrayUtils.binarySearchLowerOrEqual(parserTimestamps, targetTimestamp);
+      if (index !== undefined && index > 0) {
+        prevEntry = parser.getTraceEntry(parserTimestamps[index-1]);
+      }
+
       if (entry !== undefined) {
         traceEntries.set(parser.getTraceType(), [entry, prevEntry]);
-      } else if (parserTimestamps) {
-        const firstEntry = parser.getTraceEntry(parserTimestamps[0]);
-        traceEntries.set(parser.getTraceType(), [firstEntry, prevEntry]);
       }
     });
 

tools/winscope-ng/src/common/trace/timestamp_utils.ts

@@ -1,26 +0,0 @@
-/*
- * 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 { ArrayUtils } from "common/utils/array_utils";
-import { Timestamp } from "common/trace/timestamp";
-
-export class TimestampUtils {
-  static getClosestIndex(targetTimestamp: Timestamp, timestamps: Timestamp[]) {
-    if (timestamps === undefined) {
-      throw TypeError(`Timestamps with type "${targetTimestamp.getType()}" not available`);
-    }
-    return ArrayUtils.binarySearchLowerOrEqual(timestamps, targetTimestamp);
-  }
-}

tools/winscope-ng/src/parsers/parser_common.spec.ts

@@ -18,6 +18,7 @@ import {Parser} from "./parser";
 import {CommonTestUtils} from "test/common/utils";
 import {UnitTestUtils} from "test/unit/utils";
 import {ParserFactory} from "./parser_factory";
+import {TraceType} from "../common/trace/trace_type";
 
 describe("Parser", () => {
   it("is robust to empty trace file", async () => {
@@ -26,6 +27,20 @@ describe("Parser", () => {
     expect(parsers.length).toEqual(0);
   });
 
+  it("is robust to trace with no entries", async () => {
+    const parser = await UnitTestUtils.getParser("traces/no_entries_InputMethodClients.pb");
+
+    expect(parser.getTraceType()).toEqual(TraceType.INPUT_METHOD_CLIENTS);
+    expect(parser.getTimestamps(TimestampType.ELAPSED)).toEqual([]);
+    expect(parser.getTimestamps(TimestampType.REAL)).toEqual([]);
+
+    const timestampElapsed = new Timestamp(TimestampType.ELAPSED, 0n);
+    expect(parser.getTraceEntry(timestampElapsed)).toBeUndefined();
+
+    const timestampReal = new Timestamp(TimestampType.REAL, 0n);
+    expect(parser.getTraceEntry(timestampReal)).toBeUndefined();
+  });
+
   describe("real timestamp", () => {
     let parser: Parser;
 

tools/winscope-ng/src/test/fixtures/traces/no_entries_InputMethodClients.pb

@@ -0,0 +1 @@
+	IMCTRACE<&<15><><EFBFBD><EFBFBD><EFBFBD>

tools/winscope-ng/src/viewers/viewer_stub.ts

@@ -0,0 +1,46 @@
+/*
+ * 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 {Viewer, View, ViewType} from "./viewer";
+
+class ViewerStub implements Viewer {
+  constructor(title: string, viewContent?: string) {
+    this.title = title;
+
+    if (viewContent !== undefined) {
+      this.htmlElement = document.createElement("div");
+      this.htmlElement.innerText = viewContent;
+    } else {
+      this.htmlElement = undefined as unknown as HTMLElement;
+    }
+  }
+
+  notifyCurrentTraceEntries(entries: any) {
+    // do nothing
+  }
+
+  getViews(): View[] {
+    return [new View(ViewType.TAB, this.htmlElement, this.title)];
+  }
+
+  getDependencies(): any[] {
+    return [];
+  }
+
+  private htmlElement: HTMLElement;
+  private title: string;
+}
+
+export {ViewerStub};