Bug 1356546: Part 4 - Use StructuredCloneHolder as transport for proxied method return values. r=aswan
authorKris Maglione <maglione.k@gmail.com>
Sun, 04 Jun 2017 20:39:28 -0700
changeset 412786 c3bf2490530f5ac768040043b8d0dd55ce6c5a03
parent 412785 fe952a030435f0690bd4f64d6299201b9734cb47
child 412787 5d990834b3e42f6900564f17a6e5f8aa1cd8eedc
push id1490
push usermtabara@mozilla.com
push dateMon, 31 Jul 2017 14:08:16 +0000
treeherdermozilla-release@70e32e6bf15e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersaswan
bugs1356546
milestone55.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 1356546: Part 4 - Use StructuredCloneHolder as transport for proxied method return values. r=aswan MozReview-Commit-ID: LZ3XkamgkeF
toolkit/components/extensions/ExtensionChild.jsm
toolkit/components/extensions/ExtensionCommon.jsm
toolkit/components/extensions/ExtensionParent.jsm
--- a/toolkit/components/extensions/ExtensionChild.jsm
+++ b/toolkit/components/extensions/ExtensionChild.jsm
@@ -43,19 +43,19 @@ const {
   defineLazyGetter,
   getMessageManager,
   getUniqueId,
 } = ExtensionUtils;
 
 const {
   LocalAPIImplementation,
   LocaleData,
+  NoCloneSpreadArgs,
   SchemaAPIInterface,
   SingletonEventManager,
-  SpreadArgs,
 } = ExtensionCommon;
 
 const isContentProcess = Services.appinfo.processType == Services.appinfo.PROCESS_TYPE_CONTENT;
 
 // Copy an API object from |source| into the scope |dest|.
 function injectAPI(source, dest) {
   for (let prop in source) {
     // Skip names prefixed with '_'.
@@ -768,17 +768,19 @@ class ChildAPIManager {
         }
         break;
 
       case "API:CallResult":
         let deferred = this.callPromises.get(data.callId);
         if ("error" in data) {
           deferred.reject(data.error);
         } else {
-          deferred.resolve(new SpreadArgs(data.result));
+          let result = data.result.deserialize(this.context.cloneScope);
+
+          deferred.resolve(new NoCloneSpreadArgs(result));
         }
         this.callPromises.delete(data.callId);
         break;
     }
   }
 
   /**
    * Call a function in the parent process and ignores its return value.
--- a/toolkit/components/extensions/ExtensionCommon.jsm
+++ b/toolkit/components/extensions/ExtensionCommon.jsm
@@ -48,23 +48,47 @@ var {
   runSafeSyncWithoutClone,
   instanceOf,
 } = ExtensionUtils;
 
 XPCOMUtils.defineLazyGetter(this, "console", getConsole);
 
 var ExtensionCommon;
 
+/**
+ * A sentinel class to indicate that an array of values should be
+ * treated as an array when used as a promise resolution value, but as a
+ * spread expression (...args) when passed to a callback.
+ */
 class SpreadArgs extends Array {
   constructor(args) {
     super();
     this.push(...args);
   }
 }
 
+/**
+ * Like SpreadArgs, but also indicates that the array values already
+ * belong to the target compartment, and should not be cloned before
+ * being passed.
+ *
+ * The `unwrappedValues` property contains an Array object which belongs
+ * to the target compartment, and contains the same unwrapped values
+ * passed the NoCloneSpreadArgs constructor.
+ */
+class NoCloneSpreadArgs {
+  constructor(args) {
+    this.unwrappedValues = args;
+  }
+
+  [Symbol.iterator]() {
+    return this.unwrappedValues[Symbol.iterator]();
+  }
+}
+
 class BaseContext {
   constructor(envType, extension) {
     this.envType = envType;
     this.onClose = new Set();
     this.checkedLastError = false;
     this._lastError = null;
     this.contextId = getUniqueId();
     this.unloaded = false;
@@ -318,16 +342,18 @@ class BaseContext {
 
     if (callback) {
       promise.then(
         args => {
           if (this.unloaded) {
             dump(`Promise resolved after context unloaded\n`);
           } else if (!this.active) {
             dump(`Promise resolved while context is inactive\n`);
+          } else if (args instanceof NoCloneSpreadArgs) {
+            this.runSafeWithoutClone(callback, ...args.unwrappedValues);
           } else if (args instanceof SpreadArgs) {
             runSafe(callback, ...args);
           } else {
             runSafe(callback, args);
           }
         },
         error => {
           this.withLastError(error, () => {
@@ -343,16 +369,19 @@ class BaseContext {
     } else {
       return new this.cloneScope.Promise((resolve, reject) => {
         promise.then(
           value => {
             if (this.unloaded) {
               dump(`Promise resolved after context unloaded\n`);
             } else if (!this.active) {
               dump(`Promise resolved while context is inactive\n`);
+            } else if (value instanceof NoCloneSpreadArgs) {
+              let values = value.unwrappedValues;
+              this.runSafeWithoutClone(resolve, values.length == 1 ? values[0] : values);
             } else if (value instanceof SpreadArgs) {
               runSafe(resolve, value.length == 1 ? value[0] : value);
             } else {
               runSafe(resolve, value);
             }
           },
           value => {
             if (this.unloaded) {
@@ -1483,15 +1512,16 @@ const stylesheetMap = new DefaultMap(url
 });
 
 
 ExtensionCommon = {
   BaseContext,
   CanOfAPIs,
   LocalAPIImplementation,
   LocaleData,
+  NoCloneSpreadArgs,
   SchemaAPIInterface,
   SchemaAPIManager,
   SingletonEventManager,
   SpreadArgs,
   ignoreEvent,
   stylesheetMap,
 };
--- a/toolkit/components/extensions/ExtensionParent.jsm
+++ b/toolkit/components/extensions/ExtensionParent.jsm
@@ -619,17 +619,19 @@ ParentAPIManager = {
       let result = fun(...args);
 
       if (data.callId) {
         result = result || Promise.resolve();
 
         result.then(result => {
           result = result instanceof SpreadArgs ? [...result] : [result];
 
-          reply({result});
+          let holder = new StructuredCloneHolder(result);
+
+          reply({result: holder});
         }, error => {
           error = context.normalizeError(error);
           reply({error: {message: error.message, fileName: error.fileName}});
         });
       }
     } catch (e) {
       if (data.callId) {
         let error = context.normalizeError(e);