Merge pull request #4568 from atom/remote-proto

Keep prototype chain in remote objects

atom/browser/lib/rpc-server.js

@@ -6,11 +6,52 @@ const objectsRegistry = require('./objects-registry');
 const v8Util = process.atomBinding('v8_util');
 const IDWeakMap = process.atomBinding('id_weak_map').IDWeakMap;
 
+// The internal properties of Function.
+const FUNCTION_PROPERTIES = [
+  'length', 'name', 'arguments', 'caller', 'prototype',
+];
+
 var slice = [].slice;
 
+// Return the description of object's members:
+let getObjectMemebers = function(object) {
+  let names = Object.getOwnPropertyNames(object);
+  // For Function, we should not override following properties even though they
+  // are "own" properties.
+  if (typeof object === 'function') {
+    names = names.filter((name) => {
+      return !FUNCTION_PROPERTIES.includes(name);
+    });
+  }
+  // Map properties to descriptors.
+  return names.map((name) => {
+    let descriptor = Object.getOwnPropertyDescriptor(object, name);
+    let member = {name, enumerable: descriptor.enumerable, writable: false};
+    if (descriptor.get === undefined && typeof object[name] === 'function') {
+      member.type = 'method';
+    } else {
+      if (descriptor.set || descriptor.writable)
+        member.writable = true;
+      member.type = 'get';
+    }
+    return member;
+  });
+};
+
+// Return the description of object's prototype.
+let getObjectPrototype = function(object) {
+  let proto = Object.getPrototypeOf(object);
+  if (proto === null || proto === Object.prototype)
+    return null;
+  return {
+    members: getObjectMemebers(proto),
+    proto: getObjectPrototype(proto),
+  };
+};
+
 // Convert a real value into meta data.
 var valueToMeta = function(sender, value, optimizeSimpleObject) {
-  var el, field, i, len, meta, name;
+  var el, i, len, meta;
   if (optimizeSimpleObject == null) {
     optimizeSimpleObject = false;
   }
@@ -58,18 +99,8 @@ var valueToMeta = function(sender, value, optimizeSimpleObject) {
     // passed to renderer we would assume the renderer keeps a reference of
     // it.
     meta.id = objectsRegistry.add(sender.getId(), value);
-    meta.members = (function() {
-      var results;
-      results = [];
-      for (name in value) {
-        field = value[name];
-        results.push({
-          name: name,
-          type: typeof field
-        });
-      }
-      return results;
-    })();
+    meta.members = getObjectMemebers(value);
+    meta.proto = getObjectPrototype(value);
   } else if (meta.type === 'buffer') {
     meta.value = Array.prototype.slice.call(value, 0);
   } else if (meta.type === 'promise') {

atom/renderer/api/lib/remote.js

@@ -1,3 +1,5 @@
+'use strict';
+
 const ipcRenderer = require('electron').ipcRenderer;
 const CallbacksRegistry = require('electron').CallbacksRegistry;
 const v8Util = process.atomBinding('v8_util');
@@ -88,9 +90,59 @@ var wrapArgs = function(args, visited) {
   return Array.prototype.slice.call(args).map(valueToMeta);
 };
 
+// Populate object's members from descriptors.
+// This matches |getObjectMemebers| in rpc-server.
+let setObjectMembers = function(object, metaId, members) {
+  for (let member of members) {
+    if (object.hasOwnProperty(member.name))
+      continue;
+
+    let descriptor = { enumerable: member.enumerable };
+    if (member.type === 'method') {
+      let remoteMemberFunction = function() {
+        if (this && this.constructor === remoteMemberFunction) {
+          // Constructor call.
+          let ret = ipcRenderer.sendSync('ATOM_BROWSER_MEMBER_CONSTRUCTOR', metaId, member.name, wrapArgs(arguments));
+          return metaToValue(ret);
+        } else {
+          // Call member function.
+          let ret = ipcRenderer.sendSync('ATOM_BROWSER_MEMBER_CALL', metaId, member.name, wrapArgs(arguments));
+          return metaToValue(ret);
+        }
+      };
+      descriptor.value = remoteMemberFunction;
+    } else if (member.type === 'get') {
+      descriptor.get = function() {
+        return metaToValue(ipcRenderer.sendSync('ATOM_BROWSER_MEMBER_GET', metaId, member.name));
+      };
+
+      // Only set setter when it is writable.
+      if (member.writable) {
+        descriptor.set = function(value) {
+          ipcRenderer.sendSync('ATOM_BROWSER_MEMBER_SET', metaId, member.name, value);
+          return value;
+        };
+      }
+    }
+
+    Object.defineProperty(object, member.name, descriptor);
+  }
+};
+
+// Populate object's prototype from descriptor.
+// This matches |getObjectPrototype| in rpc-server.
+let setObjectPrototype = function(object, metaId, descriptor) {
+  if (descriptor === null)
+    return;
+  let proto = {};
+  setObjectMembers(proto, metaId, descriptor.members);
+  setObjectPrototype(proto, metaId, descriptor.proto);
+  Object.setPrototypeOf(object, proto);
+};
+
 // Convert meta data from browser into real value.
 var metaToValue = function(meta) {
-  var el, i, j, len, len1, member, ref1, ref2, results, ret;
+  var el, i, len, ref1, results, ret;
   switch (meta.type) {
     case 'value':
       return meta.value;
@@ -115,55 +167,42 @@ var metaToValue = function(meta) {
     case 'exception':
       throw new Error(meta.message + "\n" + meta.stack);
     default:
-      if (meta.type === 'function') {
-        // A shadow class to represent the remote function object.
-        ret = (function() {
-          function RemoteFunction() {
-            var obj;
-            if (this.constructor === RemoteFunction) {
-
-              // Constructor call.
-              obj = ipcRenderer.sendSync('ATOM_BROWSER_CONSTRUCTOR', meta.id, wrapArgs(arguments));
-
-              /*
-                Returning object in constructor will replace constructed object
-                with the returned object.
-                http://stackoverflow.com/questions/1978049/what-values-can-a-constructor-return-to-avoid-returning-this
-               */
-              return metaToValue(obj);
-            } else {
-
-              // Function call.
-              obj = ipcRenderer.sendSync('ATOM_BROWSER_FUNCTION_CALL', meta.id, wrapArgs(arguments));
-              return metaToValue(obj);
-            }
-          }
-
-          return RemoteFunction;
-
-        })();
-      } else {
-        ret = v8Util.createObjectWithName(meta.name);
-      }
-
-      // Polulate delegate members.
-      ref2 = meta.members;
-      for (j = 0, len1 = ref2.length; j < len1; j++) {
-        member = ref2[j];
-        if (member.type === 'function') {
-          ret[member.name] = createRemoteMemberFunction(meta.id, member.name);
-        } else {
-          Object.defineProperty(ret, member.name, createRemoteMemberProperty(meta.id, member.name));
-        }
-      }
-
       if (remoteObjectCache.has(meta.id))
         return remoteObjectCache.get(meta.id);
 
+      if (meta.type === 'function') {
+        // A shadow class to represent the remote function object.
+        let remoteFunction = function() {
+          if (this && this.constructor === remoteFunction) {
+            // Constructor call.
+            let obj = ipcRenderer.sendSync('ATOM_BROWSER_CONSTRUCTOR', meta.id, wrapArgs(arguments));
+            // Returning object in constructor will replace constructed object
+            // with the returned object.
+            // http://stackoverflow.com/questions/1978049/what-values-can-a-constructor-return-to-avoid-returning-this
+            return metaToValue(obj);
+          } else {
+            // Function call.
+            let obj = ipcRenderer.sendSync('ATOM_BROWSER_FUNCTION_CALL', meta.id, wrapArgs(arguments));
+            return metaToValue(obj);
+          }
+        };
+        ret = remoteFunction;
+      } else {
+        ret = {};
+      }
+
+      // Populate delegate members.
+      setObjectMembers(ret, meta.id, meta.members);
+      // Populate delegate prototype.
+      setObjectPrototype(ret, meta.id, meta.proto);
+
+      // Set constructor.name to object's name.
+      Object.defineProperty(ret.constructor, 'name', { value: meta.name });
+
       // Track delegate object's life time, and tell the browser to clean up
       // when the object is GCed.
       v8Util.setDestructor(ret, function() {
-        return ipcRenderer.send('ATOM_BROWSER_DEREFERENCE', meta.id);
+        ipcRenderer.send('ATOM_BROWSER_DEREFERENCE', meta.id);
       });
 
       // Remember object's id.
@@ -192,52 +231,6 @@ var metaToPlainObject = function(meta) {
   return obj;
 };
 
-// Create a RemoteMemberFunction instance.
-// This function's content should not be inlined into metaToValue, otherwise V8
-// may consider it circular reference.
-var createRemoteMemberFunction = function(metaId, name) {
-  return (function() {
-    function RemoteMemberFunction() {
-      var ret;
-      if (this.constructor === RemoteMemberFunction) {
-
-        // Constructor call.
-        ret = ipcRenderer.sendSync('ATOM_BROWSER_MEMBER_CONSTRUCTOR', metaId, name, wrapArgs(arguments));
-        return metaToValue(ret);
-      } else {
-
-        // Call member function.
-        ret = ipcRenderer.sendSync('ATOM_BROWSER_MEMBER_CALL', metaId, name, wrapArgs(arguments));
-        return metaToValue(ret);
-      }
-    }
-
-    return RemoteMemberFunction;
-
-  })();
-};
-
-// Create configuration for defineProperty.
-// This function's content should not be inlined into metaToValue, otherwise V8
-// may consider it circular reference.
-var createRemoteMemberProperty = function(metaId, name) {
-  return {
-    enumerable: true,
-    configurable: false,
-    set: function(value) {
-
-      // Set member data.
-      ipcRenderer.sendSync('ATOM_BROWSER_MEMBER_SET', metaId, name, value);
-      return value;
-    },
-    get: function() {
-
-      // Get member data.
-      return metaToValue(ipcRenderer.sendSync('ATOM_BROWSER_MEMBER_GET', metaId, name));
-    }
-  };
-};
-
 // Browser calls a callback in renderer.
 ipcRenderer.on('ATOM_RENDERER_CALLBACK', function(event, id, args) {
   return callbacksRegistry.apply(id, metaToValue(args));

spec/api-ipc-spec.js

@@ -1,3 +1,5 @@
+'use strict';
+
 const assert = require('assert');
 const path = require('path');
 
@@ -98,6 +100,41 @@ describe('ipc module', function() {
     });
   });
 
+  describe('remote class', function() {
+    let cl = remote.require(path.join(fixtures, 'module', 'class.js'));
+    let base = cl.base;
+    let derived = cl.derived;
+
+    it('can get methods', function() {
+      assert.equal(base.method(), 'method');
+    });
+
+    it('can get properties', function() {
+      assert.equal(base.readonly, 'readonly');
+    });
+
+    it('can change properties', function() {
+      assert.equal(base.value, 'old');
+      base.value = 'new';
+      assert.equal(base.value, 'new');
+      base.value = 'old';
+    });
+
+    it('has unenumerable methods', function() {
+      assert(!base.hasOwnProperty('method'));
+      assert(Object.getPrototypeOf(base).hasOwnProperty('method'));
+    });
+
+    it('keeps prototype chain in derived class', function() {
+      assert.equal(derived.method(), 'method');
+      assert.equal(derived.readonly, 'readonly');
+      assert(!derived.hasOwnProperty('method'));
+      let proto = Object.getPrototypeOf(derived);
+      assert(!proto.hasOwnProperty('method'));
+      assert(Object.getPrototypeOf(proto).hasOwnProperty('method'));
+    });
+  });
+
   describe('ipc.sender.send', function() {
     it('should work when sending an object containing id property', function(done) {
       var obj = {

spec/fixtures/module/class.js

@@ -0,0 +1,29 @@
+'use strict';
+
+let value = 'old';
+
+class BaseClass {
+  method() {
+    return 'method';
+  }
+
+  get readonly() {
+    return 'readonly';
+  }
+
+  get value() {
+    return value;
+  }
+
+  set value(val) {
+    value = val;
+  }
+}
+
+class DerivedClass extends BaseClass {
+}
+
+module.exports = {
+  base: new BaseClass,
+  derived: new DerivedClass,
+}