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 183649 990ea6bcfa07a5ca68d6439f275a18c249f222c9
parent 183648 1e495c6dba19835297494eb2a128301fe9e55848
child 183650 9bc4f004e7f004bd728ce1a26b15b9c47183dfaf
push id26799
push userphilringnalda@gmail.com
push dateSun, 18 May 2014 00:55:16 +0000
treeherdermozilla-central@00ef3a7d7aa7 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmrbkap
bugs996785
milestone32.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 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;
 };