Bug 650161 - Unify the finalization and moving GC callbacks into a weak pointer update callback r=terrence r=bholley
authorJon Coppeard <jcoppeard@mozilla.com>
Wed, 24 Sep 2014 12:54:11 +0100
changeset 207020 d63a5fe3ace7d59e1aaad2a5d64bc511793349bc
parent 207019 0b32a3831212e51d4d1692a03bace7ae68d88c41
child 207021 67061f79a38a37bab3ab8c63ac91f0322d47a9cb
push id27544
push userryanvm@gmail.com
push dateWed, 24 Sep 2014 21:10:36 +0000
treeherdermozilla-central@1735ff2bb23e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersterrence, bholley
bugs650161
milestone35.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 650161 - Unify the finalization and moving GC callbacks into a weak pointer update callback r=terrence r=bholley
js/ipc/JavaScriptChild.cpp
js/ipc/JavaScriptChild.h
js/ipc/JavaScriptParent.cpp
js/ipc/JavaScriptShared.cpp
js/ipc/JavaScriptShared.h
js/ipc/WrapperOwner.cpp
js/ipc/WrapperOwner.h
js/public/RootingAPI.h
js/src/gc/GCRuntime.h
js/src/jsapi.cpp
js/src/jsapi.h
js/src/jsfriendapi.cpp
js/src/jsgc.cpp
js/xpconnect/src/XPCJSRuntime.cpp
js/xpconnect/src/XPCMaps.cpp
js/xpconnect/src/XPCMaps.h
js/xpconnect/src/XPCWrappedNative.cpp
js/xpconnect/src/XPCWrappedNativeScope.cpp
js/xpconnect/src/xpcprivate.h
--- a/js/ipc/JavaScriptChild.cpp
+++ b/js/ipc/JavaScriptChild.cpp
@@ -16,56 +16,46 @@
 
 using namespace JS;
 using namespace mozilla;
 using namespace mozilla::jsipc;
 
 using mozilla::AutoSafeJSContext;
 
 static void
-FinalizeChild(JSFreeOp *fop, JSFinalizeStatus status, bool isCompartment, void *data)
+UpdateChildWeakPointersAfterGC(JSRuntime *rt, void *data)
 {
-    if (status == JSFINALIZE_GROUP_START) {
-        static_cast<JavaScriptChild *>(data)->finalize();
-    }
-}
-
-static void
-FixupChildAfterMovingGC(JSRuntime *rt, void *data)
-{
-    static_cast<JavaScriptChild *>(data)->fixupAfterMovingGC();
+    static_cast<JavaScriptChild *>(data)->updateWeakPointers();
 }
 
 JavaScriptChild::JavaScriptChild(JSRuntime *rt)
   : JavaScriptShared(rt),
     JavaScriptBase<PJavaScriptChild>(rt)
 {
 }
 
 JavaScriptChild::~JavaScriptChild()
 {
-    JS_RemoveFinalizeCallback(rt_, FinalizeChild);
-    JS_RemoveMovingGCCallback(rt_, FixupChildAfterMovingGC);
+    JS_RemoveWeakPointerCallback(rt_, UpdateChildWeakPointersAfterGC);
 }
 
 bool
 JavaScriptChild::init()
 {
     if (!WrapperOwner::init())
         return false;
     if (!WrapperAnswer::init())
         return false;
 
-    JS_AddFinalizeCallback(rt_, FinalizeChild, this);
-    JS_AddMovingGCCallback(rt_, FixupChildAfterMovingGC, this);
+    JS_AddWeakPointerCallback(rt_, UpdateChildWeakPointersAfterGC, this);
     return true;
 }
 
 void
