Bug 1306382 - Automatically ExposeToActiveJS when reading out of a TenuredHeap<T> r=sfink r=mccr8
authorJon Coppeard <jcoppeard@mozilla.com>
Fri, 07 Oct 2016 13:58:37 +0200
changeset 359803 6217f779742e62f225f2b9538fa82bc424a20a9f
parent 359802 09bbad43f50e67b72ee52fcfb27474f3dacf5729
child 359804 af534f12f8fee939e100f3b78b22dd712cecfbb4
push id6795
push userjlund@mozilla.com
push dateMon, 23 Jan 2017 14:19:46 +0000
treeherdermozilla-beta@76101b503191 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssfink, mccr8
bugs1306382
milestone52.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 1306382 - Automatically ExposeToActiveJS when reading out of a TenuredHeap<T> r=sfink r=mccr8
js/public/RootingAPI.h
js/src/gc/Marking.cpp
js/xpconnect/src/XPCInlines.h
js/xpconnect/src/XPCWrappedNative.cpp
js/xpconnect/src/xpcprivate.h
xpcom/base/nsCycleCollector.cpp
xpcom/base/nsCycleCollectorTraceJSHelpers.cpp
--- a/js/public/RootingAPI.h
+++ b/js/public/RootingAPI.h
@@ -139,40 +139,40 @@ class PersistentRootedBase {};
 static void* const ConstNullValue = nullptr;
 
 namespace gc {
 struct Cell;
 template<typename T>
 struct PersistentRootedMarker;
 } /* namespace gc */
 
-#define DECLARE_POINTER_COMPARISON_OPS(T)                                                \
+#define DECLARE_POINTER_COMPARISON_OPS(T)                                                         \
     bool operator==(const T& other) const { return get() == other; }                              \
     bool operator!=(const T& other) const { return get() != other; }
 
 // Important: Return a reference so passing a Rooted<T>, etc. to
 // something that takes a |const T&| is not a GC hazard.
-#define DECLARE_POINTER_CONSTREF_OPS(T)                                                  \
-    operator const T&() const { return get(); }                                                  \
+#define DECLARE_POINTER_CONSTREF_OPS(T)                                                           \
+    operator const T&() const { return get(); }                                                   \
     const T& operator->() const { return get(); }
 
 // Assignment operators on a base class are hidden by the implicitly defined
 // operator= on the derived class. Thus, define the operator= directly on the
 // class as we would need to manually pass it through anyway.
 #define DECLARE_POINTER_ASSIGN_OPS(Wrapper, T)                                                    \
     Wrapper<T>& operator=(const T& p) {                                                           \
         set(p);                                                                                   \
         return *this;                                                                             \
     }                                                                                             \
     Wrapper<T>& operator=(const Wrapper<T>& other) {                                              \
         set(other.get());                                                                         \
         return *this;                                                                             \
     }                                                                                             \
 
-#define DELETE_ASSIGNMENT_OPS(Wrapper, T)                                                 \
+#define DELETE_ASSIGNMENT_OPS(Wrapper, T)                                                         \
     template <typename S> Wrapper<T>& operator=(S) = delete;                                      \
     Wrapper<T>& operator=(const Wrapper<T>&) = delete;
 
 #define DECLARE_NONPOINTER_ACCESSOR_METHODS(ptr)                                                  \
     const T* address() const { return &(ptr); }                                                   \
     const T& get() const { return (ptr); }                                                        \
 
 #define DECLARE_NONPOINTER_MUTABLE_ACCESSOR_METHODS(ptr)                                          \
@@ -411,22 +411,35 @@ class TenuredHeap : public js::HeapBase<
         bits &= ~flagsToUnset;
     }
 
     bool hasFlag(uintptr_t flag) const {
         MOZ_ASSERT((flag & ~flagsMask) == 0);
         return (bits & flag) != 0;
     }
 
-    T getPtr() const { return reinterpret_cast<T>(bits & ~flagsMask); }
+    T unbarrieredGetPtr() const { return reinterpret_cast<T>(bits & ~flagsMask); }
     uintptr_t getFlags() const { return bits & flagsMask; }
 
+    T getPtr() const {
+        T ptr = unbarrieredGetPtr();
+        js::BarrierMethods<T>::exposeToJS(ptr);
+        return ptr;
+    }
+
     operator T() const { return getPtr(); }
     T operator->() const { return getPtr(); }
 
