Backed out 3 changesets (bug 1534847) for failing debugger on a CLOSED TREE
authorAndreea Pavel <apavel@mozilla.com>
Fri, 15 Mar 2019 00:30:59 +0200
changeset 524978 fa6af8c14ee45a20df4c3f83286b85874082f877
parent 524977 5be153ff04f4d6f9d73ef72063fe3b471727f580
child 524979 33f142760b696ee6923fa6630d6878a2121481cc
push id2032
push userffxbld-merge
push dateMon, 13 May 2019 09:36:57 +0000
treeherdermozilla-release@455c1065dcbe [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1534847
milestone67.0a1
backs outee3da141383532c5f82be55ea74fc5e0b4622ec8
27a3400989c9bce15e4c9b1422637067e34e7c6c
7cf6462189e8d4834052afa8fb9201744c5df56a
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
Backed out 3 changesets (bug 1534847) for failing debugger on a CLOSED TREE Backed out changeset ee3da1413835 (bug 1534847) Backed out changeset 27a3400989c9 (bug 1534847) Backed out changeset 7cf6462189e8 (bug 1534847)
devtools/client/debugger/new/src/actions/sources/loadSourceText.js
devtools/client/debugger/new/src/actions/types/SourceAction.js
devtools/client/debugger/new/src/reducers/sources.js
--- a/devtools/client/debugger/new/src/actions/sources/loadSourceText.js
+++ b/devtools/client/debugger/new/src/actions/sources/loadSourceText.js
@@ -1,127 +1,97 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at <http://mozilla.org/MPL/2.0/>. */
 
 // @flow
 
 import { PROMISE } from "../utils/middleware/promise";
-import { getSource, getSourcesEpoch } from "../../selectors";
+import { getSource } from "../../selectors";
 import { setBreakpointPositions } from "../breakpoints";
 
 import * as parser from "../../workers/parser";
 import { isLoaded, isOriginal } from "../../utils/source";
 import { Telemetry } from "devtools-modules";
 
+import defer from "../../utils/defer";
 import type { ThunkArgs } from "../types";
 
 import type { Source } from "../../types";
 
 const requests = new Map();
 
 // Measures the time it takes for a source to load
 const loadSourceHistogram = "DEVTOOLS_DEBUGGER_LOAD_SOURCE_MS";
 const telemetry = new Telemetry();
 
-async function loadSource(
-  state,
-  source: Source,
-  { sourceMaps, client }
-): Promise<?{
-  text: string,
-  contentType: string
-}> {
+async function loadSource(state, source: Source, { sourceMaps, client }) {
   if (isOriginal(source)) {
-    const result = await sourceMaps.getOriginalSourceText(source);
-    if (!result) {
-      // TODO: This allows pretty files to continue working the way they have
-      // been, but is very ugly. Remove this when we centralize pretty-printing
-      // in loadSource. https://github.com/firefox-devtools/debugger/issues/8071
-      if (source.isPrettyPrinted) {
-        return null;
-      }
-
-      // The way we currently try to load and select a pending selected location,
-      // it is possible that we will try to fetch original source text right after
-      // the source map has been cleared after a navigation event.
-      throw new Error("Original source text unavailable");
-    }
-    return result;
+    return sourceMaps.getOriginalSourceText(source);
   }
 
   if (!source.actors.length) {
     throw new Error("No source actor for loadSource");
   }
 
-  telemetry.start(loadSourceHistogram, source);
   const response = await client.sourceContents(source.actors[0]);
   telemetry.finish(loadSourceHistogram, source);
 
   return {
+    id: source.id,
     text: response.source,
     contentType: response.contentType || "text/javascript"
   };
 }
 
-async function loadSourceTextPromise(
-  source: Source,
-  epoch: number,
-  { dispatch, getState, client, sourceMaps }: ThunkArgs
-): Promise<?Source> {
-  if (isLoaded(source)) {
-    return source;
-  }
-
-  await dispatch({
-    type: "LOAD_SOURCE_TEXT",
-    sourceId: source.id,
-    epoch,
-    [PROMISE]: loadSource(getState(), source, { sourceMaps, client })
-  });
-
-  const newSource = getSource(getState(), source.id);
-  if (!newSource) {
-    return;
-  }
-
-  if (!newSource.isWasm && isLoaded(newSource)) {
-    parser.setSource(newSource);
-    await dispatch(setBreakpointPositions(newSource.id));
-  }
-
-  return newSource;
-}
-
 /**
  * @memberof actions/sources
  * @static
  */
-export function loadSourceText(inputSource: ?Source) {
-  return async (thunkArgs: ThunkArgs) => {
-    if (!inputSource) {
+export function loadSourceText(source: ?Source) {
+  return async ({ dispatch, getState, client, sourceMaps }: ThunkArgs) => {
+    if (!source) {
       return;
     }
-    // This ensures that the falsy check above is preserved into the IIFE
-    // below in a way that Flow is happy with.
-    const source = inputSource;
-
-    const epoch = getSourcesEpoch(thunkArgs.getState());
 
-    const id = `${epoch}:${source.id}`;
-    let promise = requests.get(id);
-    if (!promise) {
-      promise = (async () => {
-        try {
-          return await loadSourceTextPromise(source, epoch, thunkArgs);
-        } catch (e) {
-          // TODO: This swallows errors for now. Ideally we would get rid of
-          // this once we have a better handle on our async state management.
-        } finally {
-          requests.delete(id);
-        }
-      })();
-      requests.set(id, promise);
+    const id = source.id;
+    // Fetch the source text only once.
+    if (requests.has(id)) {
+      return requests.get(id);
+    }
+
+    if (isLoaded(source)) {
+      return Promise.resolve();
     }
 
-    return promise;
+    const deferred = defer();
+    requests.set(id, deferred.promise);
+
+    telemetry.start(loadSourceHistogram, source);
+    try {
+      await dispatch({
+        type: "LOAD_SOURCE_TEXT",
+        sourceId: source.id,
+        [PROMISE]: loadSource(getState(), source, { sourceMaps, client })
+      });
+    } catch (e) {
+      deferred.resolve();
+      requests.delete(id);
+      return;
+    }
+
+    const newSource = getSource(getState(), source.id);
+    if (!newSource) {
+      return;
+    }
+
+    if (!newSource.isWasm && isLoaded(newSource)) {
+      parser.setSource(newSource);
+      await dispatch(setBreakpointPositions(newSource.id));
+    }
+
+    // signal that the action is finished
+    deferred.resolve();
+    requests.delete(id);
+
+    return source;
   };
 }
--- a/devtools/client/debugger/new/src/actions/types/SourceAction.js
+++ b/devtools/client/debugger/new/src/actions/types/SourceAction.js
@@ -5,18 +5,17 @@
 // @flow
 
 import type { Source, SourceLocation } from "../../types";
 import type { PromiseAction } from "../utils/middleware/promise";
 
 export type LoadSourceAction = PromiseAction<
   {|
     +type: "LOAD_SOURCE_TEXT",
-    +sourceId: string,
-    +epoch: number
+    +sourceId: string
   |},
   Source
 >;
 export type SourceAction =
   | LoadSourceAction
   | {|
       +type: "ADD_SOURCE",
       +source: Source
--- a/devtools/client/debugger/new/src/reducers/sources.js
+++ b/devtools/client/debugger/new/src/reducers/sources.js
@@ -31,18 +31,16 @@ import { mapValues, uniqBy, uniq } from 
 export type SourcesMap = { [SourceId]: Source };
 export type SourcesMapByThread = { [ThreadId]: SourcesMap };
 
 type UrlsMap = { [string]: SourceId[] };
 type DisplayedSources = { [ThreadId]: { [SourceId]: boolean } };
 type GetDisplayedSourcesSelector = OuterState => { [ThreadId]: SourcesMap };
 
 export type SourcesState = {
-  epoch: number,
-
   // All known sources.
   sources: SourcesMap,
 
   // All sources associated with a given URL. When using source maps, multiple
   // sources can have the same URL.
   urls: UrlsMap,
 
   // For each thread, all sources in that thread that are under the project root
@@ -60,17 +58,16 @@ const emptySources = {
   sources: {},
   urls: {},
   displayed: {}
 };
 
 export function initialSourcesState(): SourcesState {
   return {
     ...emptySources,
-    epoch: 1,
     selectedLocation: undefined,
     pendingSelectedLocation: prefs.pendingSelectedLocation,
     projectDirectoryRoot: prefs.projectDirectoryRoot,
     chromeAndExtenstionsEnabled: prefs.chromeAndExtenstionsEnabled,
     focusedItem: null
   };
 }
 
@@ -162,20 +159,17 @@ function update(
         return updateSource(state, { id, isBlackBoxed });
       }
       break;
 
     case "SET_PROJECT_DIRECTORY_ROOT":
       return updateProjectDirectoryRoot(state, action.url);
 
     case "NAVIGATE":
-      return {
-        ...initialSourcesState(),
-        epoch: state.epoch + 1
-      };
+      return initialSourcesState();
 
     case "SET_FOCUSED_SOURCE_ITEM":
       return { ...state, focusedItem: action.item };
   }
 
   return state;
 }
 
@@ -297,36 +291,25 @@ function updateProjectDirectoryRoot(stat
     relativeUrl: getRelativeUrl(source, root)
   }));
 }
 
 /*
  * Update a source's loaded state fields
  * i.e. loadedState, text, error
  */
-function updateLoadedState(
-  state: SourcesState,
-  action: LoadSourceAction
-): SourcesState {
+function updateLoadedState(state, action: LoadSourceAction): SourcesState {
   const { sourceId } = action;
   let source;
 
-  // If there was a navigation between the time the action was started and
-  // completed, we don't want to update the store.
-  if (action.epoch !== state.epoch) {
-    return state;
-  }
-
   if (action.status === "start") {
     source = { id: sourceId, loadedState: "loading" };
   } else if (action.status === "error") {
     source = { id: sourceId, error: action.error, loadedState: "loaded" };
   } else {
-    // TODO: Remove this once we centralize pretty-print and this can no longer
-    // return a null value when loading a in-progress prettyprinting file.
     if (!action.value) {
       return state;
     }
 
     source = {
       id: sourceId,
       text: action.value.text,
       contentType: action.value.contentType,
@@ -534,20 +517,16 @@ export function getHasSiblingOfSameName(
 
   return getSourcesUrlsInSources(state, source.url).length > 1;
 }
 
 export function getSources(state: OuterState) {
   return state.sources.sources;
 }
 
-export function getSourcesEpoch(state: OuterState) {
-  return state.sources.epoch;
-}
-
 export function getUrls(state: OuterState) {
   return state.sources.urls;
 }
 
 export function getSourceList(state: OuterState): Source[] {
   return (Object.values(getSources(state)): any);
 }