Bug 981167 - Fix Debugger.Frame leaking ScriptFrameIter::Data on frame cache hit. (r=jimb)
authorShu-yu Guo <shu@rfrn.org>
Mon, 10 Mar 2014 19:17:24 -0700
changeset 191121 dfc89871248c89eed76f37da7c90b4644b17a0f2
parent 191120 7d62c468b48b16637872106664ba2c264fb85a63
child 191122 7265b36257e5c0bee62886ef2bcb917862aad2d2
push id474
push userasasaki@mozilla.com
push dateMon, 02 Jun 2014 21:01:02 +0000
treeherdermozilla-release@967f4cf1b31c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjimb
bugs981167
milestone30.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 981167 - Fix Debugger.Frame leaking ScriptFrameIter::Data on frame cache hit. (r=jimb)
js/src/vm/Debugger.cpp
js/src/vm/Debugger.h
--- a/js/src/vm/Debugger.cpp
+++ b/js/src/vm/Debugger.cpp
@@ -445,28 +445,41 @@ Debugger::fromChildJSObject(JSObject *ob
               obj->getClass() == &DebuggerSource_class ||
               obj->getClass() == &DebuggerObject_class ||
               obj->getClass() == &DebuggerEnv_class);
     JSObject *dbgobj = &obj->getReservedSlot(JSSLOT_DEBUGOBJECT_OWNER).toObject();
     return fromJSObject(dbgobj);
 }
 
 bool
-Debugger::getScriptFrame(JSContext *cx, AbstractFramePtr frame, MutableHandleValue vp)
-{
+Debugger::getScriptFrameWithIter(JSContext *cx, AbstractFramePtr frame,
+                                 const ScriptFrameIter *maybeIter, MutableHandleValue vp)
+{
+    MOZ_ASSERT_IF(maybeIter, maybeIter->abstractFramePtr() == frame);
+
     FrameMap::AddPtr p = frames.lookupForAdd(frame);
     if (!p) {
         /* Create and populate the Debugger.Frame object. */
         JSObject *proto = &object->getReservedSlot(JSSLOT_DEBUG_FRAME_PROTO).toObject();
         JSObject *frameobj =
             NewObjectWithGivenProto(cx, &DebuggerFrame_class, proto, nullptr);
         if (!frameobj)
             return false;
 
-        frameobj->setPrivate(frame.raw());
+        // Eagerly copy ScriptFrameIter data if we've already walked the
+        // stack.
+        if (maybeIter) {
+            AbstractFramePtr data = maybeIter->copyDataAsAbstractFramePtr();
+            if (!data)
+                return false;
+            frameobj->setPrivate(data.raw());
+        } else {
+            frameobj->setPrivate(frame.raw());
+        }
+
         frameobj->setReservedSlot(JSSLOT_DEBUGFRAME_OWNER, ObjectValue(*object));
 
         if (!frames.add(p, frame, frameobj)) {
             js_ReportOutOfMemory(cx);
             return false;
         }
     }
     vp.setObject(*p->value());
--- a/js/src/vm/Debugger.h
+++ b/js/src/vm/Debugger.h
@@ -365,16 +365,23 @@ class Debugger : private mozilla::Linked
     JSObject *newDebuggerSource(JSContext *cx, js::HandleScriptSource source);
 
     /*
      * Receive a "new script" event from the engine. A new script was compiled
      * or deserialized.
      */
     void fireNewScript(JSContext *cx, HandleScript script);
 
+    /*
+     * Gets a Debugger.Frame object. If maybeIter is non-null, we eagerly copy
+     * its data if we need to make a new Debugger.Frame.
+     */
+    bool getScriptFrameWithIter(JSContext *cx, AbstractFramePtr frame,
+                                const ScriptFrameIter *maybeIter, MutableHandleValue vp);
+
     inline Breakpoint *firstBreakpoint() const;
 
     static inline Debugger *fromOnNewGlobalObjectWatchersLink(JSCList *link);
 
   public:
     Debugger(JSContext *cx, JSObject *dbg);
     ~Debugger();
 
@@ -478,32 +485,30 @@ class Debugger : private mozilla::Linked
     bool unwrapDebuggeeValue(JSContext *cx, MutableHandleValue vp);
 
     /*
      * Store the Debugger.Frame object for frame in *vp.
      *
      * Use this if you have already access to a frame pointer without having
      * to incur the cost of walking the stack.
      */
-    bool getScriptFrame(JSContext *cx, AbstractFramePtr frame, MutableHandleValue vp);
+    bool getScriptFrame(JSContext *cx, AbstractFramePtr frame, MutableHandleValue vp) {
+        return getScriptFrameWithIter(cx, frame, nullptr, vp);
+    }
 
     /*
      * Store the Debugger.Frame object for iter in *vp. Eagerly copies a
      * ScriptFrameIter::Data.
      *
      * Use this if you had to make a ScriptFrameIter to get the required
      * frame, in which case the cost of walking the stack has already been
      * paid.
      */
     bool getScriptFrame(JSContext *cx, const ScriptFrameIter &iter, MutableHandleValue vp) {
-        AbstractFramePtr data = iter.copyDataAsAbstractFramePtr();
-        if (!data || !getScriptFrame(cx, iter.abstractFramePtr(), vp))
-            return false;
-        vp.toObject().setPrivate(data.raw());
-        return true;
+        return getScriptFrameWithIter(cx, iter.abstractFramePtr(), &iter, vp);
     }
 
     /*
      * Set |*status| and |*value| to a (JSTrapStatus, Value) pair reflecting a
      * standard SpiderMonkey call state: a boolean success value |ok|, a return
      * value |rv|, and a context |cx| that may or may not have an exception set.
      * If an exception was pending on |cx|, it is cleared (and |ok| is asserted
      * to be false).