Bug 1536196 - loadSourceText should probably be responsible for pretty-printing files. r=loganfsmyth
authorMOHIT NAGPAL <32512296+nagpalm7@users.noreply.github.com>
Tue, 19 Mar 2019 15:37:35 +0000
changeset 465038 416d1c916c8501df83408bcee3fb3e485f062d25
parent 465037 22a25f360f553b9bff4fca2c29432c7771aa5832
child 465039 b9c222097ae1e830e0d075677976463df0fd289f
push id35730
push userrmaries@mozilla.com
push dateTue, 19 Mar 2019 21:51:47 +0000
treeherdermozilla-central@4f6d8ed9e948 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersloganfsmyth
bugs1536196
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
Bug 1536196 - loadSourceText should probably be responsible for pretty-printing files. r=loganfsmyth Differential Revision: https://phabricator.services.mozilla.com/D23917
devtools/client/debugger/new/src/actions/sources/loadSourceText.js
devtools/client/debugger/new/src/actions/sources/prettyPrint.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,20 +1,26 @@
 /* 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,
+  getGeneratedSource,
+  getSourcesEpoch
+} from "../../selectors";
 import { setBreakpointPositions } from "../breakpoints";
 
+import { prettyPrintSource } from "./prettyPrint";
+
 import * as parser from "../../workers/parser";
-import { isLoaded, isOriginal } from "../../utils/source";
+import { isLoaded, isOriginal, isPretty } from "../../utils/source";
 import { Telemetry } from "devtools-modules";
 
 import type { ThunkArgs } from "../types";
 
 import type { Source } from "../../types";
 
 const requests = new Map();
 
@@ -25,26 +31,24 @@ const telemetry = new Telemetry();
 async function loadSource(
   state,
   source: Source,
   { sourceMaps, client }
 ): Promise<?{
   text: string,
   contentType: string
 }> {
+  if (isPretty(source) && isOriginal(source)) {
+    const generatedSource = getGeneratedSource(state, source);
+    return prettyPrintSource(sourceMaps, source, generatedSource);
+  }
+
   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 the
       // original source text right after the source map has been cleared
       // after a navigation event.
       throw new Error("Original source text unavailable");
     }
     return result;
   }
--- a/devtools/client/debugger/new/src/actions/sources/prettyPrint.js
+++ b/devtools/client/debugger/new/src/actions/sources/prettyPrint.js
@@ -1,37 +1,60 @@
 /* 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 assert from "../../utils/assert";
 import { recordEvent } from "../../utils/telemetry";
-import { remapBreakpoints, setBreakpointPositions } from "../breakpoints";
+import { remapBreakpoints } from "../breakpoints";
 
 import { setSymbols } from "../ast";
 import { prettyPrint } from "../../workers/pretty-print";
-import { setSource } from "../../workers/parser";
 import { getPrettySourceURL, isLoaded } from "../../utils/source";
 import { loadSourceText } from "./loadSourceText";
 import { mapFrames } from "../pause";
 import { selectSpecificLocation } from "../sources";
 
 import {
   getSource,
   getSourceFromId,
   getSourceThreads,
   getSourceByURL,
   getSelectedLocation
 } from "../../selectors";
 
 import type { Action, ThunkArgs } from "../types";
 import { selectSource } from "./select";
-import type { JsSource } from "../../types";
+import type { JsSource, Source } from "../../types";
+
+export async function prettyPrintSource(
+  sourceMaps: any,
+  prettySource: Source,
+  generatedSource: any
+) {
+  const url = getPrettySourceURL(generatedSource.url);
+  const { code, mappings } = await prettyPrint({
+    source: generatedSource,
+    url: url
+  });
+  await sourceMaps.applySourceMap(generatedSource.id, url, code, mappings);
+
+  // The source map URL service used by other devtools listens to changes to
+  // sources based on their actor IDs, so apply the mapping there too.
+  for (const sourceActor of generatedSource.actors) {
+    await sourceMaps.applySourceMap(sourceActor.actor, url, code, mappings);
+  }
+  return {
+    id: prettySource.id,
+    text: code,
+    contentType: "text/javascript"
+  };
+}
 
 export function createPrettySource(sourceId: string) {
   return async ({ dispatch, getState, sourceMaps }: ThunkArgs) => {
     const source = getSourceFromId(getState(), sourceId);
     const url = getPrettySourceURL(source.url);
     const id = await sourceMaps.generatedToOriginalId(sourceId, url);
 
     const prettySource: JsSource = {
@@ -44,36 +67,17 @@ export function createPrettySource(sourc
       contentType: "text/javascript",
       loadedState: "loading",
       introductionUrl: null,
       isExtension: false,
       actors: []
     };
 
     dispatch(({ type: "ADD_SOURCE", source: prettySource }: Action));
-    dispatch(selectSource(prettySource.id));
-
-    const { code, mappings } = await prettyPrint({ source, url });
-    await sourceMaps.applySourceMap(source.id, url, code, mappings);
-
-    // The source map URL service used by other devtools listens to changes to
-    // sources based on their actor IDs, so apply the mapping there too.
-    for (const sourceActor of source.actors) {
-      await sourceMaps.applySourceMap(sourceActor.actor, url, code, mappings);
-    }
-
-    const loadedPrettySource: JsSource = {
-      ...prettySource,
-      text: code,
-      loadedState: "loaded"
-    };
-
-    setSource(loadedPrettySource);
-    dispatch(({ type: "UPDATE_SOURCE", source: loadedPrettySource }: Action));
-    await dispatch(setBreakpointPositions(loadedPrettySource.id));
+    await dispatch(selectSource(prettySource.id));
 
     return prettySource;
   };
 }
 
 /**
  * Toggle the pretty printing of a source's text. All subsequent calls to
  * |getText| will return the pretty-toggled text. Nothing will happen for
--- a/devtools/client/debugger/new/src/reducers/sources.js
+++ b/devtools/client/debugger/new/src/reducers/sources.js
@@ -315,22 +315,16 @@ function updateLoadedState(
     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,
       loadedState: "loaded"
     };
   }