1545105 - Missing breakpoints should be removed

Status: RESOLVED INVALID

(DevTools :: Debugger, defect, P3)

Description

[Jason Laster [:jlast]]

The debugger naively sets pending breakpoints in the server assuming that if the breakpoint is no longer relevant there will be a chance to remove it via syncBreakpoint. At the moment though, we only remove breakpoints if there is no generated location AND the locations do not match. We might want to consider always removing the breakpoint if the generated location longer exists.

@@ -153,16 +179,17 @@ export function syncBreakpoint(
       newLocation

     if (!newGeneratedLocation) {
-      // We couldn't find a new mapping for the breakpoint. If there is a source
-      // mapping, remove any breakpoints for the generated location, as if the
-      // breakpoint moved. If the old generated location still maps to an
-      // original location then we don't want to add a breakpoint for it.
-      if (location.sourceUrl != generatedLocation.sourceUrl) {
-        dispatch(
-          removeBreakpointAtGeneratedLocation(cx, sourceGeneratedLocation)
-        );
-      }
+      // if (location.sourceUrl != generatedLocation.sourceUrl) {
+      dispatch(
+        removeBreakpointAtGeneratedLocation(cx, sourceGeneratedLocation)
+      );
+      // }
       return;
     }

Comment 1

[Jason Laster [:jlast]]

Brian, what do you think about this?

This bug is related to 1545103, which I just pushed to lando. So hopefully the zombie case is fixed, but I think we should still consider being more aggressive with syncing.

Comment 2

[Brian Hackett [Laid off!]]

Sure, this should be fine to do.

Comment 3

[Jason Laster [:jlast]]

Attachment: Bug 1545105 - Missing breakpoints should be removed. r=bhackett

Comment 4

[Jason Laster [:jlast]]

we decided we do not want to do this