Bug 1706265 - retain ordering of transferable objects r=jonco,smaug

Differential Revision: https://phabricator.services.mozilla.com/D129568
iaxat · Oct 27, 2021 · 06cb75e · 06cb75e
1 parent 8262a54
commit 06cb75e
Show file tree

Hide file tree

Showing 2 changed files with 60 additions and 19 deletions.
diff --git a/dom/base/test/common_postMessages.js b/dom/base/test/common_postMessages.js
@@ -304,6 +304,54 @@ function runTests(obj) {
       });
     })
 
+    // maintaining order of transferred ports
+    .then(function() {
+      if (!obj.transferableObjects) {
+        return;
+      }
+
+      // MessagePort
+      return new Promise(function(r, rr) {
+        var mcs = [];
+        const NPORTS = 50;
+        for (let i = 0; i < NPORTS; i++) {
+          mcs.push(new MessageChannel());
+        }
+        obj
+          .send(
+            42,
+            mcs.map(channel => channel.port1)
+          )
+          .then(function(received) {
+            is(
+              received.ports.length,
+              NPORTS,
+              `all ${NPORTS} ports transferred`
+            );
+            const promises = Array(NPORTS)
+              .fill()
+              .map(
+                (_, i) =>
+                  new Promise(function(subr, subrr) {
+                    mcs[i].port2.postMessage(i);
+                    received.ports[i].onmessage = e => subr(e.data == i);
+                  })
+              );
+            return Promise.all(promises);
+          })
+          .then(function(result) {
+            let in_order = 0;
+            for (const correct of result) {
+              if (correct) {
+                in_order++;
+              }
+            }
+            is(in_order, NPORTS, "All transferred ports are in order");
+          })
+          .then(r);
+      });
+    })
+
     // non transfering tests
     .then(function() {
       if (obj.transferableObjects) {

diff --git a/js/src/vm/StructuredClone.cpp b/js/src/vm/StructuredClone.cpp
@@ -497,7 +497,7 @@ struct JSStructuredCloneWriter {
         otherEntries(out.context()),
         memory(out.context()),
         transferable(out.context(), tVal),
-        transferableObjects(out.context(), TransferableObjectsSet(cx)),
+        transferableObjects(out.context(), TransferableObjectsList(cx)),
         cloneDataPolicy(cloneDataPolicy) {
     out.setCallbacks(cb, cbClosure, OwnTransferablePolicy::NoTransferables);
   }
@@ -582,17 +582,10 @@ struct JSStructuredCloneWriter {
                 SystemAllocPolicy>;
   Rooted<CloneMemory> memory;
 
-  struct TransferableObjectsHasher : public DefaultHasher<JSObject*> {
-    static inline HashNumber hash(const Lookup& l) {
-      return DefaultHasher<JSObject*>::hash(l);
-    }
-  };
-
   // Set of transferable objects
   RootedValue transferable;
-  typedef GCHashSet<JSObject*, TransferableObjectsHasher>
-      TransferableObjectsSet;
-  Rooted<TransferableObjectsSet> transferableObjects;
+  using TransferableObjectsList = GCVector<JSObject*>;
+  Rooted<TransferableObjectsList> transferableObjects;
 
   const JS::CloneDataPolicy cloneDataPolicy;
 
@@ -1171,12 +1164,12 @@ bool JSStructuredCloneWriter::parseTransferable() {
     }
 
     // No duplicates allowed
-    auto p = transferableObjects.lookupForAdd(tObj);
-    if (p) {
+    if (std::find(transferableObjects.begin(), transferableObjects.end(),
+                  tObj) != transferableObjects.end()) {
       return reportDataCloneError(JS_SCERR_DUP_TRANSFERABLE);
     }
 
-    if (!transferableObjects.add(p, tObj)) {
+    if (!transferableObjects.append(tObj)) {
       return false;
     }
   }
@@ -1898,13 +1891,13 @@ bool JSStructuredCloneWriter::writeTransferMap() {
     return false;
   }
 
-  if (!out.write(transferableObjects.count())) {
+  if (!out.write(transferableObjects.length())) {
     return false;
   }
 
   RootedObject obj(context());
-  for (auto tr = transferableObjects.all(); !tr.empty(); tr.popFront()) {
-    obj = tr.front();
+  for (auto o : transferableObjects) {
+    obj = o;
     if (!memory.put(obj, memory.count())) {
       ReportOutOfMemory(context());
       return false;
@@ -1946,14 +1939,14 @@ bool JSStructuredCloneWriter::transferOwnership() {
   point++;
   MOZ_RELEASE_ASSERT(point.canPeek());
   MOZ_ASSERT(NativeEndian::swapFromLittleEndian(point.peek()) ==
-             transferableObjects.count());
+             transferableObjects.length());
   point++;
 
   JSContext* cx = context();
   RootedObject obj(cx);
   JS::StructuredCloneScope scope = output().scope();
-  for (auto tr = transferableObjects.all(); !tr.empty(); tr.popFront()) {
-    obj = tr.front();
+  for (auto o : transferableObjects) {
+    obj = o;
 
     uint32_t tag;
     JS::TransferableOwnership ownership;