Bug 1608177 - Setting logpoints does not clean up breakpoint state properly, r=jlast.

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

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

devtools/client/webreplay/mochitest/browser_dbg_rr_logpoint-01.js

@@ -12,7 +12,8 @@ add_task(async function() {
   });
 
   await selectSource(dbg, "doc_rr_basic.html");
-  await addBreakpoint(dbg, "doc_rr_basic.html", 21, undefined, {
+  await addBreakpoint(dbg, "doc_rr_basic.html", 21);
+  await setBreakpointOptions(dbg, "doc_rr_basic.html", 21, undefined, {
     logValue: `"Logpoint Number " + number`,
   });
   await addBreakpoint(dbg, "doc_rr_basic.html", 6, undefined, {
@@ -43,5 +44,8 @@ add_task(async function() {
   await checkEvaluateInTopFrame(dbg, "number", 5);
   await reverseStepOverToLine(dbg, 20);
 
+  await addBreakpoint(dbg, "doc_rr_basic.html", 22);
+  await resumeToLine(dbg, 22);
+
   await shutdownDebugger(dbg);
 });

devtools/server/actors/breakpoint.js

@@ -91,6 +91,12 @@ BreakpointActor.prototype = {
    * Called on changes to this breakpoint's script offsets or options.
    */
   _newOffsetsOrOptions(script, offsets, oldOptions) {
+    // Clear any existing handler first in case this is called multiple times
+    // after options change.
+    for (const offset of offsets) {
+      script.clearBreakpoint(this, offset);
+    }
+
     // When replaying, logging breakpoints are handled using an API to get logged
     // messages from throughout the recording.
     if (this.threadActor.dbg.replaying && this.options.logGroupId) {
@@ -125,10 +131,7 @@ BreakpointActor.prototype = {
     }
 
     // In all other cases, this is used as a script breakpoint handler.
-    // Clear any existing handler first in case this is called multiple times
-    // after options change.
     for (const offset of offsets) {
-      script.clearBreakpoint(this, offset);
       script.setBreakpoint(offset, this);
     }
   },

devtools/server/actors/replay/control.js

@@ -2037,16 +2037,14 @@ const gControl = {
       RecordReplayControl.clearGraphics();
     } else {
       addPauseData(gPausePoint, data, /* trackCached */ true);
-      if (data.paintData) {
-        RecordReplayControl.hadRepaint(data.paintData);
-      }
+      RecordReplayControl.hadRepaint(data.paintData);
     }
     return data;
   },
 
   paint(point) {
     const data = maybeGetPauseData(point);
-    if (data && data.paintData) {
+    if (data) {
       RecordReplayControl.hadRepaint(data.paintData);
     }
   },

toolkit/recordreplay/ipc/JSControl.cpp

@@ -305,8 +305,10 @@ static bool Middleman_HadRepaint(JSContext* aCx, unsigned aArgc, Value* aVp) {
   CallArgs args = CallArgsFromVp(aArgc, aVp);
 
   if (!args.get(0).isString()) {
-    JS_ReportErrorASCII(aCx, "Bad arguments");
-    return false;
+    parent::ClearGraphics();
+
+    args.rval().setUndefined();
+    return true;
   }
 
   RootedString data(aCx, args.get(0).toString());