PR feedback; enabled new repaint-06 with a few paint points skipped (…

…for now)
replayio · Mar 19, 2024 · b33dfb0 · b33dfb0
1 parent ce518e4
commit b33dfb0
Show file tree

Hide file tree

Showing 2 changed files with 35 additions and 58 deletions.
diff --git a/packages/e2e-tests/tests/repaint-06.test.ts b/packages/e2e-tests/tests/repaint-06.test.ts
@@ -7,18 +7,19 @@ import { expect, test } from "../testFixture";
 
 test.use({ exampleKey: "doc_control_flow.html" });
 
-test.skip("repaint-06: repaints the screen screen when stepping over code that modifies the DOM", async ({
+test("repaint-06: repaints the screen screen when stepping over code that modifies the DOM", async ({
   pageWithMeta: { page, recordingId },
   exampleKey,
 }) => {
   await startTest(page, recordingId);
   await openDevToolsTab(page);
 
+  // TODO [FE-2363] Some of these points fail repaint, so they're disabled until RUN-3397 has been fixed
   const printedStrings = [
-    "catch",
-    "afterCatch",
+    // "catch",
+    // "afterCatch",
     "finally",
-    "yield 1",
+    // "yield 1",
     "generated 1",
     "yield 2",
     "generated 2",
@@ -29,17 +30,13 @@ test.skip("repaint-06: repaints the screen screen when stepping over code that m
     "within iterator",
   ];
 
-  const paints: {
-    [key: string]: {
-      before: string | null;
-      after: string | null;
-    };
+  const textToBase64DataMap: {
+    [text: string]: string | null;
   } = {};
 
   {
     await debugPrint(page, "Testing repaint graphics");
 
-    let graphicsDataUrl: string | null = null;
     let previousGraphicsDataUrl: string | null = null;
 
     for (let index = 0; index < printedStrings.length; index++) {
@@ -49,78 +46,51 @@ test.skip("repaint-06: repaints the screen screen when stepping over code that m
       const locator = await findConsoleMessage(page, text, "console-log");
       await seekToConsoleMessage(page, locator);
 
-      // Step to "Element text before..."
+      // Step from "updateText(...)" to "Element text after..."
       await stepOver(page);
       await stepOver(page);
-
-      await waitForRepaintGraphicsToLoad(page);
-
-      graphicsDataUrl = await getGraphicsDataUrl(page);
-      expect(
-        graphicsDataUrl,
-        `Expected graphics before "${string}" not to match previous after graphics`
-      ).not.toEqual(previousGraphicsDataUrl);
-      previousGraphicsDataUrl = graphicsDataUrl;
-
-      const cachedPaints = {
-        before: graphicsDataUrl,
-        after: "" as string | null,
-      };
-
-      // Step to "Element text after..."
       await stepOver(page);
       await stepOver(page);
 
       await waitForRepaintGraphicsToLoad(page);
 
-      graphicsDataUrl = await getGraphicsDataUrl(page);
+      const graphicsDataUrl = await getGraphicsDataUrl(page);
+
       expect(
         graphicsDataUrl,
-        `Expected graphics after "${string}" not to match the graphics before`
+        `Expected graphics to have been updated to reflect the text "${string}"`
       ).not.toEqual(previousGraphicsDataUrl);
-      previousGraphicsDataUrl = graphicsDataUrl;
 
-      cachedPaints.after = graphicsDataUrl;
-
-      paints[string] = cachedPaints;
+      previousGraphicsDataUrl = graphicsDataUrl;
+      textToBase64DataMap[string] = graphicsDataUrl;
     }
   }
 
   {
     await debugPrint(page, "Testing screenshot caching");
 
-    let graphicsDataUrl: string | null = null;
-
     for (let index = 0; index < printedStrings.length; index++) {
       const string = printedStrings[index];
       const text = `updateText("${string}")`;
 
       const locator = await findConsoleMessage(page, text, "console-log");
       await seekToConsoleMessage(page, locator);
 
-      // Step to "Element text before..."
+      // Step from "updateText(...)" to "Element text after..."
       await stepOver(page);
       await stepOver(page);
-
-      await waitForRepaintGraphicsToLoad(page);
-
-      graphicsDataUrl = await getGraphicsDataUrl(page);
-      expect(
-        graphicsDataUrl,
-        `Expected graphics before "${string}" to match previous cached graphics`
-      ).toEqual(paints[string]?.before);
-
-      // Step to "Element text after..."
       await stepOver(page);
       await stepOver(page);
 
       await waitForRepaintGraphicsToLoad(page);
 
-      graphicsDataUrl = await getGraphicsDataUrl(page);
+      const expectedGraphicsDataUrl = textToBase64DataMap[string];
+      const actualGraphicsDataUrl = await getGraphicsDataUrl(page);
+
       expect(
-        graphicsDataUrl,
-        `Expected graphics after "${string}" to match previous cached graphics`
-      ).toEqual(paints[string]?.after);
+        actualGraphicsDataUrl,
+        `Expected graphics for the text "${string}" to match previously cached graphics`
+      ).toEqual(expectedGraphicsDataUrl);
     }
   }
 });
diff --git a/src/ui/components/Video/imperative/MutableGraphicsState.tsx b/src/ui/components/Video/imperative/MutableGraphicsState.tsx
@@ -54,7 +54,7 @@ export const state = createState<State>({
   status: "loading",
 });
 
-let lock = new Object();
+let lock: Object | null = null;
 
 export async function updateState(
   containerElement: HTMLElement,
@@ -67,9 +67,6 @@ export async function updateState(
     time: number;
   }> = {}
 ) {
-  const localLock = new Object();
-  lock = localLock;
-
   const prevState = state.read();
 
   let {
@@ -81,6 +78,17 @@ export async function updateState(
     time = prevState.currentTime,
   } = options;
 
+  if (shallowEqual(options, { didResize })) {
+    if (lock !== null) {
+      // Don't let an event from the ResizeObserver interrupt an in-progress update;
+      // this should override new SnapShot data
+      return;
+    }
+  }
+
+  const localLock = new Object();
+  lock = localLock;
+
   let graphicsRect = prevState.graphicsRect;
   let localScale = prevState.localScale;
   let recordingScale = prevState.recordingScale;
@@ -94,15 +102,12 @@ export async function updateState(
     const naturalWidth = naturalDimensions.width;
 
     const containerRect = containerElement.getBoundingClientRect();
-    const scaledDimensions = await fitImageToContainer({
+    const scaledDimensions = fitImageToContainer({
       containerHeight: containerRect.height,
       containerWidth: containerRect.width,
       imageHeight: naturalHeight,
       imageWidth: naturalWidth,
     });
-    if (lock !== localLock) {
-      return;
-    }
 
     const clientHeight = scaledDimensions.height;
     const clientWidth = scaledDimensions.width;
@@ -132,4 +137,6 @@ export async function updateState(
   if (!shallowEqual(prevState, nextState)) {
     state.update(nextState);
   }
+
+  lock = null;
 }