Merge mozilla-central to autoland on a CLOSED TREE
authorAndreea Pavel <apavel@mozilla.com>
Sat, 06 Apr 2019 14:06:42 +0300
changeset 468270 ae30a382f499166b8f4037b0221799571aaefdaf
parent 468269 557f1dbc18ca494c059404dae75f8729763616a4 (current diff)
parent 468268 946786d08aac97761d6588669f8a1c90ace1aaf1 (diff)
child 468271 968c536eff6e86e0b429e730845c00c41f22ebaf
push id35826
push usernerli@mozilla.com
push dateSat, 06 Apr 2019 21:48:00 +0000
treeherdermozilla-central@dc53fe5c9ced [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
milestone68.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Merge mozilla-central to autoland on a CLOSED TREE
--- a/devtools/client/debugger/new/src/actions/breakpoints/index.js
+++ b/devtools/client/debugger/new/src/actions/breakpoints/index.js
@@ -175,23 +175,31 @@ export function removeBreakpointsInSourc
     for (const breakpoint of breakpoints) {
       dispatch(removeBreakpoint(breakpoint));
     }
   };
 }
 
 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));
     }
   };
 }
 
 export function toggleBreakpointAtLine(line: number) {
   return ({ dispatch, getState, client, sourceMaps }: ThunkArgs) => {
--- a/devtools/client/debugger/new/src/actions/breakpoints/modify.js
+++ b/devtools/client/debugger/new/src/actions/breakpoints/modify.js
@@ -5,29 +5,31 @@
 // @flow
 
 import {
   makeBreakpointLocation,
   makeBreakpointId,
   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 {
   Breakpoint,
   BreakpointOptions,
   BreakpointPosition,
   SourceLocation
 } from "../../types";
@@ -61,21 +63,21 @@ function clientSetBreakpoint(breakpoint:
     const breakpointLocation = makeBreakpointLocation(
       getState(),
       breakpoint.generatedLocation
     );
     return client.setBreakpoint(breakpointLocation, breakpoint.options);
   };
 }
 
-function clientRemoveBreakpoint(breakpoint: Breakpoint) {
+function clientRemoveBreakpoint(generatedLocation: SourceLocation) {
   return ({ getState, client }: ThunkArgs) => {
     const breakpointLocation = makeBreakpointLocation(
       getState(),
-      breakpoint.generatedLocation
+      generatedLocation
     );
     return client.removeBreakpoint(breakpointLocation);
   };
 }
 
 export function enableBreakpoint(initialBreakpoint: Breakpoint) {
   return async ({ dispatch, getState, client, sourceMaps }: ThunkArgs) => {
     const breakpoint = getBreakpoint(getState(), initialBreakpoint.location);
@@ -90,17 +92,18 @@ export function enableBreakpoint(initial
 
     return dispatch(clientSetBreakpoint(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");
 
     const { sourceId, column } = initialLocation;
 
     await dispatch(setBreakpointPositions({ sourceId }));
 
@@ -108,60 +111,52 @@ export function addBreakpoint(
       ? getBreakpointPositionsForLocation(getState(), initialLocation)
       : getFirstBreakpointPosition(getState(), initialLocation);
 
     if (!position) {
       return;
     }
 
     const { location, generatedLocation } = position;
-    // Both the original and generated sources must be loaded to get the
-    // breakpoint's text.
+
+    const source = getSource(getState(), location.sourceId);
+    const generatedSource = getSource(getState(), generatedLocation.sourceId);
 
-    const source = await dispatch(loadSourceById(sourceId));
-    const generatedSource = await dispatch(
-      loadSourceById(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);
 
     const id = makeBreakpointId(location);
     const breakpoint = {
       id,
       disabled,
       options,
       location,
       astLocation,
       generatedLocation,
       text,
       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 });
 
     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));
   };
 }
 
 /**
  * Remove a single breakpoint
@@ -183,17 +178,59 @@ export function removeBreakpoint(initial
       location: breakpoint.location
     });
 
     // If the breakpoint is disabled then it is not installed in the server.
     if (breakpoint.disabled) {
       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));
   };
 }
 
 /**
  * Disable a single breakpoint
  *
  * @memberof actions/breakpoints
  * @static
@@ -205,17 +242,17 @@ export function disableBreakpoint(initia
       return;
     }
 
     dispatch({
       type: "SET_BREAKPOINT",
       breakpoint: { ...breakpoint, disabled: true }
     });
 
-    return dispatch(clientRemoveBreakpoint(breakpoint));
+    return dispatch(clientRemoveBreakpoint(breakpoint.generatedLocation));
   };
 }
 
 /**
  * Update the options of a breakpoint.
  *
  * @throws {Error} "not implemented"
  * @memberof actions/breakpoints
--- a/devtools/client/debugger/new/src/actions/breakpoints/syncBreakpoint.js
+++ b/devtools/client/debugger/new/src/actions/breakpoints/syncBreakpoint.js
@@ -11,18 +11,18 @@ import {
   findFunctionByName,
   findPosition,
   makeBreakpointLocation
 } from "../../utils/breakpoint";
 
 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";
 
 import type {
   SourceLocation,
   ASTLocation,
   PendingBreakpoint,
@@ -110,35 +110,29 @@ export function syncBreakpoint(
       sourceId: generatedSourceId
     });
 
     if (
       source == generatedSource &&
       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)
         )
       );
     }
 
     const previousLocation = { ...location, sourceId };
 
     const newLocation = await findNewLocation(
       astLocation,
@@ -148,40 +142,36 @@ export function syncBreakpoint(
     );
 
     const newGeneratedLocation = await findBreakpointPosition(
       thunkArgs,
       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(sourceGeneratedLocation));
+      }
       return;
     }
 
     const isSameLocation = comparePosition(
       generatedLocation,
       newGeneratedLocation
     );
 
     // 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(
       addBreakpoint(
         newLocation,
         pendingBreakpoint.options,
         pendingBreakpoint.disabled
       )
--- a/devtools/client/debugger/new/src/actions/sources/loadSourceText.js
+++ b/devtools/client/debugger/new/src/actions/sources/loadSourceText.js
@@ -4,19 +4,20 @@
 
 // @flow
 
 import { PROMISE } from "../utils/middleware/promise";
 import {
   getSource,
   getSourceFromId,
   getGeneratedSource,
-  getSourcesEpoch
+  getSourcesEpoch,
+  getBreakpointsForSource
 } from "../../selectors";
-import { setBreakpointPositions } from "../breakpoints";
+import { setBreakpointPositions, addBreakpoint } from "../breakpoints";
 
 import { prettyPrintSource } from "./prettyPrint";
 
 import * as parser from "../../workers/parser";
 import { isLoaded, isOriginal, isPretty } from "../../utils/source";
 import {
   memoizeableAction,
   type MemoizedAction
@@ -87,16 +88,22 @@ async function loadSourceTextPromise(
 
   if (!newSource) {
     return;
   }
 
   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;
 }
 
 export function loadSourceById(sourceId: string) {
   return ({ getState, dispatch }: ThunkArgs) => {
     const source = getSourceFromId(getState(), sourceId);
--- a/devtools/client/debugger/new/src/actions/sources/prettyPrint.js
+++ b/devtools/client/debugger/new/src/actions/sources/prettyPrint.js
@@ -131,18 +131,18 @@ export function togglePrettyPrint(source
 
     if (prettySource) {
       return dispatch(selectPrettyLocation(prettySource));
     }
 
     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;
   };
 }
--- a/devtools/client/debugger/new/src/actions/sources/tests/loadSource.spec.js
+++ b/devtools/client/debugger/new/src/actions/sources/tests/loadSource.spec.js
@@ -80,18 +80,25 @@ describe("loadSourceText", () => {
 
     const location = {
       sourceId: fooOrigSource.id,
       line: 1,
       column: 0
     };
     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 }));
 
     const breakpoint2 = getBreakpointsList(getState())[0];
     expect(breakpoint2.text).toBe("var fooGen = 42;");
     expect(breakpoint2.originalText).toBe("var fooOrig = 42;");
   });
--- a/devtools/client/debugger/new/src/actions/types/BreakpointAction.js
+++ b/devtools/client/debugger/new/src/actions/types/BreakpointAction.js
@@ -4,17 +4,18 @@
 
 // @flow
 
 import type {
   Breakpoint,
   SourceLocation,
   XHRBreakpoint,
   Source,
-  BreakpointPositions
+  BreakpointPositions,
+  PendingLocation
 } from "../../types";
 
 import type { PromiseAction } from "../utils/middleware/promise";
 
 export type BreakpointAction =
   | PromiseAction<{|
       +type: "SET_XHR_BREAKPOINT",
       +breakpoint: XHRBreakpoint
@@ -43,12 +44,16 @@ export type BreakpointAction =
       +type: "SET_BREAKPOINT",
       +breakpoint: Breakpoint
     |}
   | {|
       +type: "REMOVE_BREAKPOINT",
       +location: SourceLocation
     |}
   | {|
+      +type: "REMOVE_PENDING_BREAKPOINT",
+      +location: PendingLocation
+    |}
+  | {|
       type: "ADD_BREAKPOINT_POSITIONS",
       positions: BreakpointPositions,
       source: Source
     |};
--- a/devtools/client/debugger/new/src/reducers/pending-breakpoints.js
+++ b/devtools/client/debugger/new/src/reducers/pending-breakpoints.js
@@ -24,16 +24,17 @@ import type { Action } from "../actions/
 export type PendingBreakpointsState = { [string]: PendingBreakpoint };
 
 function update(state: PendingBreakpointsState = {}, action: Action) {
   switch (action.type) {
     case "SET_BREAKPOINT":
       return setBreakpoint(state, action);
 
     case "REMOVE_BREAKPOINT":
+    case "REMOVE_PENDING_BREAKPOINT":
       return removeBreakpoint(state, action);
   }
 
   return state;
 }
 
 function setBreakpoint(state, { breakpoint }) {
   if (breakpoint.options.hidden) {
--- a/devtools/client/debugger/new/src/utils/breakpoint/index.js
+++ b/devtools/client/debugger/new/src/utils/breakpoint/index.js
@@ -44,17 +44,19 @@ export function makeBreakpointId(locatio
   return `${sourceId}:${line}:${columnString}`;
 }
 
 export function getLocationWithoutColumn(location: SourceLocation) {
   const { sourceId, line } = location;
   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 || "";
   const columnString = column || "";
 
   return `${sourceUrlString}:${line}:${columnString}`;
 }
 
--- a/gfx/2d/PathCapture.cpp
+++ b/gfx/2d/PathCapture.cpp
@@ -99,17 +99,17 @@ already_AddRefed<PathBuilder> PathCaptur
           newPathOp.mType = PathOp::OP_BEZIERTO;
           newPathOp.mP1 = mTransform->TransformPoint(aCP1);
           newPathOp.mP2 = mTransform->TransformPoint(aCP2);
           newPathOp.mP3 = mTransform->TransformPoint(aCP3);
           mVector->push_back(newPathOp);
         }
         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);
         }
         pathOpVec *mVector;
         const Matrix *mTransform;
       };
 
       ArcTransformer arcTransformer(capture->mPathOps, aTransform);
--- a/js/src/gdb/tests/test-asmjs.cpp
+++ b/js/src/gdb/tests/test-asmjs.cpp
@@ -1,41 +1,40 @@
 #include "gdb-tests.h"
 #include "jsapi.h"
 #include "js/CompilationAndEvaluation.h"
 #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"
       "    function g(n) {\n"
       "        n = n | 0;\n"
       "        return +f32[n>>2];\n"
       "    }\n"
       "    return g;\n"
       "}\n"
       "\n"
       "var func = f(this, null, new ArrayBuffer(0x10000));\n"
       "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();
 
   use(ok);
   use(rval);
 }