Bug 996785 - Distinguish local and remote CPOW objects in IPDL (r=mrbkap)
authorBill McCloskey <wmccloskey@mozilla.com>
Fri, 16 May 2014 16:40:36 -0700
changeset 183658 990ea6bcfa07a5ca68d6439f275a18c249f222c9
parent 183657 1e495c6dba19835297494eb2a128301fe9e55848
child 183659 9bc4f004e7f004bd728ce1a26b15b9c47183dfaf
push id6844
push userphilringnalda@gmail.com
push dateSun, 18 May 2014 01:12:08 +0000
treeherderfx-team@41a54c8add09 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmrbkap
bugs996785
milestone32.0a1
Bug 996785 - Distinguish local and remote CPOW objects in IPDL (r=mrbkap)
js/ipc/JavaScriptChild.cpp
js/ipc/JavaScriptChild.h
js/ipc/JavaScriptParent.cpp
js/ipc/JavaScriptParent.h
js/ipc/JavaScriptShared.cpp
js/ipc/JavaScriptShared.h
js/ipc/JavaScriptTypes.ipdlh
--- a/js/ipc/JavaScriptChild.cpp
+++ b/js/ipc/JavaScriptChild.cpp
@@ -66,26 +66,23 @@ JavaScriptChild::RecvDropObject(const Ob
     if (obj) {
         ids_.remove(obj);
         objects_.remove(objId);
     }
     return true;
 }
 
 bool
-JavaScriptChild::toId(JSContext *cx, JSObject *obj, ObjectId *idp)
+JavaScriptChild::toObjectVariant(JSContext *cx, JSObject *obj, ObjectVariant *objVarp)
 {
-    if (!obj) {
-        *idp = 0;
-        return true;
-    }
+    JS_ASSERT(obj);
 
     ObjectId id = ids_.find(obj);
     if (id) {
-        *idp = id;
+        *objVarp = RemoteObject(id);
         return true;
     }
 
     id = ++lastId_;
     if (id > MAX_CPOW_IDS) {
         JS_ReportError(cx, "CPOW id limit reached");
         return false;
     }
@@ -94,20 +91,22 @@ JavaScriptChild::toId(JSContext *cx, JSO
     if (JS_ObjectIsCallable(cx, obj))
         id |= OBJECT_IS_CALLABLE;
 
     if (!objects_.add(id, obj))
         return false;
     if (!ids_.add(cx, obj, id))
         return false;
 
-    *idp = id;
+    *objVarp = RemoteObject(id);
     return true;
 }
 
 JSObject *
-JavaScriptChild::fromId(JSContext *cx, ObjectId id)
+JavaScriptChild::fromObjectVariant(JSContext *cx, ObjectVariant objVar)
 {
+    JS_ASSERT(objVar.type() == ObjectVariant::TLocalObject);
+    ObjectId id = objVar.get_LocalObject().id();
     JSObject *obj = findObjectById(id);
     MOZ_ASSERT(obj);
     return obj;
 }
 
--- a/js/ipc/JavaScriptChild.h
+++ b/js/ipc/JavaScriptChild.h
@@ -23,18 +23,18 @@ class JavaScriptChild : public JavaScrip
     bool init();
     void trace(JSTracer *trc);
 
     bool RecvDropObject(const ObjectId &objId) MOZ_OVERRIDE;
 
     virtual void drop(JSObject *obj) { MOZ_CRASH(); }
 
   private:
-    JSObject *fromId(JSContext *cx, ObjectId id);
-    bool toId(JSContext *cx, JSObject *obj, ObjectId *idp);
+    virtual bool toObjectVariant(JSContext *cx, JSObject *obj, ObjectVariant *objVarp);
+    virtual JSObject *fromObjectVariant(JSContext *cx, ObjectVariant objVar);
 
     bool fail(JSContext *cx, ReturnStatus *rs);
     bool ok(ReturnStatus *rs);
 
   private:
     ObjectId lastId_;
     JSRuntime *rt_;
     ObjectToIdMap ids_;
--- a/js/ipc/JavaScriptParent.cpp
+++ b/js/ipc/JavaScriptParent.cpp
@@ -42,31 +42,35 @@ JavaScriptParent::init()
 {
     if (!WrapperOwner::init())
         return false;
 
     return true;
 }
 
 bool
-JavaScriptParent::toId(JSContext *cx, JSObject *obj, ObjectId *idp)
+JavaScriptParent::toObjectVariant(JSContext *cx, JSObject *obj, ObjectVariant *objVarp)
 {
+    JS_ASSERT(obj);
     obj = js::CheckedUnwrap(obj, false);
     if (!obj || !IsCPOW(obj)) {
         JS_ReportError(cx, "cannot ipc non-cpow object");
         return false;
     }
 
-    *idp = idOf(obj);
+    *objVarp = LocalObject(idOf(obj));
     return true;
 }
 
 JSObject *
-JavaScriptParent::fromId(JSContext *cx, ObjectId objId)
+JavaScriptParent::fromObjectVariant(JSContext *cx, ObjectVariant objVar)
 {
+    JS_ASSERT(objVar.type() == ObjectVariant::TRemoteObject);
+    ObjectId objId = objVar.get_RemoteObject().id();
+
     RootedObject obj(cx, findCPOWById(objId));
     if (obj) {
         if (!JS_WrapObject(cx, &obj))
             return nullptr;
         return obj;
     }
 
     if (objId > MAX_CPOW_IDS) {
--- a/js/ipc/JavaScriptParent.h
+++ b/js/ipc/JavaScriptParent.h
@@ -25,18 +25,18 @@ class JavaScriptParent : public JavaScri
     void incref();
 
     void drop(JSObject *obj);
 
     mozilla::ipc::IProtocol*
     CloneProtocol(Channel* aChannel, ProtocolCloneContext* aCtx) MOZ_OVERRIDE;
 
   private:
-    JSObject *fromId(JSContext *cx, ObjectId objId);
-    bool toId(JSContext *cx, JSObject *obj, ObjectId *idp);
+    virtual bool toObjectVariant(JSContext *cx, JSObject *obj, ObjectVariant *objVarp);
+    virtual JSObject *fromObjectVariant(JSContext *cx, ObjectVariant objVar);
 
   private:
     uintptr_t refcount_;
 };
 
 } // jsipc
 } // mozilla
 