-JavaScriptChild::finalize()
+JavaScriptChild::updateWeakPointers()
 {
     objects_.sweep();
     objectIds_.sweep();
 }
 
 JSObject *
 JavaScriptChild::scopeForTargetObjects()
 {
--- a/js/ipc/JavaScriptChild.h
+++ b/js/ipc/JavaScriptChild.h
@@ -16,17 +16,17 @@ namespace jsipc {
 
 class JavaScriptChild : public JavaScriptBase<PJavaScriptChild>
 {
   public:
     explicit JavaScriptChild(JSRuntime *rt);
     virtual ~JavaScriptChild();
 
     bool init();
-    void finalize();
+    void updateWeakPointers();
 
     void drop(JSObject *obj);
 
   protected:
     virtual bool isParent() { return false; }
     virtual JSObject *scopeForTargetObjects() MOZ_OVERRIDE;
 
   private:
--- a/js/ipc/JavaScriptParent.cpp
+++ b/js/ipc/JavaScriptParent.cpp
@@ -22,50 +22,44 @@ using namespace mozilla::jsipc;
 using namespace mozilla::dom;
 
 static void
 TraceParent(JSTracer *trc, void *data)
 {
     static_cast<JavaScriptParent *>(data)->trace(trc);
 }
 
-static void
-FixupParentAfterMovingGC(JSRuntime *rt, void *data)
-{
-    static_cast<JavaScriptParent *>(data)->fixupAfterMovingGC();
-}
-
 JavaScriptParent::JavaScriptParent(JSRuntime *rt)
   : JavaScriptShared(rt),
     JavaScriptBase<PJavaScriptParent>(rt)
 {
 }
 
 JavaScriptParent::~JavaScriptParent()
 {
     JS_RemoveExtraGCRootsTracer(rt_, TraceParent, this);
-    JS_RemoveMovingGCCallback(rt_, FixupParentAfterMovingGC);
 }
 
 bool
 JavaScriptParent::init()
 {
     if (!WrapperOwner::init())
         return false;
 
     JS_AddExtraGCRootsTracer(rt_, TraceParent, this);
-    JS_AddMovingGCCallback(rt_, FixupParentAfterMovingGC, this);
     return true;
 }
 
 void
 JavaScriptParent::trace(JSTracer *trc)
 {
-    if (active())
+    if (active()) {
         objects_.trace(trc);
+        objectIds_.trace(trc);
+    }
 }
 
 JSObject *
 JavaScriptParent::scopeForTargetObjects()
 {
     // CPWOWs from the child need to point into the parent's unprivileged junk
     // scope so that a compromised child cannot compromise the parent. In
     // practice, this means that a child process can only (a) hold parent
--- a/js/ipc/JavaScriptShared.cpp
+++ b/js/ipc/JavaScriptShared.cpp
@@ -31,26 +31,26 @@ IdToObjectMap::init()
 }
 
 void
 IdToObjectMap::trace(JSTracer *trc)
 {
     for (Table::Range r(table_.all()); !r.empty(); r.popFront()) {
         DebugOnly<JSObject *> prior = r.front().value().get();
         JS_CallObjectTracer(trc, &r.front().value(), "ipc-object");
-        MOZ_ASSERT(r.front().value() == prior);
     }
 }
 
 void
 IdToObjectMap::sweep()
 {
     for (Table::Enum e(table_); !e.empty(); e.popFront()) {
-        DebugOnly<JSObject *> prior = e.front().value().get();
-        if (JS_IsAboutToBeFinalized(&e.front().value()))
+        JS::Heap<JSObject *> *objp = &e.front().value();
+        JS_UpdateWeakPointerAfterGC(objp);
+        if (!*objp)
             e.removeFront();
     }
 }
 
 JSObject *
 IdToObjectMap::find(ObjectId id)
 {
     Table::Ptr p = table_.lookup(id);
@@ -90,21 +90,33 @@ ObjectToIdMap::init()
     if (table_)
         return true;
 
     table_ = new Table(SystemAllocPolicy());
     return table_ && table_->init(32);
 }
 
 void
+ObjectToIdMap::trace(JSTracer *trc)
+{
+    for (Table::Enum e(*table_); !e.empty(); e.popFront()) {
+        JSObject *obj = e.front().key();
+        JS_CallUnbarrieredObjectTracer(trc, &obj, "ipc-object");
+        if (obj != e.front().key())
+            e.rekeyFront(obj);
+    }
+}
+
+void
 ObjectToIdMap::sweep()
 {
     for (Table::Enum e(*table_); !e.empty(); e.popFront()) {
         JSObject *obj = e.front().key();
-        if (JS_IsAboutToBeFinalizedUnbarriered(&obj))
+        JS_UpdateWeakPointerAfterGCUnbarriered(&obj);
+        if (!obj)
             e.removeFront();
         else if (obj != e.front().key())
             e.rekeyFront(obj);
     }
 }
 
 ObjectId
 ObjectToIdMap::find(JSObject *obj)
@@ -563,14 +575,8 @@ JavaScriptShared::Wrap(JSContext *cx, Ha
         if (!toVariant(cx, v, &var))
             return false;
 
         outCpows->AppendElement(CpowEntry(str, var));
     }
 
     return true;
 }
-void JavaScriptShared::fixupAfterMovingGC()
-{
-    objects_.sweep();
-    cpows_.sweep();
-    objectIds_.sweep();
-}
--- a/js/ipc/JavaScriptShared.h
+++ b/js/ipc/JavaScriptShared.h
@@ -49,41 +49,38 @@ class IdToObjectMap
     bool init();
     void trace(JSTracer *trc);
     void sweep();
 
     bool add(ObjectId id, JSObject *obj);
     JSObject *find(ObjectId id);
     void remove(ObjectId id);
 
-    void fixupAfterMovingGC();
-
   private:
     Table table_;
 };
 
 // Map JSObjects -> ids
 class ObjectToIdMap
 {
     typedef js::PointerHasher<JSObject *, 3> Hasher;
     typedef js::HashMap<JSObject *, ObjectId, Hasher, js::SystemAllocPolicy> Table;
 
   public:
     ObjectToIdMap();
     ~ObjectToIdMap();
 
     bool init();
+    void trace(JSTracer *trc);
     void sweep();
 
     bool add(JSContext *cx, JSObject *obj, ObjectId id);
     ObjectId find(JSObject *obj);
     void remove(JSObject *obj);
 
-    void fixupAfterMovingGC();
-
   private:
     static void keyMarkCallback(JSTracer *trc, JSObject *key, void *data);
 
     Table *table_;
 };
 
 class Logging;
 
@@ -99,18 +96,16 @@ class JavaScriptShared
     void incref();
 
     static const uint32_t OBJECT_EXTRA_BITS  = 1;
     static const uint32_t OBJECT_IS_CALLABLE = (1 << 0);
 
     bool Unwrap(JSContext *cx, const InfallibleTArray<CpowEntry> &aCpows, JS::MutableHandleObject objp);
     bool Wrap(JSContext *cx, JS::HandleObject aObj, InfallibleTArray<CpowEntry> *outCpows);
 
-    void fixupAfterMovingGC();
-
   protected:
     bool toVariant(JSContext *cx, JS::HandleValue from, JSVariant *to);
     bool fromVariant(JSContext *cx, const JSVariant &from, JS::MutableHandleValue to);
 
     bool fromDescriptor(JSContext *cx, JS::Handle<JSPropertyDescriptor> desc,
                         PPropertyDescriptor *out);
     bool toDescriptor(JSContext *cx, const PPropertyDescriptor &in,
                       JS::MutableHandle<JSPropertyDescriptor> out);
--- a/js/ipc/WrapperOwner.cpp
+++ b/js/ipc/WrapperOwner.cpp
@@ -26,30 +26,37 @@ WrapperOwner::WrapperOwner(JSRuntime *rt
 static inline WrapperOwner *
 OwnerOf(JSObject *obj)
 {
     MOZ_ASSERT(IsCPOW(obj));
     return reinterpret_cast<WrapperOwner *>(GetProxyExtra(obj, 0).toPrivate());
 }
 
 ObjectId
-WrapperOwner::idOf(JSObject *obj)
+WrapperOwner::idOfUnchecked(JSObject *obj)
 {
     MOZ_ASSERT(IsCPOW(obj));
 
     Value v = GetProxyExtra(obj, 1);
     MOZ_ASSERT(v.isDouble());
 
     ObjectId objId = BitwiseCast<uint64_t>(v.toDouble());
-    MOZ_ASSERT(findCPOWById(objId) == obj);
     MOZ_ASSERT(objId);
 
     return objId;
 }
 
+ObjectId
+WrapperOwner::idOf(JSObject *obj)
+{
+    ObjectId objId = idOfUnchecked(obj);
+    MOZ_ASSERT(findCPOWById(objId) == obj);
+    return objId;
+}
+
 class CPOWProxyHandler : public BaseProxyHandler
 {
   public:
     MOZ_CONSTEXPR CPOWProxyHandler()
       : BaseProxyHandler(&family) {}
 
     virtual bool finalizeInBackground(Value priv) const MOZ_OVERRIDE {
         return false;
@@ -79,16 +86,17 @@ class CPOWProxyHandler : public BaseProx
     virtual bool call(JSContext *cx, HandleObject proxy, const CallArgs &args) const MOZ_OVERRIDE;
     virtual bool construct(JSContext *cx, HandleObject proxy, const CallArgs &args) const MOZ_OVERRIDE;
     virtual bool hasInstance(JSContext *cx, HandleObject proxy,
                              MutableHandleValue v, bool *bp) const MOZ_OVERRIDE;
     virtual bool objectClassIs(HandleObject obj, js::ESClassValue classValue,
                                JSContext *cx) const MOZ_OVERRIDE;
     virtual const char* className(JSContext *cx, HandleObject proxy) const MOZ_OVERRIDE;
     virtual void finalize(JSFreeOp *fop, JSObject *proxy) const MOZ_OVERRIDE;
+    virtual void objectMoved(JSObject *proxy, const JSObject *old) const MOZ_OVERRIDE;
     virtual bool isCallable(JSObject *obj) const MOZ_OVERRIDE;
     virtual bool isConstructor(JSObject *obj) const MOZ_OVERRIDE;
 
     static const char family;
     static const CPOWProxyHandler singleton;
 };
 
 const char CPOWProxyHandler::family = 0;
@@ -643,16 +651,22 @@ WrapperOwner::className(JSContext *cx, H
 }
 
 void
 CPOWProxyHandler::finalize(JSFreeOp *fop, JSObject *proxy) const
 {
     OwnerOf(proxy)->drop(proxy);
 }
 
+void
+CPOWProxyHandler::objectMoved(JSObject *proxy, const JSObject *old) const
+{
+    OwnerOf(proxy)->updatePointer(proxy, old);
+}
+
 bool
 CPOWProxyHandler::isCallable(JSObject *obj) const
 {
     return OwnerOf(obj)->isCallable(obj);
 }
 
 bool
 CPOWProxyHandler::isConstructor(JSObject *obj) const
@@ -673,16 +687,24 @@ WrapperOwner::drop(JSObject *obj)
     ObjectId objId = idOf(obj);
 
     cpows_.remove(objId);
     if (active())
         unused << SendDropObject(objId);
     decref();
 }
 
+void
+WrapperOwner::updatePointer(JSObject *obj, const JSObject *old)
+{
+    ObjectId objId = idOfUnchecked(obj);
+    MOZ_ASSERT(findCPOWById(objId) == old);
+    cpows_.add(objId, obj);
+}
+
 bool
 WrapperOwner::init()
 {
     if (!JavaScriptShared::init())
         return false;
 
     return true;
 }
--- a/js/ipc/WrapperOwner.h
+++ b/js/ipc/WrapperOwner.h
@@ -71,28 +71,31 @@ class WrapperOwner : public virtual Java
      * Check that |obj| is a DOM wrapper whose prototype chain contains
      * |prototypeID| at depth |depth|.
      */
     bool domInstanceOf(JSContext *cx, JSObject *obj, int prototypeID, int depth, bool *bp);
 
     bool active() { return !inactive_; }
 
     void drop(JSObject *obj);
+    void updatePointer(JSObject *obj, const JSObject *old);
 
     virtual void ActorDestroy(ActorDestroyReason why);
 
     virtual bool toObjectVariant(JSContext *cx, JSObject *obj, ObjectVariant *objVarp);
     virtual JSObject *fromObjectVariant(JSContext *cx, ObjectVariant objVar);
     JSObject *fromRemoteObjectVariant(JSContext *cx, RemoteObject objVar);
     JSObject *fromLocalObjectVariant(JSContext *cx, LocalObject objVar);
 
   protected:
     ObjectId idOf(JSObject *obj);
 
   private:
+    ObjectId idOfUnchecked(JSObject *obj);
+
     bool getPropertyNames(JSContext *cx, JS::HandleObject proxy, uint32_t flags,
                           JS::AutoIdVector &props);
 
     // Catastrophic IPC failure.
     bool ipcfail(JSContext *cx);
 
     // Check whether a return status is okay, and if not, propagate its error.
     bool ok(JSContext *cx, const ReturnStatus &status);
--- a/js/public/RootingAPI.h
+++ b/js/public/RootingAPI.h
@@ -1202,17 +1202,17 @@ class JS_PUBLIC_API(ObjectPtr)
     void init(JSObject *obj) { value = obj; }
 
     JSObject *get() const { return value; }
 
     void writeBarrierPre(JSRuntime *rt) {
         IncrementalObjectBarrier(value);
     }
 
-    bool isAboutToBeFinalized();
+    void updateWeakPointerAfterGC();
 
     ObjectPtr &operator=(JSObject *obj) {
         IncrementalObjectBarrier(value);
         value = obj;
         return *this;
     }
 
     void trace(JSTracer *trc, const char *name);
--- a/js/src/gc/GCRuntime.h
+++ b/js/src/gc/GCRuntime.h
@@ -409,18 +409,18 @@ class GCRuntime
     void resetMallocBytes();
     bool isTooMuchMalloc() const { return mallocBytes <= 0; }
     void updateMallocCounter(JS::Zone *zone, size_t nbytes);
     void onTooMuchMalloc();
 
     void setGCCallback(JSGCCallback callback, void *data);
     bool addFinalizeCallback(JSFinalizeCallback callback, void *data);
     void removeFinalizeCallback(JSFinalizeCallback func);
-    bool addMovingGCCallback(JSMovingGCCallback callback, void *data);
-    void removeMovingGCCallback(JSMovingGCCallback func);
+    bool addWeakPointerCallback(JSWeakPointerCallback callback, void *data);
+    void removeWeakPointerCallback(JSWeakPointerCallback func);
     JS::GCSliceCallback setSliceCallback(JS::GCSliceCallback callback);
 
     void setValidate(bool enable);
     void setFullCompartmentChecks(bool enable);
 
     bool isManipulatingDeadZones() { return manipulatingDeadZones; }
     void setManipulatingDeadZones(bool value) { manipulatingDeadZones = value; }
     unsigned objectsMarkedInDeadZonesCount() { return objectsMarkedInDeadZones; }
@@ -545,17 +545,17 @@ class GCRuntime
 
     void markConservativeStackRoots(JSTracer *trc, bool useSavedRoots);
 
 #ifdef DEBUG
     void checkForCompartmentMismatches();
 #endif
 
     void callFinalizeCallbacks(FreeOp *fop, JSFinalizeStatus status) const;
-    void callMovingGCCallbacks() const;
+    void callWeakPointerCallbacks() const;
 
   public:
     JSRuntime             *rt;
 
     /* Embedders can use this zone however they wish. */
     JS::Zone              *systemZone;
 
     /* List of compartments and zones (protected by the GC lock). */
@@ -800,17 +800,17 @@ class GCRuntime
     js::Vector<JSObject *, 0, js::SystemAllocPolicy>   selectedForMarking;
 #endif
 
     bool                  validate;
     bool                  fullCompartmentChecks;
 
     Callback<JSGCCallback>  gcCallback;
     CallbackVector<JSFinalizeCallback> finalizeCallbacks;
-    CallbackVector<JSMovingGCCallback> movingCallbacks;
+    CallbackVector<JSWeakPointerCallback> updateWeakPointerCallbacks;
 
     /*
      * Malloc counter to measure memory pressure for GC scheduling. It runs
      * from   maxMallocBytes down to zero.
      */
     mozilla::Atomic<ptrdiff_t, mozilla::ReleaseAcquire>   mallocBytes;
 
     /*
--- a/js/src/jsapi.cpp
+++ b/js/src/jsapi.cpp
@@ -1883,38 +1883,39 @@ JS_AddFinalizeCallback(JSRuntime *rt, JS
 
 JS_PUBLIC_API(void)
 JS_RemoveFinalizeCallback(JSRuntime *rt, JSFinalizeCallback cb)
 {
     rt->gc.removeFinalizeCallback(cb);
 }
 
 JS_PUBLIC_API(bool)
-JS_AddMovingGCCallback(JSRuntime *rt, JSMovingGCCallback cb, void *data)
+JS_AddWeakPointerCallback(JSRuntime *rt, JSWeakPointerCallback cb, void *data)
 {
     AssertHeapIsIdle(rt);
-    return rt->gc.addMovingGCCallback(cb, data);
+    return rt->gc.addWeakPointerCallback(cb, data);
 }
 
 JS_PUBLIC_API(void)
-JS_RemoveMovingGCCallback(JSRuntime *rt, JSMovingGCCallback cb)
-{
-    rt->gc.removeMovingGCCallback(cb);
-}
-
-JS_PUBLIC_API(bool)
-JS_IsAboutToBeFinalized(JS::Heap<JSObject *> *objp)
-{
-    return IsObjectAboutToBeFinalized(objp->unsafeGet());
-}
-
-JS_PUBLIC_API(bool)
-JS_IsAboutToBeFinalizedUnbarriered(JSObject **objp)
-{
-    return IsObjectAboutToBeFinalized(objp);
+JS_RemoveWeakPointerCallback(JSRuntime *rt, JSWeakPointerCallback cb)
+{
+    rt->gc.removeWeakPointerCallback(cb);
+}
+
+JS_PUBLIC_API(void)
+JS_UpdateWeakPointerAfterGC(JS::Heap<JSObject *> *objp)
+{
+    JS_UpdateWeakPointerAfterGCUnbarriered(objp->unsafeGet());
+}
+
+JS_PUBLIC_API(void)
+JS_UpdateWeakPointerAfterGCUnbarriered(JSObject **objp)
+{
+    if (IsObjectAboutToBeFinalized(objp))
+        *objp = nullptr;
 }
 
 JS_PUBLIC_API(void)
 JS_SetGCParameter(JSRuntime *rt, JSGCParamKey key, uint32_t value)
 {
     rt->gc.setParameter(key, value);
 }
 
--- a/js/src/jsapi.h
+++ b/js/src/jsapi.h
@@ -685,17 +685,17 @@ typedef enum JSFinalizeStatus {
      */
     JSFINALIZE_COLLECTION_END
 } JSFinalizeStatus;
 
 typedef void
 (* JSFinalizeCallback)(JSFreeOp *fop, JSFinalizeStatus status, bool isCompartment, void *data);
 
 typedef void
-(* JSMovingGCCallback)(JSRuntime *rt, void *data);
+(* JSWeakPointerCallback)(JSRuntime *rt, void *data);
 
 typedef bool
 (* JSInterruptCallback)(JSContext *cx);
 
 typedef void
 (* JSErrorReporter)(JSContext *cx, const char *message, JSErrorReport *report);
 
 #ifdef MOZ_TRACE_JSCALLS
@@ -2034,51 +2034,60 @@ JS_SetGCCallback(JSRuntime *rt, JSGCCall
 
 extern JS_PUBLIC_API(bool)
 JS_AddFinalizeCallback(JSRuntime *rt, JSFinalizeCallback cb, void *data);
 
 extern JS_PUBLIC_API(void)
 JS_RemoveFinalizeCallback(JSRuntime *rt, JSFinalizeCallback cb);
 
 extern JS_PUBLIC_API(bool)
-JS_AddMovingGCCallback(JSRuntime *rt, JSMovingGCCallback cb, void *data);
-
-extern JS_PUBLIC_API(void)
-JS_RemoveMovingGCCallback(JSRuntime *rt, JSMovingGCCallback cb);
-
-extern JS_PUBLIC_API(bool)
 JS_IsGCMarkingTracer(JSTracer *trc);
 
 /* For assertions only. */
 #ifdef JS_DEBUG
 extern JS_PUBLIC_API(bool)
 JS_IsMarkingGray(JSTracer *trc);
 #endif
 
 /*
- * JS_IsAboutToBeFinalized checks if the given object is going to be finalized
- * at the end of the current GC. When called outside of the context of a GC,
- * this function will return false. Typically this function is used on weak
- * references, where the reference should be nulled out or destroyed if the
- * given object is about to be finalized.
+ * Weak pointers and garbage collection
+ *
+ * Weak pointers are by their nature not marked as part of garbage collection,
+ * but they may need to be updated in two cases after a GC:
+ *
+ *  1) Their referent was found not to be live and is about to be finalized
+ *  2) Their referent has been moved by a compacting GC
  *
- * The argument to JS_IsAboutToBeFinalized is an in-out param: when the
- * function returns false, the object being referenced is still alive, but the
- * garbage collector might have moved it. In this case, the reference passed
- * to JS_IsAboutToBeFinalized will be updated to the object's new location.
+ * To handle this, any part of the system that maintain weak pointers to
+ * JavaScript GC things must register a callback with
+ * JS_(Add,Remove)WeakPointerCallback().  This callback must then call
+ * JS_UpdateWeakPointerAfterGC() on all weak pointers it knows about.
+ *
+ * The argument to JS_UpdateWeakPointerAfterGC() is an in-out param.  If the
+ * referent is about to be finalized the pointer will be set to null.  If the
+ * referent has been moved then the pointer will be updated to point to the new
+ * location.
+ *
  * Callers of this method are responsible for updating any state that is
  * dependent on the object's address. For example, if the object's address is
  * used as a key in a hashtable, then the object must be removed and
  * re-inserted with the correct hash.
  */
-extern JS_PUBLIC_API(bool)
-JS_IsAboutToBeFinalized(JS::Heap<JSObject *> *objp);
-
-extern JS_PUBLIC_API(bool)
-JS_IsAboutToBeFinalizedUnbarriered(JSObject **objp);
+
+extern JS_PUBLIC_API(bool)
+JS_AddWeakPointerCallback(JSRuntime *rt, JSWeakPointerCallback cb, void *data);
+
+extern JS_PUBLIC_API(void)
+JS_RemoveWeakPointerCallback(JSRuntime *rt, JSWeakPointerCallback cb);
+
+extern JS_PUBLIC_API(void)
+JS_UpdateWeakPointerAfterGC(JS::Heap<JSObject *> *objp);
+
+extern JS_PUBLIC_API(void)
+JS_UpdateWeakPointerAfterGCUnbarriered(JSObject **objp);
 
 typedef enum JSGCParamKey {
     /* Maximum nominal heap before last ditch GC. */
     JSGC_MAX_BYTES          = 0,
 
     /* Number of JS_malloc bytes before last ditch GC. */
     JSGC_MAX_MALLOC_BYTES   = 1,
 
--- a/js/src/jsfriendapi.cpp
+++ b/js/src/jsfriendapi.cpp
@@ -1230,20 +1230,21 @@ JS::PokeGC(JSRuntime *rt)
 JS_FRIEND_API(JSCompartment *)
 js::GetAnyCompartmentInZone(JS::Zone *zone)
 {
     CompartmentsInZoneIter comp(zone);
     JS_ASSERT(!comp.done());
     return comp.get();
 }
 
-bool
-JS::ObjectPtr::isAboutToBeFinalized()
+void
+JS::ObjectPtr::updateWeakPointerAfterGC()
 {
-    return JS_IsAboutToBeFinalized(&value);
+    if (js::gc::IsObjectAboutToBeFinalized(value.unsafeGet()))
+        value = nullptr;
 }
 
 void
 JS::ObjectPtr::trace(JSTracer *trc, const char *name)
 {
     JS_CallObjectTracer(trc, &value, name);
 }
 
--- a/js/src/jsgc.cpp
+++ b/js/src/jsgc.cpp
@@ -1618,39 +1618,39 @@ GCRuntime::callFinalizeCallbacks(FreeOp 
     for (const Callback<JSFinalizeCallback> *p = finalizeCallbacks.begin();
          p < finalizeCallbacks.end(); p++)
     {
         p->op(fop, status, !isFull, p->data);
     }
 }
 
 bool
-GCRuntime::addMovingGCCallback(JSMovingGCCallback callback, void *data)
-{
-    return movingCallbacks.append(Callback<JSMovingGCCallback>(callback, data));
+GCRuntime::addWeakPointerCallback(JSWeakPointerCallback callback, void *data)
+{
+    return updateWeakPointerCallbacks.append(Callback<JSWeakPointerCallback>(callback, data));
 }
 
 void
-GCRuntime::removeMovingGCCallback(JSMovingGCCallback callback)
-{
-    for (Callback<JSMovingGCCallback> *p = movingCallbacks.begin();
-         p < movingCallbacks.end(); p++)
+GCRuntime::removeWeakPointerCallback(JSWeakPointerCallback callback)
+{
+    for (Callback<JSWeakPointerCallback> *p = updateWeakPointerCallbacks.begin();
+         p < updateWeakPointerCallbacks.end(); p++)
     {
         if (p->op == callback) {
-            movingCallbacks.erase(p);
+            updateWeakPointerCallbacks.erase(p);
             break;
         }
     }
 }
 
 void
-GCRuntime::callMovingGCCallbacks() const
-{
-    for (const Callback<JSMovingGCCallback> *p = movingCallbacks.begin();
-         p < movingCallbacks.end(); p++)
+GCRuntime::callWeakPointerCallbacks() const
+{
+    for (const Callback<JSWeakPointerCallback> *p = updateWeakPointerCallbacks.begin();
+         p < updateWeakPointerCallbacks.end(); p++)
     {
         p->op(rt, p->data);
     }
 }
 
 JS::GCSliceCallback
 GCRuntime::setSliceCallback(JS::GCSliceCallback callback) {
     return stats.setSliceCallback(callback);
@@ -2456,17 +2456,17 @@ GCRuntime::updatePointersToRelocatedCell
     // Mark all gray roots, making sure we call the trace callback to get the
     // current set.
     if (JSTraceDataOp op = grayRootTracer.op)
         (*op)(&trc, grayRootTracer.data);
 
     MovingTracer::Sweep(&trc);
 
     // Call callbacks to get the rest of the system to fixup other untraced pointers.
-    callMovingGCCallbacks();
+    callWeakPointerCallbacks();
 }
 
 void
 GCRuntime::releaseRelocatedArenas(ArenaHeader *relocatedList)
 {
     // Release the relocated arenas, now containing only forwarding pointers
 
 #ifdef DEBUG
@@ -4631,16 +4631,17 @@ GCRuntime::beginSweepingZoneGroup()
 
     validateIncrementalMarking();
 
     FreeOp fop(rt);
 
     {
         gcstats::AutoPhase ap(stats, gcstats::PHASE_FINALIZE_START);
         callFinalizeCallbacks(&fop, JSFINALIZE_GROUP_START);
+        callWeakPointerCallbacks();
     }
 
     if (sweepingAtoms) {
         {
             gcstats::AutoPhase ap(stats, gcstats::PHASE_SWEEP_ATOMS);
             rt->sweepAtoms();
         }
         {
--- a/js/xpconnect/src/XPCJSRuntime.cpp
+++ b/js/xpconnect/src/XPCJSRuntime.cpp
@@ -784,27 +784,16 @@ XPCJSRuntime::FinalizeCallback(JSFreeOp 
     switch (status) {
         case JSFINALIZE_GROUP_START:
         {
             MOZ_ASSERT(!self->mDoingFinalization, "bad state");
 
             MOZ_ASSERT(!self->mGCIsRunning, "bad state");
             self->mGCIsRunning = true;
 
-            // Add any wrappers whose JSObjects are to be finalized to
-            // this array. Note that we do not want to be changing the
-            // refcount of these wrappers.
-            // We add them to the array now and Release the array members
-            // later to avoid the posibility of doing any JS GCThing
-            // allocations during the gc cycle.
-            self->mWrappedJSMap->UpdateWeakPointersAfterGC(self);
-
-            // Find dying scopes.
-            XPCWrappedNativeScope::UpdateWeakPointersAfterGC(self);
-
             self->mDoingFinalization = true;
             break;
         }
         case JSFINALIZE_GROUP_END:
         {
             MOZ_ASSERT(self->mDoingFinalization, "bad state");
             self->mDoingFinalization = false;
 
@@ -964,24 +953,24 @@ XPCJSRuntime::FinalizeCallback(JSFreeOp 
             self->mGCIsRunning = false;
 
             break;
         }
     }
 }
 
 /* static */ void
-XPCJSRuntime::MovingGCCallback(JSRuntime *rt, void *data)
+XPCJSRuntime::WeakPointerCallback(JSRuntime *rt, void *data)
 {
-    // Called to fixup any weak GC thing pointers that may have been moved.
+    // Called to remove any weak pointers to GC things that are about to be
+    // finalized and fixup any pointers that may have been moved.
 
     XPCJSRuntime *self = static_cast<XPCJSRuntime *>(data);
 
     self->mWrappedJSMap->UpdateWeakPointersAfterGC(self);
-    MOZ_ASSERT(self->WrappedJSToReleaseArray().IsEmpty());
 
     XPCWrappedNativeScope::UpdateWeakPointersAfterGC(self);
 }
 
 static void WatchdogMain(void *arg);
 class Watchdog;
 class WatchdogManager;
 class AutoLockWatchdog {
@@ -1542,17 +1531,17 @@ ReloadPrefsCallback(const char *pref, vo
 XPCJSRuntime::~XPCJSRuntime()
 {
     // This destructor runs before ~CycleCollectedJSRuntime, which does the
     // actual JS_DestroyRuntime() call. But destroying the runtime triggers
     // one final GC, which can call back into the runtime with various
     // callback if we aren't careful. Null out the relevant callbacks.
     js::SetActivityCallback(Runtime(), nullptr, nullptr);
     JS_RemoveFinalizeCallback(Runtime(), FinalizeCallback);
-    JS_RemoveMovingGCCallback(Runtime(), MovingGCCallback);
+    JS_RemoveWeakPointerCallback(Runtime(), WeakPointerCallback);
 
     // Clear any pending exception.  It might be an XPCWrappedJS, and if we try
     // to destroy it later we will crash.
     SetPendingException(nullptr);
 
     JS::SetGCSliceCallback(Runtime(), mPrevGCSliceCallback);
 
     xpc_DelocalizeRuntime(Runtime());
@@ -3241,17 +3230,17 @@ XPCJSRuntime::XPCJSRuntime(nsXPConnect* 
                            kStackQuota - kSystemCodeBuffer,
                            kStackQuota - kSystemCodeBuffer - kTrustedScriptBuffer);
 
     JS_SetErrorReporter(runtime, xpc::SystemErrorReporter);
     JS_SetDestroyCompartmentCallback(runtime, CompartmentDestroyedCallback);
     JS_SetCompartmentNameCallback(runtime, CompartmentNameCallback);
     mPrevGCSliceCallback = JS::SetGCSliceCallback(runtime, GCSliceCallback);
     JS_AddFinalizeCallback(runtime, FinalizeCallback, nullptr);
-    JS_AddMovingGCCallback(runtime, MovingGCCallback, this);
+    JS_AddWeakPointerCallback(runtime, WeakPointerCallback, this);
     JS_SetWrapObjectCallbacks(runtime, &WrapObjectCallbacks);
     js::SetPreserveWrapperCallback(runtime, PreserveWrapper);
 #ifdef MOZ_CRASHREPORTER
     JS_EnumerateDiagnosticMemoryRegions(DiagnosticMemoryCallback);
 #endif
 #ifdef MOZ_ENABLE_PROFILER_SPS
     if (PseudoStack *stack = mozilla_get_pseudo_stack())
         stack->sampleRuntime(runtime);
--- a/js/xpconnect/src/XPCMaps.cpp
+++ b/js/xpconnect/src/XPCMaps.cpp
@@ -83,48 +83,56 @@ HashNativeKey(PLDHashTable *table, const
 
 /***************************************************************************/
 // implement JSObject2WrappedJSMap...
 
 void
 JSObject2WrappedJSMap::UpdateWeakPointersAfterGC(XPCJSRuntime *runtime)
 {
     // Check all wrappers and update their JSObject pointer if it has been
-    // moved, or queue the wrapper for destruction if it is about to be
-    // finalized.
+    // moved, or if it is about to be finalized queue the wrapper for
+    // destruction by adding it to an array held by the runtime.
+    // Note that we do not want to be changing the refcount of these wrappers.
+    // We add them to the array now and Release the array members later to avoid
+    // the posibility of doing any JS GCThing allocations during the gc cycle.
 
     nsTArray<nsXPCWrappedJS*> &dying = runtime->WrappedJSToReleaseArray();
     MOZ_ASSERT(dying.IsEmpty());
 
     for (Map::Enum e(mTable); !e.empty(); e.popFront()) {
         nsXPCWrappedJS* wrapper = e.front().value();
         MOZ_ASSERT(wrapper, "found a null JS wrapper!");
 
         // Walk the wrapper chain and update all JSObjects.
         while (wrapper) {
 #ifdef DEBUG
             if (!wrapper->IsSubjectToFinalization()) {
+                // If a wrapper is not subject to finalization then it roots its
+                // JS object.  If so, then it will not be about to be finalized
+                // and any necessary pointer update will have already happened
+                // when it was marked.
                 JSObject *obj = wrapper->GetJSObjectPreserveColor();
                 JSObject *prior = obj;
-                MOZ_ASSERT(!JS_IsAboutToBeFinalizedUnbarriered(&obj));
-                // If a wrapper is not subject to finalization then it roots its
-                // JS object.  If so, then any necessary pointer update will
-                // have already happened when it was marked.
+                JS_UpdateWeakPointerAfterGCUnbarriered(&obj);
                 MOZ_ASSERT(obj == prior);
             }
 #endif
-            if (wrapper->IsSubjectToFinalization() && wrapper->IsObjectAboutToBeFinalized())
-                dying.AppendElement(wrapper);
+            if (wrapper->IsSubjectToFinalization()) {
+                wrapper->UpdateObjectPointerAfterGC();
+                if (!wrapper->GetJSObjectPreserveColor())
+                    dying.AppendElement(wrapper);
+            }
             wrapper = wrapper->GetNextWrapper();
         }
 
         // Remove or update the JSObject key in the table if necessary.
         JSObject *obj = e.front().key();
         JSObject *prior = obj;
-        if (JS_IsAboutToBeFinalizedUnbarriered(&obj))
+        JS_UpdateWeakPointerAfterGCUnbarriered(&obj);
+        if (!obj)
             e.removeFront();
         else if (obj != prior)
             e.rekeyFront(obj);
     }
 }
 
 void
 JSObject2WrappedJSMap::ShutdownMarker()
--- a/js/xpconnect/src/XPCMaps.h
+++ b/js/xpconnect/src/XPCMaps.h
@@ -652,21 +652,24 @@ public:
         NS_PRECONDITION(key,"bad param");
         mTable.remove(key);
     }
 
     inline uint32_t Count() { return mTable.count(); }
 
     void Sweep() {
         for (Map::Enum e(mTable); !e.empty(); e.popFront()) {
-            JSObject *updated = e.front().key();
-            if (JS_IsAboutToBeFinalizedUnbarriered(&updated) || JS_IsAboutToBeFinalized(&e.front().value()))
+            JSObject *key = e.front().key();
+            JS::Heap<JSObject *> *valuep = &e.front().value();
+            JS_UpdateWeakPointerAfterGCUnbarriered(&key);
+            JS_UpdateWeakPointerAfterGC(valuep);
+            if (!key || !*valuep)
                 e.removeFront();
-            else if (updated != e.front().key())
-                e.rekeyFront(updated);
+            else if (key != e.front().key())
+                e.rekeyFront(key);
         }
     }
 
     void Reparent(JSContext *aCx, JSObject *aNewInnerArg) {
         JS::RootedObject aNewInner(aCx, aNewInnerArg);
         for (Map::Enum e(mTable); !e.empty(); e.popFront()) {
             /*
              * We reparent wrappers that have as their parent an inner window
--- a/js/xpconnect/src/XPCWrappedNative.cpp
+++ b/js/xpconnect/src/XPCWrappedNative.cpp
@@ -935,18 +935,21 @@ XPCWrappedNative::FlatJSObjectFinalized(
     // JSObjects are about to be finalized too.
 
     XPCWrappedNativeTearOffChunk* chunk;
     for (chunk = &mFirstChunk; chunk; chunk = chunk->mNextChunk) {
         XPCWrappedNativeTearOff* to = chunk->mTearOffs;
         for (int i = XPC_WRAPPED_NATIVE_TEAROFFS_PER_CHUNK-1; i >= 0; i--, to++) {
             JSObject* jso = to->GetJSObjectPreserveColor();
             if (jso) {
-                MOZ_ASSERT(JS_IsAboutToBeFinalizedUnbarriered(&jso));
                 JS_SetPrivate(jso, nullptr);
+#ifdef DEBUG
+                JS_UpdateWeakPointerAfterGCUnbarriered(&jso);
+                MOZ_ASSERT(!jso);
+#endif
                 to->JSObjectFinalized();
             }
 
             // We also need to release any native pointers held...
             nsISupports* obj = to->GetNative();
             if (obj) {
 #ifdef XP_WIN
                 // Try to detect free'd pointer
--- a/js/xpconnect/src/XPCWrappedNativeScope.cpp
+++ b/js/xpconnect/src/XPCWrappedNativeScope.cpp
@@ -503,26 +503,28 @@ XPCWrappedNativeScope::UpdateWeakPointer
         if (cur->mWaiverWrapperMap)
             cur->mWaiverWrapperMap->Sweep();
 
         XPCWrappedNativeScope* next = cur->mNext;
 
         // Check for finalization of the global object.  Note that global
         // objects are never moved, so we don't need to handle updating the
         // object pointer here.
-        if (cur->mGlobalJSObject && cur->mGlobalJSObject.isAboutToBeFinalized()) {
-            cur->mGlobalJSObject.finalize(rt->Runtime());
-            // Move this scope from the live list to the dying list.
-            if (prev)
-                prev->mNext = next;
-            else
-                gScopes = next;
-            cur->mNext = gDyingScopes;
-            gDyingScopes = cur;
-            cur = nullptr;
+        if (cur->mGlobalJSObject) {
+            cur->mGlobalJSObject.updateWeakPointerAfterGC();
+            if (!cur->mGlobalJSObject) {
+                // Move this scope from the live list to the dying list.
+                if (prev)
+                    prev->mNext = next;
+                else
+                    gScopes = next;
+                cur->mNext = gDyingScopes;
+                gDyingScopes = cur;
+                cur = nullptr;
+            }
         }
 
         if (cur)
             prev = cur;
         cur = next;
     }
 }
 
--- a/js/xpconnect/src/xpcprivate.h
+++ b/js/xpconnect/src/xpcprivate.h
@@ -580,17 +580,17 @@ public:
     bool CustomContextCallback(JSContext *cx, unsigned operation) MOZ_OVERRIDE;
     static void GCSliceCallback(JSRuntime *rt,
                                 JS::GCProgress progress,
                                 const JS::GCDescription &desc);
     static void FinalizeCallback(JSFreeOp *fop,
                                  JSFinalizeStatus status,
                                  bool isCompartmentGC,
                                  void *data);
-    static void MovingGCCallback(JSRuntime *rt, void *data);
+    static void WeakPointerCallback(JSRuntime *rt, void *data);
 
     inline void AddVariantRoot(XPCTraceableVariant* variant);
     inline void AddWrappedJSRoot(nsXPCWrappedJS* wrappedJS);
     inline void AddObjectHolderRoot(XPCJSObjectHolder* holder);
 
     static void SuspectWrappedNative(XPCWrappedNative *wrapper,
                                      nsCycleCollectionNoteRootCallback &cb);
 
@@ -2466,17 +2466,17 @@ public:
     bool IsRootWrapper() const {return mRoot == this;}
     bool IsValid() const {return mJSObj != nullptr;}
     void SystemIsBeingShutDown();
 
     // These two methods are used by JSObject2WrappedJSMap::FindDyingJSObjects
     // to find non-rooting wrappers for dying JS objects. See the top of
     // XPCWrappedJS.cpp for more details.
     bool IsSubjectToFinalization() const {return IsValid() && mRefCnt == 1;}
-    bool IsObjectAboutToBeFinalized() {return JS_IsAboutToBeFinalized(&mJSObj);}
+    void UpdateObjectPointerAfterGC() {JS_UpdateWeakPointerAfterGC(&mJSObj);}
 
     bool IsAggregatedToNative() const {return mRoot->mOuter != nullptr;}
     nsISupports* GetAggregatedNativeObject() const {return mRoot->mOuter;}
     void SetAggregatedNativeObject(nsISupports *aNative) {
         MOZ_ASSERT(aNative);
         if (mRoot->mOuter) {
             MOZ_ASSERT(mRoot->mOuter == aNative,
                        "Only one aggregated native can be set");