Merge mozilla-central to autoland on a CLOSED TREE

devtools/client/debugger/new/src/actions/breakpoints/index.js

@@ -180,13 +180,21 @@ export function removeBreakpointsInSource(source: Source) {
 
 export function remapBreakpoints(sourceId: string) {
   return async ({ dispatch, getState, sourceMaps }: ThunkArgs) => {
-    const breakpoints = getBreakpointsList(getState());
+    const breakpoints = getBreakpointsForSource(getState(), sourceId);
     const newBreakpoints = await remapLocations(
       breakpoints,
       sourceId,
       sourceMaps
     );
 
+    // Normally old breakpoints will be clobbered if we re-add them, but when
+    // remapping we have changed the source maps and the old breakpoints will
+    // have different locations than the new ones. Manually remove the
+    // old breakpoints before adding the new ones.
+    for (const bp of breakpoints) {
+      dispatch(removeBreakpoint(bp));
+    }
+
     for (const bp of newBreakpoints) {
       await dispatch(addBreakpoint(bp.location, bp.options, bp.disabled));
     }

devtools/client/debugger/new/src/actions/breakpoints/modify.js

@@ -10,19 +10,21 @@ import {
   getASTLocation
 } from "../../utils/breakpoint";
 
-import { getTextAtPosition } from "../../utils/source";
-
 import {
   getBreakpoint,
   getBreakpointPositionsForLocation,
   getFirstBreakpointPosition,
-  getSymbols
+  getSymbols,
+  getSource,
+  getBreakpointsList,
+  getPendingBreakpointList
 } from "../../selectors";
 
-import { loadSourceById } from "../sources/loadSourceText";
 import { setBreakpointPositions } from "./breakpointPositions";
 
 import { recordEvent } from "../../utils/telemetry";
+import { comparePosition } from "../../utils/location";
+import { getTextAtPosition } from "../../utils/source";
 
 import type { ThunkArgs } from "../types";
 import type {
@@ -66,11 +68,11 @@ function clientSetBreakpoint(breakpoint: Breakpoint) {
   };
 }
 
-function clientRemoveBreakpoint(breakpoint: Breakpoint) {
+function clientRemoveBreakpoint(generatedLocation: SourceLocation) {
   return ({ getState, client }: ThunkArgs) => {
     const breakpointLocation = makeBreakpointLocation(
       getState(),
-      breakpoint.generatedLocation
+      generatedLocation
     );
     return client.removeBreakpoint(breakpointLocation);
   };
@@ -95,7 +97,8 @@ export function enableBreakpoint(initialBreakpoint: Breakpoint) {
 export function addBreakpoint(
   initialLocation: SourceLocation,
   options: BreakpointOptions = {},
-  disabled: boolean = false
+  disabled: boolean = false,
+  shouldCancel: () => boolean = () => false
 ) {
   return async ({ dispatch, getState, sourceMaps, client }: ThunkArgs) => {
     recordEvent("add_breakpoint");
@@ -113,16 +116,16 @@ export function addBreakpoint(
     }
 
     const { location, generatedLocation } = position;
-    // Both the original and generated sources must be loaded to get the
-    // breakpoint's text.
 
-    const source = await dispatch(loadSourceById(sourceId));
-    const generatedSource = await dispatch(
-      loadSourceById(generatedLocation.sourceId)
-    );
+    const source = getSource(getState(), location.sourceId);
+    const generatedSource = getSource(getState(), generatedLocation.sourceId);
+
+    if (!source || !generatedSource) {
+      return;
+    }
 
     const symbols = getSymbols(getState(), source);
-    const astLocation = await getASTLocation(source, symbols, location);
+    const astLocation = getASTLocation(source, symbols, location);
 
     const originalText = getTextAtPosition(source, location);
     const text = getTextAtPosition(generatedSource, generatedLocation);
@@ -139,16 +142,8 @@ export function addBreakpoint(
       originalText
     };
 
-    // There cannot be multiple breakpoints with the same generated location.
-    // Because a generated location cannot map to multiple original locations,
-    // the only breakpoints that can map to this generated location have the
-    // new breakpoint's |location| or |generatedLocation| as their own
-    // |location|. We will overwrite any breakpoint at |location| with the
-    // SET_BREAKPOINT action below, but need to manually remove any breakpoint
-    // at |generatedLocation|.
-    const generatedId = makeBreakpointId(breakpoint.generatedLocation);
-    if (id != generatedId && getBreakpoint(getState(), generatedLocation)) {
-      dispatch({ type: "REMOVE_BREAKPOINT", location: generatedLocation });
+    if (shouldCancel()) {
+      return;
     }
 
     dispatch({ type: "SET_BREAKPOINT", breakpoint });
@@ -156,7 +151,7 @@ export function addBreakpoint(
     if (disabled) {
       // If we just clobbered an enabled breakpoint with a disabled one, we need
       // to remove any installed breakpoint in the server.
-      return dispatch(clientRemoveBreakpoint(breakpoint));
+      return dispatch(clientRemoveBreakpoint(generatedLocation));
     }
 
     return dispatch(clientSetBreakpoint(breakpoint));
@@ -188,7 +183,49 @@ export function removeBreakpoint(initialBreakpoint: Breakpoint) {
       return;
     }
 
-    return dispatch(clientRemoveBreakpoint(breakpoint));
+    return dispatch(clientRemoveBreakpoint(breakpoint.generatedLocation));
+  };
+}
+
+/**
+ * Remove all installed, pending, and client breakpoints associated with a
+ * target generated location.
+ *
+ * @memberof actions/breakpoints
+ * @static
+ */
+export function removeBreakpointAtGeneratedLocation(target: SourceLocation) {
+  return ({ dispatch, getState, client }: ThunkArgs) => {
+    // Remove any breakpoints matching the generated location.
+    const breakpoints = getBreakpointsList(getState());
+    for (const { location, generatedLocation } of breakpoints) {
+      if (
+        generatedLocation.sourceId == target.sourceId &&
+        comparePosition(generatedLocation, target)
+      ) {
+        dispatch({
+          type: "REMOVE_BREAKPOINT",
+          location
+        });
+      }
+    }
+
+    // Remove any remaining pending breakpoints matching the generated location.
+    const pending = getPendingBreakpointList(getState());
+    for (const { location, generatedLocation } of pending) {
+      if (
+        generatedLocation.sourceUrl == target.sourceUrl &&
+        comparePosition(generatedLocation, target)
+      ) {
+        dispatch({
+          type: "REMOVE_PENDING_BREAKPOINT",
+          location
+        });
+      }
+    }
+
+    // Remove the breakpoint from the client itself.
+    return dispatch(clientRemoveBreakpoint(target));
   };
 }
 
@@ -210,7 +247,7 @@ export function disableBreakpoint(initialBreakpoint: Breakpoint) {
       breakpoint: { ...breakpoint, disabled: true }
     });
 
-    return dispatch(clientRemoveBreakpoint(breakpoint));
+    return dispatch(clientRemoveBreakpoint(breakpoint.generatedLocation));
   };
 }
 

devtools/client/debugger/new/src/actions/breakpoints/syncBreakpoint.js

@@ -16,8 +16,8 @@ import {
 import { comparePosition, createLocation } from "../../utils/location";
 
 import { originalToGeneratedId, isOriginalId } from "devtools-source-map";
-import { getSource, getBreakpoint } from "../../selectors";
-import { removeBreakpoint, addBreakpoint } from ".";
+import { getSource } from "../../selectors";
+import { addBreakpoint, removeBreakpointAtGeneratedLocation } from ".";
 
 import type { ThunkArgs } from "../types";
 import type { LoadedSymbols } from "../../reducers/types";
@@ -115,25 +115,19 @@ export function syncBreakpoint(
       location.sourceUrl != generatedLocation.sourceUrl
     ) {
       // We are handling the generated source and the pending breakpoint has a
-      // source mapping. Watch out for the case when the original source has
-      // already been processed, in which case either a breakpoint has already
-      // been added at this generated location or the client breakpoint has been
-      // removed.
+      // source mapping. Supply a cancellation callback that will abort the
+      // breakpoint if the original source was synced to a different location,
+      // in which case the client breakpoint has been removed.
       const breakpointLocation = makeBreakpointLocation(
         getState(),
         sourceGeneratedLocation
       );
-      if (
-        getBreakpoint(getState(), sourceGeneratedLocation) ||
-        !client.hasBreakpoint(breakpointLocation)
-      ) {
-        return;
-      }
       return dispatch(
         addBreakpoint(
           sourceGeneratedLocation,
           pendingBreakpoint.options,
-          pendingBreakpoint.disabled
+          pendingBreakpoint.disabled,
+          () => !client.hasBreakpoint(breakpointLocation)
         )
       );
     }
@@ -153,6 +147,13 @@ export function syncBreakpoint(
     );
 
     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(sourceGeneratedLocation));
+      }
       return;
     }
 
@@ -163,20 +164,9 @@ export function syncBreakpoint(
 
     // If the new generated location has changed from that in the pending
     // breakpoint, remove any breakpoint associated with the old generated
-    // location. This could either be in the reducer or only in the client,
-    // depending on whether the pending breakpoint has been processed for the
-    // generated source yet.
+    // location.
     if (!isSameLocation) {
-      const bp = getBreakpoint(getState(), sourceGeneratedLocation);
-      if (bp) {
-        dispatch(removeBreakpoint(bp));
-      } else {
-        const breakpointLocation = makeBreakpointLocation(
-          getState(),
-          sourceGeneratedLocation
-        );
-        client.removeBreakpoint(breakpointLocation);
-      }
+      dispatch(removeBreakpointAtGeneratedLocation(sourceGeneratedLocation));
     }
 
     return dispatch(

devtools/client/debugger/new/src/actions/sources/loadSourceText.js

@@ -9,9 +9,10 @@ import {
   getSource,
   getSourceFromId,
   getGeneratedSource,
-  getSourcesEpoch
+  getSourcesEpoch,
+  getBreakpointsForSource
 } from "../../selectors";
-import { setBreakpointPositions } from "../breakpoints";
+import { setBreakpointPositions, addBreakpoint } from "../breakpoints";
 
 import { prettyPrintSource } from "./prettyPrint";
 
@@ -92,6 +93,12 @@ async function loadSourceTextPromise(
   if (!newSource.isWasm && isLoaded(newSource)) {
     parser.setSource(newSource);
     dispatch(setBreakpointPositions({ sourceId: newSource.id }));
+
+    // Update the text in any breakpoints for this source by re-adding them.
+    const breakpoints = getBreakpointsForSource(getState(), source.id);
+    for (const { location, options, disabled } of breakpoints) {
+      await dispatch(addBreakpoint(location, options, disabled));
+    }
   }
 
   return newSource;

devtools/client/debugger/new/src/actions/sources/prettyPrint.js

@@ -136,13 +136,13 @@ export function togglePrettyPrint(sourceId: string) {
     const newPrettySource = await dispatch(createPrettySource(sourceId));
     await dispatch(selectPrettyLocation(newPrettySource));
 
-    await dispatch(remapBreakpoints(sourceId));
-
     const threads = getSourceThreads(getState(), source);
     await Promise.all(threads.map(thread => dispatch(mapFrames(thread))));
 
     await dispatch(setSymbols({ source: newPrettySource }));
 
+    await dispatch(remapBreakpoints(sourceId));
+
     return newPrettySource;
   };
 }

devtools/client/debugger/new/src/actions/sources/tests/loadSource.spec.js

@@ -85,8 +85,15 @@ describe("loadSourceText", () => {
     };
     await dispatch(actions.addBreakpoint(location, {}));
 
+    const breakpoint = getBreakpointsList(getState())[0];
+
+    expect(breakpoint.text).toBe("");
+    expect(breakpoint.originalText).toBe("");
+
+    await dispatch(actions.loadSourceText({ source: fooOrigSource }));
+
     const breakpoint1 = getBreakpointsList(getState())[0];
-    expect(breakpoint1.text).toBe("var fooGen = 42;");
+    expect(breakpoint1.text).toBe("");
     expect(breakpoint1.originalText).toBe("var fooOrig = 42;");
 
     await dispatch(actions.loadSourceText({ source: fooGenSource }));

devtools/client/debugger/new/src/actions/types/BreakpointAction.js

@@ -9,7 +9,8 @@ import type {
   SourceLocation,
   XHRBreakpoint,
   Source,
-  BreakpointPositions
+  BreakpointPositions,
+  PendingLocation
 } from "../../types";
 
 import type { PromiseAction } from "../utils/middleware/promise";
@@ -47,6 +48,10 @@ export type BreakpointAction =
       +type: "REMOVE_BREAKPOINT",
       +location: SourceLocation
     |}
+  | {|
+      +type: "REMOVE_PENDING_BREAKPOINT",
+      +location: PendingLocation
+    |}
   | {|
       type: "ADD_BREAKPOINT_POSITIONS",
       positions: BreakpointPositions,

devtools/client/debugger/new/src/reducers/pending-breakpoints.js

@@ -29,6 +29,7 @@ function update(state: PendingBreakpointsState = {}, action: Action) {
       return setBreakpoint(state, action);
 
     case "REMOVE_BREAKPOINT":
+    case "REMOVE_PENDING_BREAKPOINT":
       return removeBreakpoint(state, action);
   }
 

devtools/client/debugger/new/src/utils/breakpoint/index.js

@@ -49,7 +49,9 @@ export function getLocationWithoutColumn(location: SourceLocation) {
   return `${sourceId}:${line}`;
 }
 
-export function makePendingLocationId(location: SourceLocation) {
+type AnySourceLocation = SourceLocation | PendingLocation;
+
+export function makePendingLocationId(location: AnySourceLocation) {
   assertPendingLocation(location);
   const { sourceUrl, line, column } = location;
   const sourceUrlString = sourceUrl || "";

gfx/2d/PathCapture.cpp

@@ -104,7 +104,7 @@ already_AddRefed<PathBuilder> PathCapture::TransformedCopyToBuilder(
         }
         void LineTo(const Point &aPoint) {
           PathOp newPathOp;
-          newPathOp.mType = PathOp::OP_BEZIERTO;
+          newPathOp.mType = PathOp::OP_LINETO;
           newPathOp.mP1 = mTransform->TransformPoint(aPoint);
           mVector->push_back(newPathOp);
         }

js/src/gdb/tests/test-asmjs.cpp

@@ -4,15 +4,13 @@
 #include "js/CompileOptions.h"
 #include "js/RootingAPI.h"
 #include "js/Value.h"
+#include "mozilla/ArrayUtils.h"
 
 #include <string.h>
 
 FRAGMENT(asmjs, segfault) {
-  using namespace JS;
-
-  int line0 = __LINE__;
-  const char* bytes =
-      "\n"
+  constexpr unsigned line0 = __LINE__;
+  static const char chars[] =
       "function f(glob, ffi, heap) {\n"
       "    \"use asm\";\n"
       "    var f32 = new glob.Float32Array(heap);\n"
@@ -27,12 +25,13 @@ FRAGMENT(asmjs, segfault) {
       "func(0x10000 << 2);\n"
       "'ok'\n";
 
-  CompileOptions opts(cx);
+  JS::CompileOptions opts(cx);
   opts.setFileAndLine(__FILE__, line0 + 1);
   opts.asmJSOption = JS::AsmJSOption::Enabled;
 
-  Rooted<Value> rval(cx);
-  bool ok = JS::EvaluateUtf8(cx, opts, bytes, strlen(bytes), &rval);
+  JS::Rooted<JS::Value> rval(cx);
+  bool ok = JS::EvaluateUtf8(cx, opts, chars, mozilla::ArrayLength(chars) - 1,
+                             &rval);
 
   breakpoint();