Bug 996785 - Distinguish local and remote CPOW objects in IPDL (r=mrbkap)

js/ipc/JavaScriptChild.cpp

@@ -71,16 +71,13 @@ JavaScriptChild::RecvDropObject(const ObjectId &objId)
 }
 
 bool
-JavaScriptChild::toId(JSContext *cx, JSObject *obj, ObjectId *idp)
+JavaScriptChild::toObjectVariant(JSContext *cx, JSObject *obj, ObjectVariant *objVarp)
 {
-    if (!obj) {
+    JS_ASSERT(obj);
-        *idp = 0;
-        return true;
-    }
 
     ObjectId id = ids_.find(obj);
     if (id) {
-        *idp = id;
+        *objVarp = RemoteObject(id);
         return true;
     }
 
@@ -99,13 +96,15 @@ JavaScriptChild::toId(JSContext *cx, JSObject *obj, ObjectId *idp)
     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;

js/ipc/JavaScriptChild.h

@@ -28,8 +28,8 @@ class JavaScriptChild : public JavaScriptBase<PJavaScriptChild>
     virtual void drop(JSObject *obj) { MOZ_CRASH(); }
 
   private:
-    JSObject *fromId(JSContext *cx, ObjectId id);
+    virtual bool toObjectVariant(JSContext *cx, JSObject *obj, ObjectVariant *objVarp);
-    bool toId(JSContext *cx, JSObject *obj, ObjectId *idp);
+    virtual JSObject *fromObjectVariant(JSContext *cx, ObjectVariant objVar);
 
     bool fail(JSContext *cx, ReturnStatus *rs);
     bool ok(ReturnStatus *rs);

js/ipc/JavaScriptParent.cpp

@@ -47,21 +47,25 @@ JavaScriptParent::init()
 }
 
 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))

js/ipc/JavaScriptParent.h

@@ -30,8 +30,8 @@ class JavaScriptParent : public JavaScriptBase<PJavaScriptParent>
     CloneProtocol(Channel* aChannel, ProtocolCloneContext* aCtx) MOZ_OVERRIDE;
 
   private:
-    JSObject *fromId(JSContext *cx, ObjectId objId);
+    virtual bool toObjectVariant(JSContext *cx, JSObject *obj, ObjectVariant *objVarp);
-    bool toId(JSContext *cx, JSObject *obj, ObjectId *idp);
+    virtual JSObject *fromObjectVariant(JSContext *cx, ObjectVariant objVar);
 
   private:
     uintptr_t refcount_;

js/ipc/JavaScriptShared.cpp

@@ -194,10 +194,10 @@ JavaScriptShared::toVariant(JSContext *cx, JS::HandleValue from, JSVariant *to)
             return true;
         }
 
-        ObjectId id;
+        ObjectVariant objVar;
-        if (!toId(cx, obj, &id))
+        if (!toObjectVariant(cx, obj, &objVar))
             return false;
-        *to = ObjectVariant(id);
+        *to = objVar;
         return true;
       }
 
@@ -241,8 +241,7 @@ JavaScriptShared::fromVariant(JSContext *cx, const JSVariant &from, MutableHandl
 
         case JSVariant::TObjectVariant:
         {
-          ObjectId id = from.get_ObjectVariant().id();
+          JSObject *obj = fromObjectVariant(cx, from.get_ObjectVariant());
-          JSObject *obj = fromId(cx, id);
           if (!obj)
               return false;
           to.set(ObjectValue(*obj));
@@ -319,9 +318,9 @@ JavaScriptShared::ConvertID(const JSIID &from, nsID *to)
     to->m3[7] = from.m3_7();
 }
 
-static const uint32_t DefaultPropertyOp = 1;
+static const uint64_t DefaultPropertyOp = 1;
-static const uint32_t GetterOnlyPropertyStub = 2;
+static const uint64_t GetterOnlyPropertyStub = 2;
-static const uint32_t UnknownPropertyOp = 3;
+static const uint64_t UnknownPropertyOp = 3;
 
 bool
 JavaScriptShared::fromDescriptor(JSContext *cx, Handle<JSPropertyDescriptor> desc,
@@ -331,15 +330,18 @@ JavaScriptShared::fromDescriptor(JSContext *cx, Handle<JSPropertyDescriptor> des
     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;
@@ -351,8 +353,10 @@ JavaScriptShared::fromDescriptor(JSContext *cx, Handle<JSPropertyDescriptor> des
         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;
@@ -387,35 +391,38 @@ JavaScriptShared::toDescriptor(JSContext *cx, const PPropertyDescriptor &in,
     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);

js/ipc/JavaScriptShared.h

@@ -103,18 +103,8 @@ class JavaScriptShared
     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 bool toObjectVariant(JSContext *cx, JSObject *obj, ObjectVariant *objVarp) = 0;
-    virtual JSObject *fromId(JSContext *cx, ObjectId id) = 0;
+    virtual JSObject *fromObjectVariant(JSContext *cx, ObjectVariant objVar) = 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());
-    }
 
     static void ConvertID(const nsID &from, JSIID *to);
     static void ConvertID(const JSIID &from, nsID *to);

js/ipc/JavaScriptTypes.ipdlh

@@ -27,11 +27,22 @@ struct JSIID
     uint8_t m3_7;
 };
 
-struct ObjectVariant
+struct LocalObject
 {
     uint64_t id;
 };
 
+struct RemoteObject
+{
+    uint64_t id;
+};
+
+union ObjectVariant
+{
+    LocalObject;
+    RemoteObject;
+};
+
 struct UndefinedVariant {};
 struct NullVariant {};
 
@@ -72,11 +83,17 @@ union JSParam
     JSVariant;  /* actual value to pass through */
 };
 
+union GetterSetter
+{
+    uint64_t;
+    ObjectVariant;
+};
+
 struct PPropertyDescriptor
 {
-    uint64_t    objId;
+    ObjectVariant obj;
-    uint32_t    attrs;
+    uint32_t attrs;
-    JSVariant   value;
+    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.
@@ -86,8 +103,8 @@ struct PPropertyDescriptor
     //  1 - Default getter or setter.
     //  2 - js_GetterOnlyPropertyStub (setter only)
     //  3 - Unknown
-    uint64_t    getter;
+    GetterSetter getter;
-    uint64_t    setter;
+    GetterSetter setter;
 };
 
 struct CpowEntry