Bug 1590881 - Remove BreakpointSite .type/asWasm()/asJS() to simplify interface. r=jimb
authorLogan Smyth <loganfsmyth@gmail.com>
Tue, 12 Nov 2019 17:15:14 +0000
changeset 501708 aad72f40920e95b959c193e919269cba49bbd358
parent 501707 373ebff70ac21d0a39944739b4573c3f2d30ded9
child 501709 cd37ff69e786ef07edd241f0c8298817fe7174c2
push id114172
push userdluca@mozilla.com
push dateTue, 19 Nov 2019 11:31:10 +0000
treeherdermozilla-inbound@b5c5ba07d3db [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjimb
bugs1590881
milestone72.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
Bug 1590881 - Remove BreakpointSite .type/asWasm()/asJS() to simplify interface. r=jimb Differential Revision: https://phabricator.services.mozilla.com/D52278
js/src/debugger/DebugScript.cpp
js/src/debugger/Debugger.cpp
js/src/debugger/Debugger.h
js/src/debugger/Script.cpp
js/src/wasm/WasmDebug.cpp
--- a/js/src/debugger/DebugScript.cpp
+++ b/js/src/debugger/DebugScript.cpp
@@ -133,17 +133,16 @@ JSBreakpointSite* DebugScript::getOrCrea
 }
 
 /* static */
 void DebugScript::destroyBreakpointSite(JSFreeOp* fop, JSScript* script,
                                         jsbytecode* pc) {
   DebugScript* debug = get(script);
   JSBreakpointSite*& site = debug->breakpoints[script->pcToOffset(pc)];
   MOZ_ASSERT(site);
-  MOZ_ASSERT(site->type() == BreakpointSite::Type::JS);
   MOZ_ASSERT(site->isEmpty());
 
   site->delete_(fop);
   site = nullptr;
 
   debug->numSites--;
   if (!debug->needed()) {
     DebugAPI::destroyDebugScript(fop, script);
@@ -325,18 +324,17 @@ void DebugScript::delete_(JSFreeOp* fop,
 }
 
 #ifdef JSGC_HASH_TABLE_CHECKS
 /* static */
 void DebugAPI::checkDebugScriptAfterMovingGC(DebugScript* ds) {
   for (uint32_t i = 0; i < ds->numSites; i++) {
     JSBreakpointSite* site = ds->breakpoints[i];
     if (site) {
-      MOZ_ASSERT(site->type() == BreakpointSite::Type::JS);
-      CheckGCThingAfterMovingGC(site->asJS()->script.get());
+      CheckGCThingAfterMovingGC(site->script.get());
     }
   }
 }
 #endif  // JSGC_HASH_TABLE_CHECKS
 
 /* static */
 bool DebugAPI::stepModeEnabledSlow(JSScript* script) {
   return DebugScript::get(script)->stepperCount > 0;
--- a/js/src/debugger/Debugger.cpp
+++ b/js/src/debugger/Debugger.cpp
@@ -300,18 +300,16 @@ bool js::ParseEvalOptions(JSContext* cx,
     options.setLineno(lineno);
   }
 
   return true;
 }
 
 /*** Breakpoints ************************************************************/
 
-BreakpointSite::BreakpointSite(Type type) : type_(type) {}
-
 bool BreakpointSite::isEmpty() const { return breakpoints.isEmpty(); }
 
 void BreakpointSite::trace(JSTracer* trc) {
   for (auto p = breakpoints.begin(); p; p++) {
     p->trace(trc);
   }
 }
 
@@ -333,24 +331,16 @@ bool BreakpointSite::hasBreakpoint(Break
   for (auto p = breakpoints.begin(); p; p++) {
     if (p == bp) {
       return true;
     }
   }
   return false;
 }
 
-inline gc::Cell* BreakpointSite::owningCell() {
-  if (type() == Type::JS) {
-    return asJS()->script;
-  }
-
-  return asWasm()->instanceObject;
-}
-
 Breakpoint::Breakpoint(Debugger* debugger, HandleObject wrappedDebugger,
                        BreakpointSite* site, HandleObject handler)
     : debugger(debugger),
       wrappedDebugger(wrappedDebugger),
       site(site),
       handler(handler) {
   MOZ_ASSERT(UncheckedUnwrap(wrappedDebugger) == debugger->object);
   MOZ_ASSERT(handler->compartment() == wrappedDebugger->compartment());
@@ -378,17 +368,17 @@ void Breakpoint::remove(JSFreeOp* fop) {
   savedSite->destroyIfEmpty(fop);
 }
 
 Breakpoint* Breakpoint::nextInDebugger() { return debuggerLink.mNext; }
 
 Breakpoint* Breakpoint::nextInSite() { return siteLink.mNext; }
 
 JSBreakpointSite::JSBreakpointSite(JSScript* script, jsbytecode* pc)
-    : BreakpointSite(Type::JS), script(script), pc(pc) {
+    : script(script), pc(pc) {
   MOZ_ASSERT(!DebugAPI::hasBreakpointsAt(script, pc));
 }
 
 void JSBreakpointSite::remove(JSFreeOp* fop) {
   DebugScript::destroyBreakpointSite(fop, script, pc);
 }
 
 void JSBreakpointSite::trace(JSTracer* trc) {
@@ -397,21 +387,23 @@ void JSBreakpointSite::trace(JSTracer* t
 }
 
 void JSBreakpointSite::delete_(JSFreeOp* fop) {
   BreakpointSite::finalize(fop);
 
   fop->delete_(script, this, MemoryUse::BreakpointSite);
 }
 
+gc::Cell* JSBreakpointSite::owningCell() { return script; }
+
+Realm* JSBreakpointSite::realm() const { return script->realm(); }
+
 WasmBreakpointSite::WasmBreakpointSite(WasmInstanceObject* instanceObject_,
                                        uint32_t offset_)
-    : BreakpointSite(Type::Wasm),
-      instanceObject(instanceObject_),
-      offset(offset_) {
+    : instanceObject(instanceObject_), offset(offset_) {
   MOZ_ASSERT(instanceObject_);
   MOZ_ASSERT(instanceObject_->instance().debugEnabled());
 }
 
 void WasmBreakpointSite::trace(JSTracer* trc) {
   BreakpointSite::trace(trc);
   TraceEdge(trc, &instanceObject, "breakpoint Wasm instance");
 }
@@ -421,16 +413,20 @@ void WasmBreakpointSite::remove(JSFreeOp
 }
 
 void WasmBreakpointSite::delete_(JSFreeOp* fop) {
   BreakpointSite::finalize(fop);
 
   fop->delete_(instanceObject, this, MemoryUse::BreakpointSite);
 }
 
+gc::Cell* WasmBreakpointSite::owningCell() { return instanceObject; }
+
+Realm* WasmBreakpointSite::realm() const { return instanceObject->realm(); }
+
 /*** Debugger hook dispatch *************************************************/
 
 Debugger::Debugger(JSContext* cx, NativeObject* dbg)
     : object(dbg),
       debuggees(cx->zone()),
       uncaughtExceptionHook(nullptr),
       allowUnobservedAsmJS(false),
       collectCoverageInfo(false),
@@ -4732,29 +4728,19 @@ void Debugger::removeDebuggeeGlobal(JSFr
   }
 
   recomputeDebuggeeZoneSet();
 
   // Remove all breakpoints for the debuggee.
   Breakpoint* nextbp;
   for (Breakpoint* bp = firstBreakpoint(); bp; bp = nextbp) {
     nextbp = bp->nextInDebugger();
-    switch (bp->site->type()) {
-      case BreakpointSite::Type::JS:
-        if (bp->site->asJS()->script->realm() == global->realm()) {
-          bp->remove(fop);
-        }
-        break;
-      case BreakpointSite::Type::Wasm:
-        if (bp->site->asWasm()->instanceObject->realm() == global->realm()) {
-          bp->remove(fop);
-        }
-        break;
-      default:
-        MOZ_CRASH("unknown breakpoint type");
+
+    if (bp->site->realm() == global->realm()) {
+      bp->remove(fop);
     }
   }
   MOZ_ASSERT_IF(debuggees.empty(), !firstBreakpoint());
 
   // If we are tracking allocation sites, we need to remove the object
   // metadata callback from this global's realm.
   if (trackingAllocationSites) {
     Debugger::removeAllocationsTracking(*global);
--- a/js/src/debugger/Debugger.h
+++ b/js/src/debugger/Debugger.h
@@ -1241,57 +1241,48 @@ class WasmBreakpointSite;
  * sorts of code a breakpoint might be set in. JSBreakpointSite manages sites in
  * JSScripts, and WasmBreakpointSite manages sites in WasmInstances.
  */
 class BreakpointSite {
   friend class DebugAPI;
   friend class Breakpoint;
   friend class Debugger;
 
- public:
-  enum class Type { JS, Wasm };
-
  private:
-  Type type_;
-
   template <typename T>
   struct SiteLinkAccess {
     static mozilla::DoublyLinkedListElement<T>& Get(T* aThis) {
       return aThis->siteLink;
     }
   };
 
   // List of all js::Breakpoints at this instruction.
   using BreakpointList =
       mozilla::DoublyLinkedList<js::Breakpoint, SiteLinkAccess<js::Breakpoint>>;
   BreakpointList breakpoints;
 
-  gc::Cell* owningCell();
-
  protected:
-  BreakpointSite(Type type);
+  BreakpointSite(){};
   virtual ~BreakpointSite() {}
   void finalize(JSFreeOp* fop);
+  virtual gc::Cell* owningCell() = 0;
 
  public:
   Breakpoint* firstBreakpoint() const;
   bool hasBreakpoint(Breakpoint* bp);
-  Type type() const { return type_; }
 
   bool isEmpty() const;
   virtual void trace(JSTracer* trc);
   virtual void remove(JSFreeOp* fop) = 0;
   void destroyIfEmpty(JSFreeOp* fop) {
     if (isEmpty()) {
       remove(fop);
     }
   }
-
-  inline JSBreakpointSite* asJS();
-  inline WasmBreakpointSite* asWasm();
+  virtual Realm* realm() const = 0;
 };
 
 /*
  * A breakpoint set at a given BreakpointSite, indicating the owning debugger
  * and the handler object. A Breakpoint is a member of two linked lists: its
  * owning debugger's list and its site's list.
  */
 class Breakpoint {
@@ -1373,40 +1364,38 @@ class JSBreakpointSite : public Breakpoi
   jsbytecode* const pc;
 
  public:
   JSBreakpointSite(JSScript* script, jsbytecode* pc);
 
   void trace(JSTracer* trc) override;
   void delete_(JSFreeOp* fop);
   void remove(JSFreeOp* fop) override;
-};
+  Realm* realm() const override;
 
-inline JSBreakpointSite* BreakpointSite::asJS() {
-  MOZ_ASSERT(type() == Type::JS);
-  return static_cast<JSBreakpointSite*>(this);
-}
+ private:
+  gc::Cell* owningCell() override;
+};
 
 class WasmBreakpointSite : public BreakpointSite {
  public:
   const HeapPtr<WasmInstanceObject*> instanceObject;
   uint32_t offset;
 
  public:
   WasmBreakpointSite(WasmInstanceObject* instanceObject, uint32_t offset);
 
   void trace(JSTracer* trc) override;
   void delete_(JSFreeOp* fop);
   void remove(JSFreeOp* fop) override;
-};
+  Realm* realm() const override;
 
-inline WasmBreakpointSite* BreakpointSite::asWasm() {
-  MOZ_ASSERT(type() == Type::Wasm);
-  return static_cast<WasmBreakpointSite*>(this);
-}
+ private:
+  gc::Cell* owningCell() override;
+};
 
 Breakpoint* Debugger::firstBreakpoint() const {
   if (breakpoints.isEmpty()) {
     return nullptr;
   }
   return &(*breakpoints.begin());
 }
 
--- a/js/src/debugger/Script.cpp
+++ b/js/src/debugger/Script.cpp
@@ -2180,18 +2180,17 @@ bool DebuggerScript::CallData::getBreakp
   }
 
   for (unsigned i = 0; i < script->length(); i++) {
     JSBreakpointSite* site =
         DebugScript::getBreakpointSite(script, script->offsetToPC(i));
     if (!site) {
       continue;
     }
-    MOZ_ASSERT(site->type() == BreakpointSite::Type::JS);
-    if (!pc || site->asJS()->pc == pc) {
+    if (!pc || site->pc == pc) {
       for (Breakpoint* bp = site->firstBreakpoint(); bp;
            bp = bp->nextInSite()) {
         if (bp->debugger == dbg) {
           RootedObject handler(cx, bp->getHandler());
           if (!cx->compartment()->wrap(cx, &handler) ||
               !NewbornArrayPush(cx, arr, ObjectValue(*handler))) {
             return false;
           }
--- a/js/src/wasm/WasmDebug.cpp
+++ b/js/src/wasm/WasmDebug.cpp
@@ -255,21 +255,23 @@ void DebugState::clearBreakpointsIn(JSFr
   MOZ_ASSERT_IF(handler, instance->compartment() == handler->compartment());
 
   if (breakpointSites_.empty()) {
     return;
   }
   for (WasmBreakpointSiteMap::Enum e(breakpointSites_); !e.empty();
        e.popFront()) {
     WasmBreakpointSite* site = e.front().value();
+    MOZ_ASSERT(site->instanceObject == instance);
+
     Breakpoint* nextbp;
     for (Breakpoint* bp = site->firstBreakpoint(); bp; bp = nextbp) {
       nextbp = bp->nextInSite();
-      if (bp->site->asWasm()->instanceObject == instance &&
-          (!dbg || bp->debugger == dbg) &&
+      MOZ_ASSERT(bp->site == site);
+      if ((!dbg || bp->debugger == dbg) &&
           (!handler || bp->getHandler() == handler)) {
         bp->delete_(fop);
       }
     }
     if (site->isEmpty()) {
       fop->delete_(instance, site, MemoryUse::BreakpointSite);
       e.removeFront();
     }