Bug 797821 - Remove xpc_NewSystemInheritingJSObject and cached mJSPrototypeObject. r=mrbkap
authorBobby Holley <bobbyholley@gmail.com>
Thu, 25 Oct 2012 17:01:08 +0200
changeset 111526 23d2c7d33c79cd26d8d05896cdf2f0c86ec17266
parent 111525 1a4fb31ef6a87bb9444f2bc9687c0b1d50a9a5df
child 111527 9c24c122583b261f5298a72650558de79ee6d23b
push id93
push usernmatsakis@mozilla.com
push dateWed, 31 Oct 2012 21:26:57 +0000
reviewersmrbkap
bugs797821
milestone19.0a1
Bug 797821 - Remove xpc_NewSystemInheritingJSObject and cached mJSPrototypeObject. r=mrbkap xpc_NewSystemInheritingJSObject is totally unnecessary now that the system bit has been removed (\o/). Furthermore, the mJSPrototypeObject optimization is really dumb. it complicates tracing significantly, and we don't actually use it in any critical places: XPCWrappedNative and slim wrapper creation use a different prototype, so this is used only for the creation of tearoff reflectors (seldom/ never used), XPCWrappedNativeProto objects, and the nohelper prototype on the scope (once per scope). We could actually just pass NULL to JS_NewObject and let it deal. However, this would actually trigger a dynamic lookup for the prototype object of the associated JSClass, which isn't what we want. So we just explicitly pass in Object.prototype.
js/xpconnect/src/XPCInlines.h
js/xpconnect/src/XPCWrappedNative.cpp
js/xpconnect/src/XPCWrappedNativeProto.cpp
js/xpconnect/src/XPCWrappedNativeScope.cpp
js/xpconnect/src/xpcprivate.h
--- a/js/xpconnect/src/XPCInlines.h
+++ b/js/xpconnect/src/XPCInlines.h
@@ -593,32 +593,16 @@ xpc_ForcePropertyResolve(JSContext* cx, 
 {
     jsval prop;
 
     if (!JS_LookupPropertyById(cx, obj, id, &prop))
         return false;
     return true;
 }
 
-inline JSObject*
-xpc_NewSystemInheritingJSObject(JSContext *cx, JSClass *clasp, JSObject *proto,
-                                bool uniqueType, JSObject *parent)
-{
-    // Global creation should go through XPCWrappedNative::WrapNewGlobal().
-    MOZ_ASSERT(!(clasp->flags & JSCLASS_IS_GLOBAL));
-
-    JSObject *obj;
-    if (uniqueType) {
-        obj = JS_NewObjectWithUniqueType(cx, clasp, proto, parent);
-    } else {
-        obj = JS_NewObject(cx, clasp, proto, parent);
-    }
-    return obj;
-}
-
 inline jsid
 GetRTIdByIndex(JSContext *cx, unsigned index)
 {
   XPCJSRuntime *rt = nsXPConnect::FastGetXPConnect()->GetRuntime();
   return rt->GetStringID(index);
 }
 
 inline
--- a/js/xpconnect/src/XPCWrappedNative.cpp
+++ b/js/xpconnect/src/XPCWrappedNative.cpp
@@ -315,21 +315,17 @@ XPCWrappedNative::WrapNewGlobal(XPCCallC
     JSObject *global =  xpc::CreateGlobalObject(ccx, clasp, principal);
     if (!global)
         return NS_ERROR_FAILURE;
 
     // Immediately enter the global's compartment, so that everything else we
     // create ends up there.
     JSAutoCompartment ac(ccx, global);
 
-    // If requested, immediately initialize the standard classes on the global.
-    // We need to do this before creating a scope, because
-    // XPCWrappedNativeScope::SetGlobal resolves |Object| via
-    // JS_ResolveStandardClass. JS_InitStandardClasses asserts if any of the
-    // standard classes are already initialized, so this is a problem.
+    // If requested, initialize the standard classes on the global.
     if (initStandardClasses && ! JS_InitStandardClasses(ccx, global))
         return NS_ERROR_FAILURE;
 
     // Create a scope, but don't do any extra stuff like initializing |Components|.
     // All of that stuff happens in the caller.
     XPCWrappedNativeScope *scope = XPCWrappedNativeScope::GetNewOrUsed(ccx, global);
     MOZ_ASSERT(scope);
 
@@ -1139,17 +1135,17 @@ XPCWrappedNative::Init(XPCCallContext& c
     JSObject* protoJSObject = HasProto() ?
                                 GetProto()->GetJSProtoObject() :
                                 GetScope()->GetPrototypeNoHelper(ccx);
 
     if (!protoJSObject) {
         return false;
     }
 
-    mFlatJSObject = xpc_NewSystemInheritingJSObject(ccx, jsclazz, protoJSObject, false, parent);
+    mFlatJSObject = JS_NewObject(ccx, jsclazz, protoJSObject, parent);
     if (!mFlatJSObject)
         return false;
 
     JS_SetPrivate(mFlatJSObject, this);
 
     return FinishInit(ccx);
 }
 
@@ -2210,21 +2206,19 @@ XPCWrappedNative::InitTearOff(XPCCallCon
 }
 
 JSBool
 XPCWrappedNative::InitTearOffJSObject(XPCCallContext& ccx,
                                       XPCWrappedNativeTearOff* to)
 {
     // This is only called while locked (during XPCWrappedNative::FindTearOff).
 
-    JSObject* obj =
-        xpc_NewSystemInheritingJSObject(ccx, Jsvalify(&XPC_WN_Tearoff_JSClass),
-                                        GetScope()->GetPrototypeJSObject(),
-                                        false, mFlatJSObject);
-
+    JSObject* obj = JS_NewObject(ccx, Jsvalify(&XPC_WN_Tearoff_JSClass),
+                                 JS_GetObjectPrototype(ccx, mFlatJSObject),
+                                 mFlatJSObject);
     if (!obj)
         return false;
 
     JS_SetPrivate(obj, to);
     to->SetJSObject(obj);
     return true;
 }
 
@@ -3855,19 +3849,17 @@ ConstructSlimWrapper(XPCCallContext &ccx
 
     xpcproto->CacheOffsets(identityObj);
 
     XPCNativeScriptableInfo* si = xpcproto->GetScriptableInfo();
     JSClass* jsclazz = si->GetSlimJSClass();
     if (!jsclazz)
         return false;
 
-    wrapper = xpc_NewSystemInheritingJSObject(ccx, jsclazz,
-                                              xpcproto->GetJSProtoObject(),
-                                              false, parent);
+    wrapper = JS_NewObject(ccx, jsclazz, xpcproto->GetJSProtoObject(), parent);
     if (!wrapper)
         return false;
 
     JS_SetPrivate(wrapper, identityObj);
     SetSlimWrapperProto(wrapper, xpcproto.get());
 
     // Transfer ownership to the wrapper's private.
     aHelper.forgetCanonical();
--- a/js/xpconnect/src/XPCWrappedNativeProto.cpp
+++ b/js/xpconnect/src/XPCWrappedNativeProto.cpp
@@ -85,20 +85,19 @@ XPCWrappedNativeProto::Init(XPCCallConte
                 &XPC_WN_NoMods_NoCall_Proto_JSClass;
         }
     } else {
         jsclazz = &XPC_WN_NoMods_NoCall_Proto_JSClass;
     }
 
     JSObject *parent = mScope->GetGlobalJSObject();
 
-    mJSProtoObject =
-        xpc_NewSystemInheritingJSObject(ccx, js::Jsvalify(jsclazz),
-                                        mScope->GetPrototypeJSObject(),
-                                        true, parent);
+    mJSProtoObject = JS_NewObjectWithUniqueType(ccx, js::Jsvalify(jsclazz),
+                                                JS_GetObjectPrototype(ccx, parent),
+                                                parent);
 
     bool success = !!mJSProtoObject;
     if (success) {
         JS_SetPrivate(mJSProtoObject, this);
         if (callPostCreatePrototype)
             success = CallPostCreatePrototype(ccx);
     }
 
--- a/js/xpconnect/src/XPCWrappedNativeScope.cpp
+++ b/js/xpconnect/src/XPCWrappedNativeScope.cpp
@@ -83,21 +83,20 @@ XPCWrappedNativeScope* XPCWrappedNativeS
 XPCWrappedNativeScope*
 XPCWrappedNativeScope::GetNewOrUsed(JSContext *cx, JSObject* aGlobal)
 {
 
     XPCWrappedNativeScope* scope = FindInJSObjectScope(cx, aGlobal, true);
     if (!scope)
         scope = new XPCWrappedNativeScope(cx, aGlobal);
     else {
-        // We need to call SetGlobal in order to refresh our cached
-        // mPrototypeJSObject and to clear mPrototypeNoHelper (so we get a new
-        // new one if requested in the new scope) in the case where the global
-        // object is being reused (JS_SetAllNonReservedSlotsToUndefined has
-        // been called).  NOTE: We are only called by nsXPConnect::InitClasses.
+        // We need to call SetGlobal in order to clear mPrototypeNoHelper (so we
+        // get a new new one if requested in the new scope) in the case where
+        // the global object is being reused (JS_SetAllNonReservedSlotsToUndefined
+        // has been called). NOTE: We are only called by nsXPConnect::InitClasses.
         scope->SetGlobal(cx, aGlobal);
     }
     if (js::GetObjectClass(aGlobal)->flags & JSCLASS_XPCONNECT_GLOBAL)
         JS_SetReservedSlot(aGlobal,
                            JSCLASS_GLOBAL_SLOT_COUNT,
                            PRIVATE_TO_JSVAL(scope));
     return scope;
 }
@@ -105,17 +104,16 @@ XPCWrappedNativeScope::GetNewOrUsed(JSCo
 XPCWrappedNativeScope::XPCWrappedNativeScope(JSContext *cx,
                                              JSObject* aGlobal)
       : mWrappedNativeMap(Native2WrappedNativeMap::newMap(XPC_NATIVE_MAP_SIZE)),
         mWrappedNativeProtoMap(ClassInfo2WrappedNativeProtoMap::newMap(XPC_NATIVE_PROTO_MAP_SIZE)),
         mMainThreadWrappedNativeProtoMap(ClassInfo2WrappedNativeProtoMap::newMap(XPC_NATIVE_PROTO_MAP_SIZE)),
         mComponents(nullptr),
         mNext(nullptr),
         mGlobalJSObject(nullptr),
-        mPrototypeJSObject(nullptr),
         mPrototypeNoHelper(nullptr),
         mExperimentalBindingsEnabled(XPCJSRuntime::Get()->ExperimentalBindingsEnabled())
 {
     // add ourselves to the scopes list
     {   // scoped lock
         XPCAutoLock lock(XPCJSRuntime::Get()->GetMapLock());
 
 #ifdef DEBUG
@@ -212,24 +210,16 @@ js::Class XPC_WN_NoHelper_Proto_JSClass 
 
 void
 XPCWrappedNativeScope::SetGlobal(JSContext *cx, JSObject* aGlobal)
 {
     // We allow for calling this more than once. This feature is used by
     // nsXPConnect::InitClassesWithNewWrappedGlobal.
     mGlobalJSObject = aGlobal;
 
-    // Lookup 'globalObject.Object.prototype' for our wrapper's proto
-    JSObject *objectPrototype =
-        JS_GetObjectPrototype(cx, aGlobal);
-    if (objectPrototype)
-        mPrototypeJSObject = objectPrototype;
-    else
-        NS_ERROR("Can't get globalObject.Object.prototype");
-
     // Clear the no helper wrapper prototype object so that a new one
     // gets created if needed.
     mPrototypeNoHelper = nullptr;
 }
 
 XPCWrappedNativeScope::~XPCWrappedNativeScope()
 {
     MOZ_COUNT_DTOR(XPCWrappedNativeScope);
@@ -261,31 +251,28 @@ XPCWrappedNativeScope::~XPCWrappedNative
         mComponents->mScope = nullptr;
 
     // XXX we should assert that we are dead or that xpconnect has shutdown
     // XXX might not want to do this at xpconnect shutdown time???
     mComponents = nullptr;
 
     JSRuntime *rt = XPCJSRuntime::Get()->GetJSRuntime();
     mGlobalJSObject.finalize(rt);
-    mPrototypeJSObject.finalize(rt);
 }
 
 JSObject *
 XPCWrappedNativeScope::GetPrototypeNoHelper(XPCCallContext& ccx)
 {
     // We could create this prototype in SetGlobal(), but all scopes
     // don't need one, so we save ourselves a bit of space if we
     // create these when they're needed.
     if (!mPrototypeNoHelper) {
-        mPrototypeNoHelper =
-            xpc_NewSystemInheritingJSObject(ccx,
-                                            js::Jsvalify(&XPC_WN_NoHelper_Proto_JSClass),
-                                            mPrototypeJSObject,
-                                            false, mGlobalJSObject);
+        mPrototypeNoHelper = JS_NewObject(ccx, js::Jsvalify(&XPC_WN_NoHelper_Proto_JSClass),
+                                          JS_GetObjectPrototype(ccx, mGlobalJSObject),
+                                          mGlobalJSObject);
 
         NS_ASSERTION(mPrototypeNoHelper,
                      "Failed to create prototype for wrappers w/o a helper");
     } else {
         xpc_UnmarkGrayObject(mPrototypeNoHelper);
     }
 
     return mPrototypeNoHelper;
@@ -372,20 +359,16 @@ XPCWrappedNativeScope::StartFinalization
             if (prev)
                 prev->mNext = next;
             else
                 gScopes = next;
             cur->mNext = gDyingScopes;
             gDyingScopes = cur;
             cur = nullptr;
         } else {
-            if (cur->mPrototypeJSObject &&
-                JS_IsAboutToBeFinalized(cur->mPrototypeJSObject)) {
-                cur->mPrototypeJSObject.finalize(fop->runtime());
-            }
             if (cur->mPrototypeNoHelper &&
                 JS_IsAboutToBeFinalized(cur->mPrototypeNoHelper)) {
                 cur->mPrototypeNoHelper = nullptr;
             }
         }
         if (cur)
             prev = cur;
         cur = next;
@@ -811,17 +794,16 @@ XPCWrappedNativeScope::DebugDump(int16_t
 {
 #ifdef DEBUG
     depth-- ;
     XPC_LOG_ALWAYS(("XPCWrappedNativeScope @ %x", this));
     XPC_LOG_INDENT();
         XPC_LOG_ALWAYS(("mNext @ %x", mNext));
         XPC_LOG_ALWAYS(("mComponents @ %x", mComponents.get()));
         XPC_LOG_ALWAYS(("mGlobalJSObject @ %x", mGlobalJSObject.get()));
-        XPC_LOG_ALWAYS(("mPrototypeJSObject @ %x", mPrototypeJSObject.get()));
         XPC_LOG_ALWAYS(("mPrototypeNoHelper @ %x", mPrototypeNoHelper));
 
         XPC_LOG_ALWAYS(("mWrappedNativeMap @ %x with %d wrappers(s)",         \
                         mWrappedNativeMap,                                    \
                         mWrappedNativeMap ? mWrappedNativeMap->Count() : 0));
         // iterate contexts...
         if (depth && mWrappedNativeMap && mWrappedNativeMap->Count()) {
             XPC_LOG_INDENT();
--- a/js/xpconnect/src/xpcprivate.h
+++ b/js/xpconnect/src/xpcprivate.h
@@ -1626,23 +1626,16 @@ public:
 
     JSObject*
     GetGlobalJSObject() const
         {return xpc_UnmarkGrayObject(mGlobalJSObject);}
 
     JSObject*
     GetGlobalJSObjectPreserveColor() const {return mGlobalJSObject;}
 
-    JSObject*
-    GetPrototypeJSObject() const
-        {return xpc_UnmarkGrayObject(mPrototypeJSObject);}
-
-    JSObject*
-    GetPrototypeJSObjectPreserveColor() const {return mPrototypeJSObject;}
-
     // Getter for the prototype that we use for wrappers that have no
     // helper.
     JSObject*
     GetPrototypeNoHelper(XPCCallContext& ccx);
 
     nsIPrincipal*
     GetPrincipal() const {
         if (!mGlobalJSObject)
@@ -1671,20 +1664,16 @@ public:
 
     static void
     TraceWrappedNativesInAllScopes(JSTracer* trc, XPCJSRuntime* rt);
 
     void TraceSelf(JSTracer *trc) {
         JSObject *obj = GetGlobalJSObjectPreserveColor();
         MOZ_ASSERT(obj);
         JS_CALL_OBJECT_TRACER(trc, obj, "XPCWrappedNativeScope::mGlobalJSObject");
-
-        JSObject *proto = GetPrototypeJSObjectPreserveColor();
-        if (proto)
-            JS_CALL_OBJECT_TRACER(trc, proto, "XPCWrappedNativeScope::mPrototypeJSObject");
     }
 
     static void
     SuspectAllWrappers(XPCJSRuntime* rt, nsCycleCollectionTraversalCallback &cb);
 
     static void
     StartFinalizationPhaseOfGC(JSFreeOp *fop, XPCJSRuntime* rt);
 
@@ -1771,18 +1760,16 @@ private:
     nsRefPtr<nsXPCComponents>        mComponents;
     XPCWrappedNativeScope*           mNext;
     // The JS global object for this scope.  If non-null, this will be the
     // default parent for the XPCWrappedNatives that have us as the scope,
     // unless a PreCreate hook overrides it.  Note that this _may_ be null (see
     // constructor).
     js::ObjectPtr                    mGlobalJSObject;
 
-    // Cached value of Object.prototype
-    js::ObjectPtr                    mPrototypeJSObject;
     // Prototype to use for wrappers with no helper.
     JSObject*                        mPrototypeNoHelper;
 
     XPCContext*                      mContext;
 
     nsDataHashtable<nsDepCharHashKey, JSObject*> mCachedDOMPrototypes;
 
     JSBool mExperimentalBindingsEnabled;
@@ -4299,24 +4286,16 @@ xpc_EvalInSandbox(JSContext *cx, JSObjec
 // Inlined utilities.
 
 inline JSBool
 xpc_ForcePropertyResolve(JSContext* cx, JSObject* obj, jsid id);
 
 inline jsid
 GetRTIdByIndex(JSContext *cx, unsigned index);
 
-// Wrapper for JS_NewObject to mark the new object as system when parent is
-// also a system object. If uniqueType is specified then a new type object will
-// be created which is used only by the result, so that its property types
-// will be tracked precisely.
-inline JSObject*
-xpc_NewSystemInheritingJSObject(JSContext *cx, JSClass *clasp, JSObject *proto,
-                                bool uniqueType, JSObject *parent);
-
 nsISupports *
 XPC_GetIdentityObject(JSContext *cx, JSObject *obj);
 
 namespace xpc {
 
 class CompartmentPrivate
 {
 public: