From 6659fd5d30f8023aaf27edc5ef4e00de470ba4ad Mon Sep 17 00:00:00 2001 From: Misko Hevery Date: Tue, 24 Nov 2015 23:16:50 -0800 Subject: [PATCH] feat(wtf): add wtf support to (set/clear)Timeout/Interval/Immediate This rewrites the (set/clear)Timeout/Interval/Immediate method to render information in WTF. It also makes the code more explicit, by removing the need for bind method and instead creating explicit callbacks. The resulting code is faster, and easier to read and reason about. --- karma-browserify.conf.js | 1 + karma-microtasks.conf.js | 1 + karma.conf.js | 1 + lib/core.js | 31 +++++++- lib/patch/browser.js | 6 +- lib/patch/functions.js | 133 +++++++++++++++++++-------------- lib/utils.js | 10 --- lib/wtf.js | 51 +++++++++++++ test/patch/setInterval.spec.js | 31 +++++++- test/patch/setTimeout.spec.js | 31 ++++++-- test/wtf_mock.js | 72 ++++++++++++++++++ test/zone.spec.js | 16 +--- 12 files changed, 291 insertions(+), 93 deletions(-) create mode 100644 lib/wtf.js create mode 100644 test/wtf_mock.js diff --git a/karma-browserify.conf.js b/karma-browserify.conf.js index a473ae725..4d1929766 100644 --- a/karma-browserify.conf.js +++ b/karma-browserify.conf.js @@ -5,6 +5,7 @@ module.exports = function (config) { basePath: '', files: [ 'test/util.js', + 'test/wtf_mock.js', 'test/commonjs.spec.js', {pattern: 'test/assets/**/*.*', watched: true, served: true, included: false}, {pattern: 'lib/**/*.js', watched: true, served: false, included: false} diff --git a/karma-microtasks.conf.js b/karma-microtasks.conf.js index e78f662b0..a9901ae12 100644 --- a/karma-microtasks.conf.js +++ b/karma-microtasks.conf.js @@ -5,6 +5,7 @@ module.exports = function (config) { basePath: '', files: [ 'test/util.js', + 'test/wtf_mock.js', 'test/setup-microtask.js', 'examples/js/*.js', 'test/**/*.spec.js', diff --git a/karma.conf.js b/karma.conf.js index 7f3d57225..d599740dc 100644 --- a/karma.conf.js +++ b/karma.conf.js @@ -5,6 +5,7 @@ module.exports = function (config) { basePath: '', files: [ 'test/util.js', + 'test/wtf_mock.js', 'test/setup.js', 'example/js/*.js', //'test/lib/brick.js', diff --git a/lib/core.js b/lib/core.js index 1e39d1cc7..17e6fdf54 100644 --- a/lib/core.js +++ b/lib/core.js @@ -2,6 +2,16 @@ var keys = require('./keys'); +var deprecated = {}; + +function deprecatedWarning(key, text) { + if (!deprecated.hasOwnProperty(key)) { + deprecated[key] = true; + console.warn("DEPRECATION WARNING: '" + key + + "' is no longer supported and will be removed in next major release. " + text); + } +} + function Zone(parentZone, data) { var zone = (arguments.length) ? Object.create(parentZone) : this; @@ -70,7 +80,9 @@ Zone.prototype = { }; }, + /// @deprecated bindOnce: function (fn) { + deprecatedWarning('bindOnce', 'There is no replacement.'); var boundZone = this; return this.bind(function () { var result = fn.apply(this, arguments); @@ -120,8 +132,23 @@ Zone.prototype = { beforeTask: function () {}, onZoneCreated: function () {}, afterTask: function () {}, - enqueueTask: function () {}, - dequeueTask: function () {}, + + enqueueTask: function () { + deprecatedWarning('enqueueTask', 'Use addTask/addRepeatingTask/addMicroTask'); + }, + dequeueTask: function () { + deprecatedWarning('dequeueTask', 'Use removeTask/removeRepeatingTask/removeMicroTask'); + }, + + addTask: function(taskFn) { this.enqueueTask(taskFn); }, + removeTask: function(taskFn) { this.dequeueTask(taskFn); }, + + addRepeatingTask: function(taskFn) { this.enqueueTask(taskFn); }, + removeRepeatingTask: function(taskFn) { this.dequeueTask(taskFn); }, + + addMicrotask: function(taskFn) { this.enqueueTask(taskFn); }, + removeMicrotask: function(taskFn) { this.dequeueTask(taskFn); }, + addEventListener: function () { return this[keys.common.addEventListener].apply(this, arguments); }, diff --git a/lib/patch/browser.js b/lib/patch/browser.js index 1ce22febd..731f40d27 100644 --- a/lib/patch/browser.js +++ b/lib/patch/browser.js @@ -13,9 +13,9 @@ var fileReaderPatch = require('./file-reader'); function apply() { fnPatch.patchSetClearFunction(global, [ - 'timeout', - 'interval', - 'immediate' + 'Timeout', + 'Interval', + 'Immediate' ]); fnPatch.patchRequestAnimationFrame(global, [ diff --git a/lib/patch/functions.js b/lib/patch/functions.js index e181487d6..4148db5c0 100644 --- a/lib/patch/functions.js +++ b/lib/patch/functions.js @@ -1,58 +1,98 @@ 'use strict'; var utils = require('../utils'); +var wtf = require('../wtf'); +var Zone = require('../core').Zone; -function patchSetClearFunction(obj, fnNames) { - fnNames.map(function (name) { - return name[0].toUpperCase() + name.substr(1); - }).forEach(function (name) { +function patchSetClearFunction(window, fnNames) { + fnNames.forEach(function (name) { + var repeating = name == 'Interval'; var setName = 'set' + name; - var delegate = obj[setName]; - - if (delegate) { - var clearName = 'clear' + name; - var ids = {}; + var clearName = 'clear' + name; + var setNative = window[setName]; + var clearNative = window[clearName]; + var ids = {}; + + if (setNative) { + var wtfSetEventFn = wtf.createEvent('Zone#' + setName + '(uint32 zone, uint32 id, uint32 delay)'); + var wtfClearEventFn = wtf.createEvent('Zone#' + clearName + '(uint32 zone, uint32 id)'); + var wtfCallbackFn = wtf.createScope('Zone#cb:' + name + '(uint32 zone, uint32 id, uint32 delay)'); + + // Forward all calls from the window through the zone. + window[setName] = function () { + return global.zone[setName].apply(global.zone, arguments); + }; + window[clearName] = function () { + return global.zone[clearName].apply(global.zone, arguments); + }; - var bindArgs = setName === 'setInterval' ? utils.bindArguments : utils.bindArgumentsOnce; - global.zone[setName] = function (fn) { - var id, fnRef = fn; - arguments[0] = function () { - delete ids[id]; - return fnRef.apply(this, arguments); + // Set up zone processing for the set function. + Zone.prototype[setName] = function (fn, delay) { + // We need to save `fn` in var different then argument. This is because + // in IE9 `argument[0]` and `fn` have same identity, and assigning to + // `argument[0]` changes `fn`. + var callbackFn = fn; + if (typeof callbackFn !== 'function') { + // force the error by calling the method with wrong args + setNative.apply(window, arguments); + } + var zone = this; + var setId = null; + // wrap the callback function into the zone. + arguments[0] = function() { + var callbackZone = zone.isRootZone() || isRaf ? zone : zone.fork(); + var callbackThis = this; + var callbackArgs = arguments; + return wtf.leaveScope( + wtfCallbackFn(callbackZone.$id, setId, delay), + callbackZone.run(function() { + if (!repeating) { + delete ids[setId]; + callbackZone.removeTask(callbackFn); + } + return callbackFn.apply(callbackThis, callbackArgs); + }) + ); }; - var args = bindArgs(arguments); - id = delegate.apply(obj, args); - ids[id] = true; - return id; + if (repeating) { + zone.addRepeatingTask(callbackFn); + } else { + zone.addTask(callbackFn); + } + setId = setNative.apply(window, arguments); + ids[setId] = callbackFn; + wtfSetEventFn(zone.$id, setId, delay); + return setId; }; - obj[setName] = function () { - return global.zone[setName].apply(this, arguments); + Zone.prototype[setName + 'Unpatched'] = function() { + return setNative.apply(window, arguments); }; - var clearDelegate = obj[clearName]; - - global.zone[clearName] = function (id) { - if (ids[id]) { + // Set up zone processing for the clear function. + Zone.prototype[clearName] = function (id) { + var scope = wtfClearEventFn(this.$id, id); + if (ids.hasOwnProperty(id)) { + var callbackFn = ids[id]; delete ids[id]; - global.zone.dequeueTask(); + if (repeating) { + this.removeRepeatingTask(callbackFn); + } else { + this.removeTask(callbackFn); + } } - return clearDelegate.apply(this, arguments); + return clearNative.apply(window, arguments); }; - global.zone[setName + 'Unpatched'] = function() { - return delegate.apply(obj, arguments); + Zone.prototype[clearName + 'Unpatched'] = function() { + return clearNative.apply(window, arguments); }; - global.zone[clearName + 'Unpatched'] = function() { - return clearDelegate.apply(obj, arguments); - }; - - obj[clearName] = function () { - return global.zone[clearName].apply(this, arguments); - }; } + } + fnNames.forEach(function(args) { + patchMacroTaskMethod.apply(null, args); }); }; @@ -83,26 +123,6 @@ function patchRequestAnimationFrame(obj, fnNames) { }); }; -function patchSetFunction(obj, fnNames) { - fnNames.forEach(function (name) { - var delegate = obj[name]; - - if (delegate) { - global.zone[name] = function (fn) { - arguments[0] = function () { - return fn.apply(this, arguments); - }; - var args = utils.bindArgumentsOnce(arguments); - return delegate.apply(obj, args); - }; - - obj[name] = function () { - return zone[name].apply(this, arguments); - }; - } - }); -}; - function patchFunction(obj, fnNames) { fnNames.forEach(function (name) { var delegate = obj[name]; @@ -119,7 +139,6 @@ function patchFunction(obj, fnNames) { module.exports = { patchSetClearFunction: patchSetClearFunction, - patchSetFunction: patchSetFunction, patchRequestAnimationFrame: patchRequestAnimationFrame, patchFunction: patchFunction }; diff --git a/lib/utils.js b/lib/utils.js index b7ed30caf..11c73a7a3 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -11,15 +11,6 @@ function bindArguments(args) { return args; }; -function bindArgumentsOnce(args) { - for (var i = args.length - 1; i >= 0; i--) { - if (typeof args[i] === 'function') { - args[i] = global.zone.bindOnce(args[i]); - } - } - return args; -}; - function patchPrototype(obj, fnNames) { fnNames.forEach(function (name) { var delegate = obj[name]; @@ -199,7 +190,6 @@ function patchClass(className) { module.exports = { bindArguments: bindArguments, - bindArgumentsOnce: bindArgumentsOnce, patchPrototype: patchPrototype, patchProperty: patchProperty, patchProperties: patchProperties, diff --git a/lib/wtf.js b/lib/wtf.js new file mode 100644 index 000000000..183d3cfbc --- /dev/null +++ b/lib/wtf.js @@ -0,0 +1,51 @@ +'use strict'; + +// Detect and setup WTF. +var wtfTrace = null; +var wtfEvents = null; +var wtfEnabled = (function () { + var wtf = global['wtf']; + if (wtf) { + wtfTrace = wtf['trace']; + if (wtfTrace) { + wtfEvents = wtfTrace['events']; + return true; + } + } + return false; +})(); + +function noop() { +} + +if (wtfEnabled) { + module.exports = { + enabled: true, + createScope: function (signature, flags) { + return wtfEvents.createScope(signature, flags); + }, + createEvent: function (signature, flags) { + return wtfEvents.createInstance(signature, flags); + }, + leaveScope: function (scope, returnValue) { + wtfTrace.leaveScope(scope, returnValue); + return returnValue; + }, + beginTimeRange:function (rangeType, action) { + return wtfTrace.beginTimeRange(rangeType, action); + }, + endTimeRange: function (range) { + wtfTrace.endTimeRange(range); + } + }; +} else { + module.exports = { + enabled: false, + createScope: function (s, f) { return noop; }, + createEvent: function (s, f) { return noop; }, + leaveScope: function (s, v) { return v; }, + beginTimeRange: function (t, a) { return null; }, + endTimeRange: function (r) { } + }; +} + diff --git a/test/patch/setInterval.spec.js b/test/patch/setInterval.spec.js index ceadf00cf..de51a1f97 100644 --- a/test/patch/setInterval.spec.js +++ b/test/patch/setInterval.spec.js @@ -3,16 +3,39 @@ describe('setInterval', function () { it('should work with setInterval', function (done) { - var testZone = zone.fork(); + var testZone = zone.fork({ + addTask: function(fn) { wtfMock.log.push('addTask ' + fn.id ); }, + removeTask: function(fn) { wtfMock.log.push('removeTask ' + fn.id); }, + addRepeatingTask: function(fn) { wtfMock.log.push('addRepeatingTask ' + fn.id); }, + removeRepeatingTask: function(fn) { wtfMock.log.push('removeRepeatingTask ' + fn.id); }, + }); + var zId; + var setIntervalId = '?'; testZone.run(function() { - var cancelId = setInterval(function() { + zId = zone.$id; + var intervalFn = function () { + var zCallbackId = zone.$id; // creates implied zone in all callbacks. expect(zone).toBeDirectChildOf(testZone); clearInterval(cancelId); - done(); - }, 10); + zone.setTimeoutUnpatched(function() { + expect(wtfMock.log).toEqual([ + 'addRepeatingTask abc', + '# Zone#setInterval(' + zId + ', ' + cancelId + ', 10)', + '> Zone#cb:Interval(' + zCallbackId + ', ' + cancelId + ', 10)', + '# Zone#clearInterval(' + zCallbackId + ', ' + cancelId + ')', + 'removeRepeatingTask abc', + '< Zone#cb:Interval' + ]); + done(); + }); + }; + intervalFn.id = 'abc'; + var cancelId = setInterval(intervalFn, 10); + expect(wtfMock.log[0]).toEqual('addRepeatingTask abc'); + expect(wtfMock.log[1]).toEqual('# Zone#setInterval(' + zId + ', ' + cancelId + ', 10)'); }); }); diff --git a/test/patch/setTimeout.spec.js b/test/patch/setTimeout.spec.js index 8f7d5300b..2bb6f2973 100644 --- a/test/patch/setTimeout.spec.js +++ b/test/patch/setTimeout.spec.js @@ -4,15 +4,36 @@ describe('setTimeout', function () { it('should work with setTimeout', function (done) { - var testZone = zone.fork(); + var testZone = zone.fork({ + addTask: function(fn) { wtfMock.log.push('addTask ' + fn.id ); }, + removeTask: function(fn) { wtfMock.log.push('removeTask ' + fn.id); }, + addRepeatingTask: function(fn) { wtfMock.log.push('addRepeatingTask ' + fn.id); }, + removeRepeatingTask: function(fn) { wtfMock.log.push('removeRepeatingTask ' + fn.id); }, + }); + var zId; + var cancelId = '?'; testZone.run(function() { - - setTimeout(function() { + zId = zone.$id; + var timeoutFn = function () { + var zCallbackId = zone.$id; // creates implied zone in all callbacks. expect(zone).toBeDirectChildOf(testZone); - done(); - }, 0); + zone.setTimeoutUnpatched(function() { + expect(wtfMock.log).toEqual([ + 'addTask abc', + '# Zone#setTimeout(' + zId + ', ' + cancelId + ', 3)', + '> Zone#cb:Timeout(' + zCallbackId + ', ' + cancelId + ', 3)', + 'removeTask abc', + '< Zone#cb:Timeout' + ]); + done(); + }); + }; + timeoutFn.id = 'abc'; + cancelId = setTimeout(timeoutFn, 3); + expect(wtfMock.log[0]).toEqual('addTask abc'); + expect(wtfMock.log[1]).toEqual('# Zone#setTimeout(' + zId + ', ' + cancelId + ', 3)'); }); }); diff --git a/test/wtf_mock.js b/test/wtf_mock.js new file mode 100644 index 000000000..82846e0eb --- /dev/null +++ b/test/wtf_mock.js @@ -0,0 +1,72 @@ +'use strict'; + +var log = []; +var logArgs = []; +var wtfMock = { + log: log, + logArgs: logArgs, + reset: function () { + log.length = 0; + logArgs.length = 0; + }, + trace: { + leaveScope: function(scope, returnValue) { + return scope(returnValue); + }, + beginTimeRange: function(type, action) { + logArgs.push([]); + log.push('>>> ' + type + '[' + action + ']'); + return function() { + logArgs.push([]); + log.push('<<< ' + type); + } + }, + endTimeRange: function(range) { + range(); + }, + events: { + createScope: function(signature, flags) { + var parts = signature.split('('); + var name = parts[0]; + return function scopeFn() { + var args = []; + for(var i = arguments.length - 1; i >= 0; i--) { + var arg = arguments[i]; + if (arg !== undefined) { + args.unshift(arg); + } + } + log.push('> ' + name + '(' + args.join(', ') + ')'); + logArgs.push(args); + return function (retValue) { + log.push('< ' + name + (retValue == undefined ? '' : ' => ' + retValue)); + logArgs.push(retValue); + return retValue; + }; + }; + }, + createInstance: function(signature, flags) { + var parts = signature.split('('); + var name = parts[0]; + return function eventFn() { + var args = []; + for(var i = arguments.length - 1; i >= 0; i--) { + var arg = arguments[i]; + if (arg !== undefined) { + args.unshift(arg); + } + } + log.push('# ' + name + '(' + args.join(', ') + ')'); + logArgs.push(args); + }; + } + } + }, +} + +beforeEach(function() { + wtfMock.reset(); +}); + +window.wtfMock = wtfMock; +window.wtf = wtfMock; \ No newline at end of file diff --git a/test/zone.spec.js b/test/zone.spec.js index 62e39c124..b9a5e21c3 100644 --- a/test/zone.spec.js +++ b/test/zone.spec.js @@ -41,15 +41,12 @@ describe('Zone', function () { }); - it('should fire onZoneCreated when a zone is forked', function (done) { + it('should fire onZoneCreated when a zone is forked', function () { var createdSpy = jasmine.createSpy(); var counter = 0; var myZone = zone.fork({ onZoneCreated: function () { counter += 1; - }, - afterTask: function () { - counter -= 1; } }); @@ -57,15 +54,9 @@ describe('Zone', function () { expect(counter).toBe(0); - setTimeout(function () {}, 0); - expect(counter).toBe(1); + myZone.fork(); - setTimeout(function () { - expect(counter).toBe(0); - setTimeout(done, 5); - expect(counter).toBe(1); - }, 0); - expect(counter).toBe(2); + expect(counter).toBe(1); }); }); @@ -209,6 +200,7 @@ describe('Zone', function () { it('should execute all callbacks from root zone without forking zones', function(done) { // using setTimeout for the test which relies on patching via bind + expect(zone.isRootZone()).toBe(true); setTimeout(function() { expect(zone.isRootZone()).toBe(true); done();