--- a/js/ipc/JavaScriptShared.cpp
+++ b/js/ipc/JavaScriptShared.cpp
@@ -189,20 +189,20 @@ JavaScriptShared::toVariant(JSContext *c
         if (xpc_JSObjectIsID(cx, obj)) {
             JSIID iid;
             const nsID *id = xpc_JSObjectToID(cx, obj);
             ConvertID(*id, &iid);
             *to = iid;
             return true;
         }
 
-        ObjectId id;
-        if (!toId(cx, obj, &id))
+        ObjectVariant objVar;
+        if (!toObjectVariant(cx, obj, &objVar))
             return false;
-        *to = ObjectVariant(id);
+        *to = objVar;
         return true;
       }
 
       case JSTYPE_STRING:
       {
         nsDependentJSString dep;
         if (!dep.init(cx, from))
             return false;
@@ -236,18 +236,17 @@ JavaScriptShared::fromVariant(JSContext 
           return true;
 
         case JSVariant::TNullVariant:
           to.set(NullValue());
           return true;
 
         case JSVariant::TObjectVariant:
         {
-          ObjectId id = from.get_ObjectVariant().id();
-          JSObject *obj = fromId(cx, id);
+          JSObject *obj = fromObjectVariant(cx, from.get_ObjectVariant());
           if (!obj)
               return false;
           to.set(ObjectValue(*obj));
           return true;
         }
 
         case JSVariant::Tdouble:
           to.set(JS_NumberValue(from.get_double()));
@@ -314,50 +313,55 @@ JavaScriptShared::ConvertID(const JSIID 
     to->m3[2] = from.m3_2();
     to->m3[3] = from.m3_3();
     to->m3[4] = from.m3_4();
     to->m3[5] = from.m3_5();
     to->m3[6] = from.m3_6();
     to->m3[7] = from.m3_7();
 }
 
-static const uint32_t DefaultPropertyOp = 1;
-static const uint32_t GetterOnlyPropertyStub = 2;
-static const uint32_t UnknownPropertyOp = 3;
+static const uint64_t DefaultPropertyOp = 1;
+static const uint64_t GetterOnlyPropertyStub = 2;
+static const uint64_t UnknownPropertyOp = 3;
 
 bool
 JavaScriptShared::fromDescriptor(JSContext *cx, Handle<JSPropertyDescriptor> desc,
                                  PPropertyDescriptor *out)
 {
     out->attrs() = desc.attributes();
     if (!toVariant(cx, desc.value(), &out->value()))
         return false;
 
-    if (!toId(cx, desc.object(), &out->objId()))
+    JS_ASSERT(desc.object());
+    if (!toObjectVariant(cx, desc.object(), &out->obj()))
         return false;
 
     if (!desc.getter()) {
         out->getter() = 0;
     } else if (desc.hasGetterObject()) {
         JSObject *getter = desc.getterObject();
-        if (!toId(cx, getter, &out->getter()))
+        ObjectVariant objVar;
+        if (!toObjectVariant(cx, getter, &objVar))
             return false;
+        out->getter() = objVar;
     } else {
         if (desc.getter() == JS_PropertyStub)
             out->getter() = DefaultPropertyOp;
         else
             out->getter() = UnknownPropertyOp;
     }
 
     if (!desc.setter()) {
         out->setter() = 0;
     } else if (desc.hasSetterObject()) {
         JSObject *setter = desc.setterObject();
-        if (!toId(cx, setter, &out->setter()))
+        ObjectVariant objVar;
+        if (!toObjectVariant(cx, setter, &objVar))
             return false;
+        out->setter() = objVar;
     } else {
         if (desc.setter() == JS_StrictPropertyStub)
             out->setter() = DefaultPropertyOp;
         else if (desc.setter() == js_GetterOnlyPropertyStub)
             out->setter() = GetterOnlyPropertyStub;
         else
             out->setter() = UnknownPropertyOp;
     }
@@ -382,45 +386,48 @@ UnknownStrictPropertyStub(JSContext *cx,
 bool
 JavaScriptShared::toDescriptor(JSContext *cx, const PPropertyDescriptor &in,
                                MutableHandle<JSPropertyDescriptor> out)
 {
     out.setAttributes(in.attrs());
     if (!fromVariant(cx, in.value(), out.value()))
         return false;
     Rooted<JSObject*> obj(cx);
-    if (!fromId(cx, in.objId(), &obj))
+    obj = fromObjectVariant(cx, in.obj());
+    if (!obj)
         return false;
     out.object().set(obj);
 
-    if (!in.getter()) {
+    if (in.getter().type() == GetterSetter::Tuint64_t && !in.getter().get_uint64_t()) {
         out.setGetter(nullptr);
     } else if (in.attrs() & JSPROP_GETTER) {
         Rooted<JSObject*> getter(cx);
-        if (!fromId(cx, in.getter(), &getter))
+        getter = fromObjectVariant(cx, in.getter().get_ObjectVariant());
+        if (!getter)
             return false;
         out.setGetter(JS_DATA_TO_FUNC_PTR(JSPropertyOp, getter.get()));
     } else {
-        if (in.getter() == DefaultPropertyOp)
+        if (in.getter().get_uint64_t() == DefaultPropertyOp)
             out.setGetter(JS_PropertyStub);
         else
             out.setGetter(UnknownPropertyStub);
     }
 
-    if (!in.setter()) {
+    if (in.setter().type() == GetterSetter::Tuint64_t && !in.setter().get_uint64_t()) {
         out.setSetter(nullptr);
     } else if (in.attrs() & JSPROP_SETTER) {
         Rooted<JSObject*> setter(cx);
-        if (!fromId(cx, in.setter(), &setter))
+        setter = fromObjectVariant(cx, in.setter().get_ObjectVariant());
+        if (!setter)
             return false;
         out.setSetter(JS_DATA_TO_FUNC_PTR(JSStrictPropertyOp, setter.get()));
     } else {
-        if (in.setter() == DefaultPropertyOp)
+        if (in.setter().get_uint64_t() == DefaultPropertyOp)
             out.setSetter(JS_StrictPropertyStub);
-        else if (in.setter() == GetterOnlyPropertyStub)
+        else if (in.setter().get_uint64_t() == GetterOnlyPropertyStub)
             out.setSetter(js_GetterOnlyPropertyStub);
         else
             out.setSetter(UnknownStrictPropertyStub);
     }
 
     return true;
 }
 
--- a/js/ipc/JavaScriptShared.h
+++ b/js/ipc/JavaScriptShared.h
@@ -98,28 +98,18 @@ class JavaScriptShared
     bool fromDescriptor(JSContext *cx, JS::Handle<JSPropertyDescriptor> desc,
                         PPropertyDescriptor *out);
     bool toDescriptor(JSContext *cx, const PPropertyDescriptor &in,
                       JS::MutableHandle<JSPropertyDescriptor> out);
 
     bool convertIdToGeckoString(JSContext *cx, JS::HandleId id, nsString *to);
     bool convertGeckoStringToId(JSContext *cx, const nsString &from, JS::MutableHandleId id);
 
-    virtual bool toId(JSContext *cx, JSObject *obj, ObjectId *idp) = 0;
-    virtual JSObject *fromId(JSContext *cx, ObjectId id) = 0;
-
-    bool fromId(JSContext *cx, ObjectId id, JS::MutableHandle<JSObject*> objp) {
-        if (!id) {
-            objp.set(nullptr);
-            return true;
-        }
-
-        objp.set(fromId(cx, id));
-        return bool(objp.get());
-    }
+    virtual bool toObjectVariant(JSContext *cx, JSObject *obj, ObjectVariant *objVarp) = 0;
+    virtual JSObject *fromObjectVariant(JSContext *cx, ObjectVariant objVar) = 0;
 
     static void ConvertID(const nsID &from, JSIID *to);
     static void ConvertID(const JSIID &from, nsID *to);
 
     JSObject *findCPOWById(uint32_t objId) {
         return cpows_.find(objId);
     }
     JSObject *findObjectById(uint32_t objId) {
--- a/js/ipc/JavaScriptTypes.ipdlh
+++ b/js/ipc/JavaScriptTypes.ipdlh
@@ -22,21 +22,32 @@ struct JSIID
     uint8_t m3_2;
     uint8_t m3_3;
     uint8_t m3_4;
     uint8_t m3_5;
     uint8_t m3_6;
     uint8_t m3_7;
 };
 
-struct ObjectVariant
+struct LocalObject
 {
     uint64_t id;
 };
 
+struct RemoteObject
+{
+    uint64_t id;
+};
+
+union ObjectVariant
+{
+    LocalObject;
+    RemoteObject;
+};
+
 struct UndefinedVariant {};
 struct NullVariant {};
 
 union JSVariant
 {
     UndefinedVariant;
     NullVariant;
     ObjectVariant;
@@ -67,32 +78,38 @@ union ReturnStatus
 };
 
 union JSParam
 {
     void_t;     /* value is strictly an xpc out param */
     JSVariant;  /* actual value to pass through */
 };
 
+union GetterSetter
+{
+    uint64_t;
+    ObjectVariant;
+};
+
 struct PPropertyDescriptor
 {
-    uint64_t    objId;
-    uint32_t    attrs;
-    JSVariant   value;
+    ObjectVariant obj;
+    uint32_t attrs;
+    JSVariant value;
 
     // How to interpret these values depends on whether JSPROP_GETTER/SETTER
     // are set. If set, the corresponding value is a CPOW or 0 for NULL.
     // Otherwise, the following table is used:
     //
     //  0 - NULL
     //  1 - Default getter or setter.
     //  2 - js_GetterOnlyPropertyStub (setter only)
     //  3 - Unknown
-    uint64_t    getter;
-    uint64_t    setter;
+    GetterSetter getter;
+    GetterSetter setter;
 };
 
 struct CpowEntry
 {
   nsString name;
   JSVariant value;
 };