Bug 1607639 - Part 4: Convert introductionUrl into sourceMapBaseURL to match actual usage. r=jlast

Differential Revision: https://phabricator.services.mozilla.com/D72115
This commit is contained in:
Logan Smyth 2020-04-27 02:17:02 +00:00
parent 6af9e5261a
commit bfe7013db8
16 changed files with 85 additions and 47 deletions

View File

@ -48,7 +48,11 @@ type Range = {
};
export type SourceMapInput = {|
id: SourceId,
// This URL isn't actually used in the source-map module, but we have it
// passed in so that the Toolbox can throw a more useful error message
// if the sourcemap for a given generated source file fails to load.
url: string,
sourceMapBaseURL: string,
sourceMapURL: string,
isWasm: boolean,
|};

View File

@ -27,7 +27,7 @@ async function setupBundleFixtureAndData(name) {
const source = {
id: `${name}.js`,
sourceMapURL: `${name}.js.map`,
url: `http://example.com/${name}.js`,
sourceMapBaseURL: `http://example.com/${name}.js`,
};
require("devtools-utils/src/network-request").mockImplementationOnce(() => {

View File

@ -136,7 +136,7 @@ describe("source maps", () => {
const source = {
id: "missingmap.js",
sourceMapURL: "missingmap.js.map",
url: "http:://example.com/missingmap.js",
sourceMapBaseURL: "http:://example.com/missingmap.js",
};
networkRequest.mockImplementationOnce(() => {

View File

@ -91,7 +91,7 @@ describe("wasm source maps", () => {
test("read and transpose wasm map", async () => {
const source = {
id: "min.js",
url: "wasm:http://example.com/whatever/:min.js",
sourceMapBaseURL: "wasm:http://example.com/whatever/:min.js",
sourceMapURL: "http://example.com/whatever/min.js.map",
isWasm: true,
};

View File

@ -25,13 +25,14 @@ function hasOriginalURL(url: string): boolean {
}
function _resolveSourceMapURL(source: SourceMapInput) {
let { url = "", sourceMapURL } = source;
let { sourceMapBaseURL, sourceMapURL } = source;
sourceMapBaseURL = sourceMapBaseURL || "";
sourceMapURL = sourceMapURL || "";
if (!url) {
if (!sourceMapBaseURL) {
// If the source doesn't have a URL, don't resolve anything.
return { sourceMapURL, baseURL: sourceMapURL };
}
sourceMapURL = sourceMapURL || "";
let resolvedString;
let baseURL;
@ -41,14 +42,14 @@ function _resolveSourceMapURL(source: SourceMapInput) {
// for large inlined source-maps, and we don't actually need to parse them.
if (sourceMapURL.startsWith("data:")) {
resolvedString = sourceMapURL;
baseURL = url;
baseURL = sourceMapBaseURL;
} else {
resolvedString = new URL(
sourceMapURL,
// If the URL is a data: URL, the sourceMapURL needs to be absolute, so
// we might as well pass `undefined` to avoid parsing a potentially
// very large data: URL for no reason.
url.startsWith("data:") ? undefined : url
sourceMapBaseURL.startsWith("data:") ? undefined : sourceMapBaseURL
).toString();
baseURL = resolvedString;
}

View File

@ -106,18 +106,6 @@ function loadSourceMap(cx: Context, sourceActor: SourceActor) {
let data = null;
try {
// Unable to correctly type the result of a spread on a union type.
// See https://github.com/facebook/flow/pull/7298
let url = sourceActor.url || "";
if (!sourceActor.url && typeof sourceActor.introductionUrl === "string") {
// If the source was dynamically generated (via eval, dynamically
// created script elements, and so forth), it won't have a URL, so that
// it is not collapsed into other sources from the same place. The
// introduction URL will include the point it was constructed at,
// however, so use that for resolving any source maps in the source.
url = sourceActor.introductionUrl;
}
// Ignore sourceMapURL on scripts that are part of HTML files, since
// we currently treat sourcemaps as Source-wide, not SourceActor-specific.
const source = getSourceByActorId(getState(), sourceActor.id);
@ -126,7 +114,8 @@ function loadSourceMap(cx: Context, sourceActor: SourceActor) {
// Using source ID here is historical and eventually we'll want to
// switch to all of this being per-source-actor.
id: source.id,
url,
url: sourceActor.url || "",
sourceMapBaseURL: sourceActor.sourceMapBaseURL || "",
sourceMapURL: sourceActor.sourceMapURL || "",
isWasm: sourceActor.introductionType === "wasm",
});
@ -345,9 +334,9 @@ export function newGeneratedSources(sourceInfo: Array<GeneratedSourceData>) {
thread,
source: newId,
isBlackBoxed: source.isBlackBoxed,
sourceMapBaseURL: source.sourceMapBaseURL,
sourceMapURL: source.sourceMapURL,
url: source.url,
introductionUrl: source.introductionUrl,
introductionType: source.introductionType,
});
}

View File

@ -31,6 +31,19 @@ export function prepareSourcePayload(
makeSourceId(source, isServiceWorker)
);
source = { ...source };
// Maintain backward-compat with servers that only return introductionUrl and
// not sourceMapBaseURL.
if (
typeof source.sourceMapBaseURL === "undefined" &&
typeof (source: any).introductionUrl !== "undefined"
) {
source.sourceMapBaseURL =
source.url || (source: any).introductionUrl || null;
delete (source: any).introductionUrl;
}
return { thread: threadFront.actor, isServiceWorker, source };
}

View File

@ -101,8 +101,8 @@ export type SourcePayload = {
actor: ActorId,
url: URL | null,
isBlackBoxed: boolean,
sourceMapBaseURL: URL | null,
sourceMapURL: URL | null,
introductionUrl: URL | null,
introductionType: string | null,
extensionName: string | null,
};

View File

@ -44,6 +44,9 @@ export type SourceActor = {|
+isBlackBoxed: boolean,
// The URL that the sourcemap should be loaded relative to.
+sourceMapBaseURL: URL | null,
// The URL of the sourcemap for this source if there is one.
+sourceMapURL: URL | null,
@ -51,10 +54,6 @@ export type SourceActor = {|
// string-based source, this will not be known.
+url: URL | null,
// If this script was introduced by an eval, this will be the URL of the
// script that triggered the evaluation.
+introductionUrl: URL | null,
// The debugger's Debugger.Source API provides type information for the
// cause of this source's creation.
+introductionType: string | null,

View File

@ -133,9 +133,9 @@ function makeSourceURL(filename: string) {
}
type MakeSourceProps = {
sourceMapBaseURL?: string,
sourceMapURL?: string,
introductionType?: string,
introductionUrl?: string,
isBlackBoxed?: boolean,
};
function createMakeSource(): (
@ -155,9 +155,9 @@ function createMakeSource(): (
source: {
actor: `${name}-${index}-actor`,
url: `http://localhost:8000/examples/${name}`,
sourceMapBaseURL: props.sourceMapBaseURL || null,
sourceMapURL: props.sourceMapURL || null,
introductionType: props.introductionType || null,
introductionUrl: props.introductionUrl || null,
isBlackBoxed: !!props.isBlackBoxed,
extensionName: null,
},

View File

@ -157,12 +157,18 @@ SourceMapURLService.prototype._registerNewSource = function(source) {
return;
}
const { generatedUrl, url, actor: id, sourceMapURL } = source;
const {
generatedUrl,
url,
actor: id,
sourceMapBaseURL,
sourceMapURL,
} = source;
// |generatedUrl| comes from the actor and is extracted from the
// source code by SpiderMonkey.
const seenUrl = generatedUrl || url;
this._urls.set(seenUrl, { id, url: seenUrl, sourceMapURL });
this._urls.set(seenUrl, { id, url: seenUrl, sourceMapBaseURL, sourceMapURL });
this._idMap.set(id, seenUrl);
return seenUrl;
@ -197,9 +203,9 @@ SourceMapURLService.prototype._registerNewStyleSheet = function(sheet) {
return;
}
const { href, nodeHref, sourceMapURL, actorID: id } = sheet;
const { href, nodeHref, sourceMapBaseURL, sourceMapURL, actorID: id } = sheet;
const url = href || nodeHref;
this._urls.set(url, { id, url, sourceMapURL });
this._urls.set(url, { id, url, sourceMapBaseURL, sourceMapURL });
this._idMap.set(id, url);
return url;
@ -293,7 +299,12 @@ SourceMapURLService.prototype.originalPositionFor = async function(
}
// Call getOriginalURLs to make sure the source map has been
// fetched. We don't actually need the result of this though.
await this._sourceMapService.getOriginalURLs(urlInfo);
await this._sourceMapService.getOriginalURLs({
id: urlInfo.id,
url: urlInfo.url,
sourceMapBaseURL: urlInfo.sourceMapBaseURL,
sourceMapURL: urlInfo.sourceMapURL,
});
const location = { sourceId: urlInfo.id, line, column, sourceUrl: url };
const resolvedLocation = await this._sourceMapService.getOriginalLocation(
location

View File

@ -117,6 +117,14 @@ class StyleSheetFront extends FrontClassWithSpec(styleSheetSpec) {
get ruleCount() {
return this._form.ruleCount;
}
get sourceMapBaseURL() {
// Handle backward-compat for servers that don't return sourceMapBaseURL.
if (this._form.sourceMapBaseURL === undefined) {
return this.href || this.nodeHref;
}
return this._form.sourceMapBaseURL;
}
get sourceMapURL() {
return this._form.sourceMapURL;
}

View File

@ -3869,11 +3869,13 @@ function hasOriginalURL(url) {
function _resolveSourceMapURL(source) {
let {
url = "",
sourceMapBaseURL,
sourceMapURL
} = source;
sourceMapBaseURL = sourceMapBaseURL || "";
sourceMapURL = sourceMapURL || "";
if (!url) {
if (!sourceMapBaseURL) {
// If the source doesn't have a URL, don't resolve anything.
return {
sourceMapURL,
@ -3881,7 +3883,6 @@ function _resolveSourceMapURL(source) {
};
}
sourceMapURL = sourceMapURL || "";
let resolvedString;
let baseURL; // When the sourceMap is a data: URL, fall back to using the source's URL,
// if possible. We don't use `new URL` here because it will be _very_ slow
@ -3889,12 +3890,12 @@ function _resolveSourceMapURL(source) {
if (sourceMapURL.startsWith("data:")) {
resolvedString = sourceMapURL;
baseURL = url;
baseURL = sourceMapBaseURL;
} else {
resolvedString = new URL(sourceMapURL, // If the URL is a data: URL, the sourceMapURL needs to be absolute, so
// we might as well pass `undefined` to avoid parsing a potentially
// very large data: URL for no reason.
url.startsWith("data:") ? undefined : url).toString();
sourceMapBaseURL.startsWith("data:") ? undefined : sourceMapBaseURL).toString();
baseURL = resolvedString;
}

View File

@ -344,11 +344,17 @@ StyleEditorUI.prototype = {
return editor;
}
const { href, nodeHref, actorID: id, sourceMapURL } = styleSheet;
const url = href || nodeHref;
const {
href,
nodeHref,
actorID: id,
sourceMapURL,
sourceMapBaseURL,
} = styleSheet;
const sources = await sourceMapService.getOriginalURLs({
id,
url,
url: href || nodeHref,
sourceMapBaseURL,
sourceMapURL,
});
// A single generated sheet might map to multiple original

View File

@ -181,8 +181,10 @@ const SourceActor = ActorClassWithSpec(sourceSpec, {
const source = this._source;
let introductionUrl = null;
if (source.introductionScript) {
introductionUrl = source.introductionScript.source.url;
if (source.introductionScript && source.introductionScript.source.url) {
introductionUrl = source.introductionScript.source.url
.split(" -> ")
.pop();
}
return {
@ -190,10 +192,13 @@ const SourceActor = ActorClassWithSpec(sourceSpec, {
extensionName: this.extensionName,
url: this.url,
isBlackBoxed: this.threadActor.sources.isBlackBoxed(this.url),
// If the source was dynamically generated (via eval, dynamically
// created script elements, and so forth), it won't have a URL, so that
// it is not collapsed into other sources from the same place. The
// introduction URL will include the point it was constructed at,
// however, so use that for resolving any source maps in the source.
sourceMapBaseURL: this.url || introductionUrl || null,
sourceMapURL: source.sourceMapURL,
introductionUrl: introductionUrl
? introductionUrl.split(" -> ").pop()
: null,
introductionType: source.introductionType,
};
},

View File

@ -431,6 +431,7 @@ var StyleSheetActor = protocol.ActorClassWithSpec(styleSheetSpec, {
title: this.rawSheet.title,
system: !CssLogic.isAuthorStylesheet(this.rawSheet),
styleSheetIndex: this.styleSheetIndex,
sourceMapBaseURL: this.href || docHref || null,
sourceMapURL: this.rawSheet.sourceMapURL,
};