+    explicit operator bool() const {
+        return bool(js::BarrierMethods<T>::asGCThingOrNull(unbarrieredGetPtr()));
+    }
+    explicit operator bool() {
+        return bool(js::BarrierMethods<T>::asGCThingOrNull(unbarrieredGetPtr()));
+    }
+
     TenuredHeap<T>& operator=(T p) {
         setPtr(p);
         return *this;
     }
 
     TenuredHeap<T>& operator=(const TenuredHeap<T>& other) {
         bits = other.bits;
         return *this;
--- a/js/src/gc/Marking.cpp
+++ b/js/src/gc/Marking.cpp
@@ -434,17 +434,17 @@ JS::TraceEdge(JSTracer* trc, JS::Heap<T>
     if (InternalBarrierMethods<T>::isMarkable(*thingp->unsafeGet()))
         DispatchToTracer(trc, ConvertToBase(thingp->unsafeGet()), name);
 }
 
 JS_PUBLIC_API(void)
 JS::TraceEdge(JSTracer* trc, JS::TenuredHeap<JSObject*>* thingp, const char* name)
 {
     MOZ_ASSERT(thingp);
-    if (JSObject* ptr = thingp->getPtr()) {
+    if (JSObject* ptr = thingp->unbarrieredGetPtr()) {
         DispatchToTracer(trc, &ptr, name);
         thingp->setPtr(ptr);
     }
 }
 
 template <typename T>
 void
 js::TraceManuallyBarrieredEdge(JSTracer* trc, T* thingp, const char* name)
--- a/js/xpconnect/src/XPCInlines.h
+++ b/js/xpconnect/src/XPCInlines.h
@@ -446,41 +446,37 @@ XPCNativeSet::MatchesSetUpToInterface(co
     return false;
 }
 
 /***************************************************************************/
 
 inline
 JSObject* XPCWrappedNativeTearOff::GetJSObjectPreserveColor() const
 {
-    return mJSObject.getPtr();
+    return mJSObject.unbarrieredGetPtr();
 }
 
 inline
 JSObject* XPCWrappedNativeTearOff::GetJSObject()
 {
-    JSObject* obj = GetJSObjectPreserveColor();
-    if (obj) {
-      JS::ExposeObjectToActiveJS(obj);
-    }
-    return obj;
+    return mJSObject;
 }
 
 inline
 void XPCWrappedNativeTearOff::SetJSObject(JSObject*  JSObj)
 {
     MOZ_ASSERT(!IsMarked());
     mJSObject = JSObj;
 }
 
 inline
 void XPCWrappedNativeTearOff::JSObjectMoved(JSObject* obj, const JSObject* old)
 {
     MOZ_ASSERT(!IsMarked());
-    MOZ_ASSERT(mJSObject == old);
+    MOZ_ASSERT(mJSObject.unbarrieredGetPtr() == old);
     mJSObject = obj;
 }
 
 inline
 XPCWrappedNativeTearOff::~XPCWrappedNativeTearOff()
 {
     MOZ_COUNT_DTOR(XPCWrappedNativeTearOff);
     MOZ_ASSERT(!(GetInterface() || GetNative() || GetJSObjectPreserveColor()),
--- a/js/xpconnect/src/XPCWrappedNative.cpp
+++ b/js/xpconnect/src/XPCWrappedNative.cpp
@@ -935,17 +935,17 @@ XPCWrappedNative::FlatJSObjectFinalized(
 
     Release();
 }
 
 void
 XPCWrappedNative::FlatJSObjectMoved(JSObject* obj, const JSObject* old)
 {
     JS::AutoAssertGCCallback inCallback(obj);
-    MOZ_ASSERT(mFlatJSObject == old);
+    MOZ_ASSERT(mFlatJSObject.unbarrieredGetPtr() == old);
 
     nsWrapperCache* cache = nullptr;
     CallQueryInterface(mIdentity, &cache);
     if (cache)
         cache->UpdateWrapper(obj, old);
 
     mFlatJSObject = obj;
 }
@@ -2160,17 +2160,17 @@ NS_IMETHODIMP XPCWrappedNative::DebugDum
         } else
             XPC_LOG_ALWAYS(("Scope @ %x", GetScope()));
 
         if (depth && mSet)
             mSet->DebugDump(depth);
         else
             XPC_LOG_ALWAYS(("mSet @ %x", mSet.get()));
 
-        XPC_LOG_ALWAYS(("mFlatJSObject of %x", mFlatJSObject.getPtr()));
+        XPC_LOG_ALWAYS(("mFlatJSObject of %x", mFlatJSObject.unbarrieredGetPtr()));
         XPC_LOG_ALWAYS(("mIdentity of %x", mIdentity.get()));
         XPC_LOG_ALWAYS(("mScriptableInfo @ %x", mScriptableInfo));
 
         if (depth && mScriptableInfo) {
             XPC_LOG_INDENT();
             XPC_LOG_ALWAYS(("mScriptable @ %x", mScriptableInfo->GetCallback()));
             XPC_LOG_ALWAYS(("mFlags of %x", (uint32_t)mScriptableInfo->GetFlags()));
             XPC_LOG_ALWAYS(("mJSClass @ %x", mScriptableInfo->GetJSClass()));
--- a/js/xpconnect/src/xpcprivate.h
+++ b/js/xpconnect/src/xpcprivate.h
@@ -1790,33 +1790,30 @@ public:
 
     nsISupports*
     GetIdentityObject() const {return mIdentity;}
 
     /**
      * This getter clears the gray bit before handing out the JSObject which
      * means that the object is guaranteed to be kept alive past the next CC.
      */
-    JSObject*
-    GetFlatJSObject() const
-    {
-        JS::ExposeObjectToActiveJS(mFlatJSObject);
-        return mFlatJSObject;
-    }
+    JSObject* GetFlatJSObject() const { return mFlatJSObject; }
 
     /**
      * This getter does not change the color of the JSObject meaning that the
      * object returned is not guaranteed to be kept alive past the next CC.
      *
      * This should only be called if you are certain that the return value won't
      * be passed into a JS API function and that it won't be stored without
      * being rooted (or otherwise signaling the stored value to the CC).
      */
     JSObject*
-    GetFlatJSObjectPreserveColor() const {return mFlatJSObject;}
+    GetFlatJSObjectPreserveColor() const {
+        return mFlatJSObject.unbarrieredGetPtr();
+    }
 
     XPCNativeSet*
     GetSet() const {return mSet;}
 
     void
     SetSet(already_AddRefed<XPCNativeSet> set) {mSet = set;}
 
     static XPCWrappedNative* Get(JSObject* obj) {
@@ -1894,19 +1891,20 @@ public:
 
     void Mark() const {}
 
     inline void TraceInside(JSTracer* trc) {
         if (HasProto())
             GetProto()->TraceSelf(trc);
         else
             GetScope()->TraceSelf(trc);
-        if (mFlatJSObject && JS_IsGlobalObject(mFlatJSObject))
-        {
-            xpc::TraceXPCGlobal(trc, mFlatJSObject);
+
+        JSObject* obj = mFlatJSObject.unbarrieredGetPtr();
+        if (obj && JS_IsGlobalObject(obj)) {
+            xpc::TraceXPCGlobal(trc, obj);
         }
     }
 
     void TraceJS(JSTracer* trc) {
         TraceInside(trc);
     }
 
     void TraceSelf(JSTracer* trc) {
--- a/xpcom/base/nsCycleCollector.cpp
+++ b/xpcom/base/nsCycleCollector.cpp
@@ -2719,17 +2719,17 @@ public:
                      void* aClosure) const override
   {
     AppendJSObjectToPurpleBuffer(*aObject);
   }
 
   virtual void Trace(JS::TenuredHeap<JSObject*>* aObject, const char* aName,
                      void* aClosure) const override
   {
-    AppendJSObjectToPurpleBuffer(*aObject);
+    AppendJSObjectToPurpleBuffer(aObject->unbarrieredGetPtr());
   }
 
   virtual void Trace(JS::Heap<JSString*>* aString, const char* aName,
                      void* aClosure) const override
   {
   }
 
   virtual void Trace(JS::Heap<JSScript*>* aScript, const char* aName,
--- a/xpcom/base/nsCycleCollectorTraceJSHelpers.cpp
+++ b/xpcom/base/nsCycleCollectorTraceJSHelpers.cpp
@@ -67,18 +67,18 @@ TraceCallbackFunc::Trace(JSObject** aPtr
     mCallback(JS::GCCellPtr(*aPtr), aName, aClosure);
   }
 }
 
 void
 TraceCallbackFunc::Trace(JS::TenuredHeap<JSObject*>* aPtr, const char* aName,
                          void* aClosure) const
 {
-  if (aPtr->getPtr()) {
-    mCallback(JS::GCCellPtr(aPtr->getPtr()), aName, aClosure);
+  if (*aPtr) {
+    mCallback(JS::GCCellPtr(aPtr->unbarrieredGetPtr()), aName, aClosure);
   }
 }
 
 void
 TraceCallbackFunc::Trace(JS::Heap<JSFunction*>* aPtr, const char* aName,
                          void* aClosure) const
 {
   if (*aPtr) {