Bug 1581665 - Tidy up handling of pause and paint data, r=jlast.

Differential Revision: https://phabricator.services.mozilla.com/D46087

--HG--
extra : moz-landing-system : lando

devtools/server/actors/replay/control.js

@@ -238,7 +238,7 @@ ChildProcess.prototype = {
       return;
     }
     RecordReplayControl.waitUntilPaused(this.id, maybeCreateCheckpoint);
-    assert(this.paused);
+    assert(this.paused || this.crashed);
   },
 
   // Add a checkpoint for this child to save.
@@ -323,7 +323,6 @@ function lookupChild(id) {
   if (id == gMainChild.id) {
     return gMainChild;
   }
-  assert(gReplayingChildren[id]);
   return gReplayingChildren[id];
 }
 
@@ -1263,6 +1262,7 @@ function resume(forward) {
       maybeResumeRecording();
       return;
     }
+    ensureFlushed();
   }
   if (
     gPausePoint.checkpoint == FirstCheckpointId &&
@@ -1827,7 +1827,12 @@ function Initialize(recordingChildId) {
 function ManifestFinished(id, response) {
   try {
     dumpv(`ManifestFinished #${id} ${stringify(response)}`);
-    lookupChild(id).manifestFinished(response);
+    const child = lookupChild(id);
+    if (child) {
+      child.manifestFinished(response);
+    } else {
+      // Ignore messages from child processes that we have marked as crashed.
+    }
   } catch (e) {
     dump(`ERROR: ManifestFinished threw exception: ${e} ${e.stack}\n`);
   }
@@ -2005,7 +2010,7 @@ const gControl = {
     return gDebuggerRequests;
   },
 
-  getPauseData() {
+  getPauseDataAndRepaint() {
     // If the child has not arrived at the pause point yet, see if there is
     // cached pause data for this point already which we can immediately return.
     if (gPauseMode == PauseModes.ARRIVING && !gDebuggerRequests.length) {
@@ -2014,11 +2019,21 @@ const gControl = {
         // After the child pauses, it will need to generate the pause data so
         // that any referenced objects will be instantiated.
         addDebuggerRequest({ type: "pauseData" });
+        RecordReplayControl.hadRepaint(data.paintData);
         return data;
       }
     }
     gControl.maybeSwitchToReplayingChild();
-    return gControl.sendRequest({ type: "pauseData" });
+    const data = gControl.sendRequest({ type: "pauseData" });
+    if (data.unhandledDivergence) {
+      RecordReplayControl.clearGraphics();
+    } else {
+      addPauseData(gPausePoint, data, /* trackCached */ true);
+      if (data.paintData) {
+        RecordReplayControl.hadRepaint(data.paintData);
+      }
+    }
+    return data;
   },
 
   paint(point) {
@@ -2028,35 +2043,6 @@ const gControl = {
     }
   },
 
-  repaint() {
-    if (!gPausePoint) {
-      return;
-    }
-    if (
-      gMainChild.paused &&
-      pointEquals(gPausePoint, gMainChild.pausePoint())
-    ) {
-      // Flush the recording if we are repainting because we interrupted things
-      // and will now rewind.
-      if (gMainChild.recording) {
-        ensureFlushed();
-      }
-      return;
-    }
-    const data = maybeGetPauseData(gPausePoint);
-    if (data && data.paintData) {
-      RecordReplayControl.hadRepaint(data.paintData);
-    } else {
-      gControl.maybeSwitchToReplayingChild();
-      const rv = gControl.sendRequest({ type: "repaint" });
-      if (rv && rv.length) {
-        RecordReplayControl.hadRepaint(rv);
-      } else {
-        RecordReplayControl.clearGraphics();
-      }
-    }
-  },
-
   isPausedAtDebuggerStatement() {
     const point = gControl.pausePoint();
     if (point) {

devtools/server/actors/replay/debugger.js

@@ -476,9 +476,6 @@ ReplayDebugger.prototype = {
       // There is no preferred direction of travel after an explicit pause.
       this._direction = Direction.NONE;
 
-      // Update graphics according to the current state of the child.
-      this._control.repaint();
-
       // If breakpoint handlers for the pause haven't been called yet, don't
       // call them at all.
       this._cancelPerformPause = true;
@@ -537,13 +534,14 @@ ReplayDebugger.prototype = {
   },
 
   // Fill in the debugger with (hopefully) all data the client/server need to
-  // pause at the current location.
+  // pause at the current location. This also updates graphics to match the
+  // current location.
   _capturePauseData() {
     if (this._pool.frames.length) {
       return;
     }
 
-    const pauseData = this._control.getPauseData();
+    const pauseData = this._control.getPauseDataAndRepaint();
     if (!pauseData.frames) {
       return;
     }

devtools/server/actors/replay/replay.js

@@ -366,8 +366,6 @@ Services.obs.addObserver(
         contents.arguments.forEach(v =>
           contents.argumentsData.addValue(v, PropertyLevels.FULL)
         );
-
-        ClearPausedState();
       }
 
       newConsoleMessage(contents);
@@ -911,12 +909,6 @@ function getDebuggeeValue(value) {
   return value;
 }
 
-// eslint-disable-next-line no-unused-vars
-function ClearPausedState() {
-  gPausedObjects = new IdMap();
-  gDereferencedObjects = new Map();
-}
-
 ///////////////////////////////////////////////////////////////////////////////
 // Manifest Management
 ///////////////////////////////////////////////////////////////////////////////
@@ -1015,7 +1007,6 @@ const gManifestStartHandlers = {
   getPauseData() {
     divergeFromRecording();
     const data = getPauseData();
-    data.paintData = RecordReplayControl.repaint();
     RecordReplayControl.manifestFinished(data);
   },
 
@@ -1034,12 +1025,7 @@ const gManifestStartHandlers = {
     const displayName = formatDisplayName(frame);
     const rv = frame.evalWithBindings(`[${text}]`, { displayName });
 
-    let pauseData;
-    if (!skipPauseData) {
-      pauseData = getPauseData();
-      pauseData.paintData = RecordReplayControl.repaint();
-      ClearPausedState();
-    }
+    const pauseData = skipPauseData ? undefined : getPauseData();
 
     let result;
     if (rv.return) {
@@ -1189,6 +1175,12 @@ function HitCheckpoint(id) {
   gLastCheckpoint = id;
   const point = currentExecutionPoint();
 
+  // Reset paused state at each checkpoint. In order to reach the checkpoint we
+  // must have unpaused, and resetting the state allows these objects to be
+  // collected by the GC.
+  gPausedObjects = new IdMap();
+  gDereferencedObjects = new Map();
+
   try {
     processManifestAfterCheckpoint(point);
   } catch (e) {
@@ -1778,13 +1770,16 @@ PreviewedObjects.prototype = {
 // as the server will end up needing to make more requests before the client can
 // finish pausing.
 function getPauseData() {
+  const paintData = RecordReplayControl.repaint();
+
   const numFrames = countScriptFrames();
   if (!numFrames) {
-    return {};
+    return { paintData };
   }
 
   const rv = new PreviewedObjects();
 
+  rv.paintData = paintData;
   rv.frames = [];
   rv.scripts = {};
   rv.offsetMetadata = [];