Bug 981167 - Fix Debugger.Frame leaking ScriptFrameIter::Data on frame cache hit. (r=jimb)
☠☠ backed out by fb23f20f9c6c ☠ ☠
authorShu-yu Guo <shu@rfrn.org>
Mon, 10 Mar 2014 01:04:04 -0700
changeset 189948 7de39a071ca6202971d48a614c0a28b389739ef4
parent 189899 19839a359db1d8511b3f86c3b0a4b051dd0cab3e
child 189949 00d4459b6380a87cbd7faefe05cf411198dfd75f
push id3503
push userraliiev@mozilla.com
push dateMon, 28 Apr 2014 18:51:11 +0000
treeherdermozilla-beta@c95ac01e332e [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).