From c8239c08d7ad7d375683fd85745b30da789bb591 Mon Sep 17 00:00:00 2001 From: Julien Gilli Date: Fri, 13 Feb 2015 14:38:59 -0800 Subject: [PATCH] console: allow Object.prototype fields as labels This is a backport of 6c3647c38d73f729ce85633a0440cd939d93dea2 from v0.12 to v0.10. Console.prototype.timeEnd() returns NaN if the timer label corresponds to a property on Object.prototype. In v0.12, this was fixed by using Object.create(null) to construct the _times object However, the version of V8 in the v0.10 branch makes this fix not work as expected. In v0.10, this commit changes the _times object into a array of objects of the form: { label: someLabel, time: staringWallClockTime } someLabel can thus be any string, including any string that represents any Object.prototype field. Fixes #9116. PR: #9215 PR-URL: https://github.com/joyent/node/pull/9215 Reviewed-By: Colin Ihrig --- lib/console.js | 41 ++++++++++++++++++++++++++++++------- test/simple/test-console.js | 29 ++++++++++++++++++-------- 2 files changed, 55 insertions(+), 15 deletions(-) diff --git a/lib/console.js b/lib/console.js index 51d5a36f92b..3d22dcded23 100644 --- a/lib/console.js +++ b/lib/console.js @@ -20,6 +20,7 @@ // USE OR OTHER DEALINGS IN THE SOFTWARE. var util = require('util'); +var assert = require('assert'); function Console(stdout, stderr) { if (!(this instanceof Console)) { @@ -40,7 +41,7 @@ function Console(stdout, stderr) { Object.defineProperty(this, '_stdout', prop); prop.value = stderr; Object.defineProperty(this, '_stderr', prop); - prop.value = {}; + prop.value = []; Object.defineProperty(this, '_times', prop); // bind the prototype functions to this Console instance @@ -70,18 +71,44 @@ Console.prototype.dir = function(object) { }; +function findTimeWithLabel(times, label) { + assert(Array.isArray(times), 'times must be an Array'); + assert(typeof label === 'string', 'label must be a string'); + + var found; + + times.some(function findTime(item) { + if (item.label === label) { + found = item; + return true; + } + }); + + return found; +} + Console.prototype.time = function(label) { - this._times[label] = Date.now(); + var timeEntry = findTimeWithLabel(this._times, label); + var wallClockTime = Date.now(); + if (!timeEntry) { + this._times.push({ label: label, time: wallClockTime }); + } else { + timeEntry.time = wallClockTime; + } }; - Console.prototype.timeEnd = function(label) { - var time = this._times[label]; - if (!time) { + var wallClockTimeEnd = Date.now(); + var timeEntry = findTimeWithLabel(this._times, label); + + if (!timeEntry) { throw new Error('No such label: ' + label); + } else { + assert(typeof timeEntry.time === 'number', + 'start time must be a number'); + var duration = wallClockTimeEnd - timeEntry.time; + this.log('%s: %dms', label, duration); } - var duration = Date.now() - time; - this.log('%s: %dms', label, duration); }; diff --git a/test/simple/test-console.js b/test/simple/test-console.js index bef8772a8f2..2c45a06bb9b 100644 --- a/test/simple/test-console.js +++ b/test/simple/test-console.js @@ -46,6 +46,22 @@ console.log({slashes: '\\\\'}); console._stderr = process.stdout; console.trace('This is a %j %d', { formatted: 'trace' }, 10, 'foo'); +assert.throws(function () { + console.timeEnd('no such label'); +}); + +assert.doesNotThrow(function () { + console.time('label'); + console.timeEnd('label'); +}); + +console.time('__proto__'); +console.timeEnd('__proto__'); +console.time('constructor'); +console.timeEnd('constructor'); +console.time('hasOwnProperty'); +console.timeEnd('hasOwnProperty'); + global.process.stdout.write = stdout_write; assert.equal('foo\n', strings.shift()); @@ -55,11 +71,8 @@ assert.equal("{ slashes: '\\\\\\\\' }\n", strings.shift()); assert.equal('Trace: This is a {"formatted":"trace"} 10 foo', strings.shift().split('\n').shift()); -assert.throws(function () { - console.timeEnd('no such label'); -}); - -assert.doesNotThrow(function () { - console.time('label'); - console.timeEnd('label'); -}); +assert.ok(/^label: \d+ms$/.test(strings.shift().trim())); +assert.ok(/^__proto__: \d+ms$/.test(strings.shift().trim())); +assert.ok(/^constructor: \d+ms$/.test(strings.shift().trim())); +assert.ok(/^hasOwnProperty: \d+ms$/.test(strings.shift().trim())); +assert.equal(strings.length, 0);