[RPP] Fix the Entry coordinate missing when the trace is loaded

The arrow was broken when the connected even was off screen on load. Example in the bug.

While I was at it, I also refactored the function that rerenders the Overlay.

Bug: 395567067
Change-Id: I851978cde04658269edd60c9bf0661c8af123e36
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6429296
Reviewed-by: Jack Franklin <[email protected]>
Auto-Submit: Alina Varkki <[email protected]>
Commit-Queue: Jack Franklin <[email protected]>

Author: AlinaVarkki

front_end/panels/timeline/overlays/OverlaysImpl.ts

@@ -1148,30 +1148,32 @@ export class Overlays extends EventTarget {
       const entryToWrapper = component.entryToWrapper();
 
       if (entryTo && entryToWrapper) {
-        let toEntryX = 0;
+        let toEntryX = this.xPixelForEventStartOnChart(entryTo) ?? 0;
         // If the 'to' entry is visible, set the entry Y as an arrow coordinate to point to. If not, get the canvas edge coordate to point the arrow to.
         let toEntryY = this.#yCoordinateForNotVisibleEntry(entryTo);
-
-        if (entryToVisibility) {
-          const toEntryParams = this.#positionEntryBorderOutlineType(entryTo, entryToWrapper);
-
-          if (toEntryParams) {
-            const toEntryHeight = toEntryParams?.entryHeight;
-            const toEntryWidth = toEntryParams?.entryWidth;
-            const toCutOffHeight = toEntryParams?.cutOffHeight;
-            toEntryX = toEntryParams?.x;
-            toEntryY = toEntryParams?.y;
-
-            component.toEntryCoordinateAndDimensions = {
-              x: toEntryX,
-              y: toEntryY,
-              length: toEntryWidth,
-              height: toEntryHeight - toCutOffHeight,
-            };
-          } else {
-            // Something went if the entry is visible and we cannot get its' parameters.
-            return;
-          }
+        const toEntryParams = this.#positionEntryBorderOutlineType(entryTo, entryToWrapper);
+
+        if (toEntryParams) {
+          const toEntryHeight = toEntryParams?.entryHeight;
+          const toEntryWidth = toEntryParams?.entryWidth;
+          const toCutOffHeight = toEntryParams?.cutOffHeight;
+          toEntryX = toEntryParams?.x;
+          toEntryY = toEntryParams?.y;
+
+          component.toEntryCoordinateAndDimensions = {
+            x: toEntryX,
+            y: toEntryY,
+            length: toEntryWidth,
+            height: toEntryHeight - toCutOffHeight,
+          };
+        } else {
+          // if the entry exists and we cannot get its' parameters, it is probably loaded and is off screen.
+          // In this case, assign the coordinates so we can draw the arrow in the right direction.
+          component.toEntryCoordinateAndDimensions = {
+            x: toEntryX,
+            y: toEntryY,
+          };
+          return;
         }
 
       } else {

front_end/panels/timeline/overlays/components/EntriesLinkOverlay.ts

@@ -43,8 +43,8 @@ export class EntriesLinkOverlay extends HTMLElement {
   #connector: SVGLineElement|null = null;
   #entryFromWrapper: HTMLElement|null = null;
   #entryToWrapper: HTMLElement|null = null;
-  #entryFromConnector: SVGCircleElement|null = null;
-  #entryToConnector: SVGCircleElement|null = null;
+  #entryFromCirleConnector: SVGCircleElement|null = null;
+  #entryToCircleConnector: SVGCircleElement|null = null;
   #entryFromVisible = true;
   #entryToVisible = true;
   #canvasRect: DOMRect|null = null;
@@ -73,8 +73,8 @@ export class EntriesLinkOverlay extends HTMLElement {
     this.#connector = this.#connectorLineContainer?.querySelector('line') ?? null;
     this.#entryFromWrapper = this.#shadow.querySelector('.from-highlight-wrapper') ?? null;
     this.#entryToWrapper = this.#shadow.querySelector('.to-highlight-wrapper') ?? null;
-    this.#entryFromConnector = this.#connectorLineContainer?.querySelector('.entryFromConnector') ?? null;
-    this.#entryToConnector = this.#connectorLineContainer?.querySelector('.entryToConnector') ?? null;
+    this.#entryFromCirleConnector = this.#connectorLineContainer?.querySelector('.entryFromConnector') ?? null;
+    this.#entryToCircleConnector = this.#connectorLineContainer?.querySelector('.entryToConnector') ?? null;
     this.#linkState = linkCreationNotStartedState;
     this.#render();
   }
@@ -117,13 +117,13 @@ export class EntriesLinkOverlay extends HTMLElement {
     this.#coordinateFrom = {x: fromEntryParams.x, y: fromEntryParams.y};
     this.#fromEntryDimensions = {width: fromEntryParams.length, height: fromEntryParams.height};
     this.#updateCreateLinkBox();
-    this.#redrawConnectionArrow();
+    this.#redrawAllEntriesLinkParts();
   }
 
   set entriesVisibility(entriesVisibility: {fromEntryVisibility: boolean, toEntryVisibility: boolean}) {
     this.#entryFromVisible = entriesVisibility.fromEntryVisibility;
     this.#entryToVisible = entriesVisibility.toEntryVisibility;
-    this.#redrawConnectionArrow();
+    this.#redrawAllEntriesLinkParts();
   }
 
   // The arrow might be pointing either to an entry or an empty space.
@@ -137,7 +137,7 @@ export class EntriesLinkOverlay extends HTMLElement {
     }
 
     this.#updateCreateLinkBox();
-    this.#redrawConnectionArrow();
+    this.#redrawAllEntriesLinkParts();
   }
 
   set fromEntryIsSource(x: boolean) {
@@ -156,97 +156,121 @@ export class EntriesLinkOverlay extends HTMLElement {
     this.#render();
   }
 
-  #redrawConnectionArrow(): void {
-    if (!this.#connector || !this.#entryFromWrapper || !this.#entryToWrapper || !this.#entryFromConnector ||
-        !this.#entryToConnector) {
-      console.error('`connector` element is missing.');
+  /*
+    Redraw all parts of the EntriesLink overlay
+     _________
+    |__entry__|o\      <-- 'from 'entry wrapper and the circle connector next to it
+                 \
+                  \    <-- Arrow Connector
+                   \   ________________
+                    ➘ o|_____entry______|  <-- 'to' entry wrapper and the circle connector next to it
+  */
+  #redrawAllEntriesLinkParts(): void {
+    if (!this.#connector || !this.#entryFromWrapper || !this.#entryToWrapper || !this.#entryFromCirleConnector ||
+        !this.#entryToCircleConnector) {
+      console.error('one of the required Entries Link elements is missing.');
       return;
     }
 
     if (this.#linkState === Trace.Types.File.EntriesLinkState.CREATION_NOT_STARTED) {
-      this.#entryFromConnector.setAttribute('visibility', 'hidden');
-      this.#entryToConnector.setAttribute('visibility', 'hidden');
+      this.#entryFromCirleConnector.setAttribute('visibility', 'hidden');
+      this.#entryToCircleConnector.setAttribute('visibility', 'hidden');
+      this.#connector.style.display = 'none';
       return;
     }
 
+    this.#setEntriesWrappersVisibility();
+    this.#setConnectorCirclesVisibility();
+    this.#setArrowConnectorStyle();
+    this.#positionConnectorLineAndCircles();
+
+    this.#render();
+  }
+
+  // Only draw the entry wrapper if that entry is visible
+  #setEntriesWrappersVisibility(): void {
+    if (!this.#entryFromWrapper || !this.#entryToWrapper) {
+      return;
+    }
+    this.#entryFromWrapper.style.visibility = this.#entryFromVisible ? 'visible' : 'hidden';
+    this.#entryToWrapper.style.visibility = this.#entryToVisible ? 'visible' : 'hidden';
+  }
+
+  // Draw the entry connector circles:
+  //  - The entry the arrow is connecting to is the connection source
+  //  - That entry currently is visible
+  //  - There is enough space for the connector circle
+  #setConnectorCirclesVisibility(): void {
+    if (!this.#toEntryDimensions || !this.#entryFromCirleConnector || !this.#entryToCircleConnector) {
+      return;
+    }
     // If the user is zoomed out, the connector circles can be as large as the
     // event itself. So if the rectangle for this entry is too small, we
     // don't draw the circles.
     const minWidthToDrawConnectorCircles = 8;
-
-    // We do not draw the connectors if the entry is not visible, or if the
-    // entry we are connecting to isn't the actual source entry.
-    // We also don't draw them if an entry is completely hidden, in which case
-    // we aren't drawing the arrows, so it doesn't make sense to draw the
-    // connectors.
     const drawFromEntryConnectorCircle = this.#entryFromVisible && !this.#arrowHidden && this.#fromEntryIsSource &&
         this.#fromEntryDimensions.width >= minWidthToDrawConnectorCircles;
-
-    const widthOfToEntry = this.#toEntryDimensions?.width ?? 0;
     const drawToEntryConnectorCircle = !this.#arrowHidden && this.#entryToVisible && this.#toEntryIsSource &&
-        widthOfToEntry >= minWidthToDrawConnectorCircles && !this.#arrowHidden;
+        this.#toEntryDimensions?.width >= minWidthToDrawConnectorCircles && !this.#arrowHidden;
+
+    this.#entryFromCirleConnector.setAttribute('visibility', drawFromEntryConnectorCircle ? 'visible' : 'hidden');
+    this.#entryToCircleConnector.setAttribute('visibility', drawToEntryConnectorCircle ? 'visible' : 'hidden');
+  }
+
+  #setArrowConnectorStyle(): void {
+    if (!this.#connector) {
+      return;
+    }
+
+    // If neither entry is visible, do not display the connector
+    this.#connector.style.display = (this.#entryFromVisible || this.#entryToVisible) ? 'block' : 'none';
+    this.#connector.setAttribute('stroke-width', '2');
+
+    const arrowColor = ThemeSupport.ThemeSupport.instance().getComputedValue('--color-text-primary');
+
+    // Use a solid stroke if the 'to' entry's dimensions are unknown (during link creation) or if both entries are visible.
+    if (!this.#toEntryDimensions || (this.#entryFromVisible && this.#entryToVisible)) {
+      this.#connector.setAttribute('stroke', arrowColor);
+      return;
+    }
 
-    this.#entryFromConnector.setAttribute('visibility', drawFromEntryConnectorCircle ? 'visible' : 'hidden');
-    this.#entryToConnector.setAttribute('visibility', drawToEntryConnectorCircle ? 'visible' : 'hidden');
+    // If one entry is not visible and one is, fade the arrow.
+    if (this.#entryFromVisible && !this.#entryToVisible) {
+      this.#connector.setAttribute('stroke', 'url(#fromVisibleLineGradient)');
+    } else if (this.#entryToVisible && !this.#entryFromVisible) {
+      this.#connector.setAttribute('stroke', 'url(#toVisibleLineGradient)');
+    }
+  }
+
+  #positionConnectorLineAndCircles(): void {
+    if (!this.#connector || !this.#entryFromCirleConnector || !this.#entryToCircleConnector) {
+      return;
+    }
 
     // If the entry is visible, the entry arrow starts from the middle of the right edge of the entry (end on the X axis and middle of the Y axis).
     // If not, draw it to the y coordinate of the entry and the edge of the timeline so it is pointing in the direction of the entry.
     const halfFromEntryHeight = this.#fromEntryDimensions.height / 2;
-    if (this.#entryFromVisible) {
-      const endConnectionPointX = String(this.#coordinateFrom.x + this.#fromEntryDimensions.width);
-      const endConnectionPointY = String(this.#coordinateFrom.y + halfFromEntryHeight);
+    const fromX = this.#coordinateFrom.x + this.#fromEntryDimensions.width;
+    const fromY = this.#coordinateFrom.y + halfFromEntryHeight;
 
-      this.#connector.setAttribute('x1', endConnectionPointX);
-      this.#connector.setAttribute('y1', endConnectionPointY);
+    this.#connector.setAttribute('x1', fromX.toString());
+    this.#connector.setAttribute('y1', fromY.toString());
 
-      this.#entryFromConnector.setAttribute('cx', endConnectionPointX);
-      this.#entryFromConnector.setAttribute('cy', endConnectionPointY);
-      this.#entryFromWrapper.style.visibility = 'visible';
-    } else {
-      this.#connector.setAttribute('x1', (this.#coordinateFrom.x + this.#fromEntryDimensions.width).toString());
-      this.#connector.setAttribute('y1', String(this.#coordinateFrom.y + halfFromEntryHeight));
-      this.#entryFromWrapper.style.visibility = 'hidden';
-    }
+    this.#entryFromCirleConnector.setAttribute('cx', fromX.toString());
+    this.#entryFromCirleConnector.setAttribute('cy', fromY.toString());
 
     // If the arrow is pointing to the entry and that entry is visible, point it to the middle of the entry.
     // If the entry is not visible, point the arrow to the edge of the screen towards the entry.
     // Otherwise, the arrow is following the mouse so we assign it to the provided coordinates.
-    if (this.#toEntryDimensions && this.#entryToVisible) {
-      const connectionPointX = String(this.#coordinateTo.x);
-      const connectionPointY = String(this.#coordinateTo.y + this.#toEntryDimensions.height / 2);
-
-      this.#connector.setAttribute('x2', connectionPointX);
-      this.#connector.setAttribute('y2', connectionPointY);
+    const toX = this.#coordinateTo.x;
+    const toY = this.#toEntryDimensions ? this.#coordinateTo.y + (this.#toEntryDimensions?.height ?? 0) / 2 :
+                                          this.#coordinateTo.y;
 
-      this.#entryToConnector.setAttribute('cx', connectionPointX);
-      this.#entryToConnector.setAttribute('cy', connectionPointY);
+    this.#connector.setAttribute('x2', toX.toString());
+    this.#connector.setAttribute('y2', toY.toString());
 
-      this.#entryToWrapper.style.visibility = 'visible';
-    } else {
-      this.#entryToWrapper.style.visibility = 'hidden';
-      this.#connector.setAttribute('x2', this.#coordinateTo.x.toString());
-      // If `toEntryDimensions` exist, the arrow points to the entry and we need to take its height into account.
-      // Otherwise, it is following the mouse.
-      if (this.#toEntryDimensions) {
-        const halfToEntryHeight = this.#toEntryDimensions.height / 2;
-        this.#connector.setAttribute('y2', String(this.#coordinateTo.y + halfToEntryHeight));
-      } else {
-        this.#connector.setAttribute('y2', (this.#coordinateTo.y).toString());
-      }
-    }
-
-    this.#connector.setAttribute('stroke-width', '2');
-
-    if (this.#toEntryDimensions && this.#entryFromVisible && !this.#entryToVisible) {
-      this.#connector.setAttribute('stroke', 'url(#fromVisibleLineGradient)');
-    } else if (this.#toEntryDimensions && this.#entryToVisible && !this.#entryFromVisible) {
-      this.#connector.setAttribute('stroke', 'url(#toVisibleLineGradient)');
-    } else {
-      const arrowColor = ThemeSupport.ThemeSupport.instance().getComputedValue('--color-text-primary');
-      this.#connector.setAttribute('stroke', arrowColor);
-    }
-
-    this.#render();
+    this.#entryToCircleConnector.setAttribute('cx', toX.toString());
+    this.#entryToCircleConnector.setAttribute('cy', toY.toString());
   }
 
   /*