Bug 1456604 - Add DifferentProcessForIndexedDB scope to handle backwards compatibility with SameProcessSameThread clones, r=jorendorff
authorSteve Fink <sfink@mozilla.com>
Tue, 24 Apr 2018 15:21:47 -0700
changeset 472502 7b6f6fcaaa177324d1827f9be431e23e9e0573ad
parent 472501 6529717260a226e15e02da023bfced594a763204
child 472503 e0fcc5bcaf2454e2e2a26a0388863ff746eb02b8
push id1728
push userjlund@mozilla.com
push dateMon, 18 Jun 2018 21:12:27 +0000
treeherdermozilla-release@c296fde26f5f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjorendorff
bugs1456604
milestone61.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 1456604 - Add DifferentProcessForIndexedDB scope to handle backwards compatibility with SameProcessSameThread clones, r=jorendorff
dom/indexedDB/IDBObjectStore.cpp
js/public/StructuredClone.h
js/src/builtin/TestingFunctions.cpp
js/src/tests/non262/extensions/clone-errors.js
js/src/tests/non262/extensions/clone-transferables.js
js/src/vm/StructuredClone.cpp
--- a/dom/indexedDB/IDBObjectStore.cpp
+++ b/dom/indexedDB/IDBObjectStore.cpp
@@ -68,17 +68,17 @@ using namespace mozilla::ipc;
 struct IDBObjectStore::StructuredCloneWriteInfo
 {
   JSAutoStructuredCloneBuffer mCloneBuffer;
   nsTArray<StructuredCloneFile> mFiles;
   IDBDatabase* mDatabase;
   uint64_t mOffsetToKeyProp;
 
   explicit StructuredCloneWriteInfo(IDBDatabase* aDatabase)
-    : mCloneBuffer(JS::StructuredCloneScope::DifferentProcess, nullptr,
+    : mCloneBuffer(JS::StructuredCloneScope::DifferentProcessForIndexedDB, nullptr,
                    nullptr)
     , mDatabase(aDatabase)
     , mOffsetToKeyProp(0)
   {
     MOZ_ASSERT(aDatabase);
 
     MOZ_COUNT_CTOR(StructuredCloneWriteInfo);
   }
@@ -1326,17 +1326,17 @@ IDBObjectStore::DeserializeValue(JSConte
     nullptr,
     nullptr,
     nullptr
   };
 
   // FIXME: Consider to use StructuredCloneHolder here and in other
   //        deserializing methods.
   if (!JS_ReadStructuredClone(aCx, aCloneReadInfo.mData, JS_STRUCTURED_CLONE_VERSION,
-                              JS::StructuredCloneScope::DifferentProcess,
+                              JS::StructuredCloneScope::DifferentProcessForIndexedDB,
                               aValue, &callbacks, &aCloneReadInfo)) {
     return false;
   }
 
   return true;
 }
 
 namespace {
@@ -1499,17 +1499,17 @@ private:
       nullptr,
       nullptr,
       nullptr,
       nullptr
     };
 
     if (!JS_ReadStructuredClone(aCx, mCloneReadInfo.mData,
                                 JS_STRUCTURED_CLONE_VERSION,
-                                JS::StructuredCloneScope::DifferentProcess,
+                                JS::StructuredCloneScope::DifferentProcessForIndexedDB,
                                 aValue, &callbacks, &mCloneReadInfo)) {
       return NS_ERROR_DOM_DATA_CLONE_ERR;
     }
 
     return NS_OK;
   }
 
   void
@@ -1613,17 +1613,17 @@ private:
       nullptr,
       nullptr,
       nullptr,
       nullptr
     };
 
     if (!JS_ReadStructuredClone(aCx, mCloneReadInfo.mData,
                                 JS_STRUCTURED_CLONE_VERSION,
-                                JS::StructuredCloneScope::DifferentProcess,
+                                JS::StructuredCloneScope::DifferentProcessForIndexedDB,
                                 aValue, &callbacks, &mCloneReadInfo)) {
       return NS_ERROR_DOM_DATA_CLONE_ERR;
     }
 
     return NS_OK;
   }
 
   void
