Bug 1476032: Neuter StructuredCloneHolder objects after deserializing. r=aswan
authorKris Maglione <maglione.k@gmail.com>
Tue, 22 Jan 2019 13:01:22 -0800
changeset 515377 005a8fce14e71875a1e784d7adf3ec73fe5e694b
parent 515376 a3b8503e6109c7e6e81cee27af25b9ec1b01d74d
child 515378 2d8247673f5d837ef5a5203c62c6f008389317f3
push id1953
push userffxbld-merge
push dateMon, 11 Mar 2019 12:10:20 +0000
treeherdermozilla-release@9c35dcbaa899 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersaswan
bugs1476032
milestone66.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 1476032: Neuter StructuredCloneHolder objects after deserializing. r=aswan Differential Revision: https://phabricator.services.mozilla.com/D17287
dom/base/StructuredCloneBlob.cpp
dom/base/StructuredCloneBlob.h
dom/base/test/unit/test_structuredcloneholder.js
dom/chrome-webidl/StructuredCloneHolder.webidl
toolkit/components/extensions/ExtensionChild.jsm
toolkit/components/extensions/ExtensionCommon.jsm
toolkit/components/extensions/ExtensionStorage.jsm
toolkit/components/extensions/ExtensionStorageIDB.jsm
toolkit/components/extensions/MessageChannel.jsm
toolkit/components/extensions/Schemas.jsm
toolkit/components/extensions/parent/ext-storage.js
--- a/dom/base/StructuredCloneBlob.cpp
+++ b/dom/base/StructuredCloneBlob.cpp
@@ -13,19 +13,21 @@
 #include "mozilla/dom/StructuredCloneTags.h"
 #include "mozilla/Maybe.h"
 #include "mozilla/UniquePtr.h"
 #include "xpcpublic.h"
 
 namespace mozilla {
 namespace dom {
 
-StructuredCloneBlob::StructuredCloneBlob()
-    : StructuredCloneHolder(CloningSupported, TransferringNotSupported,
-                            StructuredCloneScope::DifferentProcess) {}
+StructuredCloneBlob::StructuredCloneBlob() {
+  mHolder.emplace(Holder::CloningSupported,
+                  Holder::TransferringNotSupported,
+                  Holder::StructuredCloneScope::DifferentProcess);
+}
 
 StructuredCloneBlob::~StructuredCloneBlob() {
   UnregisterWeakMemoryReporter(this);
 }
 
 /* static */ already_AddRefed<StructuredCloneBlob>
 StructuredCloneBlob::Constructor(GlobalObject& aGlobal, JS::HandleValue aValue,
                                  JS::HandleObject aTargetGlobal,
@@ -58,66 +60,76 @@ StructuredCloneBlob::Constructor(GlobalO
       aRv.NoteJSContextException(cx);
       return nullptr;
     }
 
     ar.emplace(cx, obj);
     value = JS::ObjectValue(*obj);
   }
 
-  holder->Write(cx, value, aRv);
+  holder->mHolder->Write(cx, value, aRv);
   if (aRv.Failed()) {
     return nullptr;
   }
 
   return holder.forget();
 }
 
 void StructuredCloneBlob::Deserialize(JSContext* aCx,
                                       JS::HandleObject aTargetScope,
+                                      bool aKeepData,
                                       JS::MutableHandleValue aResult,
                                       ErrorResult& aRv) {
   JS::RootedObject scope(aCx, js::CheckedUnwrap(aTargetScope));
   if (!scope) {
     js::ReportAccessDenied(aCx);
     aRv.NoteJSContextException(aCx);
     return;
   }
 
+  if (!mHolder.isSome()) {
+    aRv.Throw(NS_ERROR_NOT_INITIALIZED);
+    return;
+  }
+
   {
     JSAutoRealm ar(aCx, scope);
 
-    Read(xpc::NativeGlobal(scope), aCx, aResult, aRv);
+    mHolder->Read(xpc::NativeGlobal(scope), aCx, aResult, aRv);
     if (aRv.Failed()) {
       return;
     }
   }
 
+  if (!aKeepData) {
+    mHolder.reset();
+  }
+
   if (!JS_WrapValue(aCx, aResult)) {
     aResult.set(JS::UndefinedValue());
     aRv.NoteJSContextException(aCx);
   }
 }
 
 /* static */ JSObject* StructuredCloneBlob::ReadStructuredClone(
     JSContext* aCx, JSStructuredCloneReader* aReader,
     StructuredCloneHolder* aHolder) {
   JS::RootedObject obj(aCx);
   {
     RefPtr<StructuredCloneBlob> holder = StructuredCloneBlob::Create();
 
-    if (!holder->ReadStructuredCloneInternal(aCx, aReader, aHolder) ||
+    if (!holder->mHolder->ReadStructuredCloneInternal(aCx, aReader, aHolder) ||
         !holder->WrapObject(aCx, nullptr, &obj)) {
       return nullptr;
     }
   }
   return obj.get();
 }
 
-bool StructuredCloneBlob::ReadStructuredCloneInternal(
+bool StructuredCloneBlob::Holder::ReadStructuredCloneInternal(
     JSContext* aCx, JSStructuredCloneReader* aReader,
     StructuredCloneHolder* aHolder) {
   uint32_t length;
   uint32_t version;
   if (!JS_ReadUint32Pair(aReader, &length, &version)) {
     return false;
   }
 
@@ -150,16 +162,25 @@ bool StructuredCloneBlob::ReadStructured
   mBuffer->adopt(std::move(data), version, &StructuredCloneHolder::sCallbacks);
 
   return true;
 }
 
 bool StructuredCloneBlob::WriteStructuredClone(JSContext* aCx,
                                                JSStructuredCloneWriter* aWriter,
                                                StructuredCloneHolder* aHolder) {
+  if (mHolder.isNothing()) {
+    return false;
+  }
+  return mHolder->WriteStructuredClone(aCx, aWriter, aHolder);
+}
+
+bool StructuredCloneBlob::Holder::WriteStructuredClone(JSContext* aCx,
+                                                       JSStructuredCloneWriter* aWriter,
+                                                       StructuredCloneHolder* aHolder) {
   auto& data = mBuffer->data();
   if (!JS_WriteUint32Pair(aWriter, SCTAG_DOM_STRUCTURED_CLONE_HOLDER, 0) ||
       !JS_WriteUint32Pair(aWriter, data.Size(), JS_STRUCTURED_CLONE_VERSION) ||
       !JS_WriteUint32Pair(aWriter, aHolder->BlobImpls().Length(),
                           BlobImpls().Length())) {
     return false;
   }
 
@@ -174,19 +195,24 @@ bool StructuredCloneBlob::WrapObject(JSC
                                      JS::HandleObject aGivenProto,
                                      JS::MutableHandleObject aResult) {
   return StructuredCloneHolder_Binding::Wrap(aCx, this, aGivenProto, aResult);
 }
 
 NS_IMETHODIMP
 StructuredCloneBlob::CollectReports(nsIHandleReportCallback* aHandleReport,
                                     nsISupports* aData, bool aAnonymize) {
+  size_t size = MallocSizeOf(this);
+  if (mHolder.isSome()) {
+    size += mHolder->SizeOfExcludingThis(MallocSizeOf);
+  }
+
   MOZ_COLLECT_REPORT("explicit/dom/structured-clone-holder", KIND_HEAP,
                      UNITS_BYTES,
-                     MallocSizeOf(this) + SizeOfExcludingThis(MallocSizeOf),
+                     size,
                      "Memory used by StructuredCloneHolder DOM objects.");
 
   return NS_OK;
 }
 
 NS_IMPL_ISUPPORTS(StructuredCloneBlob, nsIMemoryReporter)
 
 }  // namespace dom
--- a/dom/base/StructuredCloneBlob.h
+++ b/dom/base/StructuredCloneBlob.h
@@ -8,24 +8,24 @@
 #define mozilla_dom_StructuredCloneBlob_h
 
 #include "mozilla/dom/BindingDeclarations.h"
 #include "mozilla/dom/StructuredCloneHolder.h"
 #include "mozilla/dom/StructuredCloneHolderBinding.h"
 
 #include "jsapi.h"
 
+#include "mozilla/Maybe.h"
 #include "nsIMemoryReporter.h"
 #include "nsISupports.h"
 
 namespace mozilla {
 namespace dom {
 
-class StructuredCloneBlob final : public nsIMemoryReporter,
-                                  public StructuredCloneHolder {
+class StructuredCloneBlob final : public nsIMemoryReporter {
   MOZ_DEFINE_MALLOC_SIZE_OF(MallocSizeOf)
 
  public:
   NS_DECL_ISUPPORTS
   NS_DECL_NSIMEMORYREPORTER
 
   static JSObject* ReadStructuredClone(JSContext* aCx,
                                        JSStructuredCloneReader* aReader,
@@ -33,37 +33,48 @@ class StructuredCloneBlob final : public
   bool WriteStructuredClone(JSContext* aCx, JSStructuredCloneWriter* aWriter,
                             StructuredCloneHolder* aHolder);
 
   static already_AddRefed<StructuredCloneBlob> Constructor(
       GlobalObject& aGlobal, JS::HandleValue aValue,
       JS::HandleObject aTargetGlobal, ErrorResult& aRv);
 
   void Deserialize(JSContext* aCx, JS::HandleObject aTargetScope,
-                   JS::MutableHandleValue aResult, ErrorResult& aRv);
+                   bool aKeepData, JS::MutableHandleValue aResult,
+                   ErrorResult& aRv);
 
   nsISupports* GetParentObject() const { return nullptr; }
   JSObject* GetWrapper() const { return nullptr; }
 
   bool WrapObject(JSContext* aCx, JS::Handle<JSObject*> aGivenProto,
                   JS::MutableHandleObject aResult);
 
  protected:
   virtual ~StructuredCloneBlob();
 
  private:
   explicit StructuredCloneBlob();
 
+  class Holder : public StructuredCloneHolder {
+  public:
+    using StructuredCloneHolder::StructuredCloneHolder;
+
+    bool ReadStructuredCloneInternal(JSContext* aCx,
+                                     JSStructuredCloneReader* aReader,
+                                     StructuredCloneHolder* aHolder);
+
+    bool WriteStructuredClone(JSContext* aCx, JSStructuredCloneWriter* aWriter,
+                              StructuredCloneHolder* aHolder);
+  };
+
+  Maybe<Holder> mHolder;
+
   static already_AddRefed<StructuredCloneBlob> Create() {
     RefPtr<StructuredCloneBlob> holder = new StructuredCloneBlob();
     RegisterWeakMemoryReporter(holder);
     return holder.forget();
   }
-
-  bool ReadStructuredCloneInternal(JSContext* aCx,
-                                   JSStructuredCloneReader* aReader,
-                                   StructuredCloneHolder* aHolder);
 };
 
 }  // namespace dom
 }  // namespace mozilla
 
 #endif  // mozilla_dom_StructuredCloneBlob_h
--- a/dom/base/test/unit/test_structuredcloneholder.js
+++ b/dom/base/test/unit/test_structuredcloneholder.js
@@ -12,17 +12,17 @@ add_task(async function test_structuredC
 
   const obj = {foo: [{bar: "baz"}]};
 
   let holder = new StructuredCloneHolder(obj);
 
 
   // Test same-compartment deserialization
 
-  let res = holder.deserialize(global);
+  let res = holder.deserialize(global, true);
 
   notEqual(res, obj, "Deserialized result is a different object from the original");
 
   deepEqual(res, obj, "Deserialized result is deeply equivalent to the original");
 
   equal(Cu.getObjectPrincipal(res), Cu.getObjectPrincipal(global),
         "Deserialized result has the correct principal");
 
@@ -30,17 +30,17 @@ add_task(async function test_structuredC
   // Test non-object-value round-trip.
 
   equal(new StructuredCloneHolder("foo").deserialize(global), "foo",
         "Round-tripping non-object values works as expected");
 
 
   // Test cross-compartment deserialization
 
-  res = holder.deserialize(sandbox);
+  res = holder.deserialize(sandbox, true);
 
   notEqual(res, obj, "Cross-compartment-deserialized result is a different object from the original");
 
   deepEqual(res, obj, "Cross-compartment-deserialized result is deeply equivalent to the original");
 
   equal(Cu.getObjectPrincipal(res), principal,
         "Cross-compartment-deserialized result has the correct principal");
 
@@ -55,18 +55,30 @@ add_task(async function test_structuredC
 
   Services.cpmm.sendAsyncMessage(MSG, holder);
 
   res = await resultPromise;
 
   ok(res.data instanceof StructuredCloneHolder,
      "Sending structured clone holders through message managers works as expected");
 
-  deepEqual(res.data.deserialize(global), obj,
+  deepEqual(res.data.deserialize(global, true), obj,
             "Sending structured clone holders through message managers works as expected");
+
+  // Test that attempting to deserialize a neutered holder throws.
+
+  deepEqual(holder.deserialize(global), obj, "Deserialized result is correct when discarding data");
+
+  Assert.throws(() => holder.deserialize(global),
+                err => err.result == Cr.NS_ERROR_NOT_INITIALIZED,
+                "Attempting to deserialize neutered holder throws");
+
+  Assert.throws(() => holder.deserialize(global, true),
+                err => err.result == Cr.NS_ERROR_NOT_INITIALIZED,
+                "Attempting to deserialize neutered holder throws");
 });
 
 // Test that X-rays passed to an exported function are serialized
 // through their exported wrappers.
 add_task(async function test_structuredCloneHolder_xray() {
   let principal = Services.scriptSecurityManager.createCodebasePrincipal(
     Services.io.newURI("http://example.com/"), {});
 
--- a/dom/chrome-webidl/StructuredCloneHolder.webidl
+++ b/dom/chrome-webidl/StructuredCloneHolder.webidl
@@ -16,12 +16,15 @@
   * The serialization happens in the compartment of the given global or, if no
   * global is provided, the compartment of the data value.
   */
  Constructor(any data, optional object? global = null)]
 interface StructuredCloneHolder {
   /**
    * Deserializes the structured clone data in the scope of the given global,
    * and returns the result.
+   *
+   * If `keepData` is true, the structured clone data is preserved, and can be
+   * deserialized repeatedly. Otherwise, it is immediately discarded.
    */
   [Throws]
-  any deserialize(object global);
+  any deserialize(object global, optional boolean keepData = false);
 };
--- a/toolkit/components/extensions/ExtensionChild.jsm
+++ b/toolkit/components/extensions/ExtensionChild.jsm
@@ -191,18 +191,18 @@ class Port {
           });
         },
       }).api(),
 
       onMessage: new EventManager({
         context: this.context,
         name: "Port.onMessage",
         register: fire => {
-          return this.registerOnMessage(holder => {
-            let msg = holder.deserialize(this.context.cloneScope);
+          return this.registerOnMessage((holder, isLastHandler) => {
+            let msg = holder.deserialize(this.context.cloneScope, !isLastHandler);
             fire.asyncWithoutClone(msg, portObj);
           });
         },
       }).api(),
 
       get error() {
         return portError;
       },
@@ -250,19 +250,19 @@ class Port {
    * Register a callback that is called when a message is received. The callback
    * is automatically unregistered when the port or context is closed.
    *
    * @param {function} callback Called when a message is received.
    * @returns {function} Function to unregister the listener.
    */
   registerOnMessage(callback) {
     let handler = Object.assign({
-      receiveMessage: ({data}) => {
+      receiveMessage: ({data}, isLastHandler) => {
         if (this.context.active && !this.disconnected) {
-          callback(data);
+          callback(data, isLastHandler);
         }
       },
     }, this.handlerBase);
 
     let unregister = () => {
       this.unregisterMessageFuncs.delete(unregister);
       MessageChannel.removeListener(this.receiverMMs, "Extension:Port:PostMessage", handler);
     };
@@ -429,31 +429,31 @@ class Messenger {
               return false;
             }
 
             // Ignore the message if it was sent by this Messenger.
             return (sender.contextId !== this.context.contextId &&
                     filter(sender, recipient));
           },
 
-          receiveMessage: ({target, data: holder, sender, recipient, channelId}) => {
+          receiveMessage: ({target, data: holder, sender, recipient, channelId}, isLastHandler) => {
             if (!this.context.active) {
               return;
             }
 
             let sendResponse;
             let response = undefined;
             let promise = new Promise(resolve => {
               sendResponse = value => {
                 resolve(value);
                 response = promise;
               };
             });
 
-            let message = holder.deserialize(this.context.cloneScope);
+            let message = holder.deserialize(this.context.cloneScope, !isLastHandler);
             holder = null;
 
             sender = Cu.cloneInto(sender, this.context.cloneScope);
             sendResponse = Cu.exportFunction(sendResponse, this.context.cloneScope);
 
             // Note: We intentionally do not use runSafe here so that any
             // errors are propagated to the message sender.
             let result = fire.raw(message, sender, sendResponse);
--- a/toolkit/components/extensions/ExtensionCommon.jsm
+++ b/toolkit/components/extensions/ExtensionCommon.jsm
@@ -1316,17 +1316,17 @@ class SchemaAPIManager extends EventEmit
       manifestKeys: this.manifestKeys,
       eventModules: this.eventModules,
       schemaURLs: this.schemaURLs,
     });
   }
 
   initModuleData(moduleData) {
     if (!this._modulesJSONLoaded) {
-      let data = moduleData.deserialize({});
+      let data = moduleData.deserialize({}, true);
 
       this.modules = data.modules;
       this.modulePaths = data.modulePaths;
       this.manifestKeys = data.manifestKeys;
       this.eventModules = new DefaultMap(() => new Set(),
                                          data.eventModules);
       this.schemaURLs = data.schemaURLs;
     }
--- a/toolkit/components/extensions/ExtensionStorage.jsm
+++ b/toolkit/components/extensions/ExtensionStorage.jsm
@@ -414,17 +414,17 @@ var ExtensionStorage = {
    *        extension scope directly.
    * @returns {object}
    */
   deserializeForContext(context, items) {
     let result = new context.cloneScope.Object();
     for (let [key, value] of Object.entries(items)) {
       if (value && typeof value === "object" &&
           Cu.getClassName(value, true) === "StructuredCloneHolder") {
-        value = value.deserialize(context.cloneScope);
+        value = value.deserialize(context.cloneScope, true);
       } else {
         value = Cu.cloneInto(value, context.cloneScope);
       }
       result[key] = value;
     }
     return result;
   },
 };
--- a/toolkit/components/extensions/ExtensionStorageIDB.jsm
+++ b/toolkit/components/extensions/ExtensionStorageIDB.jsm
@@ -562,17 +562,17 @@ this.ExtensionStorageIDB = {
 
           if (!parentResult.backendEnabled) {
             result = {backendEnabled: false};
           } else {
             result = {
               ...parentResult,
               // In the child process, we need to deserialize the storagePrincipal
               // from the StructuredCloneHolder used to send it across the processes.
-              storagePrincipal: parentResult.storagePrincipal.deserialize(this),
+              storagePrincipal: parentResult.storagePrincipal.deserialize(this, true),
             };
           }
 
           // Cache the result once we know that it has been resolved. The promise returned by
           // context.childManager.callParentAsyncFunction will be dead when context.cloneScope
           // is destroyed. To keep a promise alive in the cache, we wrap the result in an
           // independent promise.
           this.selectedBackendPromises.set(extension, Promise.resolve(result));
--- a/toolkit/components/extensions/MessageChannel.jsm
+++ b/toolkit/components/extensions/MessageChannel.jsm
@@ -872,19 +872,19 @@ this.MessageChannel = {
       // Note: We use `new Promise` rather than `Promise.resolve` here
       // so that errors from the handler are trapped and converted into
       // rejected promises.
       return new Promise(resolve => {
         resolve(handlers[0].receiveMessage(data));
       });
     }
 
-    let responses = handlers.map(handler => {
+    let responses = handlers.map((handler, i) => {
       try {
-        return handler.receiveMessage(data);
+        return handler.receiveMessage(data, i + 1 == handlers.length);
       } catch (e) {
         return Promise.reject(e);
       }
     });
     data = null;
     responses = responses.filter(response => response !== undefined);
 
     switch (responseType) {
--- a/toolkit/components/extensions/Schemas.jsm
+++ b/toolkit/components/extensions/Schemas.jsm
@@ -2945,17 +2945,17 @@ class SchemaRoot extends Namespace {
 
     return type.parseSchema(this, schema, path, allowedProperties);
   }
 
   parseSchemas() {
     for (let [key, schema] of this.schemaJSON.entries()) {
       try {
         if (typeof schema.deserialize === "function") {
-          schema = schema.deserialize(global);
+          schema = schema.deserialize(global, isParentProcess);
 
           // If we're in the parent process, we need to keep the
           // StructuredCloneHolder blob around in order to send to future child
           // processes. If we're in a child, we have no further use for it, so
           // just store the deserialized schema data in its place.
           if (!isParentProcess) {
             this.schemaJSON.set(key, schema);
           }
--- a/toolkit/components/extensions/parent/ext-storage.js
+++ b/toolkit/components/extensions/parent/ext-storage.js
@@ -69,17 +69,17 @@ this.storage = class extends ExtensionAP
         local: {
           async callMethodInParentProcess(method, args) {
             const res = await ExtensionStorageIDB.selectBackend({extension});
             if (!res.backendEnabled) {
               return ExtensionStorage[method](extension.id, ...args);
             }
 
             const persisted = extension.hasPermission("unlimitedStorage");
-            const db = await ExtensionStorageIDB.open(res.storagePrincipal.deserialize(this), persisted);
+            const db = await ExtensionStorageIDB.open(res.storagePrincipal.deserialize(this, true), persisted);
             const changes = await db[method](...args);
             if (changes) {
               ExtensionStorageIDB.notifyListeners(extension.id, changes);
             }
             return changes;
           },
           // Private storage.local JSONFile backend methods (used internally by the child
           // ext-storage.js module).