Bug 1564168 - [jsdbg2] Debugger.prototype.enabled should be removed.

Differential Revision: https://phabricator.services.mozilla.com/D39414

--HG--
extra : moz-landing-system : lando

Author: Jason Laster

js/ductwork/debugger/tests/test_nativewrappers.js

@@ -34,5 +34,5 @@ function run_test()
     g.stopMe(doc.createEvent("MouseEvent"));
   } + ")()");
 
-  dbg.enabled = false;
+  dbg.removeAllDebuggees();
 }

js/examples/jorendb.js

@@ -247,7 +247,7 @@ function evalCommand(expr) {
 }
 
 function quitCommand() {
-    dbg.enabled = false;
+    dbg.removeAllDebuggees();
     quit(0);
 }
 
@@ -300,16 +300,10 @@ function doPrint(expr, style) {
               ? debuggeeGlobalWrapper.executeInGlobalWithBindings(expr, debuggeeValues)
               : focusedFrame.evalWithBindings(expr, debuggeeValues));
     if (cv === null) {
-        if (!dbg.enabled)
-            return [cv];
         print("Debuggee died.");
     } else if ('return' in cv) {
-        if (!dbg.enabled)
-            return [undefined];
         showDebuggeeValue(cv.return, style);
     } else {
-        if (!dbg.enabled)
-            return [cv];
         print("Exception caught. (To rethrow it, type 'throw'.)");
         lastExc = cv.throw;
         showDebuggeeValue(lastExc, style);
@@ -324,7 +318,7 @@ function printCommand(rest) {
 function keysCommand(rest) { return doPrint("Object.keys(" + rest + ")"); }
 
 function detachCommand() {
-    dbg.enabled = false;
+    dbg.removeAllDebuggees();
     return [undefined];
 }
 
@@ -355,14 +349,10 @@ function throwCommand(rest) {
     } else {
         var cv = saveExcursion(function () { return focusedFrame.eval(rest); });
         if (cv === null) {
-            if (!dbg.enabled)
-                return [cv];
             print("Debuggee died while determining what to throw. Stopped.");
         } else if ('return' in cv) {
             return [{throw: cv.return}];
         } else {
-            if (!dbg.enabled)
-                return [cv];
             print("Exception determining what to throw. Stopped.");
             showDebuggeeValue(cv.throw);
         }
@@ -439,14 +429,10 @@ function forcereturnCommand(rest) {
     } else {
         var cv = saveExcursion(function () { return f.eval(rest); });
         if (cv === null) {
-            if (!dbg.enabled)
-                return [cv];
             print("Debuggee died while determining what to forcereturn. Stopped.");
         } else if ('return' in cv) {
             return [{return: cv.return}];
         } else {
-            if (!dbg.enabled)
-                return [cv];
             print("Error determining what to forcereturn. Stopped.");
             showDebuggeeValue(cv.throw);
         }

js/src/debugger/Debugger.cpp

@@ -368,9 +368,7 @@ Breakpoint::Breakpoint(Debugger* debugger, BreakpointSite* site,
 
 void Breakpoint::destroy(JSFreeOp* fop,
                          MayDestroySite mayDestroySite /* true */) {
-  if (debugger->enabled) {
-    site->dec(fop);
-  }
+  site->dec(fop);
   debugger->breakpoints.remove(this);
   site->breakpoints.remove(this);
   gc::Cell* cell = site->owningCellUnbarriered();
@@ -425,7 +423,6 @@ Debugger::Debugger(JSContext* cx, NativeObject* dbg)
     : object(dbg),
       debuggees(cx->zone()),
       uncaughtExceptionHook(nullptr),
-      enabled(true),
       allowUnobservedAsmJS(false),
       collectCoverageInfo(false),
       observedGCs(cx->zone()),
@@ -682,9 +679,8 @@ static bool DebuggerExists(
 
 /* static */
 bool Debugger::hasLiveHook(GlobalObject* global, Hook which) {
-  return DebuggerExists(global, [=](Debugger* dbg) {
-    return dbg->enabled && dbg->getHook(which);
-  });
+  return DebuggerExists(global,
+                        [=](Debugger* dbg) { return dbg->getHook(which); });
 }
 
 /* static */
@@ -722,10 +718,6 @@ JSObject* Debugger::getHook(Hook hook) const {
 }
 
 bool Debugger::hasAnyLiveHooks(JSRuntime* rt) const {
-  if (!enabled) {
-    return false;
-  }
-
   // A onNewGlobalObject hook does not hold its Debugger live, so its behavior
   // is nondeterministic. This behavior is not satisfying, but it is at least
   // documented.
@@ -964,7 +956,7 @@ bool DebugAPI::slowPathOnLeaveFrame(JSContext* cx, AbstractFramePtr frame,
       Debugger* dbg = Debugger::fromChildJSObject(frameobj);
       EnterDebuggeeNoExecute nx(cx, *dbg, adjqi);
 
-      if (dbg->enabled && frameobj->isLive() && frameobj->onPopHandler()) {
+      if (frameobj->isLive() && frameobj->onPopHandler()) {
         OnPopHandler* handler = frameobj->onPopHandler();
 
         Maybe<AutoRealm> ar;
@@ -2237,7 +2229,7 @@ ResumeMode Debugger::dispatchHook(JSContext* cx, HookIsEnabledFun hookIsEnabled,
   if (GlobalObject::DebuggerVector* debuggers = global->getDebuggers()) {
     for (auto p = debuggers->begin(); p != debuggers->end(); p++) {
       Debugger* dbg = *p;
-      if (dbg->enabled && hookIsEnabled(dbg)) {
+      if (hookIsEnabled(dbg)) {
         if (!triggered.append(ObjectValue(*dbg->toJSObject()))) {
           return ResumeMode::Terminate;
         }
@@ -2258,7 +2250,7 @@ ResumeMode Debugger::dispatchHook(JSContext* cx, HookIsEnabledFun hookIsEnabled,
   for (Value* p = triggered.begin(); p != triggered.end(); p++) {
     Debugger* dbg = Debugger::fromJSObject(&p->toObject());
     EnterDebuggeeNoExecute nx(cx, *dbg, adjqi);
-    if (dbg->debuggees.has(global) && dbg->enabled && hookIsEnabled(dbg)) {
+    if (dbg->debuggees.has(global) && hookIsEnabled(dbg)) {
       ResumeMode resumeMode = fireHook(dbg);
       adjqi.runJobs();
       if (resumeMode != ResumeMode::Continue) {
@@ -2370,8 +2362,8 @@ ResumeMode DebugAPI::onTrap(JSContext* cx, MutableHandleValue vp) {
         continue;
       }
 
-      // There are two reasons we have to check whether dbg is enabled and
-      // debugging global.
+      // There are two reasons we have to check whether dbg is debugging
+      // global.
       //
       // One is just that one breakpoint handler can disable other Debuggers
       // or remove debuggees.
@@ -2380,8 +2372,7 @@ ResumeMode DebugAPI::onTrap(JSContext* cx, MutableHandleValue vp) {
       // specific global--until they are executed. Only now do we know which
       // global the script is running against.
       Debugger* dbg = bp->debugger;
-      bool hasDebuggee = dbg->enabled && dbg->debuggees.has(global);
-      if (hasDebuggee) {
+      if (dbg->debuggees.has(global)) {
         Maybe<AutoRealm> ar;
         ar.emplace(cx, dbg->object);
         EnterDebuggeeNoExecute nx(cx, *dbg, adjqi);
@@ -2676,7 +2667,7 @@ Maybe<double> DebugAPI::allocationSamplingProbability(GlobalObject* global) {
     // this is safe as long as dbgp does not escape.
     Debugger* dbgp = p->unbarrieredGet();
 
-    if (dbgp->trackingAllocationSites && dbgp->enabled) {
+    if (dbgp->trackingAllocationSites) {
       foundAnyDebuggers = true;
       probability = std::max(dbgp->allocationSamplingProbability, probability);
     }
@@ -2712,7 +2703,7 @@ bool DebugAPI::slowPathOnLogAllocationSite(JSContext* cx, HandleObject obj,
     // such that the vector gets reallocated.
     MOZ_ASSERT(dbgs.begin() == begin);
 
-    if ((*dbgp)->trackingAllocationSites && (*dbgp)->enabled &&
+    if ((*dbgp)->trackingAllocationSites &&
         !(*dbgp)->appendAllocationSite(cx, obj, frame, when)) {
       return false;
     }
@@ -2730,7 +2721,7 @@ bool Debugger::isDebuggeeUnbarriered(const Realm* realm) const {
 bool Debugger::appendAllocationSite(JSContext* cx, HandleObject obj,
                                     HandleSavedFrame frame,
                                     mozilla::TimeStamp when) {
-  MOZ_ASSERT(trackingAllocationSites && enabled);
+  MOZ_ASSERT(trackingAllocationSites);
 
   AutoRealm ar(cx, object);
   RootedObject wrappedFrame(cx, frame);
@@ -3257,21 +3248,21 @@ bool Debugger::hookObservesAllExecution(Hook which) {
 }
 
 Debugger::IsObserving Debugger::observesAllExecution() const {
-  if (enabled && !!getHook(OnEnterFrame)) {
+  if (!!getHook(OnEnterFrame)) {
     return Observing;
   }
   return NotObserving;
 }
 
 Debugger::IsObserving Debugger::observesAsmJS() const {
-  if (enabled && !allowUnobservedAsmJS) {
+  if (!allowUnobservedAsmJS) {
     return Observing;
   }
   return NotObserving;
 }
 
 Debugger::IsObserving Debugger::observesCoverage() const {
-  if (enabled && collectCoverageInfo) {
+  if (collectCoverageInfo) {
     return Observing;
   }
   return NotObserving;
@@ -3388,7 +3379,7 @@ bool DebugAPI::isObservedByDebuggerTrackingAllocations(
       // Use unbarrieredGet() to prevent triggering read barrier while
       // collecting, this is safe as long as dbg does not escape.
       Debugger* dbg = p->unbarrieredGet();
-      if (dbg->trackingAllocationSites && dbg->enabled) {
+      if (dbg->trackingAllocationSites) {
         return true;
       }
     }
@@ -3900,71 +3891,6 @@ static Debugger* Debugger_fromThisValue(JSContext* cx, const CallArgs& args,
   Debugger* dbg = Debugger_fromThisValue(cx, args, fnname); \
   if (!dbg) return false
 
-/* static */
-bool Debugger::getEnabled(JSContext* cx, unsigned argc, Value* vp) {
-  THIS_DEBUGGER(cx, argc, vp, "get enabled", args, dbg);
-  args.rval().setBoolean(dbg->enabled);
-  return true;
-}
-
-/* static */
-bool Debugger::setEnabled(JSContext* cx, unsigned argc, Value* vp) {
-  THIS_DEBUGGER(cx, argc, vp, "set enabled", args, dbg);
-  if (!args.requireAtLeast(cx, "Debugger.set enabled", 1)) {
-    return false;
-  }
-
-  bool wasEnabled = dbg->enabled;
-  dbg->enabled = ToBoolean(args[0]);
-
-  if (wasEnabled != dbg->enabled) {
-    if (dbg->trackingAllocationSites) {
-      if (wasEnabled) {
-        dbg->removeAllocationsTrackingForAllDebuggees();
-      } else {
-        if (!dbg->addAllocationsTrackingForAllDebuggees(cx)) {
-          dbg->enabled = false;
-          return false;
-        }
-      }
-    }
-
-    for (Breakpoint* bp = dbg->firstBreakpoint(); bp;
-         bp = bp->nextInDebugger()) {
-      if (!wasEnabled) {
-        bp->site->inc(cx->runtime()->defaultFreeOp());
-      } else {
-        bp->site->dec(cx->runtime()->defaultFreeOp());
-      }
-    }
-
-    // Add or remove ourselves from the runtime's list of Debuggers
-    // that care about new globals.
-    if (dbg->getHook(OnNewGlobalObject)) {
-      if (!wasEnabled) {
-        cx->runtime()->onNewGlobalObjectWatchers().pushBack(dbg);
-      } else {
-        cx->runtime()->onNewGlobalObjectWatchers().remove(dbg);
-      }
-    }
-
-    // Ensure the compartment is observable if we are re-enabling a
-    // Debugger with hooks that observe all execution.
-    if (!dbg->updateObservesAllExecutionOnDebuggees(
-            cx, dbg->observesAllExecution())) {
-      return false;
-    }
-
-    // Note: To toogle code coverage, we currently need to have no live
-    // stack frame, thus the coverage does not depend on the enabled flag.
-
-    dbg->updateObservesAsmJSOnDebuggees(dbg->observesAsmJS());
-  }
-
-  args.rval().setUndefined();
-  return true;
-}
-
 /* static */
 bool Debugger::getHookImpl(JSContext* cx, CallArgs& args, Debugger& dbg,
                            Hook which) {
@@ -4092,13 +4018,11 @@ bool Debugger::setOnNewGlobalObject(JSContext* cx, unsigned argc, Value* vp) {
 
   // Add or remove ourselves from the runtime's list of Debuggers that care
   // about new globals.
-  if (dbg->enabled) {
-    JSObject* newHook = dbg->getHook(OnNewGlobalObject);
-    if (!oldHook && newHook) {
-      cx->runtime()->onNewGlobalObjectWatchers().pushBack(dbg);
-    } else if (oldHook && !newHook) {
-      cx->runtime()->onNewGlobalObjectWatchers().remove(dbg);
-    }
+  JSObject* newHook = dbg->getHook(OnNewGlobalObject);
+  if (!oldHook && newHook) {
+    cx->runtime()->onNewGlobalObjectWatchers().pushBack(dbg);
+  } else if (oldHook && !newHook) {
+    cx->runtime()->onNewGlobalObjectWatchers().remove(dbg);
   }
 
   return true;
@@ -4626,13 +4550,13 @@ bool Debugger::addDebuggeeGlobal(JSContext* cx, Handle<GlobalObject*> global) {
   });
 
   // (5)
-  if (trackingAllocationSites && enabled &&
+  if (trackingAllocationSites &&
       !Debugger::addAllocationsTracking(cx, global)) {
     return false;
   }
 
   auto allocationsTrackingGuard = MakeScopeExit([&] {
-    if (trackingAllocationSites && enabled) {
+    if (trackingAllocationSites) {
       Debugger::removeAllocationsTracking(*global);
     }
   });
@@ -6055,7 +5979,6 @@ bool Debugger::adoptSource(JSContext* cx, unsigned argc, Value* vp) {
 }
 
 const JSPropertySpec Debugger::properties[] = {
-    JS_PSGS("enabled", Debugger::getEnabled, Debugger::setEnabled, 0),
     JS_PSGS("onDebuggerStatement", Debugger::getOnDebuggerStatement,
             Debugger::setOnDebuggerStatement, 0),
     JS_PSGS("onExceptionUnwind", Debugger::getOnExceptionUnwind,
@@ -6270,16 +6193,13 @@ bool Debugger::observesFrame(const FrameIter& iter) const {
 }
 
 bool Debugger::observesScript(JSScript* script) const {
-  if (!enabled) {
-    return false;
-  }
   // Don't ever observe self-hosted scripts: the Debugger API can break
   // self-hosted invariants.
   return observesGlobal(&script->global()) && !script->selfHosted();
 }
 
 bool Debugger::observesWasm(wasm::Instance* instance) const {
-  if (!enabled || !instance->debugEnabled()) {
+  if (!instance->debugEnabled()) {
     return false;
   }
   return observesGlobal(&instance->object()->global());
@@ -6792,7 +6712,7 @@ JS_PUBLIC_API bool FireOnGarbageCollectionHookRequired(JSContext* cx) {
   AutoCheckCannotGC noGC;
 
   for (Debugger* dbg : cx->runtime()->debuggerList()) {
-    if (dbg->enabled && dbg->observedGC(cx->runtime()->gc.majorGCCount()) &&
+    if (dbg->observedGC(cx->runtime()->gc.majorGCCount()) &&
         dbg->getHook(Debugger::OnGarbageCollection)) {
       return true;
     }
@@ -6812,7 +6732,7 @@ JS_PUBLIC_API bool FireOnGarbageCollectionHook(
     AutoCheckCannotGC noGC;
 
     for (Debugger* dbg : cx->runtime()->debuggerList()) {
-      if (dbg->enabled && dbg->observedGC(data->majorGCNumber()) &&
+      if (dbg->observedGC(data->majorGCNumber()) &&
           dbg->getHook(Debugger::OnGarbageCollection)) {
         if (!triggered.append(dbg->object)) {
           JS_ReportOutOfMemory(cx);