diff --git a/js/mainApiMgr.js b/js/mainApiMgr.js index a21d7aa8..45f4e00d 100644 --- a/js/mainApiMgr.js +++ b/js/mainApiMgr.js @@ -15,12 +15,18 @@ const apiCmds = apiEnums.cmds; const apiName = apiEnums.apiName; const apiProxyCmds = apiEnums.proxyCmds +// can be overridden for testing +let checkValidWindow = true; + /** * Ensure events comes from a window that we have created. * @param {EventEmitter} event node emitter event to be tested * @return {Boolean} returns true if exists otherwise false */ function isValidWindow(event) { + if (!checkValidWindow) { + return true; + } var result = false; if (event && event.sender) { // validate that event sender is from window we created @@ -87,7 +93,7 @@ electron.ipcMain.on(apiName, (event, arg) => { const Notify = require('./notify/notifyImpl.js'); // holds all project classes that can be created. -const api = { +let api = { Notify: Notify } @@ -127,12 +133,14 @@ electron.ipcMain.on(apiProxyCmds.createObject, function(event, args) { let objId = uniqueId(); liveObjs[objId] = obj; - obj.addEventListener('destroy', function() { - var liveObj = liveObjs[objId]; - if (liveObj) { - delete liveObjs[objId]; - } - }); + if (typeof obj.addEventListener === 'function') { + obj.addEventListener('destroy', function() { + var liveObj = liveObjs[objId]; + if (liveObj) { + delete liveObjs[objId]; + } + }); + } setResult(objId); } else { @@ -206,7 +214,6 @@ electron.ipcMain.on(apiProxyCmds.invokeMethod, function(event, args) { result = classType[args.methodName](...funcArgs); } else { let obj = liveObjs[args.objId]; - if (!args.methodName || !obj[args.methodName]) { event.sender.send(apiProxyCmds.invokeResult, { error: 'no such method', @@ -423,3 +430,15 @@ electron.ipcMain.on(apiProxyCmds.removeEvent, function(event, args) { obj.removeEventListener(args.eventName, callbackFunc); } }); + +function addNewInterface(name, interfaceClass) { + api[name] = interfaceClass; +} + +// expose these methods primarily for testing... +module.exports = { + addNewInterface: addNewInterface, + shouldCheckValidWindow: function(shouldCheck) { + checkValidWindow = shouldCheck; + } +} diff --git a/js/notify/notifyImpl.js b/js/notify/notifyImpl.js index 67dfbaff..c32faa78 100644 --- a/js/notify/notifyImpl.js +++ b/js/notify/notifyImpl.js @@ -50,10 +50,6 @@ class Notify { this.destroy(); } - destroy() { - this.emitter.removeAllListeners(); - } - static get permission() { return 'granted'; } @@ -73,6 +69,13 @@ class Notify { // // private stuff below here // + + destroy() { + this.emitter.removeAllListeners(); + // allow live instance to be destroyed + this.emitter.emit('destroy'); + } + } module.exports = Notify; diff --git a/js/notify/notifyInterface.js b/js/notify/notifyInterface.js index 079de8ba..39135e4c 100644 --- a/js/notify/notifyInterface.js +++ b/js/notify/notifyInterface.js @@ -32,12 +32,6 @@ class Notify { */ close() {} - /** - * call to clean up ref held by main process to notification. - * note: calling close will also invoke destroy. - */ - destroy() {} - /** * This returns a promise and is always 'granted' * @return {promise} promise fullfilled with 'granted' diff --git a/js/preload/createProxy.js b/js/preload/createProxy.js index 7910c438..ca994b26 100644 --- a/js/preload/createProxy.js +++ b/js/preload/createProxy.js @@ -62,7 +62,6 @@ let constructorHandler = { set: instanceSetHandler } - // work like to incorporate something like https://github.com/EvolveLabs/electron-weak // here to tell when object is destroyed so we can ipc main process to // loss ref to liveObj. @@ -76,7 +75,7 @@ let constructorHandler = { function instanceGetHandler(target, name) { // all methods and getters we support should be on the prototype - let prototype = Reflect.getPrototypeOf(target); + let prototype = Object.getPrototypeOf(target); let desc = Object.getOwnPropertyDescriptor(prototype, name); // does this have a "getter" @@ -127,8 +126,8 @@ function addEventHandler(target) { function removeEventHandler(target) { return function(eventName, callback) { - if (target._callbacks && target._callback.has(callback)) { - let callbackObj = target._callback.get(callback); + if (target._callbacks && target._callbacks.has(callback)) { + let callbackObj = target._callbacks.get(callback); let args = { eventName: eventName, @@ -233,7 +232,7 @@ function getHandler(target, property, isStatic) { } function instanceSetHandler(target, property, value) { - let prototype = Reflect.getPrototypeOf(target); + let prototype = Object.getPrototypeOf(target); let desc = Object.getOwnPropertyDescriptor(prototype, property); if (desc && desc.set) { diff --git a/tests/__mocks__/electron.js b/tests/__mocks__/electron.js index e8c59293..c86fff0f 100644 --- a/tests/__mocks__/electron.js +++ b/tests/__mocks__/electron.js @@ -1,9 +1,14 @@ const path = require('path'); +const EventEmitter = require('events'); + +let ipcEmitter = new EventEmitter(); // use config provided by test framework function pathToConfigDir() { return path.join(__dirname, '/../fixtures'); } + +// electron app mock... const app = { getAppPath: pathToConfigDir, getPath: function(type) { @@ -11,6 +16,46 @@ const app = { return path.join(pathToConfigDir(), '/Symphony.exe'); } return pathToConfigDir(); + }, + on: function() { + // no-op + } +} + +// simple ipc mocks for render and main process ipc using +// nodes' EventEmitter +const ipcMain = { + on: function(event, cb) { + ipcEmitter.on(event, cb); + } +} + +const ipcRenderer = { + sendSync: function(event, args) { + let listeners = ipcEmitter.listeners(event); + if (listeners.length > 0) { + let listener = listeners[0]; + var eventArg = {} + listener(eventArg, args); + return eventArg.returnValue; + } + return null; + }, + send: function(event, args) { + var senderEvent = { + sender: { + send: function(event, arg) { + ipcEmitter.emit(event, arg); + } + } + } + ipcEmitter.emit(event, senderEvent, args); + }, + on: function(eventName, cb) { + ipcEmitter.on(eventName, cb); + }, + removeListener: function(eventName, cb) { + ipcEmitter.removeListener(eventName, cb); } } @@ -18,6 +63,8 @@ module.exports = { require: jest.genMockFunction(), match: jest.genMockFunction(), app: app, + ipcMain: ipcMain, + ipcRenderer: ipcRenderer, remote: jest.genMockFunction(), dialog: jest.genMockFunction() }; diff --git a/tests/apiTests/proxy-polyfill.js b/tests/apiTests/proxy-polyfill.js new file mode 100644 index 00000000..ad14d0c8 --- /dev/null +++ b/tests/apiTests/proxy-polyfill.js @@ -0,0 +1,182 @@ + +// Note: fork of polyfill since it doesn't handle creating +// a Proxy from an existing Proxy. +// ToDo: need to submit PR to https://github.com/GoogleChrome/proxy-polyfill + +/* + * Copyright 2016 Google Inc. All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not + * use this file except in compliance with the License. You may obtain a copy of + * the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations under + * the License. + */ + +'use strict'; + +(function(scope) { + if (scope['Proxy']) { + return; + } + let lastRevokeFn = null; + + /** + * @param {*} o + * @return {boolean} whether this is probably a (non-null) Object + */ + function isObject(o) { + return o ? (typeof o == 'object' || typeof o == 'function') : false; + } + + /** + * @constructor + * @param {!Object} target + * @param {{apply, construct, get, set}} handler + */ + scope.Proxy = function(target, handler) { + + if (!isObject(target) || !isObject(handler)) { + throw new TypeError('Cannot create proxy with a non-object as target or handler'); + } + + // Construct revoke function, and set lastRevokeFn so that Proxy.revocable can steal it. + // The caller might get the wrong revoke function if a user replaces or wraps scope.Proxy + // to call itself, but that seems unlikely especially when using the polyfill. + let throwRevoked = function() {}; + lastRevokeFn = function() { + throwRevoked = function(trap) { + throw new TypeError(`Cannot perform '${trap}' on a proxy that has been revoked`); + }; + }; + + // Fail on unsupported traps: Chrome doesn't do this, but ensure that users of the polyfill + // are a bit more careful. Copy the internal parts of handler to prevent user changes. + let unsafeHandler = handler; + handler = {'get': null, 'set': null, 'apply': null, 'construct': null}; + for (let k in unsafeHandler) { + if (!(k in handler)) { + throw new TypeError(`Proxy polyfill does not support trap '${k}'`); + } + handler[k] = unsafeHandler[k]; + } + if (typeof unsafeHandler == 'function') { + // Allow handler to be a function (which has an 'apply' method). This matches what is + // probably a bug in native versions. It treats the apply call as a trap to be configured. + handler.apply = unsafeHandler.apply.bind(unsafeHandler); + } + + // Define proxy as this, or a Function (if either it's callable, or apply is set). + // TODO(samthor): Closure compiler doesn't know about 'construct', attempts to rename it. + let proxy = this; + let isMethod = false; + let targetIsFunction = typeof target == 'function'; + if (handler.apply || handler['construct'] || targetIsFunction) { + proxy = function Proxy() { + let usingNew = (this && this.constructor === proxy); + throwRevoked(usingNew ? 'construct' : 'apply'); + + if (usingNew && handler['construct']) { + return handler['construct'].call(this, target, arguments); + } else if (!usingNew && handler.apply) { + return handler.apply(target, this, arguments); + } else if (targetIsFunction) { + // since the target was a function, fallback to calling it directly. + if (usingNew) { + // inspired by answers to https://stackoverflow.com/q/1606797 + let all = Array.prototype.slice.call(arguments); + all.unshift(target); // pass class as first arg to constructor, although irrelevant + // nb. cast to convince Closure compiler that this is a constructor + let f = /** @type {!Function} */ (target.bind.apply(target, all)); + return new f(); + } + return target.apply(this, arguments); + } + throw new TypeError(usingNew ? 'not a constructor' : 'not a function'); + }; + isMethod = true; + } + + // Create default getters/setters. Create different code paths as handler.get/handler.set can't + // change after creation. + let getter = handler.get ? function(prop) { + throwRevoked('get'); + return handler.get(this, prop, proxy); + } : function(prop) { + throwRevoked('get'); + return this[prop]; + }; + let setter = handler.set ? function(prop, value) { + throwRevoked('set'); + let status = handler.set(this, prop, value, proxy); + if (!status) { + // TODO(samthor): If the calling code is in strict mode, throw TypeError. + // It's (sometimes) possible to work this out, if this code isn't strict- try to load the + // callee, and if it's available, that code is non-strict. However, this isn't exhaustive. + } + } : function(prop, value) { + throwRevoked('set'); + this[prop] = value; + }; + + // Clone direct properties (i.e., not part of a prototype). + let propertyNames = Object.getOwnPropertyNames(target); + if (target.__proto__) { + propertyNames = propertyNames.concat(Object.getOwnPropertyNames(target.__proto__)) + } + let propertyMap = {}; + propertyNames.forEach(function(prop) { + if (isMethod && prop in proxy) { + return; // ignore properties already here, e.g. 'bind', 'prototype' etc + } + let real = Object.getOwnPropertyDescriptor(target, prop); + let desc = { + enumerable: real && !!real.enumerable, + get: getter.bind(target, prop), + set: setter.bind(target, prop), + }; + Object.defineProperty(proxy, prop, desc); + propertyMap[prop] = true; + }); + + // Set the prototype, or clone all prototype methods (always required if a getter is provided). + // TODO(samthor): We don't allow prototype methods to be set. It's (even more) awkward. + // An alternative here would be to _just_ clone methods to keep behavior consistent. + let prototypeOk = true; + if (Object.setPrototypeOf) { + Object.setPrototypeOf(proxy, Object.getPrototypeOf(target)); + } else if (proxy.__proto__) { + proxy.__proto__ = target.__proto__; + } else { + prototypeOk = false; + } + if (handler.get || !prototypeOk) { + for (let k in target) { + if (propertyMap[k]) { + continue; + } + Object.defineProperty(proxy, k, {get: getter.bind(target, k)}); + } + } + + // The Proxy polyfill cannot handle adding new properties. Seal the target and proxy. + Object.seal(target); + Object.seal(proxy); + + return proxy; // nb. if isMethod is true, proxy != this + }; + + scope.Proxy.revocable = function(target, handler) { + let p = new scope.Proxy(target, handler); + return {'proxy': p, 'revoke': lastRevokeFn}; + }; + + scope.Proxy['revocable'] = scope.Proxy.revocable; + scope['Proxy'] = scope.Proxy; +})(typeof module !== 'undefined' && module['exports'] ? global : window); diff --git a/tests/apiTests/proxyAPI.test.js b/tests/apiTests/proxyAPI.test.js new file mode 100644 index 00000000..da319531 --- /dev/null +++ b/tests/apiTests/proxyAPI.test.js @@ -0,0 +1,304 @@ +const EventEmitter = require('events'); + +const createProxy = require('../../js/preload/createProxy.js'); +const mainApiMgr = require('../../js/mainApiMgr.js'); + +mainApiMgr.shouldCheckValidWindow(false); + +// need polyfil for html5 Proxy +require('./proxy-polyfill'); + +class testInterface { + constructor(arg1, arg2) {} + + getArg1() {}; + + addToArg2(value) {} + + get argumentOne() {} + + set newArgOneValue(newValue) {} + + static staticMethodSum(a, b) {} + + static get staticGetter() {} + + addEventListener(event,cb) {} + + removeEventListener(event,cb) {} + + emitEvent(event) {} + + close() {} + + destroy() {} +} + +class testImpl { + constructor(arg1, arg2) { + this._arg1 = arg1; + this._arg2 = arg2; + this.emitter = new EventEmitter(); + } + + getArg1() { + return this._arg1; + }; + + addToArg2(value) { + return this._arg2 + value; + } + + get argumentOne() { + return this._arg1; + } + + set newArgOneValue(newValue) { + this._arg1 = newValue; + } + + static staticMethodSum(a, b) { + return a + b; + } + + static get staticGetter() { + return 'hello world'; + } + + addEventListener(event, cb) { + this.emitter.on(event, cb); + } + + removeEventListener(event,cb) { + this.emitter.removeListener(event, cb); + } + + emitEvent(event) { + this.emitter.emit(event); + } + + close() { + this.emitter.emit('destroy'); + } + + destroy() {} +} + +mainApiMgr.addNewInterface('testInterface', testImpl); + +describe('proxy tests...', function() { + var inst; + var TestInterfaceProxy; + + const arg1 = 3, arg2 = 2; + + beforeEach(function() { + TestInterfaceProxy = createProxy(testInterface); + inst = new TestInterfaceProxy(arg1, arg2); + }); + + test('getArg1 method', function(done) { + inst.getArg1().then(function(result) { + expect(result).toBe(arg1); + done(); + }); + }); + + test('addToArg2 method', function(done) { + inst.addToArg2(4).then(function(result) { + expect(result).toBe(arg2 + 4); + done(); + }); + }); + + test('getter: argumentOne', function(done) { + inst.argumentOne.then(function(result) { + expect(result).toBe(arg1); + done(); + }); + }); + + test('setter: newArgOneValue', function(done) { + inst.newArgOneValue = 10; + inst.argumentOne.then(function(result) { + expect(result).toBe(10); + done(); + }); + }); + + test('static method', function(done) { + TestInterfaceProxy.staticMethodSum(5, 6).then(function(result) { + expect(result).toBe(11); + done(); + }); + }); + + test('static getter', function(done) { + TestInterfaceProxy.staticGetter.then(function(result) { + expect(result).toBe('hello world'); + done(); + }); + }); + + test('should call click handler', function(done) { + inst.addEventListener('click', function() { + done(); + }); + + inst.emitEvent('click'); + }); + + test('should call click handler twice', function(done) { + var timesCalled = 0; + inst.addEventListener('click', function() { + timesCalled++; + if (timesCalled === 2) { + done(); + } + }); + + inst.emitEvent('click'); + inst.emitEvent('click'); + }); + + test('should only call close handler', function(done) { + inst.addEventListener('click', function() { + // shouldn't hit here + expect(false).toBe(true); + }); + + inst.addEventListener('close', function() { + done(); + }); + + inst.emitEvent('close'); + }); + + test('should not emit event addEventHandler', function(done) { + inst.addEventListener('click', function() { + // shouldn't hit here + expect(false).toBe(true); + }); + + inst.emitEvent('wrong-event'); + setTimeout(done, 500); + }); + + test('should not call click handler after removed', function(done) { + function onClick() { + // shouldn't hit here + expect(true).toBe(false); + } + inst.addEventListener('click', onClick); + inst.removeEventListener('click', onClick); + inst.emitEvent('click'); + setTimeout(done, 500); + }); + + test('should call click handler after add, remove, add', function(done) { + function onClick() { + done(); + } + inst.addEventListener('click', onClick); + inst.removeEventListener('click', onClick); + inst.addEventListener('click', onClick); + inst.emitEvent('click'); + }); +}); + +describe('proxy test with multiple instances...', function() { + var inst1, inst2; + var TestInterfaceProxy; + + const arg1 = 3, arg2 = 2; + + beforeEach(function() { + TestInterfaceProxy = createProxy(testInterface); + inst1 = new TestInterfaceProxy(arg1, arg2); + inst2 = new TestInterfaceProxy(arg1, arg2); + }); + + test('should have indepdendent setters', function(done) { + inst1.newArgOneValue = 10; + inst2.newArgOneValue = 5; + inst1.argumentOne.then(function(result) { + expect(result).toBe(10); + inst2.argumentOne.then(function(result) { + expect(result).toBe(5); + done(); + }); + }); + }); + + test('should only call event handler for inst2', function(done) { + inst1.addEventListener('click', function() { + // shouldn't hit here + expect(true).toBe(false); + }); + inst2.addEventListener('click', function() { + done(); + }); + + inst2.emitEvent('click'); + }); + + test('should call event handler for inst1 and inst2', function(done) { + let isInst1Clicked = false; + let isInst2Clicked = false; + inst1.addEventListener('click', function() { + if (isInst1Clicked) { return; } + isInst1Clicked = true; + clicked(); + }); + inst2.addEventListener('click', function() { + if (isInst2Clicked) { return; } + isInst2Clicked = true; + clicked(); + }); + + function clicked() { + if (isInst1Clicked && isInst1Clicked) { + done(); + } + } + + inst1.emitEvent('click'); + inst2.emitEvent('click'); + }); +}); + +describe('proxy destroy tests...', function() { + var inst, arg1 = 5, arg2 = 4; + var TestInterfaceProxy; + + beforeEach(function() { + TestInterfaceProxy = createProxy(testInterface); + inst = new TestInterfaceProxy(arg1, arg2); + }); + + test('can not use inst after destroy is invoked', function(done) { + inst.destroy(); + + inst.getArg1() + .then(function() { + // shouldn't get here + }) + .catch(function(err) { + expect(err).toBe('method called failed: calling obj is not present') + done(); + }); + }); + + test('destroy from implementation side', function() { + inst.close(); + + inst.getArg1() + .then(function() { + // shouldn't get here + }) + .catch(function(err) { + expect(err).toBe('method called failed: calling obj is not present') + done(); + }); + }); +});