From 55da592d85c2baf8d8818010c41d9738c97013d2 Mon Sep 17 00:00:00 2001 From: Cosmin Sabou Date: Mon, 27 Aug 2018 18:52:57 +0300 Subject: [PATCH] Backed out 4 changesets (bug 956376) for causing a spike of failures in bug 1438778. a=backout Backed out changeset 9cb7826b8f28 (bug 956376) Backed out changeset 37f94ae472d6 (bug 956376) Backed out changeset d96d56907ce0 (bug 956376) Backed out changeset ee0b4798b061 (bug 956376) --- devtools/server/actors/promises.js | 4 +- devtools/server/actors/replay/debugger.js | 20 +- devtools/server/actors/replay/replay.js | 38 +-- devtools/server/actors/thread.js | 18 +- js/src/doc/Debugger/Debugger.md | 20 +- .../tests/debug/Debugger-findSources-01.js | 4 - .../tests/debug/Debugger-findSources-02.js | 15 - .../tests/debug/Debugger-findSources-03.js | 19 -- js/src/vm/Debugger.cpp | 271 ++++-------------- js/src/vm/Debugger.h | 3 - 10 files changed, 106 insertions(+), 306 deletions(-) delete mode 100644 js/src/jit-test/tests/debug/Debugger-findSources-01.js delete mode 100644 js/src/jit-test/tests/debug/Debugger-findSources-02.js delete mode 100644 js/src/jit-test/tests/debug/Debugger-findSources-03.js diff --git a/devtools/server/actors/promises.js b/devtools/server/actors/promises.js index 0d81f984d6b8..02930f18add3 100644 --- a/devtools/server/actors/promises.js +++ b/devtools/server/actors/promises.js @@ -65,8 +65,8 @@ var PromisesActor = protocol.ActorClassWithSpec(promisesSpec, { this._newPromises = []; this._promisesSettled = []; - this.dbg.findSources().forEach(source => { - this.parentActor.sources.createSourceActors(source); + this.dbg.findScripts().forEach(s => { + this.parentActor.sources.createSourceActors(s.source); }); this.dbg.onNewScript = s => { diff --git a/devtools/server/actors/replay/debugger.js b/devtools/server/actors/replay/debugger.js index 24ae60fa0988..8018f66460e2 100644 --- a/devtools/server/actors/replay/debugger.js +++ b/devtools/server/actors/replay/debugger.js @@ -170,23 +170,11 @@ ReplayDebugger.prototype = { ///////////////////////////////////////////////////////// _getSource(id) { - const source = this._scriptSources[id]; - if (source) { - return source; + if (!this._scriptSources[id]) { + const data = this._sendRequest({ type: "getSource", id }); + this._scriptSources[id] = new ReplayDebuggerScriptSource(this, data); } - return this._addSource(this._sendRequest({ type: "getSource", id })); - }, - - _addSource(data) { - if (!this._scriptSources[data.id]) { - this._scriptSources[data.id] = new ReplayDebuggerScriptSource(this, data); - } - return this._scriptSources[data.id]; - }, - - findSources() { - const data = this._sendRequest({ type: "findSources" }); - return data.map(source => this._addSource(source)); + return this._scriptSources[id]; }, ///////////////////////////////////////////////////////// diff --git a/devtools/server/actors/replay/replay.js b/devtools/server/actors/replay/replay.js index 4e32440e0642..4f63f57cff80 100644 --- a/devtools/server/actors/replay/replay.js +++ b/devtools/server/actors/replay/replay.js @@ -465,22 +465,6 @@ function getScriptData(id) { }; } -function getSourceData(id) { - const source = gScriptSources.getObject(id); - const introductionScript = gScripts.getId(source.introductionScript); - return { - id: id, - text: source.text, - url: source.url, - displayURL: source.displayURL, - elementAttributeName: source.elementAttributeName, - introductionScript, - introductionOffset: introductionScript ? source.introductionOffset : undefined, - introductionType: source.introductionType, - sourceMapURL: source.sourceMapURL, - }; -} - function forwardToScript(name) { return request => gScripts.getObject(request.id)[name](request.value); } @@ -511,16 +495,20 @@ const gRequestHandlers = { return RecordReplayControl.getContent(request.url); }, - findSources(request) { - const sources = []; - gScriptSources.forEach((id) => { - sources.push(getSourceData(id)); - }); - return sources; - }, - getSource(request) { - return getSourceData(request.id); + const source = gScriptSources.getObject(request.id); + const introductionScript = gScripts.getId(source.introductionScript); + return { + id: request.id, + text: source.text, + url: source.url, + displayURL: source.displayURL, + elementAttributeName: source.elementAttributeName, + introductionScript, + introductionOffset: introductionScript ? source.introductionOffset : undefined, + introductionType: source.introductionType, + sourceMapURL: source.sourceMapURL, + }; }, getObject(request) { diff --git a/devtools/server/actors/thread.js b/devtools/server/actors/thread.js index 2c268755df0b..9afe2f721365 100644 --- a/devtools/server/actors/thread.js +++ b/devtools/server/actors/thread.js @@ -1111,12 +1111,22 @@ const ThreadActor = ActorClassWithSpec(threadSpec, { }, /** - * Get the source lists from the debugger. + * Get the script and source lists from the debugger. */ _discoverSources: function() { - const sources = this.dbg.findSources(); - return Promise.all(sources.map(source => { - return this.sources.createSourceActors(source); + // Only get one script per Debugger.Source. + const sourcesToScripts = new Map(); + const scripts = this.dbg.findScripts(); + + for (let i = 0, len = scripts.length; i < len; i++) { + const s = scripts[i]; + if (s.source) { + sourcesToScripts.set(s.source, s); + } + } + + return Promise.all([...sourcesToScripts.values()].map(script => { + return this.sources.createSourceActors(script.source); })); }, diff --git a/js/src/doc/Debugger/Debugger.md b/js/src/doc/Debugger/Debugger.md index 6a627d26ad1e..73e73315fd9b 100644 --- a/js/src/doc/Debugger/Debugger.md +++ b/js/src/doc/Debugger/Debugger.md @@ -357,10 +357,26 @@ other kinds of objects. [visible frame][vf] currently on the calling thread's stack, or `null` if there are no visible frames on the stack. -findSources() -: Return an array of all [`Debugger.Source`][source] instances of all debuggee +findSources([query]) (not yet implemented) +: Return an array of all [`Debugger.Source`][source] instances matching + query. Each source appears only once in the array. Query + is an object whose properties restrict which sources are returned; a + source must meet all the criteria given by query to be returned. + If query is omitted, we return all sources of all debuggee scripts. + Query may have the following properties: + + `url` + : The source's `url` property must be equal to this value. + + `global` + : The source must have been evaluated in the scope of the given global + object. If this property's value is a [`Debugger.Object`][object] instance + belonging to this `Debugger` instance, then its referent is used. If the + object is not a global object, then the global in whose scope it was + allocated is used. + Note that the result may include sources that can no longer ever be used by the debuggee: say, eval code that has finished running, or source for unreachable functions. Whether such sources appear can be diff --git a/js/src/jit-test/tests/debug/Debugger-findSources-01.js b/js/src/jit-test/tests/debug/Debugger-findSources-01.js deleted file mode 100644 index 4cac711e52dc..000000000000 --- a/js/src/jit-test/tests/debug/Debugger-findSources-01.js +++ /dev/null @@ -1,4 +0,0 @@ -// In a debugger with no debuggees, findSources should return no scripts. - -const dbg = new Debugger; -assertEq(dbg.findSources().length, 0); diff --git a/js/src/jit-test/tests/debug/Debugger-findSources-02.js b/js/src/jit-test/tests/debug/Debugger-findSources-02.js deleted file mode 100644 index da71bf17b958..000000000000 --- a/js/src/jit-test/tests/debug/Debugger-findSources-02.js +++ /dev/null @@ -1,15 +0,0 @@ -// In a debugger with scripts, findSources finds the script source. - -const g = newGlobal(); -// Declare a function in order to keep the script source alive across GC. -g.evaluate(`function fa() {}`, { fileName: "a.js" }); -g.evaluate(`function fb() {}`, { fileName: "b.js" }); -g.evaluate(`function fc() {}`, { fileName: "c.js" }); - -const dbg = new Debugger(); -const gw = dbg.addDebuggee(g); - -const sources = dbg.findSources(); -assertEq(sources.filter(s => s.url == "a.js").length, 1); -assertEq(sources.filter(s => s.url == "b.js").length, 1); -assertEq(sources.filter(s => s.url == "c.js").length, 1); diff --git a/js/src/jit-test/tests/debug/Debugger-findSources-03.js b/js/src/jit-test/tests/debug/Debugger-findSources-03.js deleted file mode 100644 index 2a8d3cb55bdb..000000000000 --- a/js/src/jit-test/tests/debug/Debugger-findSources-03.js +++ /dev/null @@ -1,19 +0,0 @@ -// In a debugger with multiple debuggees, findSources finds script sources across all debuggees. - -const g1 = newGlobal(); -const g2 = newGlobal(); -// Declare a function in order to keep the script source alive across GC. -g1.evaluate(`function fa() {}`, { fileName: "a.js" }); -g1.evaluate(`function fb() {}`, { fileName: "b.js" }); -g2.evaluate(`function fc() {}`, { fileName: "c.js" }); -g2.evaluate(`function fd() {}`, { fileName: "d.js" }); - -const dbg = new Debugger(); -const g1w = dbg.addDebuggee(g1); -const g2w = dbg.addDebuggee(g2); - -const sources = dbg.findSources(); -assertEq(dbg.findSources().filter(s => s.url == "a.js").length, 1); -assertEq(dbg.findSources().filter(s => s.url == "b.js").length, 1); -assertEq(dbg.findSources().filter(s => s.url == "c.js").length, 1); -assertEq(dbg.findSources().filter(s => s.url == "d.js").length, 1); diff --git a/js/src/vm/Debugger.cpp b/js/src/vm/Debugger.cpp index eae8d0349646..c1581c6a67ee 100644 --- a/js/src/vm/Debugger.cpp +++ b/js/src/vm/Debugger.cpp @@ -4188,73 +4188,19 @@ Debugger::removeDebuggeeGlobal(FreeOp* fop, GlobalObject* global, static inline DebuggerSourceReferent GetSourceReferent(JSObject* obj); -class MOZ_STACK_CLASS Debugger::QueryBase -{ - protected: - QueryBase(JSContext* cx, Debugger* dbg) - : cx(cx), - debugger(dbg), - iterMarker(&cx->runtime()->gc), - realms(cx->zone()), - oom(false) - {} - - // The context in which we should do our work. - JSContext* cx; - - // The debugger for which we conduct queries. - Debugger* debugger; - - // Require the set of realms to stay fixed while this query is alive. - gc::AutoEnterIteration iterMarker; - - using RealmSet = HashSet, ZoneAllocPolicy>; - - // A script must be in one of these realms to match the query. - RealmSet realms; - - // Indicates whether OOM has occurred while matching. - bool oom; - - bool addRealm(Realm* realm) { - return realms.put(realm); - } - - // Arrange for this query to match only scripts that run in |global|. - bool matchSingleGlobal(GlobalObject* global) { - MOZ_ASSERT(realms.count() == 0); - if (!addRealm(global->realm())) { - ReportOutOfMemory(cx); - return false; - } - return true; - } - - // Arrange for this ScriptQuery to match all scripts running in debuggee - // globals. - bool matchAllDebuggeeGlobals() { - MOZ_ASSERT(realms.count() == 0); - // Build our realm set from the debugger's set of debuggee globals. - for (WeakGlobalObjectSet::Range r = debugger->debuggees.all(); !r.empty(); r.popFront()) { - if (!addRealm(r.front()->realm())) { - ReportOutOfMemory(cx); - return false; - } - } - return true; - } -}; - /* * A class for parsing 'findScripts' query arguments and searching for * scripts that match the criteria they represent. */ -class MOZ_STACK_CLASS Debugger::ScriptQuery : public Debugger::QueryBase +class MOZ_STACK_CLASS Debugger::ScriptQuery { public: /* Construct a ScriptQuery to use matching scripts for |dbg|. */ - ScriptQuery(JSContext* cx, Debugger* dbg) - : QueryBase(cx, dbg), + ScriptQuery(JSContext* cx, Debugger* dbg): + cx(cx), + debugger(dbg), + iterMarker(&cx->runtime()->gc), + realms(cx->zone()), url(cx), displayURLString(cx), hasSource(false), @@ -4265,7 +4211,8 @@ class MOZ_STACK_CLASS Debugger::ScriptQuery : public Debugger::QueryBase innermostForRealm(cx->zone()), scriptVector(cx, ScriptVector(cx)), lazyScriptVector(cx, LazyScriptVector(cx)), - wasmInstanceVector(cx, WasmInstanceObjectVector(cx)) + wasmInstanceVector(cx, WasmInstanceObjectVector(cx)), + oom(false) {} /* @@ -4489,6 +4436,20 @@ class MOZ_STACK_CLASS Debugger::ScriptQuery : public Debugger::QueryBase } private: + /* The context in which we should do our work. */ + JSContext* cx; + + /* The debugger for which we conduct queries. */ + Debugger* debugger; + + /* Require the set of realms to stay fixed while the ScriptQuery is alive. */ + gc::AutoEnterIteration iterMarker; + + using RealmSet = HashSet, ZoneAllocPolicy>; + + /* A script must be in one of these realms to match the query. */ + RealmSet realms; + /* If this is a string, matching scripts have urls equal to it. */ RootedValue url; @@ -4538,6 +4499,39 @@ class MOZ_STACK_CLASS Debugger::ScriptQuery : public Debugger::QueryBase */ Rooted wasmInstanceVector; + /* Indicates whether OOM has occurred while matching. */ + bool oom; + + bool addRealm(Realm* realm) { + return realms.put(realm); + } + + /* Arrange for this ScriptQuery to match only scripts that run in |global|. */ + bool matchSingleGlobal(GlobalObject* global) { + MOZ_ASSERT(realms.count() == 0); + if (!addRealm(global->realm())) { + ReportOutOfMemory(cx); + return false; + } + return true; + } + + /* + * Arrange for this ScriptQuery to match all scripts running in debuggee + * globals. + */ + bool matchAllDebuggeeGlobals() { + MOZ_ASSERT(realms.count() == 0); + // Build our realm set from the debugger's set of debuggee globals. + for (WeakGlobalObjectSet::Range r = debugger->debuggees.all(); !r.empty(); r.popFront()) { + if (!addRealm(r.front()->realm())) { + ReportOutOfMemory(cx); + return false; + } + } + return true; + } + /* * Given that parseQuery or omittedQuery has been called, prepare to match * scripts. Set urlCString and displayURLChars as appropriate. @@ -4771,160 +4765,6 @@ Debugger::findScripts(JSContext* cx, unsigned argc, Value* vp) return true; } -/* - * A class for searching sources for 'findSources'. - */ -class MOZ_STACK_CLASS Debugger::SourceQuery : public Debugger::QueryBase -{ - public: - using SourceSet = JS::GCHashSet, - ZoneAllocPolicy>; - - SourceQuery(JSContext* cx, Debugger* dbg) - : QueryBase(cx, dbg), - sources(cx, SourceSet(cx->zone())) - {} - - bool findSources() { - if (!matchAllDebuggeeGlobals()) - return false; - - Realm* singletonRealm = nullptr; - if (realms.count() == 1) - singletonRealm = realms.all().front(); - - // Search each realm for debuggee scripts. - MOZ_ASSERT(sources.empty()); - oom = false; - IterateScripts(cx, singletonRealm, this, considerScript); - IterateLazyScripts(cx, singletonRealm, this, considerLazyScript); - if (oom) { - ReportOutOfMemory(cx); - return false; - } - - // TODO: Until such time that wasm modules are real ES6 modules, - // unconditionally consider all wasm toplevel instance scripts. - for (WeakGlobalObjectSet::Range r = debugger->allDebuggees(); !r.empty(); r.popFront()) { - for (wasm::Instance* instance : r.front()->realm()->wasm.instances()) { - consider(instance->object()); - if (oom) { - ReportOutOfMemory(cx); - return false; - } - } - } - - return true; - } - - Handle foundSources() const { - return sources; - } - - private: - Rooted sources; - - static void considerScript(JSRuntime* rt, void* data, JSScript* script, - const JS::AutoRequireNoGC& nogc) { - SourceQuery* self = static_cast(data); - self->consider(script, nogc); - } - - static void considerLazyScript(JSRuntime* rt, void* data, LazyScript* lazyScript, - const JS::AutoRequireNoGC& nogc) { - SourceQuery* self = static_cast(data); - self->consider(lazyScript, nogc); - } - - void consider(JSScript* script, const JS::AutoRequireNoGC& nogc) { - if (oom || script->selfHosted()) - return; - Realm* realm = script->realm(); - if (!realms.has(realm)) - return; - - if (!script->sourceObject()) - return; - - ScriptSourceObject* source = - &UncheckedUnwrap(script->sourceObject())->as(); - if (!sources.put(source)) - oom = true; - } - - void consider(LazyScript* lazyScript, const JS::AutoRequireNoGC& nogc) { - if (oom) - return; - Realm* realm = lazyScript->realm(); - if (!realms.has(realm)) - return; - - // If the script is already delazified, it should already be handled. - if (lazyScript->maybeScript()) - return; - - ScriptSourceObject* source = &lazyScript->sourceObject(); - if (!sources.put(source)) - oom = true; - } - - void consider(WasmInstanceObject* instanceObject) { - if (oom) - return; - - if (!sources.put(instanceObject)) - oom = true; - } -}; - -static inline DebuggerSourceReferent -AsSourceReferent(JSObject* obj) -{ - if (obj->is()) { - return AsVariant(&obj->as()); - } - return AsVariant(&obj->as()); -} - -/* static */ bool -Debugger::findSources(JSContext* cx, unsigned argc, Value* vp) -{ - THIS_DEBUGGER(cx, argc, vp, "findSources", args, dbg); - - if (gc::GCRuntime::temporaryAbortIfWasmGc(cx)) { - JS_ReportErrorASCII(cx, "API temporarily unavailable under wasm gc"); - return false; - } - - SourceQuery query(cx, dbg); - if (!query.findSources()) - return false; - - Handle sources(query.foundSources()); - - size_t resultLength = sources.count(); - RootedArrayObject result(cx, NewDenseFullyAllocatedArray(cx, resultLength)); - if (!result) - return false; - - result->ensureDenseInitializedLength(cx, 0, resultLength); - - size_t i = 0; - for (auto iter = sources.get().iter(); !iter.done(); iter.next()) { - Rooted sourceReferent(cx, AsSourceReferent(iter.get())); - RootedObject sourceObject(cx, dbg->wrapVariantReferent(cx, sourceReferent)); - if (!sourceObject) - return false; - result->setDenseElement(i, ObjectValue(*sourceObject)); - i++; - } - - args.rval().setObject(*result); - return true; -} - /* * A class for parsing 'findObjects' query arguments and searching for objects * that match the criteria they represent. @@ -5341,7 +5181,6 @@ const JSFunctionSpec Debugger::methods[] = { JS_FN("getNewestFrame", Debugger::getNewestFrame, 0, 0), JS_FN("clearAllBreakpoints", Debugger::clearAllBreakpoints, 0, 0), JS_FN("findScripts", Debugger::findScripts, 1, 0), - JS_FN("findSources", Debugger::findSources, 1, 0), JS_FN("findObjects", Debugger::findObjects, 1, 0), JS_FN("findAllGlobals", Debugger::findAllGlobals, 0, 0), JS_FN("makeGlobalObjectReference", Debugger::makeGlobalObjectReference, 1, 0), diff --git a/js/src/vm/Debugger.h b/js/src/vm/Debugger.h index 94ff729b1e7e..ff8f18e4a6b6 100644 --- a/js/src/vm/Debugger.h +++ b/js/src/vm/Debugger.h @@ -566,9 +566,7 @@ class Debugger : private mozilla::LinkedListElement uint32_t traceLoggerScriptedCallsLastDrainedSize; uint32_t traceLoggerScriptedCallsLastDrainedIteration; - class QueryBase; class ScriptQuery; - class SourceQuery; class ObjectQuery; MOZ_MUST_USE bool addDebuggeeGlobal(JSContext* cx, Handle obj); @@ -720,7 +718,6 @@ class Debugger : private mozilla::LinkedListElement static bool getNewestFrame(JSContext* cx, unsigned argc, Value* vp); static bool clearAllBreakpoints(JSContext* cx, unsigned argc, Value* vp); static bool findScripts(JSContext* cx, unsigned argc, Value* vp); - static bool findSources(JSContext* cx, unsigned argc, Value* vp); static bool findObjects(JSContext* cx, unsigned argc, Value* vp); static bool findAllGlobals(JSContext* cx, unsigned argc, Value* vp); static bool makeGlobalObjectReference(JSContext* cx, unsigned argc, Value* vp);