Bug 1071177 - Support symbol keys and throw exception on unique symbols with CPOWs. r=billm
authorTom Schuster <evilpies@gmail.com>
Thu, 16 Oct 2014 18:39:38 +0200
changeset 210796 f5b05c63480d804fb91dd9a8a7ee54421695282f
parent 210795 8c7d2cd4fa067369938f25c582491cf546c6d198
child 210797 553c113c8f4e81045287d0f4cdb0df97afa6a4af
push id1
push userroot
push dateMon, 20 Oct 2014 17:29:22 +0000
reviewersbillm
bugs1071177
milestone36.0a1
Bug 1071177 - Support symbol keys and throw exception on unique symbols with CPOWs. r=billm
content/base/test/chrome/cpows_child.js
content/base/test/chrome/cpows_parent.xul
js/ipc/JavaScriptBase.h
js/ipc/JavaScriptShared.cpp
js/ipc/PJavaScript.ipdl
js/ipc/WrapperAnswer.cpp
js/ipc/WrapperAnswer.h
js/ipc/WrapperOwner.cpp
js/ipc/WrapperOwner.h
--- a/content/base/test/chrome/cpows_child.js
+++ b/content/base/test/chrome/cpows_child.js
@@ -7,16 +7,19 @@ var done_count = 0;
 var is_remote;
 
 (function start() {
     [is_remote] = sendSyncMessage("cpows:is_remote");
     parent_test();
     error_reporting_test();
     dom_test();
     xray_test();
+    if (typeof Symbol === "function") {
+      symbol_test();
+    }
     compartment_test();
     regexp_test();
     sync_test();
     async_test();
     rpc_test();
     nested_sync_test();
     // The sync-ness of this call is important, because otherwise
     // we tear down the child's document while we are
@@ -114,25 +117,23 @@ function xray_test()
 
   sendSyncMessage("cpows:xray_test", {}, {element: element});
 }
 
 function symbol_test()
 {
   let iterator = Symbol.iterator;
   let named = Symbol.for("cpow-test");
-  // let unique = Symbol();
 
   let object = {
     [iterator]: iterator,
     [named]: named,
-    // [unique]: unique,
-    // "unique": unique
   };
-  sendSyncMessage("cpows:symbol_test", {}, object);
+  let test = ['a'];
+  sendSyncMessage("cpows:symbol_test", {}, {object: object, test: test});
 }
 
 // Parent->Child references should go X->parent.privilegedJunkScope->child.privilegedJunkScope->Y
 // Child->Parent references should go X->child.privilegedJunkScope->parent.unprivilegedJunkScope->Y
 function compartment_test()
 {
   // This test primarily checks various compartment invariants for CPOWs, and
   // doesn't make sense to run in-process.
--- a/content/base/test/chrome/cpows_parent.xul
+++ b/content/base/test/chrome/cpows_parent.xul
@@ -201,20 +201,26 @@
     }
 
     function recvXrayTest(message) {
       let element = message.objects.element;
       is(element.foo, undefined, "DOM element does not expose content properties");
     }
 
     function recvSymbolTest(message) {
-      let object = message.objects;
+      let object = message.objects.object;
       is(object[Symbol.iterator], Symbol.iterator, "Should use Symbol.iterator");
       is(Symbol.keyFor(object[Symbol.for("cpow-test")]), "cpow-test", "Symbols aren't registered correctly");
-      // is(object.unique, object[object.unique], "Unique symbols as ids and values don't seem to work");
+      let symbols = Object.getOwnPropertySymbols(object);
+      is(symbols.length, 2, "Object should have two symbol keys");
+      let test = undefined;
+      for (let x of message.objects.test) {
+        test = x;
+      }
+      is(test, "a", "for .. of iteration should work");
     }
 
     let systemGlobal = this;
     function recvCompartmentTest(message) {
       let getUnprivilegedObject = message.objects.getUnprivilegedObject;
       let testParentObject = message.objects.testParentObject;
 
       // Make sure that parent->child CPOWs live in the parent's privileged junk scope.
@@ -302,17 +308,19 @@
       mm.addMessageListener("cpows:reenter_sync", recvReenterSyncMessage);
       mm.addMessageListener("cpows:done", recvDoneMessage);
       mm.addMessageListener("cpows:fail", recvFailMessage);
       mm.addMessageListener("cpows:parent_test", recvParentTest);
       mm.addMessageListener("cpows:error_reporting_test", recvErrorReportingTest);
       mm.addMessageListener("cpows:dom_test", recvDomTest);
       mm.addMessageListener("cpows:dom_test_after_gc", recvDomTestAfterGC);
       mm.addMessageListener("cpows:xray_test", recvXrayTest);
-      mm.addMessageListener("cpows:symbol_test", recvSymbolTest);
+      if (typeof Symbol === "function") {
+        mm.addMessageListener("cpows:symbol_test", recvSymbolTest);
+      }
       mm.addMessageListener("cpows:compartment_test", recvCompartmentTest);
       mm.addMessageListener("cpows:regexp_test", recvRegExpTest);
       mm.addMessageListener("cpows:lifetime_test_1", recvLifetimeTest1);
       mm.addMessageListener("cpows:lifetime_test_2", recvLifetimeTest2);
       mm.loadFrameScript("chrome://mochitests/content/chrome/content/base/test/chrome/cpows_child.js", true);
     }
 
     function start() {
--- a/js/ipc/JavaScriptBase.h
+++ b/js/ipc/JavaScriptBase.h
@@ -97,18 +97,18 @@ class JavaScriptBase : public WrapperOwn
     bool RecvClassName(const uint64_t &objId, nsString *result) {
         return Answer::RecvClassName(ObjectId::deserialize(objId), result);
     }
     bool RecvRegExpToShared(const uint64_t &objId, ReturnStatus *rs, nsString *source, uint32_t *flags) {
         return Answer::RecvRegExpToShared(ObjectId::deserialize(objId), rs, source, flags);
     }
 
     bool RecvGetPropertyKeys(const uint64_t &objId, const uint32_t &flags,
-                             ReturnStatus *rs, nsTArray<nsString> *names) {
-        return Answer::RecvGetPropertyKeys(ObjectId::deserialize(objId), flags, rs, names);
+                             ReturnStatus *rs, nsTArray<JSIDVariant> *ids) {
+        return Answer::RecvGetPropertyKeys(ObjectId::deserialize(objId), flags, rs, ids);
     }
     bool RecvInstanceOf(const uint64_t &objId, const JSIID &iid,
                           ReturnStatus *rs, bool *instanceof) {
         return Answer::RecvInstanceOf(ObjectId::deserialize(objId), iid, rs, instanceof);
     }
     bool RecvDOMInstanceOf(const uint64_t &objId, const int &prototypeID, const int &depth,
                              ReturnStatus *rs, bool *instanceof) {
         return Answer::RecvDOMInstanceOf(ObjectId::deserialize(objId), prototypeID, depth, rs, instanceof);
@@ -195,18 +195,18 @@ class JavaScriptBase : public WrapperOwn
     }
 
     bool SendRegExpToShared(const ObjectId &objId, ReturnStatus *rs,
                             nsString *source, uint32_t *flags) {
         return Base::SendRegExpToShared(objId.serialize(), rs, source, flags);
     }
 
     bool SendGetPropertyKeys(const ObjectId &objId, const uint32_t &flags,
-                             ReturnStatus *rs, nsTArray<nsString> *names) {
-        return Base::SendGetPropertyKeys(objId.serialize(), flags, rs, names);
+                             ReturnStatus *rs, nsTArray<JSIDVariant> *ids) {
+        return Base::SendGetPropertyKeys(objId.serialize(), flags, rs, ids);
     }
     bool SendInstanceOf(const ObjectId &objId, const JSIID &iid,
                         ReturnStatus *rs, bool *instanceof) {
         return Base::SendInstanceOf(objId.serialize(), iid, rs, instanceof);
     }
     bool SendDOMInstanceOf(const ObjectId &objId, const int &prototypeID, const int &depth,
                            ReturnStatus *rs, bool *instanceof) {
         return Base::SendDOMInstanceOf(objId.serialize(), prototypeID, depth, rs, instanceof);
--- a/js/ipc/JavaScriptShared.cpp
+++ b/js/ipc/JavaScriptShared.cpp
@@ -435,17 +435,18 @@ JavaScriptShared::toSymbolVariant(JSCont
     }
     if (code == SymbolCode::InSymbolRegistry) {
         nsAutoJSString autoStr;
         if (!autoStr.init(cx, GetSymbolDescription(sym)))
             return false;
         *symVarp = RegisteredSymbol(autoStr);
         return true;
     }
-    MOZ_CRASH("unique symbols not yet implemented");
+
+    JS_ReportError(cx, "unique symbol can't be used with CPOW");
     return false;
 }
 
 JS::Symbol *
 JavaScriptShared::fromSymbolVariant(JSContext *cx, SymbolVariant symVar)
 {
     switch (symVar.type()) {
       case SymbolVariant::TWellKnownSymbol: {
--- a/js/ipc/PJavaScript.ipdl
+++ b/js/ipc/PJavaScript.ipdl
@@ -37,17 +37,17 @@ both:
 
     prio(high) sync IsExtensible(uint64_t objId) returns (ReturnStatus rs, bool result);
     prio(high) sync CallOrConstruct(uint64_t objId, JSParam[] argv, bool construct) returns (ReturnStatus rs, JSVariant result, JSParam[] outparams);
     prio(high) sync HasInstance(uint64_t objId, JSVariant v) returns (ReturnStatus rs, bool has);
     prio(high) sync ObjectClassIs(uint64_t objId, uint32_t classValue) returns (bool result);
     prio(high) sync ClassName(uint64_t objId) returns (nsString name);
     prio(high) sync RegExpToShared(uint64_t objId) returns (ReturnStatus rs, nsString source, uint32_t flags);
 
-    prio(high) sync GetPropertyKeys(uint64_t objId, uint32_t flags) returns (ReturnStatus rs, nsString[] names);
+    prio(high) sync GetPropertyKeys(uint64_t objId, uint32_t flags) returns (ReturnStatus rs, JSIDVariant[] ids);
     prio(high) sync InstanceOf(uint64_t objId, JSIID iid) returns (ReturnStatus rs, bool instanceof);
     prio(high) sync DOMInstanceOf(uint64_t objId, int prototypeID, int depth) returns (ReturnStatus rs, bool instanceof);
 
     prio(high) sync IsCallable(uint64_t objId) returns (bool result);
     prio(high) sync IsConstructor(uint64_t objId) returns (bool result);
 
 parent:
     async __delete__();
--- a/js/ipc/WrapperAnswer.cpp
+++ b/js/ipc/WrapperAnswer.cpp
@@ -575,17 +575,17 @@ WrapperAnswer::RecvRegExpToShared(const 
 
     *flags = JS_GetRegExpFlags(cx, obj);
 
     return ok(rs);
 }
 
 bool
 WrapperAnswer::RecvGetPropertyKeys(const ObjectId &objId, const uint32_t &flags,
-                                   ReturnStatus *rs, nsTArray<nsString> *names)
+                                   ReturnStatus *rs, nsTArray<JSIDVariant> *ids)
 {
     AutoSafeJSContext cx;
     JSAutoRequest request(cx);
 
     RootedObject obj(cx, findObjectById(cx, objId));
     if (!obj)
         return fail(cx, rs);
 
@@ -593,21 +593,21 @@ WrapperAnswer::RecvGetPropertyKeys(const
 
     LOG("%s.getPropertyKeys()", ReceiverObj(objId));
 
     AutoIdVector props(cx);
     if (!js::GetPropertyKeys(cx, obj, flags, &props))
         return fail(cx, rs);
 
     for (size_t i = 0; i < props.length(); i++) {
-        nsString name;
-        if (!convertIdToGeckoString(cx, props[i], &name))
+        JSIDVariant id;
+        if (!toJSIDVariant(cx, props[i], &id))
             return fail(cx, rs);
 
-        names->AppendElement(name);
+        ids->AppendElement(id);
     }
 
     return ok(rs);
 }
 
 bool
 WrapperAnswer::RecvInstanceOf(const ObjectId &objId, const JSIID &iid, ReturnStatus *rs,
                               bool *instanceof)
--- a/js/ipc/WrapperAnswer.h
+++ b/js/ipc/WrapperAnswer.h
@@ -50,17 +50,17 @@ class WrapperAnswer : public virtual Jav
                              nsTArray<JSParam> *outparams);
     bool RecvHasInstance(const ObjectId &objId, const JSVariant &v, ReturnStatus *rs, bool *bp);
     bool RecvObjectClassIs(const ObjectId &objId, const uint32_t &classValue,
                            bool *result);
     bool RecvClassName(const ObjectId &objId, nsString *result);
     bool RecvRegExpToShared(const ObjectId &objId, ReturnStatus *rs, nsString *source, uint32_t *flags);
 
     bool RecvGetPropertyKeys(const ObjectId &objId, const uint32_t &flags,
-                             ReturnStatus *rs, nsTArray<nsString> *names);
+                             ReturnStatus *rs, nsTArray<JSIDVariant> *ids);
     bool RecvInstanceOf(const ObjectId &objId, const JSIID &iid,
                         ReturnStatus *rs, bool *instanceof);
     bool RecvDOMInstanceOf(const ObjectId &objId, const int &prototypeID, const int &depth,
                            ReturnStatus *rs, bool *instanceof);
 
     bool RecvIsCallable(const ObjectId &objId, bool *result);
     bool RecvIsConstructor(const ObjectId &objId, bool *result);
 
--- a/js/ipc/WrapperOwner.cpp
+++ b/js/ipc/WrapperOwner.cpp
@@ -227,17 +227,17 @@ CPOWProxyHandler::ownPropertyKeys(JSCont
                                   AutoIdVector &props) const
 {
     FORWARD(ownPropertyKeys, (cx, proxy, props));
 }
 
 bool
 WrapperOwner::ownPropertyKeys(JSContext *cx, HandleObject proxy, AutoIdVector &props)
 {
-    return getPropertyKeys(cx, proxy, JSITER_OWNONLY | JSITER_HIDDEN, props);
+    return getPropertyKeys(cx, proxy, JSITER_OWNONLY | JSITER_HIDDEN | JSITER_SYMBOLS, props);
 }
 
 bool
 CPOWProxyHandler::delete_(JSContext *cx, HandleObject proxy, HandleId id, bool *bp) const
 {
     FORWARD(delete_, (cx, proxy, id, bp));
 }
 
@@ -771,30 +771,30 @@ WrapperOwner::init()
 }
 
 bool
 WrapperOwner::getPropertyKeys(JSContext *cx, HandleObject proxy, uint32_t flags, AutoIdVector &props)
 {
     ObjectId objId = idOf(proxy);
 
     ReturnStatus status;
-    InfallibleTArray<nsString> names;
-    if (!SendGetPropertyKeys(objId, flags, &status, &names))
+    InfallibleTArray<JSIDVariant> ids;
+    if (!SendGetPropertyKeys(objId, flags, &status, &ids))
         return ipcfail(cx);
 
     LOG_STACK();
 
     if (!ok(cx, status))
         return false;
 
-    for (size_t i = 0; i < names.Length(); i++) {
-        RootedId name(cx);
-        if (!convertGeckoStringToId(cx, names[i], &name))
+    for (size_t i = 0; i < ids.Length(); i++) {
+        RootedId id(cx);
+        if (!fromJSIDVariant(cx, ids[i], &id))
             return false;
-        if (!props.append(name))
+        if (!props.append(id))
             return false;
     }
 
     return true;
 }
 
 namespace mozilla {
 namespace jsipc {
--- a/js/ipc/WrapperOwner.h
+++ b/js/ipc/WrapperOwner.h
@@ -141,17 +141,17 @@ class WrapperOwner : public virtual Java
                                  ReturnStatus *rs, bool *bp) = 0;
     virtual bool SendObjectClassIs(const ObjectId &objId, const uint32_t &classValue,
                                    bool *result) = 0;
     virtual bool SendClassName(const ObjectId &objId, nsString *result) = 0;
     virtual bool SendRegExpToShared(const ObjectId &objId, ReturnStatus *rs, nsString *source,
                                     uint32_t *flags) = 0;
 
     virtual bool SendGetPropertyKeys(const ObjectId &objId, const uint32_t &flags,
-                                     ReturnStatus *rs, nsTArray<nsString> *names) = 0;
+                                     ReturnStatus *rs, nsTArray<JSIDVariant> *ids) = 0;
     virtual bool SendInstanceOf(const ObjectId &objId, const JSIID &iid,
                                 ReturnStatus *rs, bool *instanceof) = 0;
     virtual bool SendDOMInstanceOf(const ObjectId &objId, const int &prototypeID, const int &depth,
                                    ReturnStatus *rs, bool *instanceof) = 0;
 
     virtual bool SendIsCallable(const ObjectId &objId, bool *result) = 0;
     virtual bool SendIsConstructor(const ObjectId &objId, bool *result) = 0;
 };