author | James Long <longster@gmail.com> |
Wed, 20 Jul 2016 14:57:13 -0400 | |
changeset 306080 | f272791a6d75ec9d551f9c73493eaa39021e5827 |
parent 306079 | 0cca5d80caa9f04d491c28b388f2f396fd45fe5c |
child 306081 | d8384a5bd4dfd6371e3d0d17fa0a91367a20dc7a |
push id | 79765 |
push user | cbook@mozilla.com |
push date | Thu, 21 Jul 2016 14:26:34 +0000 |
treeherder | mozilla-inbound@ab54bfc55266 [default view] [failures only] |
perfherder | [talos] [build metrics] [platform microbench] (compared to previous push) |
reviewers | fitzgen |
bugs | 1238173 |
milestone | 50.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
|
devtools/server/actors/script.js | file | annotate | diff | comparison | revisions | |
devtools/server/actors/source.js | file | annotate | diff | comparison | revisions | |
devtools/server/actors/utils/ScriptStore.js | file | annotate | diff | comparison | revisions | |
devtools/server/actors/utils/moz.build | file | annotate | diff | comparison | revisions | |
devtools/server/tests/unit/test_ScriptStore.js | file | annotate | diff | comparison | revisions | |
devtools/server/tests/unit/xpcshell.ini | file | annotate | diff | comparison | revisions |
--- a/devtools/server/actors/script.js +++ b/devtools/server/actors/script.js @@ -16,17 +16,16 @@ const { ObjectActor, createValueGrip, lo const { SourceActor, getSourceURL } = require("devtools/server/actors/source"); const { DebuggerServer } = require("devtools/server/main"); const { ActorClassWithSpec } = require("devtools/shared/protocol"); const DevToolsUtils = require("devtools/shared/DevToolsUtils"); const { assert, dumpn, update, fetch } = DevToolsUtils; const promise = require("promise"); const PromiseDebugging = require("PromiseDebugging"); const xpcInspector = require("xpcInspector"); -const ScriptStore = require("./utils/ScriptStore"); const { DevToolsWorker } = require("devtools/shared/worker/worker"); const object = require("sdk/util/object"); const { threadSpec } = require("devtools/shared/specs/script"); const { defer, resolve, reject, all } = promise; loader.lazyGetter(this, "Debugger", () => { let Debugger = require("Debugger"); @@ -489,24 +488,16 @@ const ThreadActor = ActorClassWithSpec(t if (!this._threadLifetimePool) { this._threadLifetimePool = new ActorPool(this.conn); this.conn.addActorPool(this._threadLifetimePool); this._threadLifetimePool.objectActors = new WeakMap(); } return this._threadLifetimePool; }, - get scripts() { - if (!this._scripts) { - this._scripts = new ScriptStore(); - this._scripts.addScripts(this.dbg.findScripts()); - } - return this._scripts; - }, - get sources() { return this._parent.sources; }, get youngestFrame() { if (this.state != "paused") { return null; } @@ -640,18 +631,16 @@ const ThreadActor = ActorClassWithSpec(t try { // Put ourselves in the paused state. let packet = this._paused(); if (!packet) { return { error: "notAttached" }; } packet.why = { type: "attached" }; - this._restoreBreakpoints(); - // Send the response to the attach request now (rather than // returning it), because we're going to start a nested event loop // here. this.conn.send(packet); // Start a nested event loop. this._pushThreadPause(); @@ -1179,17 +1168,18 @@ const ThreadActor = ActorClassWithSpec(t _breakOnEnter: function (script) { let offsets = script.getAllOffsets(); for (let line = 0, n = offsets.length; line < n; line++) { if (offsets[line]) { // N.B. Hidden breakpoints do not have an original location, and are not // stored in the breakpoint actor map. let actor = new BreakpointActor(this); this.threadLifetimePool.addActor(actor); - let scripts = this.scripts.getScriptsBySourceAndLine(script.source, line); + + let scripts = this.dbg.findScripts({ source: script.source, line: line }); let entryPoints = findEntryPointsForLine(scripts, line); setBreakpointAtEntryPoints(actor, entryPoints); this._hiddenBreakpoints.set(actor.actorID, actor); break; } } }, @@ -1313,17 +1303,18 @@ const ThreadActor = ActorClassWithSpec(t }, /** * Get the script and source lists from the debugger. */ _discoverSources: function () { // Only get one script per Debugger.Source. const sourcesToScripts = new Map(); - const scripts = this.scripts.getAllScripts(); + const scripts = this.dbg.findScripts(); + for (let i = 0, len = scripts.length; i < len; i++) { let s = scripts[i]; if (s.source) { sourcesToScripts.set(s.source, s); } } return all([...sourcesToScripts.values()].map(script => { @@ -1919,46 +1910,27 @@ const ThreadActor = ActorClassWithSpec(t from: this.actorID, type: name, source: source.form() }); } }, /** - * Restore any pre-existing breakpoints to the sources that we have access to. - */ - _restoreBreakpoints: function () { - if (this.breakpointActorMap.size === 0) { - return; - } - - for (let s of this.scripts.getSources()) { - this._addSource(s); - } - }, - - /** * Add the provided source to the server cache. * * @param aSource Debugger.Source * The source that will be stored. * @returns true, if the source was added; false otherwise. */ _addSource: function (aSource) { if (!this.sources.allowSource(aSource) || this._debuggerSourcesSeen.has(aSource)) { return false; } - // The scripts must be added to the ScriptStore before restoring - // breakpoints. If we try to add them to the ScriptStore any later, we can - // accidentally set a breakpoint in a top level script as a "closest match" - // because we wouldn't have added the child scripts to the ScriptStore yet. - this.scripts.addScripts(this.dbg.findScripts({ source: aSource })); - let sourceActor = this.sources.createNonSourceMappedActor(aSource); let bpActors = [...this.breakpointActorMap.findActors()]; if (this._options.useSourceMaps) { let promises = []; // Go ahead and establish the source actors for this script, which // fetches sourcemaps if available and sends onNewSource
--- a/devtools/server/actors/source.js +++ b/devtools/server/actors/source.js @@ -180,17 +180,16 @@ let SourceActor = ActorClassWithSpec(sou get isInlineSource() { return this._isInlineSource; }, get threadActor() { return this._threadActor; }, get sources() { return this._threadActor.sources; }, get dbg() { return this.threadActor.dbg; }, - get scripts() { return this.threadActor.scripts; }, get source() { return this._source; }, get generatedSource() { return this._generatedSource; }, get breakpointActorMap() { return this.threadActor.breakpointActorMap; }, get url() { if (this.source) { return getSourceURL(this.source, this.threadActor._parent.window); } return this._originalUrl; @@ -439,17 +438,17 @@ let SourceActor = ActorClassWithSpec(sou /** * Extract all executable offsets from the given script * @param String url - extract offsets of the script with this url * @param Boolean onlyLine - will return only the line number * @return Set - Executable offsets/lines of the script **/ getExecutableOffsets: function (source, onlyLine) { let offsets = new Set(); - for (let s of this.threadActor.scripts.getScriptsBySource(source)) { + for (let s of this.dbg.findScripts({ source })) { for (let offset of s.getAllColumnOffsets()) { offsets.add(onlyLine ? offset.lineNumber : offset); } } return offsets; }, @@ -713,20 +712,27 @@ let SourceActor = ActorClassWithSpec(sou const { originalLocation } = actor; const { originalLine, originalSourceActor } = originalLocation; if (!this.isSourceMapped) { if (!this._setBreakpointAtGeneratedLocation( actor, GeneratedLocation.fromOriginalLocation(originalLocation) )) { - const scripts = this.scripts.getScriptsBySourceActorAndLine( - this, - originalLine - ); + const query = { line: originalLine }; + // For most cases, we have a real source to query for. The + // only time we don't is for HTML pages. In that case we want + // to query for scripts in an HTML page based on its URL, as + // there could be several sources within an HTML page. + if (this.source) { + query.source = this.source; + } else { + query.url = this.url; + } + const scripts = this.dbg.findScripts(query); // Never do breakpoint sliding for column breakpoints. // Additionally, never do breakpoint sliding if no scripts // exist on this line. // // Sliding can go horribly wrong if we always try to find the // next line with valid entry points in the entire file. // Scripts may be completely GCed and we never knew they @@ -831,21 +837,25 @@ let SourceActor = ActorClassWithSpec(sou _setBreakpointAtGeneratedLocation: function (actor, generatedLocation) { let { generatedSourceActor, generatedLine, generatedColumn, generatedLastColumn } = generatedLocation; - // Find all scripts that match the given source actor and line number. - let scripts = this.scripts.getScriptsBySourceActorAndLine( - generatedSourceActor, - generatedLine - ); + // Find all scripts that match the given source actor and line + // number. + const query = { line: generatedLine }; + if (generatedSourceActor.source) { + query.source = generatedSourceActor.source; + } else { + query.url = generatedSourceActor.url; + } + let scripts = this.dbg.findScripts(query); scripts = scripts.filter((script) => !actor.hasScript(script)); // Find all entry points that correspond to the given location. let entryPoints = []; if (generatedColumn === undefined) { // This is a line breakpoint, so we are interested in all offsets // that correspond to the given line number.
deleted file mode 100644 --- a/devtools/server/actors/utils/ScriptStore.js +++ /dev/null @@ -1,219 +0,0 @@ -/* -*- indent-tabs-mode: nil; js-indent-level: 2; js-indent-level: 2 -*- */ -/* vim: set ft=javascript ts=2 et sw=2 tw=80: */ -/* 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/. */ - -"use strict"; - -const { noop } = require("devtools/shared/DevToolsUtils"); - -/** - * A `ScriptStore` is a cache of `Debugger.Script` instances. It holds strong - * references to the cached scripts to alleviate the GC-sensitivity issues that - * plague `Debugger.prototype.findScripts`, but this means that its lifetime - * must be managed carefully. It is the `ScriptStore` user's responsibility to - * ensure that the `ScriptStore` stays up to date. - * - * Implementation Notes: - * - * The ScriptStore's prototype methods are very hot, in general. To help the - * JIT, they avoid ES6-isms and higher-order iteration functions, for the most - * part. You might be wondering why we don't maintain indices on, say, - * Debugger.Source for faster querying, if these methods are so hot. First, the - * hottest method is actually just getting all scripts; second, populating the - * store becomes prohibitively expensive. So we fall back to linear queries - * (which isn't so bad, because Debugger.prototype.findScripts is also linear). - */ -function ScriptStore() { - // Set of every Debugger.Script in the cache. - this._scripts = new NoDeleteSet; -} - -module.exports = ScriptStore; - -ScriptStore.prototype = { - // Populating a ScriptStore. - - /** - * Add one script to the cache. - * - * @param Debugger.Script script - */ - addScript(script) { - this._scripts.add(script); - }, - - /** - * Add many scripts to the cache at once. - * - * @param Array scripts - * The set of Debugger.Scripts to add to the cache. - */ - addScripts(scripts) { - for (var i = 0, len = scripts.length; i < len; i++) { - this.addScript(scripts[i]); - } - }, - - // Querying a ScriptStore. - - /** - * Get all the sources for which we have scripts cached. - * - * @returns Array of Debugger.Source - */ - getSources() { - return [...new Set(this._scripts.items.map(s => s.source))]; - }, - - /** - * Get all the scripts in the cache. - * - * @returns read-only Array of Debugger.Script. - * - * NB: The ScriptStore retains ownership of the returned array, and the - * ScriptStore's consumers MUST NOT MODIFY its contents! - */ - getAllScripts() { - return this._scripts.items; - }, - - getScriptsBySourceActor(sourceActor) { - return sourceActor.source ? - this.getScriptsBySource(sourceActor.source) : - this.getScriptsByURL(sourceActor._originalUrl); - }, - - getScriptsBySourceActorAndLine(sourceActor, line) { - return sourceActor.source ? - this.getScriptsBySourceAndLine(sourceActor.source, line) : - this.getScriptsByURLAndLine(sourceActor._originalUrl, line); - }, - - /** - * Get all scripts produced from the given source. - * - * @oaram Debugger.Source source - * @returns Array of Debugger.Script - */ - getScriptsBySource(source) { - var results = []; - var scripts = this._scripts.items; - var length = scripts.length; - for (var i = 0; i < length; i++) { - if (scripts[i].source === source) { - results.push(scripts[i]); - } - } - return results; - }, - - /** - * Get all scripts produced from the given source whose source code definition - * spans the given line. - * - * @oaram Debugger.Source source - * @param Number line - * @returns Array of Debugger.Script - */ - getScriptsBySourceAndLine(source, line) { - var results = []; - var scripts = this._scripts.items; - var length = scripts.length; - for (var i = 0; i < length; i++) { - var script = scripts[i]; - if (script.source === source && - script.startLine <= line && - (script.startLine + script.lineCount) > line) { - results.push(script); - } - } - return results; - }, - - /** - * Get all scripts defined by a source at the given URL. - * - * @param String url - * @returns Array of Debugger.Script - */ - getScriptsByURL(url) { - var results = []; - var scripts = this._scripts.items; - var length = scripts.length; - for (var i = 0; i < length; i++) { - if (scripts[i].url === url) { - results.push(scripts[i]); - } - } - return results; - }, - - /** - * Get all scripts defined by a source a the given URL and whose source code - * definition spans the given line. - * - * @param String url - * @param Number line - * @returns Array of Debugger.Script - */ - getScriptsByURLAndLine(url, line) { - var results = []; - var scripts = this._scripts.items; - var length = scripts.length; - for (var i = 0; i < length; i++) { - var script = scripts[i]; - if (script.url === url && - script.startLine <= line && - (script.startLine + script.lineCount) > line) { - results.push(script); - } - } - return results; - }, -}; - - -/** - * A set which can only grow, and does not support the delete operation. - * Provides faster iteration than the native Set by maintaining an array of all - * items, in addition to the internal set of all items, which allows direct - * iteration (without the iteration protocol and calling into C++, which are - * both expensive). - */ -function NoDeleteSet() { - this._set = new Set(); - this.items = []; -} - -NoDeleteSet.prototype = { - /** - * An array containing every item in the set for convenience and faster - * iteration. This is public for reading only, and consumers MUST NOT modify - * this array! - */ - items: null, - - /** - * Add an item to the set. - * - * @param any item - */ - add(item) { - if (!this._set.has(item)) { - this._set.add(item); - this.items.push(item); - } - }, - - /** - * Return true if the item is in the set, false otherwise. - * - * @param any item - * @returns Boolean - */ - has(item) { - return this._set.has(item); - } -};
--- a/devtools/server/actors/utils/moz.build +++ b/devtools/server/actors/utils/moz.build @@ -5,13 +5,12 @@ # file, You can obtain one at http://mozilla.org/MPL/2.0/. DevToolsModules( 'actor-registry-utils.js', 'audionodes.json', 'automation-timeline.js', 'make-debugger.js', 'map-uri-to-addon-id.js', - 'ScriptStore.js', 'stack.js', 'TabSources.js', 'walker-search.js' )
deleted file mode 100644 --- a/devtools/server/tests/unit/test_ScriptStore.js +++ /dev/null @@ -1,168 +0,0 @@ -/* -*- js-indent-level: 2; indent-tabs-mode: nil -*- */ -/* Any copyright is dedicated to the Public Domain. - http://creativecommons.org/publicdomain/zero/1.0/ */ - -// Test the functionality of ScriptStore. - -const ScriptStore = require("devtools/server/actors/utils/ScriptStore"); - -// Fixtures - -const firstSource = "firstSource"; -const secondSource = "secondSource"; -const thirdSource = "thirdSource"; - -const scripts = new Set([ - { - url: "a.js", - source: firstSource, - startLine: 1, - lineCount: 100, - global: "g1" - }, - { - url: "a.js", - source: firstSource, - startLine: 1, - lineCount: 40, - global: "g1" - }, - { - url: "a.js", - source: firstSource, - startLine: 50, - lineCount: 100, - global: "g1" - }, - { - url: "a.js", - source: firstSource, - startLine: 60, - lineCount: 90, - global: "g1" - }, - { - url: "index.html", - source: secondSource, - startLine: 150, - lineCount: 1, - global: "g2" - }, - { - url: "index.html", - source: thirdSource, - startLine: 200, - lineCount: 100, - global: "g2" - }, - { - url: "index.html", - source: thirdSource, - startLine: 250, - lineCount: 10, - global: "g2" - }, - { - url: "index.html", - source: thirdSource, - startLine: 275, - lineCount: 5, - global: "g2" - } -]); - -function contains(script, line) { - return script.startLine <= line && - line < script.startLine + script.lineCount; -} - -function run_test() { - testAddScript(); - testAddScripts(); - testGetSources(); - testGetScriptsBySource(); - testGetScriptsBySourceAndLine(); - testGetScriptsByURL(); - testGetScriptsByURLAndLine(); -} - -function testAddScript() { - const ss = new ScriptStore(); - - for (let s of scripts) { - ss.addScript(s); - } - - equal(ss.getAllScripts().length, scripts.size); - - for (let s of ss.getAllScripts()) { - ok(scripts.has(s)); - } -} - -function testAddScripts() { - const ss = new ScriptStore(); - ss.addScripts([...scripts]); - - equal(ss.getAllScripts().length, scripts.size); - - for (let s of ss.getAllScripts()) { - ok(scripts.has(s)); - } -} - -function testGetSources() { - const ss = new ScriptStore(); - ss.addScripts([...scripts]); - - const expected = new Set([firstSource, secondSource, thirdSource]); - const actual = ss.getSources(); - equal(expected.size, actual.length); - - for (let s of actual) { - ok(expected.has(s)); - expected.delete(s); - } -} - -function testGetScriptsBySource() { - const ss = new ScriptStore(); - ss.addScripts([...scripts]); - - const expected = [...scripts].filter(s => s.source === thirdSource); - const actual = ss.getScriptsBySource(thirdSource); - - deepEqual(actual, expected); -} - -function testGetScriptsBySourceAndLine() { - const ss = new ScriptStore(); - ss.addScripts([...scripts]); - - const expected = [...scripts].filter( - s => s.source === firstSource && contains(s, 65)); - const actual = ss.getScriptsBySourceAndLine(firstSource, 65); - - deepEqual(actual, expected); -} - -function testGetScriptsByURL() { - const ss = new ScriptStore(); - ss.addScripts([...scripts]); - - const expected = [...scripts].filter(s => s.url === "index.html"); - const actual = ss.getScriptsByURL("index.html"); - - deepEqual(actual, expected); -} - -function testGetScriptsByURLAndLine() { - const ss = new ScriptStore(); - ss.addScripts([...scripts]); - - const expected = [...scripts].filter( - s => s.url === "index.html" && contains(s, 250)); - const actual = ss.getScriptsByURLAndLine("index.html", 250); - - deepEqual(actual, expected); -}
--- a/devtools/server/tests/unit/xpcshell.ini +++ b/devtools/server/tests/unit/xpcshell.ini @@ -32,17 +32,16 @@ support-files = setBreakpoint-on-line-with-no-offsets-in-gcd-script.js addons/web-extension/manifest.json addons/web-extension2/manifest.json [test_addon_reload.js] [test_addons_actor.js] [test_animation_name.js] [test_animation_type.js] -[test_ScriptStore.js] [test_actor-registry-actor.js] [test_nesting-01.js] [test_nesting-02.js] [test_nesting-03.js] [test_forwardingprefix.js] [test_getyoungestframe.js] [test_nsjsinspector.js] [test_dbgactor.js]