Bug 1539817 - Uplift Debugger Backend improvements. r=loganfsmyth a=pascalc
authorBrian Hackett <bhackett1024@gmail.com>
Wed, 03 Apr 2019 16:15:54 +0300
changeset 525891 55fcffe7c78859441841194cad8635abc000ac3f
parent 525890 9179d76ad6c23f3a3494187b5d65daa2f3a1f85b
child 525892 10df2c9f7ce4c076dc4d6d7db9a1b8af23da8f2b
push id2032
push userffxbld-merge
push dateMon, 13 May 2019 09:36:57 +0000
treeherdermozilla-release@455c1065dcbe [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersloganfsmyth, pascalc
bugs1539817, 1536201, 1535071, 1536618
milestone67.0
Bug 1539817 - Uplift Debugger Backend improvements. r=loganfsmyth a=pascalc Bug 1536201 - Avoid calling findScripts() in a loop. r=jlast Bug 1535071 - Use the debugger global scope for worker error reporting if necessary, r=smaug,baku. Bug 1536618 Part 2 - Tolerate sources from any debugger in getOrCreateSourceActor, r=loganfsmyth. Bug 1536618 Part 1 - Add Debugger.adoptSource(), r=jorendorff.
devtools/server/actors/replay/debugger.js
devtools/server/actors/source.js
devtools/server/actors/utils/TabSources.js
dom/script/ScriptSettings.cpp
dom/workers/RuntimeService.cpp
dom/workers/WorkerCommon.h
js/src/doc/Debugger/Debugger.md
js/src/vm/Debugger.cpp
js/src/vm/Debugger.h
--- a/devtools/server/actors/replay/debugger.js
+++ b/devtools/server/actors/replay/debugger.js
@@ -539,16 +539,21 @@ ReplayDebugger.prototype = {
   },
 
   findSources() {
     this._ensurePaused();
     const data = this._sendRequest({ type: "findSources" });
     return data.map(source => this._addSource(source));
   },
 
+  adoptSource(source) {
+    assert(source._dbg == this);
+    return source;
+  },
+
   /////////////////////////////////////////////////////////
   // Object methods
   /////////////////////////////////////////////////////////
 
   _getObject(id) {
     if (id && !this._objects[id]) {
       const data = this._sendRequest({ type: "getObject", id });
       switch (data.kind) {
--- a/devtools/server/actors/source.js
+++ b/devtools/server/actors/source.js
@@ -287,31 +287,29 @@ const SourceActor = ActorClassWithSpec(s
         column: startColumn = 0,
       } = {},
       end: {
         line: endLine = Infinity,
         column: endColumn = Infinity,
       } = {},
     } = query || {};
 
-    let scripts;
-    if (Number.isFinite(endLine)) {
-      const found = new Set();
-      for (let line = startLine; line <= endLine; line++) {
-        for (const script of this._findDebuggeeScripts({ line })) {
-          found.add(script);
-        }
-      }
-      scripts = Array.from(found);
-    } else {
-      scripts = this._findDebuggeeScripts();
-    }
+    const scripts = this._findDebuggeeScripts();
 
     const positions = [];
     for (const script of scripts) {
+      // This purely a performance boost to avoid needing to build an array
+      // of breakable points for scripts when we know we don't need it.
+      if (
+        script.startLine > endLine ||
+        script.startLine + script.lineCount < startLine
+      ) {
+        continue;
+      }
+
       const offsets = script.getPossibleBreakpoints();
       for (const { lineNumber, columnNumber } of offsets) {
         if (
           lineNumber < startLine ||
           (lineNumber === startLine && columnNumber < startColumn) ||
           lineNumber > endLine ||
           (lineNumber === endLine && columnNumber > endColumn)
         ) {
--- a/devtools/server/actors/utils/TabSources.js
+++ b/devtools/server/actors/utils/TabSources.js
@@ -143,16 +143,29 @@ TabSources.prototype = {
       throw new Error("getSource: could not find source actor for " +
                       (source.url || "source"));
     }
 
     return sourceActor;
   },
 
   getOrCreateSourceActor(source) {
+    // Tolerate the source coming from a different Debugger than the one
+    // associated with the thread.
+    try {
+      source = this._thread.dbg.adoptSource(source);
+    } catch (e) {
+      // We can't create actors for sources in the same compartment as the
+      // thread's Debugger.
+      if (/is in the same compartment as this debugger/.test(e)) {
+        return null;
+      }
+      throw e;
+    }
+
     if (this.hasSourceActor(source)) {
       return this.getSourceActor(source);
     }
     return this.createSourceActor(source);
   },
 
   getSourceActorByInternalSourceId: function(id) {
     if (!this._sourcesByInternalSourceId) {
--- a/dom/script/ScriptSettings.cpp
+++ b/dom/script/ScriptSettings.cpp
@@ -478,16 +478,21 @@ void AutoJSAPI::ReportException() {
   // to be in a realm when we fetch the pending exception. In this case,
   // we enter the privileged junk scope and don't dispatch any error events.
   JS::Rooted<JSObject*> errorGlobal(cx(), JS::CurrentGlobalOrNull(cx()));
   if (!errorGlobal) {
     if (mIsMainThread) {
       errorGlobal = xpc::PrivilegedJunkScope();
     } else {
       errorGlobal = GetCurrentThreadWorkerGlobal();
+      if (!errorGlobal) {
+        // We might be reporting an error in debugger code that ran before the
+        // worker's global was created. Use the debugger global instead.
+        errorGlobal = GetCurrentThreadWorkerDebuggerGlobal();
+      }
     }
   }
   MOZ_ASSERT(JS_IsGlobalObject(errorGlobal));
   JSAutoRealm ar(cx(), errorGlobal);
   JS::Rooted<JS::Value> exn(cx());
   js::ErrorReport jsReport(cx());
   if (StealException(&exn) &&
       jsReport.init(cx(), exn, js::ErrorReport::WithSideEffects)) {
--- a/dom/workers/RuntimeService.cpp
+++ b/dom/workers/RuntimeService.cpp
@@ -2475,10 +2475,22 @@ JSObject* GetCurrentThreadWorkerGlobal()
   }
   WorkerGlobalScope* scope = wp->GlobalScope();
   if (!scope) {
     return nullptr;
   }
   return scope->GetGlobalJSObject();
 }
 
+JSObject* GetCurrentThreadWorkerDebuggerGlobal() {
+  WorkerPrivate* wp = GetCurrentThreadWorkerPrivate();
+  if (!wp) {
+    return nullptr;
+  }
+  WorkerDebuggerGlobalScope* scope = wp->DebuggerGlobalScope();
+  if (!scope) {
+    return nullptr;
+  }
+  return scope->GetGlobalJSObject();
+}
+
 }  // namespace dom
 }  // namespace mozilla
--- a/dom/workers/WorkerCommon.h
+++ b/dom/workers/WorkerCommon.h
@@ -27,16 +27,18 @@ WorkerPrivate* GetCurrentThreadWorkerPri
 bool IsCurrentThreadRunningWorker();
 
 bool IsCurrentThreadRunningChromeWorker();
 
 JSContext* GetCurrentWorkerThreadJSContext();
 
 JSObject* GetCurrentThreadWorkerGlobal();
 
+JSObject* GetCurrentThreadWorkerDebuggerGlobal();
+
 void CancelWorkersForWindow(nsPIDOMWindowInner* aWindow);
 
 void FreezeWorkersForWindow(nsPIDOMWindowInner* aWindow);
 
 void ThawWorkersForWindow(nsPIDOMWindowInner* aWindow);
 
 void SuspendWorkersForWindow(nsPIDOMWindowInner* aWindow);
 
--- a/js/src/doc/Debugger/Debugger.md
+++ b/js/src/doc/Debugger/Debugger.md
@@ -477,16 +477,20 @@ other kinds of objects.
      equivalent debuggee value owned by this `Debugger`.
 
      If `value` is a primitive value, return it unchanged. If `value` is a
      `Debugger.Object` owned by an arbitrary `Debugger`, return an equivalent
      `Debugger.Object` owned by this `Debugger`. Otherwise, if `value` is some
      other kind of object, and hence not a proper debuggee value, throw a
      TypeError instead.
 
+<code>adoptSource(<i>source</i>)</code>
+:    Given `source` of type `Debugger.Source` which is owned by an arbitrary
+     `Debugger`, return an equivalent `Debugger.Source` owned by this `Debugger`.
+
 ## Static methods of the Debugger Object
 
 The functions described below are not called with a `this` value.
 
 <code id="isCompilableUnit">isCompilableUnit(<i>source</i>)</code>
 :   Given a string of source code, designated by <i>source</i>, return false if
     the string might become a valid JavaScript statement with the addition of
     more lines. Otherwise return true. The intent is to support interactive
--- a/js/src/vm/Debugger.cpp
+++ b/js/src/vm/Debugger.cpp
@@ -5644,16 +5644,79 @@ bool Debugger::adoptDebuggeeValue(JSCont
       return false;
     }
   }
 
   args.rval().set(v);
   return true;
 }
 
+class DebuggerAdoptSourceMatcher {
+  JSContext* cx_;
+  Debugger* dbg_;
+
+ public:
+  explicit DebuggerAdoptSourceMatcher(JSContext* cx, Debugger* dbg)
+      : cx_(cx), dbg_(dbg) {}
+
+  using ReturnType = JSObject*;
+
+  ReturnType match(HandleScriptSourceObject source) {
+    if (source->compartment() == cx_->compartment()) {
+      JS_ReportErrorASCII(cx_, "Source is in the same compartment as this debugger");
+      return nullptr;
+    }
+    return dbg_->wrapSource(cx_, source);
+  }
+  ReturnType match(Handle<WasmInstanceObject*> wasmInstance) {
+    if (wasmInstance->compartment() == cx_->compartment()) {
+      JS_ReportErrorASCII(cx_, "WasmInstance is in the same compartment as this debugger");
+      return nullptr;
+    }
+    return dbg_->wrapWasmSource(cx_, wasmInstance);
+  }
+};
+
+static inline NativeObject* GetSourceReferentRawObject(JSObject* obj);
+static inline DebuggerSourceReferent GetSourceReferent(JSObject* obj);
+
+bool Debugger::adoptSource(JSContext* cx, unsigned argc, Value* vp) {
+  THIS_DEBUGGER(cx, argc, vp, "adoptSource", args, dbg);
+  if (!args.requireAtLeast(cx, "Debugger.adoptSource", 1)) {
+    return false;
+  }
+
+  RootedObject obj(cx, NonNullObject(cx, args[0]));
+  if (!obj) {
+    return false;
+  }
+
+  obj = UncheckedUnwrap(obj);
+  if (obj->getClass() != &DebuggerSource_class) {
+    JS_ReportErrorASCII(cx, "Argument is not a Debugger.Source");
+    return false;
+  }
+
+  if (!GetSourceReferentRawObject(obj)) {
+    JS_ReportErrorASCII(cx, "Argument is Debugger.Source.prototype");
+    return false;
+  }
+
+  Rooted<DebuggerSourceReferent> referent(cx, GetSourceReferent(obj));
+
+  DebuggerAdoptSourceMatcher matcher(cx, dbg);
+  JSObject* res = referent.match(matcher);
+  if (!res) {
+    return false;
+  }
+
+  args.rval().setObject(*res);
+  return true;
+}
+
 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,
             Debugger::setOnExceptionUnwind, 0),
     JS_PSGS("onNewScript", Debugger::getOnNewScript, Debugger::setOnNewScript,
             0),
@@ -5685,16 +5748,17 @@ const JSFunctionSpec Debugger::methods[]
     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),
     JS_FN("adoptDebuggeeValue", Debugger::adoptDebuggeeValue, 1, 0),
+    JS_FN("adoptSource", Debugger::adoptSource, 1, 0),
     JS_FS_END};
 
 const JSFunctionSpec Debugger::static_methods[]{
     JS_FN("isCompilableUnit", Debugger::isCompilableUnit, 1, 0),
     JS_FN("recordReplayProcessKind", Debugger::recordReplayProcessKind, 0, 0),
     JS_FS_END};
 
 /*** Debugger.Script ********************************************************/
--- a/js/src/vm/Debugger.h
+++ b/js/src/vm/Debugger.h
@@ -782,16 +782,17 @@ class Debugger : private mozilla::Linked
   static bool endTraceLogger(JSContext* cx, unsigned argc, Value* vp);
   static bool isCompilableUnit(JSContext* cx, unsigned argc, Value* vp);
   static bool recordReplayProcessKind(JSContext* cx, unsigned argc, Value* vp);
 #ifdef NIGHTLY_BUILD
   static bool setupTraceLogger(JSContext* cx, unsigned argc, Value* vp);
   static bool drainTraceLogger(JSContext* cx, unsigned argc, Value* vp);
 #endif
   static bool adoptDebuggeeValue(JSContext* cx, unsigned argc, Value* vp);
+  static bool adoptSource(JSContext* cx, unsigned argc, Value* vp);
   static bool construct(JSContext* cx, unsigned argc, Value* vp);
   static const JSPropertySpec properties[];
   static const JSFunctionSpec methods[];
   static const JSFunctionSpec static_methods[];
 
   static void removeFromFrameMapsAndClearBreakpointsIn(JSContext* cx,
                                                        AbstractFramePtr frame,
                                                        bool suspending = false);