From 357b30e6bdf377ea7bdb2278d0e3a160aeb27683 Mon Sep 17 00:00:00 2001
From: Kean Mariotti <keanmariotti@google.com>
Date: Fri, 23 Dec 2022 08:40:23 +0000
Subject: [PATCH] Remove legacy data and properties view from layer trace root

- Use OnPush Angular change detection strategy in SF viewer (better perf and avoids change loop errors)
- Hide PropertyGroupsComponent when root layer is selected
- Add formatting for "raw" long numbers

Fix: b/254044321
Test: npm run build:all && npm run test:all
Change-Id: I56274973a8d42c28e49e4a4a92fd6ae09a6693db
---
 .../trace/flickerlib/ObjectFormatter.ts       | 11 +++++++
 .../flickerlib/layers/LayerTraceEntry.ts      |  6 ++--
 tools/winscope-ng/src/styles.css              |  4 ---
 .../components/properties.component.ts        | 14 ++++++---
 .../src/viewers/components/tree.component.ts  | 22 +++++++-------
 .../viewer_surface_flinger/presenter.ts       | 30 ++++++++++++++-----
 .../viewers/viewer_surface_flinger/ui_data.ts |  1 +
 .../viewer_surface_flinger.component.ts       |  6 ++--
 .../viewer_surface_flinger.ts                 | 18 +++++------
 .../viewer_window_manager.component.ts        |  2 +-
 10 files changed, 71 insertions(+), 43 deletions(-)

diff --git a/tools/winscope-ng/src/common/trace/flickerlib/ObjectFormatter.ts b/tools/winscope-ng/src/common/trace/flickerlib/ObjectFormatter.ts
index 31d7578d2..1ac19e66b 100644
--- a/tools/winscope-ng/src/common/trace/flickerlib/ObjectFormatter.ts
+++ b/tools/winscope-ng/src/common/trace/flickerlib/ObjectFormatter.ts
@@ -18,6 +18,7 @@ import {
     toSize, toActiveBuffer, toColor, toColor3, toPoint, toPointF, toRect,
     toRectF, toRegion, toMatrix22, toTransform, toInsets
 } from './common';
+import {ArrayUtils} from "common/utils/array_utils";
 import { PropertiesDump } from "viewers/common/ui_tree_utils";
 import intDefMapping from
     '../../../../../../../prebuilts/misc/common/winscope/intDefMapping.json';
@@ -176,6 +177,16 @@ export default class ObjectFormatter {
             }
         }
 
+        // Raw long number (no type name, no constructor name, no useful toString() method)
+        if (ArrayUtils.equal(Object.keys(obj).sort(), ["high_", "low_"])) {
+            const high = BigInt(obj.high_) << 32n;
+            let low = BigInt(obj.low_);
+            if (low < 0) {
+                low = -low;
+            }
+            return (high | low).toString();
+        }
+
         return null;
     }
 
