Bug 976763 - Change DescribeStack not to return JSScripts or JSFunctions (r=bz)
authorLuke Wagner <luke@mozilla.com>
Tue, 25 Feb 2014 09:43:14 -0600
changeset 171048 85a14a1db91ebb94d57ef0e65cbc5d8347d40b2c
parent 171047 960a7f055c95d6d304a2f2e82f7ffebedf36414d
child 171049 0aa56e2a5f816a30da48701f73d03060ee9692b2
push id270
push userpvanderbeken@mozilla.com
push dateThu, 06 Mar 2014 09:24:21 +0000
reviewersbz
bugs976763
milestone30.0a1
Bug 976763 - Change DescribeStack not to return JSScripts or JSFunctions (r=bz)
dom/bindings/Exceptions.cpp
dom/promise/PromiseCallback.cpp
js/jsd/jsd_obj.cpp
js/jsd/jsd_scpt.cpp
js/public/OldDebugAPI.h
js/src/jsapi-tests/testScriptInfo.cpp
js/src/jsobj.cpp
js/src/vm/OldDebugAPI.cpp
--- a/dom/bindings/Exceptions.cpp
+++ b/dom/bindings/Exceptions.cpp
@@ -255,18 +255,18 @@ NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(St
 NS_IMPL_CYCLE_COLLECTION_UNLINK_END
 NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN(StackDescriptionOwner)
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE_SCRIPT_OBJECTS
 NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END
 NS_IMPL_CYCLE_COLLECTION_TRACE_BEGIN(StackDescriptionOwner)
   JS::StackDescription* desc = tmp->mDescription;
   if (tmp->mDescription) {
     for (size_t i = 0; i < desc->nframes; ++i) {
-      NS_IMPL_CYCLE_COLLECTION_TRACE_JS_MEMBER_CALLBACK(mDescription->frames[i].script());
-      NS_IMPL_CYCLE_COLLECTION_TRACE_JS_MEMBER_CALLBACK(mDescription->frames[i].fun());
+      NS_IMPL_CYCLE_COLLECTION_TRACE_JS_MEMBER_CALLBACK(mDescription->frames[i].markedLocation1());
+      NS_IMPL_CYCLE_COLLECTION_TRACE_JS_MEMBER_CALLBACK(mDescription->frames[i].markedLocation2());
     }
   }
 NS_IMPL_CYCLE_COLLECTION_TRACE_END
 
 class JSStackFrame : public nsIStackFrame
 {
 public:
   NS_DECL_CYCLE_COLLECTING_ISUPPORTS
@@ -372,23 +372,18 @@ NS_IMETHODIMP JSStackFrame::GetLanguageN
   return NS_OK;
 }
 
 /* readonly attribute string filename; */
 NS_IMETHODIMP JSStackFrame::GetFilename(nsACString& aFilename)
 {
   if (!mFilenameInitialized) {
     JS::FrameDescription& desc = mStackDescription->FrameAt(mIndex);
-    if (desc.script()) {
-      ThreadsafeAutoSafeJSContext cx;
-      JSAutoCompartment ac(cx, desc.script());
-      const char* filename = JS_GetScriptFilename(cx, desc.script());
-      if (filename) {
-        mFilename.Assign(filename);
-      }
+    if (const char *filename = desc.filename()) {
+      mFilename.Assign(filename);
     }
     mFilenameInitialized = true;
   }
 
   // The filename must be set to null if empty.
   if (mFilename.IsEmpty()) {
     aFilename.SetIsVoid(true);
   } else {
@@ -398,24 +393,18 @@ NS_IMETHODIMP JSStackFrame::GetFilename(
   return NS_OK;
 }
 
 /* readonly attribute string name; */
 NS_IMETHODIMP JSStackFrame::GetName(nsACString& aFunction)
 {
   if (!mFunnameInitialized) {
     JS::FrameDescription& desc = mStackDescription->FrameAt(mIndex);
-    if (desc.fun() && desc.script()) {
-      ThreadsafeAutoSafeJSContext cx;
-      JSAutoCompartment ac(cx, desc.script());
-      JS::Rooted<JSFunction*> fun(cx, desc.fun());
-      JS::Rooted<JSString*> funid(cx, JS_GetFunctionDisplayId(fun));
-      if (funid) {
-        CopyUTF16toUTF8(JS_GetStringCharsZ(cx, funid), mFunname);
-      }
+    if (JSFlatString *name = desc.funDisplayName()) {
+      CopyUTF16toUTF8(JS_GetFlatStringChars(name), mFunname);
     }
     mFunnameInitialized = true;
   }
 
   // The function name must be set to null if empty.
   if (mFunname.IsEmpty()) {
     aFunction.SetIsVoid(true);
   } else {
--- a/dom/promise/PromiseCallback.cpp
+++ b/dom/promise/PromiseCallback.cpp
@@ -212,17 +212,17 @@ WrapperPromiseCallback::Call(JS::Handle<
         JSAutoCompartment ac(cx, unwrapped);
         if (JS_ObjectIsFunction(cx, unwrapped)) {
           JS::Rooted<JS::Value> asValue(cx, JS::ObjectValue(*unwrapped));
           JS::Rooted<JSFunction*> func(cx, JS_ValueToFunction(cx, asValue));
 
           MOZ_ASSERT(func);
           JSScript* script = JS_GetFunctionScript(cx, func);
           if (script) {
-            fileName = JS_GetScriptFilename(cx, script);
+            fileName = JS_GetScriptFilename(script);
             lineNumber = JS_GetScriptBaseLineNumber(cx, script);
           }
         }
       }
 
       // We're back in aValue's compartment here.
       JS::Rooted<JSString*> stack(cx, JS_GetEmptyString(JS_GetRuntime(cx)));
       JS::Rooted<JSString*> fn(cx, JS_NewStringCopyZ(cx, fileName));
--- a/js/jsd/jsd_obj.cpp
+++ b/js/jsd/jsd_obj.cpp
@@ -92,17 +92,17 @@ jsd_Constructing(JSDContext* jsdc, JSCon
 
     JSD_LOCK_OBJECTS(jsdc);
     jsdobj = jsd_GetJSDObjectForJSObject(jsdc, obj);
     if( jsdobj && !jsdobj->ctorURL )
     {
         script = frame.script();
         if( script )
         {
-            ctorURL = JS_GetScriptFilename(cx, script);
+            ctorURL = JS_GetScriptFilename(script);
             if( ctorURL )
                 jsdobj->ctorURL = jsd_AddAtom(jsdc, ctorURL);
 
             JSD_LOCK_SCRIPTS(jsdc);
             jsdscript = jsd_FindOrCreateJSDScript(jsdc, cx, script, frame);
             JSD_UNLOCK_SCRIPTS(jsdc);
             if( jsdscript && (ctorNameStr = jsd_GetScriptFunctionId(jsdc, jsdscript)) ) {
                 if( (ctorName = JS_EncodeString(cx, ctorNameStr)) ) {
--- a/js/jsd/jsd_scpt.cpp
+++ b/js/jsd/jsd_scpt.cpp
@@ -57,17 +57,17 @@ static JSDScript*
     lineno = (unsigned) JS_GetScriptBaseLineNumber(cx, script);
     if( lineno == 0 )
         return nullptr;
 
     jsdscript = (JSDScript*) calloc(1, sizeof(JSDScript));
     if( ! jsdscript )
         return nullptr;
 
-    raw_filename = JS_GetScriptFilename(cx,script);
+    raw_filename = JS_GetScriptFilename(script);
 
     JS_HashTableAdd(jsdc->scriptsTable, (void *)script, (void *)jsdscript);
     JS_APPEND_LINK(&jsdscript->links, &jsdc->scripts);
     jsdscript->jsdc         = jsdc;
     jsdscript->script       = script;  
     jsdscript->lineBase     = lineno;
     jsdscript->lineExtent   = (unsigned)NOT_SET_YET;
     jsdscript->data         = nullptr;
--- a/js/public/OldDebugAPI.h
+++ b/js/public/OldDebugAPI.h
@@ -8,16 +8,17 @@
 #define js_OldDebugAPI_h
 
 /*
  * JS debugger API.
  */
 
 #include "mozilla/NullPtr.h"
 
+#include "jsapi.h"
 #include "jsbytecode.h"
 
 #include "js/CallArgs.h"
 #include "js/TypeDecls.h"
 
 class JSAtom;
 class JSFreeOp;
 
@@ -25,50 +26,54 @@ namespace js {
 class StackFrame;
 class ScriptFrameIter;
 }
 
 // Raw JSScript* because this needs to be callable from a signal handler.
 extern JS_PUBLIC_API(unsigned)
 JS_PCToLineNumber(JSContext *cx, JSScript *script, jsbytecode *pc);
 
+extern JS_PUBLIC_API(const char *)
+JS_GetScriptFilename(JSScript *script);
+
 namespace JS {
 
 class FrameDescription
 {
   public:
-    FrameDescription(JSScript *script, JSFunction *fun, jsbytecode *pc)
-        : script_(script)
-        , fun_(fun)
-        , pc_(pc)
-        , linenoComputed(false)
-    {
-    }
-
     explicit FrameDescription(const js::ScriptFrameIter& iter);
 
     unsigned lineno() {
         if (!linenoComputed) {
             lineno_ = JS_PCToLineNumber(nullptr, script_, pc_);
             linenoComputed = true;
         }
         return lineno_;
     }
 
-    Heap<JSScript*> &script() {
+    const char *filename() const {
+        return JS_GetScriptFilename(script_);
+    }
+
+    JSFlatString *funDisplayName() const {
+        return funDisplayName_ ? JS_ASSERT_STRING_IS_FLAT(funDisplayName_) : nullptr;
+    }
+
+    // Both these locations should be traced during GC but otherwise not used;
+    // they are implementation details.
+    Heap<JSScript*> &markedLocation1() {
         return script_;
     }
-
-    Heap<JSFunction*> &fun() {
-        return fun_;
+    Heap<JSString*> &markedLocation2() {
+        return funDisplayName_;
     }
 
   private:
     Heap<JSScript*> script_;
-    Heap<JSFunction*> fun_;
+    Heap<JSString*> funDisplayName_;
     jsbytecode *pc_;
     unsigned lineno_;
     bool linenoComputed;
 };
 
 struct StackDescription
 {
     unsigned nframes;
@@ -292,19 +297,16 @@ JS_GetParentOrScopeChain(JSContext *cx, 
  * of any scope (returned via JS_GetFrameScopeChain or JS_GetFrameCalleeObject)
  * from "Proxy" to "Call", "Block", "With" etc.
  */
 extern JS_PUBLIC_API(const char *)
 JS_GetDebugClassName(JSObject *obj);
 
 /************************************************************************/
 
-extern JS_PUBLIC_API(const char *)
-JS_GetScriptFilename(JSContext *cx, JSScript *script);
-
 extern JS_PUBLIC_API(const jschar *)
 JS_GetScriptSourceMap(JSContext *cx, JSScript *script);
 
 extern JS_PUBLIC_API(unsigned)
 JS_GetScriptBaseLineNumber(JSContext *cx, JSScript *script);
 
 extern JS_PUBLIC_API(unsigned)
 JS_GetScriptLineExtent(JSContext *cx, JSScript *script);
--- a/js/src/jsapi-tests/testScriptInfo.cpp
+++ b/js/src/jsapi-tests/testScriptInfo.cpp
@@ -33,17 +33,17 @@ BEGIN_TEST(testScriptInfo)
                                                  options));
 
     CHECK(script);
 
     jsbytecode *start = JS_LineNumberToPC(cx, script, startLine);
     CHECK_EQUAL(JS_GetScriptBaseLineNumber(cx, script), startLine);
     CHECK_EQUAL(JS_PCToLineNumber(cx, script, start), startLine);
     CHECK_EQUAL(JS_GetScriptLineExtent(cx, script), 11);
-    CHECK(strcmp(JS_GetScriptFilename(cx, script), __FILE__) == 0);
+    CHECK(strcmp(JS_GetScriptFilename(script), __FILE__) == 0);
     const jschar *sourceMap = JS_GetScriptSourceMap(cx, script);
     CHECK(sourceMap);
     CHECK(CharsMatch(sourceMap, "http://example.com/path/to/source-map.json"));
 
     return true;
 }
 static bool
 CharsMatch(const jschar *p, const char *q)
--- a/js/src/jsobj.cpp
+++ b/js/src/jsobj.cpp
@@ -5879,17 +5879,17 @@ js_DumpStackFrame(JSContext *cx, StackFr
 
 JS_FRIEND_API(void)
 js_DumpBacktrace(JSContext *cx)
 {
     Sprinter sprinter(cx);
     sprinter.init();
     size_t depth = 0;
     for (ScriptFrameIter i(cx); !i.done(); ++i, ++depth) {
-        const char *filename = JS_GetScriptFilename(cx, i.script());
+        const char *filename = JS_GetScriptFilename(i.script());
         unsigned line = JS_PCToLineNumber(cx, i.script(), i.pc());
         JSScript *script = i.script();
         sprinter.printf("#%d %14p   %s:%d (%p @ %d)\n",
                         depth, (i.isJit() ? 0 : i.interpFrame()), filename, line,
                         script, script->pcToOffset(i.pc()));
     }
     fprintf(stdout, "%s", sprinter.string());
 }
--- a/js/src/vm/OldDebugAPI.cpp
+++ b/js/src/vm/OldDebugAPI.cpp
@@ -550,17 +550,17 @@ JS_GetDebugClassName(JSObject *obj)
     if (obj->is<DebugScopeObject>())
         return obj->as<DebugScopeObject>().scope().getClass()->name;
     return obj->getClass()->name;
 }
 
 /************************************************************************/
 
 JS_PUBLIC_API(const char *)
-JS_GetScriptFilename(JSContext *cx, JSScript *script)
+JS_GetScriptFilename(JSScript *script)
 {
     return script->filename();
 }
 
 JS_PUBLIC_API(const jschar *)
 JS_GetScriptSourceMap(JSContext *cx, JSScript *script)
 {
     ScriptSource *source = script->scriptSource();
@@ -920,17 +920,24 @@ js_CallContextDebugHandler(JSContext *cx
 }
 
 /*
  * A contructor that crates a FrameDescription from a ScriptFrameIter, to avoid
  * constructing a FrameDescription on the stack just to append it to a vector.
  * FrameDescription contains Heap<T> fields that should not live on the stack.
  */
 JS::FrameDescription::FrameDescription(const ScriptFrameIter& iter)
-  : script_(iter.script()), fun_(iter.maybeCallee()), pc_(iter.pc()), linenoComputed(false) {}
+  : script_(iter.script()),
+    funDisplayName_(nullptr),
+    pc_(iter.pc()),
+    linenoComputed(false)
+{
+    if (JSFunction *fun = iter.maybeCallee())
+        funDisplayName_ = fun->displayAtom();
+}
 
 JS_PUBLIC_API(JS::StackDescription *)
 JS::DescribeStack(JSContext *cx, unsigned maxFrames)
 {
     Vector<FrameDescription> frames(cx);
 
     for (NonBuiltinScriptFrameIter i(cx); !i.done(); ++i) {
         if (!frames.append(i))