diff --git a/devtools/server/actors/webconsole/eval-with-debugger.js b/devtools/server/actors/webconsole/eval-with-debugger.js index d2fc63b4c0ab..59313dd3db00 100644 --- a/devtools/server/actors/webconsole/eval-with-debugger.js +++ b/devtools/server/actors/webconsole/eval-with-debugger.js @@ -5,6 +5,7 @@ "use strict"; +const Debugger = require("Debugger"); const DevToolsUtils = require("devtools/shared/DevToolsUtils"); loader.lazyRequireGetter( @@ -162,7 +163,7 @@ exports.evalWithDebugger = function(string, options = {}, webConsole) { ); if (options.eager) { - allowSideEffects(dbg, sideEffectData); + allowSideEffects(sideEffectData); } // Attempt to initialize any declarations found in the evaluated string @@ -268,17 +269,17 @@ function preventSideEffects(dbg) { throw new Error("Debugger has hook installed"); } - const data = { - executedScripts: new Set(), - debuggees: dbg.getDebuggees(), - - handler: { - hit: () => null, - }, - }; + // Note: It is critical for debuggee performance that we implement all of + // this debuggee tracking logic with a separate Debugger instance. + // Bug 1617666 arises otherwise if we set an onEnterFrame hook on the + // existing debugger object and then later clear it. + const newDbg = new Debugger(); + for (const debuggee of dbg.getDebuggees()) { + newDbg.addDebuggee(debuggee.unsafeDereference()); + } // TODO: re-enable addAllGlobalsAsDebuggees(bug #1610532) - // dbg.addAllGlobalsAsDebuggees(); + // newDbg.addAllGlobalsAsDebuggees(); const timeoutDuration = 100; const endTime = Date.now() + timeoutDuration; @@ -290,7 +291,12 @@ function preventSideEffects(dbg) { return ++count % 100 === 0 && Date.now() > endTime; } - dbg.onEnterFrame = frame => { + const executedScripts = new Set(); + const handler = { + hit: () => null, + }; + + newDbg.onEnterFrame = frame => { if (shouldCancel()) { return null; } @@ -303,19 +309,22 @@ function preventSideEffects(dbg) { const script = frame.script; - if (data.executedScripts.has(script)) { + if (executedScripts.has(script)) { return undefined; } - data.executedScripts.add(script); + executedScripts.add(script); const offsets = script.getEffectfulOffsets(); for (const offset of offsets) { - script.setBreakpoint(offset, data.handler); + script.setBreakpoint(offset, handler); } return undefined; }; + // The debugger only calls onNativeCall handlers on the debugger that is + // explicitly calling eval, so we need to add this hook on "dbg" even though + // the rest of our hooks work via "newDbg". dbg.onNativeCall = (callee, reason) => { // Getters are never considered effectful, and setters are always effectful. // Natives called normally are handled with a whitelist. @@ -330,22 +339,15 @@ function preventSideEffects(dbg) { return null; }; - return data; + return { + dbg, + newDbg, + }; } -function allowSideEffects(dbg, data) { - for (const script of data.executedScripts) { - script.clearBreakpoint(data.handler); - } - - for (const global of dbg.getDebuggees()) { - if (!data.debuggees.includes(global)) { - dbg.removeDebuggee(global); - } - } - - dbg.onEnterFrame = undefined; - dbg.onNativeCall = undefined; +function allowSideEffects(data) { + data.dbg.onNativeCall = undefined; + data.newDbg.removeAllDebuggees(); } // Native functions which are considered to be side effect free.