Bug 815803. Now that JSSLOT_PROXY_PRIVATE == 0, simplify binding code that had to deal with different slot indices on proxy and non-proxy objects. r=peterv
authorBoris Zbarsky <bzbarsky@mit.edu>
Thu, 06 Dec 2012 15:21:19 -0500
changeset 125095 adde19c975eb8d87dfbb015a196e389fcfdfd161
parent 125094 0e57f424507196393880c9e3617fca51e6da62a1
child 125096 4e8386f1328677967909d23e2360bf1a9acffa26
push id297
push userlsblakk@mozilla.com
push dateTue, 26 Mar 2013 17:28:00 +0000
treeherdermozilla-release@64d7b45c34e6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerspeterv
bugs815803
milestone20.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 815803. Now that JSSLOT_PROXY_PRIVATE == 0, simplify binding code that had to deal with different slot indices on proxy and non-proxy objects. r=peterv
dom/bindings/BindingUtils.cpp
dom/bindings/BindingUtils.h
dom/bindings/Codegen.py
dom/bindings/DOMJSProxyHandler.cpp
dom/workers/Worker.cpp
dom/workers/WorkerScope.cpp
js/xpconnect/src/XPCQuickStubs.h
js/xpconnect/src/XPCWrappedNativeScope.cpp
js/xpconnect/src/nsXPConnect.cpp
--- a/dom/bindings/BindingUtils.cpp
+++ b/dom/bindings/BindingUtils.cpp
@@ -573,18 +573,18 @@ JSBool
 ThrowingConstructor(JSContext* cx, unsigned argc, JS::Value* vp)
 {
   return ThrowErrorMessage(cx, MSG_ILLEGAL_CONSTRUCTOR);
 }
 
 inline const NativePropertyHooks*
 GetNativePropertyHooks(JSContext *cx, JSObject *obj, DOMObjectType& type)
 {
-  const DOMClass* domClass;
-  if (GetDOMClass(obj, domClass) != eNonDOMObject) {
+  const DOMClass* domClass = GetDOMClass(obj);
+  if (domClass) {
     type = eInstance;
     return domClass->mNativeHooks;
   }
 
   if (JS_ObjectIsFunction(cx, obj)) {
     MOZ_ASSERT(JS_IsNativeFunction(obj, Constructor));
     type = eInterface;
     const JS::Value& v =
--- a/dom/bindings/BindingUtils.h
+++ b/dom/bindings/BindingUtils.h
@@ -86,97 +86,65 @@ IsDOMIfaceAndProtoClass(const JSClass* c
 }
 
 inline bool
 IsDOMIfaceAndProtoClass(const js::Class* clasp)
 {
   return IsDOMIfaceAndProtoClass(Jsvalify(clasp));
 }
 
-// It's ok for eRegularDOMObject and eProxyDOMObject to be the same, but
-// eNonDOMObject should always be different from the other two. This enum
-// shouldn't be used to differentiate between non-proxy and proxy bindings.
-enum DOMObjectSlot {
-  eNonDOMObject = -1,
-  eRegularDOMObject = DOM_OBJECT_SLOT,
-  eProxyDOMObject = DOM_PROXY_OBJECT_SLOT
-};
-
+MOZ_STATIC_ASSERT(DOM_OBJECT_SLOT == js::JSSLOT_PROXY_PRIVATE,
+                  "JSSLOT_PROXY_PRIVATE doesn't match DOM_OBJECT_SLOT.  "
+                  "Expect bad things");
 template <class T>
 inline T*
-UnwrapDOMObject(JSObject* obj, DOMObjectSlot slot)
+UnwrapDOMObject(JSObject* obj)
 {
-  MOZ_ASSERT(slot != eNonDOMObject,
+  MOZ_ASSERT(IsDOMClass(js::GetObjectClass(obj)) || IsDOMProxy(obj),
              "Don't pass non-DOM objects to this function");
 
-#ifdef DEBUG
-  if (IsDOMClass(js::GetObjectClass(obj))) {
-    MOZ_ASSERT(slot == eRegularDOMObject);
-  } else {
-    MOZ_ASSERT(IsDOMProxy(obj));
-    MOZ_ASSERT(slot == eProxyDOMObject);
-  }
-#endif
-
-  JS::Value val = js::GetReservedSlot(obj, slot);
+  JS::Value val = js::GetReservedSlot(obj, DOM_OBJECT_SLOT);
   // XXXbz/khuey worker code tries to unwrap interface objects (which have
   // nothing here).  That needs to stop.
   // XXX We don't null-check UnwrapObject's result; aren't we going to crash
   // anyway?
   if (val.isUndefined()) {
     return NULL;
   }
   
   return static_cast<T*>(val.toPrivate());
 }
 
-// Only use this with a new DOM binding object (either proxy or regular).
 inline const DOMClass*
 GetDOMClass(JSObject* obj)
 {
   js::Class* clasp = js::GetObjectClass(obj);
   if (IsDOMClass(clasp)) {
     return &DOMJSClass::FromJSClass(clasp)->mClass;
   }
 
-  MOZ_ASSERT(IsDOMProxy(obj));
-  js::BaseProxyHandler* handler = js::GetProxyHandler(obj);
-  return &static_cast<DOMProxyHandler*>(handler)->mClass;
-}
-
-inline DOMObjectSlot
-GetDOMClass(JSObject* obj, const DOMClass*& result)
-{
-  js::Class* clasp = js::GetObjectClass(obj);
-  if (IsDOMClass(clasp)) {
-    result = &DOMJSClass::FromJSClass(clasp)->mClass;
-    return eRegularDOMObject;
-  }
-
   if (js::IsObjectProxyClass(clasp) || js::IsFunctionProxyClass(clasp)) {
     js::BaseProxyHandler* handler = js::GetProxyHandler(obj);
     if (handler->family() == ProxyFamily()) {
-      result = &static_cast<DOMProxyHandler*>(handler)->mClass;
-      return eProxyDOMObject;
+      return &static_cast<DOMProxyHandler*>(handler)->mClass;
     }
   }
 
-  return eNonDOMObject;
+  return nullptr;
 }
 
 inline bool
 UnwrapDOMObjectToISupports(JSObject* obj, nsISupports*& result)
 {
-  const DOMClass* clasp;
-  DOMObjectSlot slot = GetDOMClass(obj, clasp);
-  if (slot == eNonDOMObject || !clasp->mDOMObjectIsISupports) {
+  const DOMClass* clasp = GetDOMClass(obj);
+  if (!clasp || !clasp->mDOMObjectIsISupports) {
     return false;
   }
  
-  result = UnwrapDOMObject<nsISupports>(obj, slot);
+  result = UnwrapDOMObject<nsISupports>(obj);
   return true;
 }
 
 inline bool
 IsDOMObject(JSObject* obj)
 {
   js::Class* clasp = js::GetObjectClass(obj);
   return IsDOMClass(clasp) || IsDOMProxy(obj, clasp);
@@ -186,43 +154,42 @@ IsDOMObject(JSObject* obj)
 // (for example, overload resolution uses unwrapping to tell what sort
 // of thing it's looking at).
 // U must be something that a T* can be assigned to (e.g. T* or an nsRefPtr<T>).
 template <prototypes::ID PrototypeID, class T, typename U>
 MOZ_ALWAYS_INLINE nsresult
 UnwrapObject(JSContext* cx, JSObject* obj, U& value)
 {
   /* First check to see whether we have a DOM object */
-  const DOMClass* domClass;
-  DOMObjectSlot slot = GetDOMClass(obj, domClass);
-  if (slot == eNonDOMObject) {
+  const DOMClass* domClass = GetDOMClass(obj);
+  if (!domClass) {
     /* Maybe we have a security wrapper or outer window? */
     if (!js::IsWrapper(obj)) {
       /* Not a DOM object, not a wrapper, just bail */
       return NS_ERROR_XPC_BAD_CONVERT_JS;
     }
 
     obj = xpc::Unwrap(cx, obj, false);
     if (!obj) {
       return NS_ERROR_XPC_SECURITY_MANAGER_VETO;
     }
     MOZ_ASSERT(!js::IsWrapper(obj));
-    slot = GetDOMClass(obj, domClass);
-    if (slot == eNonDOMObject) {
+    domClass = GetDOMClass(obj);
+    if (!domClass) {
       /* We don't have a DOM object */
       return NS_ERROR_XPC_BAD_CONVERT_JS;
     }
   }
 
   /* This object is a DOM object.  Double-check that it is safely
      castable to T by checking whether it claims to inherit from the
      class identified by protoID. */
   if (domClass->mInterfaceChain[PrototypeTraits<PrototypeID>::Depth] ==
       PrototypeID) {
-    value = UnwrapDOMObject<T>(obj, slot);
+    value = UnwrapDOMObject<T>(obj);
     return NS_OK;
   }
 
   /* It's the wrong sort of DOM object */
   return NS_ERROR_XPC_BAD_CONVERT_JS;
 }
 
 inline bool
@@ -538,21 +505,20 @@ WrapNewBindingObject(JSContext* cx, JSOb
       // try to communicate triedToWrap to the caller, but in practice
       // callers seem to be testing JS_IsExceptionPending(cx) to
       // figure out whether WrapObject() threw instead.
       return false;
     }
   }
 
 #ifdef DEBUG
-  const DOMClass* clasp = nullptr;
-  DOMObjectSlot slot = GetDOMClass(obj, clasp);
-  // slot can be eNonDOMObject if the cache contained a non-DOM object from a
+  const DOMClass* clasp = GetDOMClass(obj);
+  // clasp can be null if the cache contained a non-DOM object from a
   // different compartment than scope.
-  if (slot != eNonDOMObject) {
+  if (clasp) {
     // Some sanity asserts about our object.  Specifically:
     // 1)  If our class claims we're nsISupports, we better be nsISupports
     //     XXXbz ideally, we could assert that reinterpret_cast to nsISupports
     //     does the right thing, but I don't see a way to do it.  :(
     // 2)  If our class doesn't claim we're nsISupports we better be
     //     reinterpret_castable to nsWrapperCache.
     MOZ_ASSERT(clasp, "What happened here?");
     MOZ_ASSERT_IF(clasp->mDOMObjectIsISupports, IsISupports<T>::Value);
--- a/dom/bindings/Codegen.py
+++ b/dom/bindings/Codegen.py
@@ -744,17 +744,17 @@ class CGAbstractClassHook(CGAbstractStat
     'this' unwrapping as it assumes that the unwrapped type is always known.
     """
     def __init__(self, descriptor, name, returnType, args):
         CGAbstractStaticMethod.__init__(self, descriptor, name, returnType,
                                         args)
 
     def definition_body_prologue(self):
         return """
-  %s* self = UnwrapDOMObject<%s>(obj, eRegularDOMObject);
+  %s* self = UnwrapDOMObject<%s>(obj);
 """ % (self.descriptor.nativeType, self.descriptor.nativeType)
 
     def definition_body(self):
         return self.definition_body_prologue() + self.generate_code()
 
     def generate_code(self):
         # Override me
         assert(False)
--- a/dom/bindings/DOMJSProxyHandler.cpp
+++ b/dom/bindings/DOMJSProxyHandler.cpp
@@ -62,17 +62,17 @@ DOMProxyHandler::EnsureExpandoObject(JSC
     }
 
     XPCWrappedNativeScope* scope = xpc::GetObjectScope(obj);
     if (!scope->RegisterDOMExpandoObject(obj)) {
       return NULL;
     }
 
     nsWrapperCache* cache;
-    CallQueryInterface(UnwrapDOMObject<nsISupports>(obj, eProxyDOMObject), &cache);
+    CallQueryInterface(UnwrapDOMObject<nsISupports>(obj), &cache);
     cache->SetPreservingWrapper(true);
 
     js::SetProxyExtra(obj, JSPROXYSLOT_EXPANDO, ObjectValue(*expando));
   }
   return expando;
 }
 
 bool
--- a/dom/workers/Worker.cpp
+++ b/dom/workers/Worker.cpp
@@ -212,29 +212,27 @@ private:
   {
     return ConstructInternal(aCx, aArgc, aVp, false, Class());
   }
 
   static void
   Finalize(JSFreeOp* aFop, JSObject* aObj)
   {
     JS_ASSERT(JS_GetClass(aObj) == Class());
-    WorkerPrivate* worker =
-      UnwrapDOMObject<WorkerPrivate>(aObj, eRegularDOMObject);
+    WorkerPrivate* worker = UnwrapDOMObject<WorkerPrivate>(aObj);
     if (worker) {
       worker->_finalize(aFop);
     }
   }
 
   static void
   Trace(JSTracer* aTrc, JSObject* aObj)
   {
     JS_ASSERT(JS_GetClass(aObj) == Class());
-    WorkerPrivate* worker =
-      UnwrapDOMObject<WorkerPrivate>(aObj, eRegularDOMObject);
+    WorkerPrivate* worker = UnwrapDOMObject<WorkerPrivate>(aObj);
     if (worker) {
       worker->_trace(aTrc);
     }
   }
 
   static JSBool
   Terminate(JSContext* aCx, unsigned aArgc, jsval* aVp)
   {
@@ -356,46 +354,44 @@ private:
   ~ChromeWorker();
 
   static WorkerPrivate*
   GetInstancePrivate(JSContext* aCx, JSObject* aObj, const char* aFunctionName)
   {
     if (aObj) {
       JSClass* classPtr = JS_GetClass(aObj);
       if (classPtr == Class()) {
-        return UnwrapDOMObject<WorkerPrivate>(aObj, eRegularDOMObject);
+        return UnwrapDOMObject<WorkerPrivate>(aObj);
       }
     }
 
     return Worker::GetInstancePrivate(aCx, aObj, aFunctionName);
   }
 
   static JSBool
   Construct(JSContext* aCx, unsigned aArgc, jsval* aVp)
   {
     return ConstructInternal(aCx, aArgc, aVp, true, Class());
   }
 
   static void
   Finalize(JSFreeOp* aFop, JSObject* aObj)
   {
     JS_ASSERT(JS_GetClass(aObj) == Class());
-    WorkerPrivate* worker =
-      UnwrapDOMObject<WorkerPrivate>(aObj, eRegularDOMObject);
+    WorkerPrivate* worker = UnwrapDOMObject<WorkerPrivate>(aObj);
     if (worker) {
       worker->_finalize(aFop);
     }
   }
 
   static void
   Trace(JSTracer* aTrc, JSObject* aObj)
   {
     JS_ASSERT(JS_GetClass(aObj) == Class());
-    WorkerPrivate* worker =
-      UnwrapDOMObject<WorkerPrivate>(aObj, eRegularDOMObject);
+    WorkerPrivate* worker = UnwrapDOMObject<WorkerPrivate>(aObj);
     if (worker) {
       worker->_trace(aTrc);
     }
   }
 };
 
 DOMJSClass ChromeWorker::sClass = {
   { "ChromeWorker",
@@ -413,17 +409,17 @@ DOMJSClass ChromeWorker::sClass = {
 };
 
 WorkerPrivate*
 Worker::GetInstancePrivate(JSContext* aCx, JSObject* aObj,
                            const char* aFunctionName)
 {
   JSClass* classPtr = JS_GetClass(aObj);
   if (classPtr == Class() || classPtr == ChromeWorker::Class()) {
-    return UnwrapDOMObject<WorkerPrivate>(aObj, eRegularDOMObject);
+    return UnwrapDOMObject<WorkerPrivate>(aObj);
   }
 
   JS_ReportErrorNumber(aCx, js_GetErrorMessage, NULL, JSMSG_INCOMPATIBLE_PROTO,
                        Class()->name, aFunctionName, classPtr->name);
   return NULL;
 }
 
 } // anonymous namespace
--- a/dom/workers/WorkerScope.cpp
+++ b/dom/workers/WorkerScope.cpp
@@ -765,18 +765,17 @@ private:
     return true;
   }
 
   static DedicatedWorkerGlobalScope*
   GetInstancePrivate(JSContext* aCx, JSObject* aObj, const char* aFunctionName)
   {
     JSClass* classPtr = JS_GetClass(aObj);
     if (classPtr == Class()) {
-      return UnwrapDOMObject<DedicatedWorkerGlobalScope>(aObj,
-                                                         eRegularDOMObject);
+      return UnwrapDOMObject<DedicatedWorkerGlobalScope>(aObj);
     }
 
     JS_ReportErrorNumber(aCx, js_GetErrorMessage, NULL,
                          JSMSG_INCOMPATIBLE_PROTO, Class()->name, aFunctionName,
                          classPtr->name);
     return NULL;
   }
 
@@ -801,29 +800,29 @@ private:
     return true;
   }
 
   static void
   Finalize(JSFreeOp* aFop, JSObject* aObj)
   {
     JS_ASSERT(JS_GetClass(aObj) == Class());
     DedicatedWorkerGlobalScope* scope =
-      UnwrapDOMObject<DedicatedWorkerGlobalScope>(aObj, eRegularDOMObject);
+      UnwrapDOMObject<DedicatedWorkerGlobalScope>(aObj);
     if (scope) {
       DestroyProtoAndIfaceCache(aObj);
       scope->_finalize(aFop);
     }
   }
 
   static void
   Trace(JSTracer* aTrc, JSObject* aObj)
   {
     JS_ASSERT(JS_GetClass(aObj) == Class());
     DedicatedWorkerGlobalScope* scope =
-      UnwrapDOMObject<DedicatedWorkerGlobalScope>(aObj, eRegularDOMObject);
+      UnwrapDOMObject<DedicatedWorkerGlobalScope>(aObj);
     if (scope) {
       TraceProtoAndIfaceCache(aTrc, aObj);
       scope->_trace(aTrc);
     }
   }
 
   static JSBool
   PostMessage(JSContext* aCx, unsigned aArgc, jsval* aVp)
@@ -889,17 +888,17 @@ WorkerGlobalScope::GetInstancePrivate(JS
 {
   JSClass* classPtr = JS_GetClass(aObj);
 
   // We can only make DedicatedWorkerGlobalScope, not WorkerGlobalScope, so this
   // should never happen.
   JS_ASSERT(classPtr != Class());
 
   if (classPtr == DedicatedWorkerGlobalScope::Class()) {
-    return UnwrapDOMObject<DedicatedWorkerGlobalScope>(aObj, eRegularDOMObject);
+    return UnwrapDOMObject<DedicatedWorkerGlobalScope>(aObj);
   }
 
   JS_ReportErrorNumber(aCx, js_GetErrorMessage, NULL, JSMSG_INCOMPATIBLE_PROTO,
                        sClass.name, aFunctionName, classPtr->name);
   return NULL;
 }
 
 } /* anonymous namespace */
--- a/js/xpconnect/src/XPCQuickStubs.h
+++ b/js/xpconnect/src/XPCQuickStubs.h
@@ -447,20 +447,19 @@ castNativeFromWrapper(JSContext *cx,
     } else if (cur && IS_SLIM_WRAPPER(cur)) {
         native = static_cast<nsISupports*>(xpc_GetJSPrivate(cur));
         if (!native || !HasBitInInterfacesBitmap(cur, interfaceBit)) {
             native = nullptr;
         } else if (lccx) {
             lccx->SetWrapper(cur);
         }
     } else if (cur && protoDepth >= 0) {
-        const mozilla::dom::DOMClass* domClass;
-        mozilla::dom::DOMObjectSlot slot =
-            mozilla::dom::GetDOMClass(cur, domClass);
-        native = mozilla::dom::UnwrapDOMObject<nsISupports>(cur, slot);
+        const mozilla::dom::DOMClass* domClass =
+            mozilla::dom::GetDOMClass(cur);
+        native = mozilla::dom::UnwrapDOMObject<nsISupports>(cur);
         if (native &&
             (uint32_t)domClass->mInterfaceChain[protoDepth] != protoID) {
             native = nullptr;
         }
     } else {
         native = nullptr;
     }
 
--- a/js/xpconnect/src/XPCWrappedNativeScope.cpp
+++ b/js/xpconnect/src/XPCWrappedNativeScope.cpp
@@ -330,20 +330,19 @@ WrappedNativeSuspecter(JSDHashTable *tab
 }
 
 static PLDHashOperator
 SuspectDOMExpandos(nsPtrHashKey<JSObject> *key, void *arg)
 {
     nsCycleCollectionTraversalCallback *cb =
       static_cast<nsCycleCollectionTraversalCallback *>(arg);
     JSObject* obj = key->GetKey();
-    const dom::DOMClass* clasp;
-    dom::DOMObjectSlot slot = GetDOMClass(obj, clasp);
-    MOZ_ASSERT(slot != dom::eNonDOMObject && clasp->mDOMObjectIsISupports);
-    nsISupports* native = dom::UnwrapDOMObject<nsISupports>(obj, slot);
+    const dom::DOMClass* clasp = dom::GetDOMClass(obj);
+    MOZ_ASSERT(clasp && clasp->mDOMObjectIsISupports);
+    nsISupports* native = dom::UnwrapDOMObject<nsISupports>(obj);
     cb->NoteXPCOMRoot(native);
     return PL_DHASH_NEXT;
 }
 
 // static
 void
 XPCWrappedNativeScope::SuspectAllWrappers(XPCJSRuntime* rt,
                                           nsCycleCollectionTraversalCallback& cb)
--- a/js/xpconnect/src/nsXPConnect.cpp
+++ b/js/xpconnect/src/nsXPConnect.cpp
@@ -754,24 +754,23 @@ NoteGCThingXPCOMChildren(js::Class *clas
     }
     // XXX This test does seem fragile, we should probably whitelist classes
     //     that do hold a strong reference, but that might not be possible.
     else if (clasp->flags & JSCLASS_HAS_PRIVATE &&
              clasp->flags & JSCLASS_PRIVATE_IS_NSISUPPORTS) {
         NS_CYCLE_COLLECTION_NOTE_EDGE_NAME(cb, "xpc_GetJSPrivate(obj)");
         cb.NoteXPCOMChild(static_cast<nsISupports*>(xpc_GetJSPrivate(obj)));
     } else {
-        const DOMClass* domClass;
-        DOMObjectSlot slot = GetDOMClass(obj, domClass);
-        if (slot != eNonDOMObject) {
+        const DOMClass* domClass = GetDOMClass(obj);
+        if (domClass) {
             NS_CYCLE_COLLECTION_NOTE_EDGE_NAME(cb, "UnwrapDOMObject(obj)");
             if (domClass->mDOMObjectIsISupports) {
-                cb.NoteXPCOMChild(UnwrapDOMObject<nsISupports>(obj, slot));
+                cb.NoteXPCOMChild(UnwrapDOMObject<nsISupports>(obj));
             } else if (domClass->mParticipant) {
-                cb.NoteNativeChild(UnwrapDOMObject<void>(obj, slot),
+                cb.NoteNativeChild(UnwrapDOMObject<void>(obj),
                                    domClass->mParticipant);
             }
         }
     }
 }
 
 enum TraverseSelect {
     TRAVERSE_CPP,