[RPP] Update the Entry Label Annotation in the ModificationsManager when the label changes

Update the state of the ModificationsManager annotation which, in turn,  updates the label in the sidebar as the label is edited.

Bug: 349530077
Change-Id: Iadcb1768e5673a3840348a8b861c1695943aa08e
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/5687112
Reviewed-by: Jack Franklin <jacktfranklin@chromium.org>
Commit-Queue: Alina Varkki <alinavarkki@chromium.org>

Author: AlinaVarkki

front_end/panels/timeline/ModificationsManager.ts

@@ -11,23 +11,16 @@ import {type EntryLabel, type TimelineOverlay} from './Overlays.js';
 const modificationsManagerByTraceIndex: ModificationsManager[] = [];
 let activeManager: ModificationsManager|null;
 
-// Event dispatched after an annotation was added.
-// The event argument is the Overlay that needs to be created by `Overlays.ts`.
-export class AnnotationAddedEvent extends Event {
-  static readonly eventName = 'annotationaddedevent';
+export type UpdateAction = 'Remove'|'Add'|'UpdateLabel';
 
-  constructor(public addedAnnotationOverlay: TimelineOverlay) {
-    super(AnnotationAddedEvent.eventName);
-  }
-}
-
-// Event dispatched after an annotation was removed.
-// The event argument is the Overlay that needs to be removed from `Overlays.ts`.
-export class AnnotationRemovedEvent extends Event {
-  static readonly eventName = 'annotationremovedevent';
+// Event dispatched after an annotation was added, removed or updated.
+// The event argument is the Overlay that needs to be created,removed
+// or updated by `Overlays.ts` and the action that needs to be applied to it.
+export class AnnotationModifiedEvent extends Event {
+  static readonly eventName = 'annotationmodifiedevent';
 
-  constructor(public removedAnnotationOverlay: TimelineOverlay) {
-    super(AnnotationRemovedEvent.eventName);
+  constructor(public overlay: TimelineOverlay, public action: UpdateAction) {
+    super(AnnotationModifiedEvent.eventName);
   }
 }
 
@@ -125,7 +118,7 @@ export class ModificationsManager extends EventTarget {
     this.#overlayForAnnotation.set(newAnnotation, newOverlay);
 
     // TODO: When we have more annotations, check the annotation type and create the appropriate one
-    this.dispatchEvent(new AnnotationAddedEvent(newOverlay));
+    this.dispatchEvent(new AnnotationModifiedEvent(newOverlay, 'Add'));
   }
 
   removeAnnotationOverlay(removedOverlay: TimelineOverlay): void {
@@ -135,7 +128,20 @@ export class ModificationsManager extends EventTarget {
       return;
     }
     this.#overlayForAnnotation.delete(annotationForRemovedOverlay);
-    this.dispatchEvent(new AnnotationRemovedEvent(removedOverlay));
+    this.dispatchEvent(new AnnotationModifiedEvent(removedOverlay, 'Remove'));
+  }
+
+  updateAnnotationOverlay(updatedOverlay: TimelineOverlay): void {
+    const annotationForUpdatedOverlay = this.#getAnnotationByOverlay(updatedOverlay);
+    if (!annotationForUpdatedOverlay) {
+      console.warn('Annotation for updated Overlay does not exist');
+      return;
+    }
+
+    if (updatedOverlay.type === 'ENTRY_LABEL') {
+      annotationForUpdatedOverlay.label = updatedOverlay.label;
+    }
+    this.dispatchEvent(new AnnotationModifiedEvent(updatedOverlay, 'UpdateLabel'));
   }
 
   #getAnnotationByOverlay(overlay: TimelineOverlay): TraceEngine.Types.File.Annotation|null {

front_end/panels/timeline/Overlays.test.ts

@@ -13,6 +13,7 @@ import {TraceLoader} from '../../testing/TraceLoader.js';
 import * as RenderCoordinator from '../../ui/components/render_coordinator/render_coordinator.js';
 import * as PerfUI from '../../ui/legacy/components/perf_ui/perf_ui.js';
 
+import * as Components from './components/components.js';
 import * as Timeline from './timeline.js';
 
 const coordinator = RenderCoordinator.RenderCoordinator.RenderCoordinator.instance();
@@ -217,16 +218,20 @@ describeWithEnvironment('Overlays', () => {
 
       const currManager = Timeline.ModificationsManager.ModificationsManager.activeManager();
       // The Annotations Overlays are added through the ModificationsManager listener
-      currManager?.addEventListener(Timeline.ModificationsManager.AnnotationAddedEvent.eventName, event => {
-        const addedOverlay = (event as Timeline.ModificationsManager.AnnotationAddedEvent).addedAnnotationOverlay;
-        overlays.add(addedOverlay);
+      currManager?.addEventListener(Timeline.ModificationsManager.AnnotationModifiedEvent.eventName, event => {
+        const {overlay, action} = (event as Timeline.ModificationsManager.AnnotationModifiedEvent);
+        if (action === 'Add') {
+          overlays.add(overlay);
+        }
         overlays.update();
       });
 
       // When an annotation overlay is remomved, this event is dispatched to the Modifications Manager.
-      overlays.addEventListener(Timeline.Overlays.AnnotationOverlayRemoveEvent.eventName, event => {
-        const addedOverlay = (event as Timeline.Overlays.AnnotationOverlayRemoveEvent).overlay;
-        overlays.remove(addedOverlay);
+      overlays.addEventListener(Timeline.Overlays.AnnotationOverlayActionEvent.eventName, event => {
+        const {overlay, action} = (event as Timeline.Overlays.AnnotationOverlayActionEvent);
+        if (action === 'Remove') {
+          overlays.remove(overlay);
+        }
         overlays.update();
       });
 
@@ -393,6 +398,35 @@ describeWithEnvironment('Overlays', () => {
       assert.strictEqual(overlays.overlaysForEntry(event).length, 0);
     });
 
+    it('Update label overlay when the label changes', async function() {
+      const traceParsedData = await TraceLoader.traceEngine(this, 'web-dev.json.gz');
+      const {overlays, container, charts} = setupChartWithDimensionsAndAnnotationOverlayListeners(traceParsedData);
+      const event = charts.mainProvider.eventByIndex(50);
+      assert.isOk(event);
+
+      // Create an entry label overlay
+      Timeline.ModificationsManager.ModificationsManager.activeManager()?.createAnnotation({
+        type: 'ENTRY_LABEL',
+        entry: event as TraceEngine.Types.TraceEvents.TraceEventData,
+        label: '',
+      });
+      overlays.update();
+
+      // Ensure that the overlay was created.
+      const overlayDOM = container.querySelector<HTMLElement>('.overlay-type-ENTRY_LABEL');
+      assert.isOk(overlayDOM);
+      const component = overlayDOM?.querySelector('devtools-entry-label-overlay');
+      assert.isOk(component?.shadowRoot);
+
+      component.connectedCallback();
+      component.dispatchEvent(new Components.EntryLabelOverlay.EntryLabelChangeEvent('new label'));
+
+      const updatedOverlay = overlays.overlaysForEntry(event)[0] as Timeline.Overlays.EntryLabel;
+      assert.isOk(updatedOverlay);
+      // Make sure the label was updated in the Overlay Object
+      assert.strictEqual(updatedOverlay.label, 'new label');
+    });
+
     it('can render an overlay for a time range', async function() {
       const traceParsedData = await TraceLoader.traceEngine(this, 'web-dev.json.gz');
       const {overlays, container} = setupChartWithDimensionsAndAnnotationOverlayListeners(traceParsedData);

front_end/panels/timeline/Overlays.ts

@@ -122,13 +122,14 @@ export interface TimelineCharts {
 }
 
 // An event dispatched when one of the Annotation Overlays (overlay created by the user,
-// ex. EntryLabel) is removed. When one of the Annotation Overlays is removed,
+// ex. EntryLabel) is removed or updated. When one of the Annotation Overlays is removed or updated,
 // ModificationsManager listens to this event and updates the current annotations.
-export class AnnotationOverlayRemoveEvent extends Event {
-  static readonly eventName = 'annotationoverlayremoveevent';
+export type UpdateAction = 'Remove'|'Update';
+export class AnnotationOverlayActionEvent extends Event {
+  static readonly eventName = 'annotationoverlayactionsevent';
 
-  constructor(public overlay: TimelineOverlay) {
-    super(AnnotationOverlayRemoveEvent.eventName);
+  constructor(public overlay: TimelineOverlay, public action: UpdateAction) {
+    super(AnnotationOverlayActionEvent.eventName);
   }
 }
 
@@ -656,7 +657,12 @@ export class Overlays extends EventTarget {
       case 'ENTRY_LABEL': {
         const component = new Components.EntryLabelOverlay.EntryLabelOverlay(overlay.label);
         component.addEventListener(Components.EntryLabelOverlay.EmptyEntryLabelRemoveEvent.eventName, () => {
-          this.dispatchEvent(new AnnotationOverlayRemoveEvent(overlay));
+          this.dispatchEvent(new AnnotationOverlayActionEvent(overlay, 'Remove'));
+        });
+        component.addEventListener(Components.EntryLabelOverlay.EntryLabelChangeEvent.eventName, event => {
+          const newLabel = (event as Components.EntryLabelOverlay.EntryLabelChangeEvent).newLabel;
+          overlay.label = newLabel;
+          this.dispatchEvent(new AnnotationOverlayActionEvent(overlay, 'Update'));
         });
         div.appendChild(component);
         return div;

front_end/panels/timeline/TimelineFlameChartView.ts

@@ -18,7 +18,7 @@ import {CountersGraph} from './CountersGraph.js';
 import {SHOULD_SHOW_EASTER_EGG} from './EasterEgg.js';
 import {ModificationsManager} from './ModificationsManager.js';
 import {
-  AnnotationOverlayRemoveEvent,
+  AnnotationOverlayActionEvent,
   Overlays,
   type TimelineOverlay,
   type TimeRangeLabel,
@@ -196,9 +196,13 @@ export class TimelineFlameChartView extends UI.Widget.VBox implements PerfUI.Fla
       },
     });
 
-    this.#overlays.addEventListener(AnnotationOverlayRemoveEvent.eventName, event => {
-      const overlay = (event as AnnotationOverlayRemoveEvent).overlay;
-      ModificationsManager.activeManager()?.removeAnnotationOverlay(overlay);
+    this.#overlays.addEventListener(AnnotationOverlayActionEvent.eventName, event => {
+      const {overlay, action} = (event as AnnotationOverlayActionEvent);
+      if (action === 'Remove') {
+        ModificationsManager.activeManager()?.removeAnnotationOverlay(overlay);
+      } else if (action === 'Update') {
+        ModificationsManager.activeManager()?.updateAnnotationOverlay(overlay);
+      }
     });
 
     this.networkPane = new UI.Widget.VBox();

front_end/panels/timeline/TimelinePanel.ts

@@ -58,7 +58,7 @@ import {SHOULD_SHOW_EASTER_EGG} from './EasterEgg.js';
 import {Tracker} from './FreshRecording.js';
 import historyToolbarButtonStyles from './historyToolbarButton.css.js';
 import {IsolateSelector} from './IsolateSelector.js';
-import {AnnotationAddedEvent, AnnotationRemovedEvent, ModificationsManager} from './ModificationsManager.js';
+import {AnnotationModifiedEvent, ModificationsManager} from './ModificationsManager.js';
 import {cpuprofileJsonGenerator, traceJsonGenerator} from './SaveFileFormatter.js';
 import {NodeNamesUpdated, SourceMapsResolver} from './SourceMapsResolver.js';
 import {type Client, TimelineController} from './TimelineController.js';
@@ -1294,15 +1294,13 @@ export class TimelinePanel extends UI.Panel.Panel implements Client, TimelineMod
       }
 
       // Add ModificationsManager listeners for annotations change to update the Annotation Overlays.
-      currentManager?.addEventListener(AnnotationAddedEvent.eventName, event => {
-        const addedOverlay = (event as AnnotationAddedEvent).addedAnnotationOverlay;
-        this.flameChart.addOverlay(addedOverlay);
-        this.#sideBar.setAnnotationsTabContent(currentManager.getAnnotations());
-      });
-
-      currentManager?.addEventListener(AnnotationRemovedEvent.eventName, event => {
-        const removedOverlay = (event as AnnotationRemovedEvent).removedAnnotationOverlay;
-        this.flameChart.removeOverlay(removedOverlay);
+      currentManager?.addEventListener(AnnotationModifiedEvent.eventName, event => {
+        const {overlay, action} = (event as AnnotationModifiedEvent);
+        if (action === 'Add') {
+          this.flameChart.addOverlay(overlay);
+        } else if (action === 'Remove') {
+          this.flameChart.removeOverlay(overlay);
+        }
         this.#sideBar.setAnnotationsTabContent(currentManager.getAnnotations());
       });
 

front_end/panels/timeline/components/EntryLabelOverlay.ts

@@ -14,6 +14,14 @@ export class EmptyEntryLabelRemoveEvent extends Event {
   }
 }
 
+export class EntryLabelChangeEvent extends Event {
+  static readonly eventName = 'entrylabelchangeevent';
+
+  constructor(public newLabel: string) {
+    super(EntryLabelChangeEvent.eventName);
+  }
+}
+
 export class EntryLabelOverlay extends HTMLElement {
   // The label is angled on the left from the centre of the entry it belongs to.
   // `LABEL_AND_CONNECTOR_SHIFT_LENGTH` specifies how many pixels to the left it is shifted.
@@ -25,7 +33,7 @@ export class EntryLabelOverlay extends HTMLElement {
   static readonly LABEL_AND_CONNECTOR_HEIGHT =
       EntryLabelOverlay.LABEL_HEIGHT + EntryLabelOverlay.LABEL_PADDING * 2 + EntryLabelOverlay.LABEL_CONNECTOR_HEIGHT;
   // Set the max label length to avoid labels that could signicantly increase the file size.
-  static readonly MAX_LABEL_LENGTH = 100;
+  static readonly MAX_LABEL_LENGTH = 1;
 
   static readonly litTagName = LitHtml.literal`devtools-entry-label-overlay`;
   readonly #shadow = this.attachShadow({mode: 'open'});
@@ -37,6 +45,7 @@ export class EntryLabelOverlay extends HTMLElement {
 
   #labelPartsWrapper: HTMLElement|null = null;
   #labelBox: HTMLElement|null = null;
+  #label: string;
 
   /*
 The entry label overlay consists of 3 parts - the label part with the label string inside,
@@ -62,22 +71,29 @@ Otherwise, the entry label overlay object only gets repositioned.
     super();
     this.#render();
     this.#labelPartsWrapper = this.#shadow.querySelector<HTMLElement>('.label-parts-wrapper');
+    this.#label = label;
     this.#drawLabel(label);
     this.#drawConnector();
   }
 
   connectedCallback(): void {
     this.#shadow.adoptedStyleSheets = [styles];
-    this.#labelBox?.addEventListener('keydown', this.#handleLabelInputKeyDown);
-    this.#labelBox?.addEventListener('paste', this.#handleLabelInputPaste);
   }
 
-  disconnectedCallback(): void {
-    this.#labelBox?.removeEventListener('keydown', this.#handleLabelInputKeyDown);
-    this.#labelBox?.removeEventListener('paste', this.#handleLabelInputPaste);
+  #handleLabelInputKeyUp(): void {
+    // If the label changed on key up, dispatch label changed event
+    const labelBoxTextContent = this.#labelBox?.textContent ?? '';
+    if (labelBoxTextContent !== this.#label) {
+      this.#label = labelBoxTextContent;
+      this.dispatchEvent(new EntryLabelChangeEvent(this.#label));
+    }
   }
 
   #handleLabelInputKeyDown(event: KeyboardEvent): boolean {
+    if (!this.#labelBox) {
+      return false;
+    }
+
     const allowedKeysAfterReachingLenLimit = [
       'Backspace',
       'Delete',
@@ -89,12 +105,13 @@ Otherwise, the entry label overlay object only gets repositioned.
     // Therefore, if the new key is `Enter` key, treat it
     // as the end of the label input and blur the input field.
     if (event.key === 'Enter' || event.key === 'Escape') {
-      this.dispatchEvent(new FocusEvent('blur', {bubbles: true}));
+      this.#labelBox.dispatchEvent(new FocusEvent('blur', {bubbles: true}));
       return false;
     }
 
     // If the max limit is not reached, return true
-    if (!this.textContent || this.textContent.length <= EntryLabelOverlay.MAX_LABEL_LENGTH) {
+    if (this.#labelBox.textContent !== null &&
+        this.#labelBox.textContent.length <= EntryLabelOverlay.MAX_LABEL_LENGTH) {
       return true;
     }
 
@@ -114,21 +131,21 @@ Otherwise, the entry label overlay object only gets repositioned.
     event.preventDefault();
 
     const clipboardData = event.clipboardData;
-    if (!clipboardData) {
+    if (!clipboardData || !this.#labelBox) {
       return;
     }
 
     const pastedText = clipboardData.getData('text');
 
-    const newText = this.textContent + pastedText;
+    const newText = this.#labelBox.textContent + pastedText;
     const trimmedText = newText.slice(0, EntryLabelOverlay.MAX_LABEL_LENGTH + 1);
 
-    this.textContent = trimmedText;
+    this.#labelBox.textContent = trimmedText;
 
     // Reset the selection to the end
     const selection = window.getSelection();
     const range = document.createRange();
-    range.selectNodeContents(this);
+    range.selectNodeContents(this.#labelBox);
     range.collapse(false);
     selection?.removeAllRanges();
     selection?.addRange(range);
@@ -262,6 +279,9 @@ Otherwise, the entry label overlay object only gets repositioned.
             class="label-box"
             @dblclick=${() => this.#setLabelEditabilityAndRemoveEmptyLabel(true)}
             @blur=${() => this.#setLabelEditabilityAndRemoveEmptyLabel(false)}
+            @keydown=${this.#handleLabelInputKeyDown}
+            @paste=${this.#handleLabelInputPaste}
+            @keyup=${this.#handleLabelInputKeyUp}
             contenteditable=${this.#isLabelEditable}>
           </span>
           <svg class="connectorContainer">