diff --git a/tools/winscope-ng/src/common/trace/flickerlib/layers/LayerTraceEntry.ts b/tools/winscope-ng/src/common/trace/flickerlib/layers/LayerTraceEntry.ts
index fd60f3901..1a6d61eab 100644
--- a/tools/winscope-ng/src/common/trace/flickerlib/layers/LayerTraceEntry.ts
+++ b/tools/winscope-ng/src/common/trace/flickerlib/layers/LayerTraceEntry.ts
@@ -21,8 +21,8 @@ import { TimeUtils } from "common/utils/time_utils";
 import { ElapsedTimestamp, RealTimestamp } from "common/trace/timestamp";
 
 LayerTraceEntry.fromProto = function (
-    protos: any[],
-    displayProtos: any[],
+    protos: object[],
+    displayProtos: object[],
     elapsedTimestamp: bigint,
     vSyncId: number,
     hwcBlob: string,
@@ -49,7 +49,7 @@ LayerTraceEntry.fromProto = function (
     return entry;
 }
 
-function addAttributes(entry: LayerTraceEntry, protos: any, useElapsedTime = false) {
+function addAttributes(entry: LayerTraceEntry, protos: object[], useElapsedTime = false) {
     entry.kind = "entry";
     // Avoid parsing the entry root because it is an array of layers
     // containing all trace information, this slows down the property tree.
diff --git a/tools/winscope-ng/src/styles.css b/tools/winscope-ng/src/styles.css
index 2b49ea3b7..fadb05262 100644
--- a/tools/winscope-ng/src/styles.css
+++ b/tools/winscope-ng/src/styles.css
@@ -39,7 +39,3 @@ app-root {
     flex-direction: row;
     overflow: auto;
 }
-
-viewer-surface-flinger .properties-view .view-header {
-    flex: 3;
-}
diff --git a/tools/winscope-ng/src/viewers/components/properties.component.ts b/tools/winscope-ng/src/viewers/components/properties.component.ts
index 1a7cfec87..ced3b6271 100644
--- a/tools/winscope-ng/src/viewers/components/properties.component.ts
+++ b/tools/winscope-ng/src/viewers/components/properties.component.ts
@@ -22,7 +22,8 @@ import { PropertiesTreeNode, Terminal} from "viewers/common/ui_tree_utils";
 @Component({
   selector: "properties-view",
   template: `
-    <div class="view-header">
+    <div class="view-header"
+         [class.view-header-with-property-groups]="displayPropertyGroups">
       <div class="title-filter">
         <h2 class="properties-title mat-title">Properties</h2>
 
@@ -49,7 +50,7 @@ import { PropertiesTreeNode, Terminal} from "viewers/common/ui_tree_utils";
       </div>
 
       <property-groups
-        *ngIf="itemIsSelected() && propertyGroups"
+        *ngIf="itemIsSelected() && displayPropertyGroups"
         class="property-groups"
         [item]="selectedFlickerItem"
       ></property-groups>
@@ -58,7 +59,8 @@ import { PropertiesTreeNode, Terminal} from "viewers/common/ui_tree_utils";
     <mat-divider></mat-divider>
 
     <div class="properties-content">
-      <h3 *ngIf="objectKeys(propertiesTree).length > 0 && isProtoDump" class="properties-title mat-subheading-2">Properties - Proto Dump</h3>
+      <h3 *ngIf="objectKeys(propertiesTree).length > 0 && isProtoDump"
+          class="properties-title mat-subheading-2">Properties - Proto Dump</h3>
 
       <div class="tree-wrapper">
         <tree-view
@@ -80,6 +82,10 @@ import { PropertiesTreeNode, Terminal} from "viewers/common/ui_tree_utils";
         margin-bottom: 12px;
       }
 
+      .view-header-with-property-groups {
+        flex: 3;
+      }
+
       .title-filter {
         display: flex;
         flex-direction: row;
@@ -121,7 +127,7 @@ export class PropertiesComponent {
   @Input() userOptions: UserOptions = {};
   @Input() propertiesTree: PropertiesTreeNode = {};
   @Input() selectedFlickerItem: TraceTreeNode | null = null;
-  @Input() propertyGroups = false;
+  @Input() displayPropertyGroups = false;
   @Input() isProtoDump = false;
 
   constructor(
diff --git a/tools/winscope-ng/src/viewers/components/tree.component.ts b/tools/winscope-ng/src/viewers/components/tree.component.ts
index 0f41214ba..7136c4e2a 100644
--- a/tools/winscope-ng/src/viewers/components/tree.component.ts
+++ b/tools/winscope-ng/src/viewers/components/tree.component.ts
@@ -13,10 +13,10 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-import { Component, Inject, Input, Output, ElementRef, EventEmitter, ChangeDetectionStrategy } from "@angular/core";
+import {Component, Inject, Input, Output, ElementRef, EventEmitter, ChangeDetectionStrategy} from "@angular/core";
 import { PersistentStore } from "common/utils/persistent_store";
 import { nodeStyles, treeNodeDataViewStyles } from "viewers/components/styles/node.styles";
-import { UiTreeUtils, UiTreeNode, HierarchyTreeNode, PropertiesTreeNode } from "viewers/common/ui_tree_utils";
+import { UiTreeUtils, UiTreeNode, HierarchyTreeNode } from "viewers/common/ui_tree_utils";
 import { TraceType } from "common/trace/trace_type";
 
 @Component({
@@ -76,8 +76,12 @@ export class TreeComponent {
   diffClass = UiTreeUtils.diffClass;
   isHighlighted = UiTreeUtils.isHighlighted;
 
-  @Input() item?: UiTreeNode;
+  // TODO (b/263779536): this array is passed down from viewers/presenters and is used to generate
+  //  an identifier supposed to be unique for each viewer. Let's just use a proper identifier
+  //  instead. Each viewer/presenter could pass down a random magic number, an UUID, ...
   @Input() dependencies: Array<TraceType> = [];
+
+  @Input() item?: UiTreeNode;
   @Input() store!: PersistentStore;
   @Input() isFlattened? = false;
   @Input() initialDepth = 0;
@@ -117,7 +121,7 @@ export class TreeComponent {
   }
 
   constructor(
-    @Inject(ElementRef) public elementRef: ElementRef,
+    @Inject(ElementRef) public elementRef: ElementRef
   ) {
     this.nodeElement = elementRef.nativeElement.querySelector(".node");
     this.nodeElement?.addEventListener("mousedown", this.nodeMouseDownEventListener);
@@ -168,18 +172,14 @@ export class TreeComponent {
   }
 
   private updateHighlightedItems() {
-    if (this.item instanceof HierarchyTreeNode) {
-      if (this.item.stableId) {
-        this.highlightedItemChange.emit(`${this.item.stableId}`);
-      } else if (!this.item.stableId) {
-        //this.selectedTreeChange.emit(this.item);
-      }
+    if (this.item?.stableId) {
+      this.highlightedItemChange.emit(`${this.item.stableId}`);
     }
   }
 
   public isPinned() {
     if (this.item instanceof HierarchyTreeNode) {
-      return this.pinnedItems?.map(item => `${item.id}`).includes(`${this.item.id}`);
+      return this.pinnedItems?.map(item => `${item.stableId}`).includes(`${this.item.stableId}`);
     }
     return false;
   }
diff --git a/tools/winscope-ng/src/viewers/viewer_surface_flinger/presenter.ts b/tools/winscope-ng/src/viewers/viewer_surface_flinger/presenter.ts
index 02cb00f3b..d3226de72 100644
--- a/tools/winscope-ng/src/viewers/viewer_surface_flinger/presenter.ts
+++ b/tools/winscope-ng/src/viewers/viewer_surface_flinger/presenter.ts
@@ -1,4 +1,3 @@
-
 /*
  * Copyright (C) 2022 The Android Open Source Project
  *
@@ -31,7 +30,7 @@ export class Presenter {
   constructor(notifyViewCallback: NotifyViewCallbackType, private storage: Storage) {
     this.notifyViewCallback = notifyViewCallback;
     this.uiData = new UiData([TraceType.SURFACE_FLINGER]);
-    this.notifyViewCallback(this.uiData);
+    this.copyUiDataAndNotifyView();
   }
 
   public updatePinnedItems(pinnedItem: HierarchyTreeNode) {
@@ -43,7 +42,7 @@ export class Presenter {
     }
     this.updatePinnedIds(pinnedId);
     this.uiData.pinnedItems = this.pinnedItems;
-    this.notifyViewCallback(this.uiData);
+    this.copyUiDataAndNotifyView();
   }
 
   public updateHighlightedItems(id: string) {
@@ -54,20 +53,20 @@ export class Presenter {
       this.highlightedItems.push(id);
     }
     this.uiData.highlightedItems = this.highlightedItems;
-    this.notifyViewCallback(this.uiData);
+    this.copyUiDataAndNotifyView();
   }
 
   public updateHierarchyTree(userOptions: UserOptions) {
     this.hierarchyUserOptions = userOptions;
     this.uiData.hierarchyUserOptions = this.hierarchyUserOptions;
     this.uiData.tree = this.generateTree();
-    this.notifyViewCallback(this.uiData);
+    this.copyUiDataAndNotifyView();
   }
 
   public filterHierarchyTree(filterString: string) {
     this.hierarchyFilter = TreeUtils.makeNodeFilter(filterString);
     this.uiData.tree = this.generateTree();
-    this.notifyViewCallback(this.uiData);
+    this.copyUiDataAndNotifyView();
   }
 
   public updatePropertiesTree(userOptions: UserOptions) {
@@ -101,7 +100,7 @@ export class Presenter {
         this.uiData.tree = this.generateTree();
       }
     }
-    this.notifyViewCallback(this.uiData);
+    this.copyUiDataAndNotifyView();
   }
 
   private generateRects(): Rectangle[] {
@@ -160,8 +159,9 @@ export class Presenter {
     if (this.selectedHierarchyTree) {
       this.uiData.propertiesTree = this.getTreeWithTransformedProperties(this.selectedHierarchyTree);
       this.uiData.selectedLayer = this.selectedLayer;
+      this.uiData.displayPropertyGroups = this.shouldDisplayPropertyGroups(this.selectedLayer);
     }
-    this.notifyViewCallback(this.uiData);
+    this.copyUiDataAndNotifyView();
   }
 
   private generateTree() {
@@ -252,6 +252,20 @@ export class Presenter {
     return transformedTree;
   }
 
+  private shouldDisplayPropertyGroups(selectedLayer: Layer): boolean {
+    // Do not display property groups when the root layer is selected. The root layer doesn't
+    // provide property groups info (visibility, geometry transforms, ...).
+    const isRoot = selectedLayer === this.entry;
+    return !isRoot;
+  }
+
+  private copyUiDataAndNotifyView() {
+    // Create a shallow copy of the data, otherwise the Angular OnPush change detection strategy
+    // won't detect the new input
+    const copy = Object.assign({}, this.uiData);
+    this.notifyViewCallback(copy);
+  }
+
   private readonly notifyViewCallback: NotifyViewCallbackType;
   private uiData: UiData;
   private hierarchyFilter: FilterType = TreeUtils.makeNodeFilter("");
diff --git a/tools/winscope-ng/src/viewers/viewer_surface_flinger/ui_data.ts b/tools/winscope-ng/src/viewers/viewer_surface_flinger/ui_data.ts
index 6af67f615..18bc9b2b1 100644
--- a/tools/winscope-ng/src/viewers/viewer_surface_flinger/ui_data.ts
+++ b/tools/winscope-ng/src/viewers/viewer_surface_flinger/ui_data.ts
@@ -30,6 +30,7 @@ export class UiData {
   tree: HierarchyTreeNode | null = null;
   propertiesTree: PropertiesTreeNode | null = null;
   selectedLayer: Layer = {};
+  displayPropertyGroups = true;
 
   constructor(dependencies?: Array<TraceType>) {
     this.dependencies = dependencies ?? [];
diff --git a/tools/winscope-ng/src/viewers/viewer_surface_flinger/viewer_surface_flinger.component.ts b/tools/winscope-ng/src/viewers/viewer_surface_flinger/viewer_surface_flinger.component.ts
index 55295bb24..35bc1bd76 100644
--- a/tools/winscope-ng/src/viewers/viewer_surface_flinger/viewer_surface_flinger.component.ts
+++ b/tools/winscope-ng/src/viewers/viewer_surface_flinger/viewer_surface_flinger.component.ts
@@ -14,6 +14,7 @@
  * limitations under the License.
  */
 import {
+  ChangeDetectionStrategy,
   Component,
   Input,
 } from "@angular/core";
@@ -24,6 +25,7 @@ import { PersistentStore } from "common/utils/persistent_store";
 
 @Component({
   selector: "viewer-surface-flinger",
+  changeDetection: ChangeDetectionStrategy.OnPush,
   template: `
     <div class="card-grid">
       <rects-view
@@ -49,7 +51,7 @@ import { PersistentStore } from "common/utils/persistent_store";
         [userOptions]="inputData?.propertiesUserOptions ?? {}"
         [propertiesTree]="inputData?.propertiesTree ?? {}"
         [selectedFlickerItem]="inputData?.selectedLayer ?? {}"
-        [propertyGroups]="true"
+        [displayPropertyGroups]="inputData?.displayPropertyGroups"
         [isProtoDump]="true"
       ></properties-view>
     </div>
@@ -67,7 +69,7 @@ import { PersistentStore } from "common/utils/persistent_store";
   ]
 })
 export class ViewerSurfaceFlingerComponent {
-  @Input() inputData: UiData | null = null;
+  @Input() inputData?: UiData;
   @Input() store: PersistentStore = new PersistentStore();
   @Input() active = false;
   TRACE_INFO = TRACE_INFO;
diff --git a/tools/winscope-ng/src/viewers/viewer_surface_flinger/viewer_surface_flinger.ts b/tools/winscope-ng/src/viewers/viewer_surface_flinger/viewer_surface_flinger.ts
index 606cc4f5c..d605361f6 100644
--- a/tools/winscope-ng/src/viewers/viewer_surface_flinger/viewer_surface_flinger.ts
+++ b/tools/winscope-ng/src/viewers/viewer_surface_flinger/viewer_surface_flinger.ts
@@ -13,22 +13,24 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-import {TraceType} from "common/trace/trace_type";
-import {View, Viewer, ViewType} from "viewers/viewer";
 import {Presenter} from "./presenter";
 import {UiData} from "./ui_data";
+import {TraceType} from "common/trace/trace_type";
+import {View, Viewer, ViewType} from "viewers/viewer";
 import {ViewerEvents} from "viewers/common/viewer_events";
 
 class ViewerSurfaceFlinger implements Viewer {
+  public static readonly DEPENDENCIES: TraceType[] = [TraceType.SURFACE_FLINGER];
+  private htmlElement: HTMLElement;
+  private presenter: Presenter;
+
   constructor(storage: Storage) {
     this.htmlElement = document.createElement("viewer-surface-flinger");
+
     this.presenter = new Presenter((uiData: UiData) => {
-      // Angular does not deep watch @Input properties. Clearing inputData to null before repopulating
-      // automatically ensures that the UI will change via the Angular change detection cycle. Without
-      // resetting, Angular does not auto-detect that inputData has changed.
-      (this.htmlElement as any).inputData = null;
       (this.htmlElement as any).inputData = uiData;
     }, storage);
+
     this.htmlElement.addEventListener(ViewerEvents.HierarchyPinnedChange, (event) => this.presenter.updatePinnedItems(((event as CustomEvent).detail.pinnedItem)));
     this.htmlElement.addEventListener(ViewerEvents.HighlightedChange, (event) => this.presenter.updateHighlightedItems(`${(event as CustomEvent).detail.id}`));
     this.htmlElement.addEventListener(ViewerEvents.HierarchyUserOptionsChange, (event) => this.presenter.updateHierarchyTree((event as CustomEvent).detail.userOptions));
@@ -49,10 +51,6 @@ class ViewerSurfaceFlinger implements Viewer {
   public getDependencies(): TraceType[] {
     return ViewerSurfaceFlinger.DEPENDENCIES;
   }
-
-  public static readonly DEPENDENCIES: TraceType[] = [TraceType.SURFACE_FLINGER];
-  private htmlElement: HTMLElement;
-  private presenter: Presenter;
 }
 
 export {ViewerSurfaceFlinger};
diff --git a/tools/winscope-ng/src/viewers/viewer_window_manager/viewer_window_manager.component.ts b/tools/winscope-ng/src/viewers/viewer_window_manager/viewer_window_manager.component.ts
index 023f03ac0..1869e3760 100644
--- a/tools/winscope-ng/src/viewers/viewer_window_manager/viewer_window_manager.component.ts
+++ b/tools/winscope-ng/src/viewers/viewer_window_manager/viewer_window_manager.component.ts
@@ -65,7 +65,7 @@ import { PersistentStore } from "common/utils/persistent_store";
   ]
 })
 export class ViewerWindowManagerComponent {
-  @Input() inputData: UiData | null = null;
+  @Input() inputData?: UiData;
   @Input() store: PersistentStore = new PersistentStore();
   @Input() active = false;
   TRACE_INFO = TRACE_INFO;