--- a/js/public/StructuredClone.h
+++ b/js/public/StructuredClone.h
@@ -142,28 +142,38 @@ enum class StructuredCloneScope : uint32
      * serialized data will be treated as `Send` but not `Copy`.
      *
      * When reading, this means the same thing as SameProcessSameThread;
      * the distinction only matters when writing.
      */
     SameProcessDifferentThread,
 
     /**
-     * The broadest scope.
-     *
      * When writing, this means we're writing for an audience in a different
      * process. Produce serialized data that can be sent to other processes,
      * bitwise copied, or even stored as bytes in a database and read by later
-     * versions of Firefox years from now. Transferable objects are limited to
-     * ArrayBuffers, whose contents are copied into the serialized data (rather
-     * than just writing a pointer).
+     * versions of Firefox years from now. The HTML5 spec refers to this as
+     * "ForStorage" as in StructuredSerializeForStorage, though we use
+     * DifferentProcess for IPC as well as storage.
+     *
+     * Transferable objects are limited to ArrayBuffers, whose contents are
+     * copied into the serialized data (rather than just writing a pointer).
      *
      * When reading, this means: Do not accept pointers.
      */
-    DifferentProcess
+    DifferentProcess,
+
+    /**
+     * Handle a backwards-compatibility case with IndexedDB (bug 1434308): when
+     * reading, this means to treat legacy SameProcessSameThread data as if it
+     * were DifferentProcess.
+     *
+     * Do not use this for writing; use DifferentProcess instead.
+     */
+    DifferentProcessForIndexedDB
 };
 
 enum TransferableOwnership {
     /** Transferable data has not been filled in yet */
     SCTAG_TMO_UNFILLED = 0,
 
     /** Structured clone buffer does not yet own the data */
     SCTAG_TMO_UNOWNED = 1,
@@ -390,16 +400,19 @@ class MOZ_NON_MEMMOVABLE JS_PUBLIC_API(J
     // The constructor must be infallible but SystemAllocPolicy is not, so both
     // the initial size and initial capacity of the BufferList must be zero.
     explicit JSStructuredCloneData()
         : bufList_(0, 0, kStandardCapacity, js::SystemAllocPolicy())
         , callbacks_(nullptr)
         , closure_(nullptr)
         , ownTransferables_(OwnTransferablePolicy::NoTransferables)
     {}
+
+    // Steal the raw data from a BufferList. In this case, we don't know the
+    // scope and none of the callback info is assigned yet.
     MOZ_IMPLICIT JSStructuredCloneData(BufferList&& buffers)
         : bufList_(mozilla::Move(buffers))
         , callbacks_(nullptr)
         , closure_(nullptr)
         , ownTransferables_(OwnTransferablePolicy::NoTransferables)
     {}
     JSStructuredCloneData(JSStructuredCloneData&& other) = default;
     JSStructuredCloneData& operator=(JSStructuredCloneData&& other) = default;
--- a/js/src/builtin/TestingFunctions.cpp
+++ b/js/src/builtin/TestingFunctions.cpp
@@ -2986,16 +2986,18 @@ ParseCloneScope(JSContext* cx, HandleStr
         return scope;
 
     if (strcmp(scopeStr.ptr(), "SameProcessSameThread") == 0)
         scope.emplace(JS::StructuredCloneScope::SameProcessSameThread);
     else if (strcmp(scopeStr.ptr(), "SameProcessDifferentThread") == 0)
         scope.emplace(JS::StructuredCloneScope::SameProcessDifferentThread);
     else if (strcmp(scopeStr.ptr(), "DifferentProcess") == 0)
         scope.emplace(JS::StructuredCloneScope::DifferentProcess);
+    else if (strcmp(scopeStr.ptr(), "DifferentProcessForIndexedDB") == 0)
+        scope.emplace(JS::StructuredCloneScope::DifferentProcessForIndexedDB);
 
     return scope;
 }
 
 static bool
 Serialize(JSContext* cx, unsigned argc, Value* vp)
 {
     CallArgs args = CallArgsFromVp(argc, vp);
@@ -5586,30 +5588,32 @@ gc::ZealModeHelpText),
 "  (wasm) programs."),
 
     JS_FN_HELP("serialize", Serialize, 1, 0,
 "serialize(data, [transferables, [policy]])",
 "  Serialize 'data' using JS_WriteStructuredClone. Returns a structured\n"
 "  clone buffer object. 'policy' may be an options hash. Valid keys:\n"
 "    'SharedArrayBuffer' - either 'allow' (the default) or 'deny'\n"
 "      to specify whether SharedArrayBuffers may be serialized.\n"
-"    'scope' - SameProcessSameThread, SameProcessDifferentThread, or\n"
-"      DifferentProcess. Determines how some values will be serialized.\n"
-"      Clone buffers may only be deserialized with a compatible scope.\n"
-"      NOTE - For DifferentProcess, must also set SharedArrayBuffer:'deny'\n"
-"      if data contains any shared memory object."),
+"    'scope' - SameProcessSameThread, SameProcessDifferentThread,\n"
+"      DifferentProcess, or DifferentProcessForIndexedDB. Determines how some\n"
+"      values will be serialized. Clone buffers may only be deserialized with a\n"
+"      compatible scope. NOTE - For DifferentProcess/DifferentProcessForIndexedDB,\n"
+"      must also set SharedArrayBuffer:'deny' if data contains any shared memory\n"
+"      object."),
 
     JS_FN_HELP("deserialize", Deserialize, 1, 0,
 "deserialize(clonebuffer[, opts])",
 "  Deserialize data generated by serialize. 'opts' is an options hash with one\n"
 "  recognized key 'scope', which limits the clone buffers that are considered\n"
 "  valid. Allowed values: 'SameProcessSameThread', 'SameProcessDifferentThread',\n"
-"  and 'DifferentProcess'. So for example, a DifferentProcess clone buffer\n"
-"  may be deserialized in any scope, but a SameProcessSameThread clone buffer\n"
-"  cannot be deserialized in a DifferentProcess scope."),
+"  'DifferentProcess', and 'DifferentProcessForIndexedDB'. So for example, a\n"
+"  DifferentProcessForIndexedDB clone buffer may be deserialized in any scope, but\n"
+"  a SameProcessSameThread clone buffer cannot be deserialized in a\n"
+"  DifferentProcess scope."),
 
     JS_FN_HELP("detachArrayBuffer", DetachArrayBuffer, 1, 0,
 "detachArrayBuffer(buffer)",
 "  Detach the given ArrayBuffer object from its memory, i.e. as if it\n"
 "  had been transferred to a WebWorker."),
 
     JS_FN_HELP("helperThreadCount", HelperThreadCount, 0, 0,
 "helperThreadCount()",
--- a/js/src/tests/non262/extensions/clone-errors.js
+++ b/js/src/tests/non262/extensions/clone-errors.js
@@ -20,23 +20,23 @@ check(function () {});
 check(new Proxy({}, {}));
 
 // A failing getter.
 check({get x() { throw new Error("fail"); }});
 
 // Mismatched scopes.
 for (let [write_scope, read_scope] of [['SameProcessSameThread', 'SameProcessDifferentThread'],
                                        ['SameProcessSameThread', 'DifferentProcess'],
+                                       ['SameProcessDifferentThread', 'DifferentProcessForIndexedDB'],
                                        ['SameProcessDifferentThread', 'DifferentProcess']])
 {
   var ab = new ArrayBuffer(12);
   var buffer = serialize(ab, [ab], { scope: write_scope });
   var caught = false;
   try {
     deserialize(buffer, { scope: read_scope });
   } catch (exc) {
     caught = true;
   }
-  // The scope check is disabled by bug 1433642. It will be re-added in bug 1456604.
-  // assertEq(caught, true, `${write_scope} clone buffer should not be deserializable as ${read_scope}`);
+  assertEq(caught, true, `${write_scope} clone buffer should not be deserializable as ${read_scope}`);
 }
 
 reportCompare(0, 0, "ok");
--- a/js/src/tests/non262/extensions/clone-transferables.js
+++ b/js/src/tests/non262/extensions/clone-transferables.js
@@ -1,18 +1,22 @@
 // |reftest| skip-if(!xulRuntime.shell)
 // Any copyright is dedicated to the Public Domain.
 // http://creativecommons.org/licenses/publicdomain/
 
 function* buffer_options() {
-  for (var scope of ["SameProcessSameThread", "SameProcessDifferentThread", "DifferentProcess"]) {
-    for (var size of [0, 8, 16, 200, 1000, 4096, 8192, 65536]) {
-      yield { scope, size };
+    for (var scope of ["SameProcessSameThread",
+                       "SameProcessDifferentThread",
+                       "DifferentProcess",
+                       "DifferentProcessForIndexedDB"])
+    {
+        for (var size of [0, 8, 16, 200, 1000, 4096, 8192, 65536]) {
+            yield { scope, size };
+        }
     }
-  }
 }
 
 
 function test() {
     for (var {scope, size} of buffer_options()) {
         var old = new ArrayBuffer(size);
         var copy = deserialize(serialize([old, old], [old], { scope }), { scope });
         assertEq(old.byteLength, 0);
--- a/js/src/vm/StructuredClone.cpp
+++ b/js/src/vm/StructuredClone.cpp
@@ -1701,17 +1701,19 @@ JSStructuredCloneWriter::transferOwnersh
                 return false;
             }
 
             if (arrayBuffer->isDetached()) {
                 JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr, JSMSG_TYPED_ARRAY_DETACHED);
                 return false;
             }
 
-            if (scope == JS::StructuredCloneScope::DifferentProcess) {
+            if (scope == JS::StructuredCloneScope::DifferentProcess ||
+                scope == JS::StructuredCloneScope::DifferentProcessForIndexedDB)
+            {
                 // Write Transferred ArrayBuffers in DifferentProcess scope at
                 // the end of the clone buffer, and store the offset within the
                 // buffer to where the ArrayBuffer was written. Note that this
                 // will invalidate the current position iterator.
 
                 size_t pointOffset = out.offset(point);
                 tag = SCTAG_TRANSFER_MAP_STORED_ARRAY_BUFFER;
                 ownership = JS::SCTAG_TMO_UNOWNED;
@@ -2388,29 +2390,43 @@ JSStructuredCloneReader::readHeader()
         return in.reportTruncated();
 
     JS::StructuredCloneScope storedScope;
     if (tag == SCTAG_HEADER) {
         MOZ_ALWAYS_TRUE(in.readPair(&tag, &data));
         storedScope = JS::StructuredCloneScope(data);
     } else {
         // Old structured clone buffer. We must have read it from disk.
-        storedScope = JS::StructuredCloneScope::DifferentProcess;
+        storedScope = JS::StructuredCloneScope::DifferentProcessForIndexedDB;
     }
 
-    if (storedScope != JS::StructuredCloneScope::SameProcessSameThread &&
-        storedScope != JS::StructuredCloneScope::SameProcessDifferentThread &&
-        storedScope != JS::StructuredCloneScope::DifferentProcess)
+    if (storedScope < JS::StructuredCloneScope::SameProcessSameThread ||
+        storedScope > JS::StructuredCloneScope::DifferentProcessForIndexedDB)
     {
         JS_ReportErrorNumberASCII(context(), GetErrorMessage, nullptr, JSMSG_SC_BAD_SERIALIZED_DATA,
                                   "invalid structured clone scope");
         return false;
     }
 
-    // Do not check storedScope due to bug 1434308, until bug 1456604 is fixed.
+    if (storedScope == JS::StructuredCloneScope::SameProcessSameThread &&
+        allowedScope == JS::StructuredCloneScope::DifferentProcessForIndexedDB)
+    {
+        // Bug 1434308 - allow stored IndexedDB clones to contain what is
+        // essentially DifferentProcess data, but labeled as
+        // SameProcessSameThread (because that's what old code wrote.)
+        allowedScope = JS::StructuredCloneScope::DifferentProcess;
+        return true;
+    }
+
+    if (storedScope < allowedScope) {
+        JS_ReportErrorNumberASCII(context(), GetErrorMessage, nullptr,
+                                  JSMSG_SC_BAD_SERIALIZED_DATA,
+                                  "incompatible structured clone scope");
+        return false;
+    }
 
     return true;
 }
 
 bool
 JSStructuredCloneReader::readTransferMap()
 {
     JSContext* cx = context();
@@ -2441,17 +2457,19 @@ JSStructuredCloneReader::readTransferMap
         if (!in.readPtr(&content))
             return false;
 
         uint64_t extraData;
         if (!in.read(&extraData))
             return false;
 
         if (tag == SCTAG_TRANSFER_MAP_ARRAY_BUFFER) {
-            if (allowedScope == JS::StructuredCloneScope::DifferentProcess) {
+            if (allowedScope == JS::StructuredCloneScope::DifferentProcess ||
+                allowedScope == JS::StructuredCloneScope::DifferentProcessForIndexedDB)
+            {
                 // Transferred ArrayBuffers in a DifferentProcess clone buffer
                 // are treated as if they weren't Transferred at all. We should
                 // only see SCTAG_TRANSFER_MAP_STORED_ARRAY_BUFFER.
                 ReportDataCloneError(cx, callbacks, JS_SCERR_TRANSFERABLE);
                 return false;
             }
 
             size_t nbytes = extraData;