From d1293540e13573eb9ea5f90730bb9c9710c345db Mon Sep 17 00:00:00 2001 From: Jason Bedard Date: Tue, 9 Jun 2015 02:53:22 -0700 Subject: [PATCH] perf(copy): only validate/clear user specified destination Closes #12068 --- src/Angular.js | 169 +++++++++++++++++++++++--------------------- test/AngularSpec.js | 13 ++++ 2 files changed, 103 insertions(+), 79 deletions(-) diff --git a/src/Angular.js b/src/Angular.js index 2dcdd9c6b177..bc33a994ef6f 100644 --- a/src/Angular.js +++ b/src/Angular.js @@ -802,100 +802,111 @@ function arrayRemove(array, value) { */ -function copy(source, destination, stackSource, stackDest) { - if (isWindow(source) || isScope(source)) { - throw ngMinErr('cpws', - "Can't copy! Making copies of Window or Scope instances is not supported."); - } - if (isTypedArray(destination)) { - throw ngMinErr('cpta', - "Can't copy! TypedArray destination cannot be mutated."); - } - - if (!destination) { - destination = source; - if (isObject(source)) { - var index; - if (stackSource && (index = stackSource.indexOf(source)) !== -1) { - return stackDest[index]; - } +function copy(source, destination) { + var stackSource = []; + var stackDest = []; - // TypedArray, Date and RegExp have specific copy functionality and must be - // pushed onto the stack before returning. - // Array and other objects create the base object and recurse to copy child - // objects. The array/object will be pushed onto the stack when recursed. - if (isArray(source)) { - return copy(source, [], stackSource, stackDest); - } else if (isTypedArray(source)) { - destination = new source.constructor(source); - } else if (isDate(source)) { - destination = new Date(source.getTime()); - } else if (isRegExp(source)) { - destination = new RegExp(source.source, source.toString().match(/[^\/]*$/)[0]); - destination.lastIndex = source.lastIndex; - } else if (isFunction(source.cloneNode)) { - destination = source.cloneNode(true); - } else { - var emptyObject = Object.create(getPrototypeOf(source)); - return copy(source, emptyObject, stackSource, stackDest); - } + if (destination) { + if (isTypedArray(destination)) { + throw ngMinErr('cpta', "Can't copy! TypedArray destination cannot be mutated."); + } + if (source === destination) { + throw ngMinErr('cpi', "Can't copy! Source and destination are identical."); + } - if (stackDest) { - stackSource.push(source); - stackDest.push(destination); - } + // Empty the destination object + if (isArray(destination)) { + destination.length = 0; + } else { + forEach(destination, function(value, key) { + if (key !== '$$hashKey') { + delete destination[key]; + } + }); } - } else { - if (source === destination) throw ngMinErr('cpi', - "Can't copy! Source and destination are identical."); - stackSource = stackSource || []; - stackDest = stackDest || []; + stackSource.push(source); + stackDest.push(destination); + return copyRecurse(source, destination); + } - if (isObject(source)) { - stackSource.push(source); - stackDest.push(destination); - } + return copyElement(source); + function copyRecurse(source, destination) { + var h = destination.$$hashKey; var result, key; if (isArray(source)) { - destination.length = 0; - for (var i = 0; i < source.length; i++) { - destination.push(copy(source[i], null, stackSource, stackDest)); + for (var i = 0, ii = source.length; i < ii; i++) { + destination.push(copyElement(source[i])); } - } else { - var h = destination.$$hashKey; - if (isArray(destination)) { - destination.length = 0; - } else { - forEach(destination, function(value, key) { - delete destination[key]; - }); + } else if (isBlankObject(source)) { + // createMap() fast path --- Safe to avoid hasOwnProperty check because prototype chain is empty + for (key in source) { + destination[key] = copyElement(source[key]); } - if (isBlankObject(source)) { - // createMap() fast path --- Safe to avoid hasOwnProperty check because prototype chain is empty - for (key in source) { - destination[key] = copy(source[key], null, stackSource, stackDest); - } - } else if (source && typeof source.hasOwnProperty === 'function') { - // Slow path, which must rely on hasOwnProperty - for (key in source) { - if (source.hasOwnProperty(key)) { - destination[key] = copy(source[key], null, stackSource, stackDest); - } + } else if (source && typeof source.hasOwnProperty === 'function') { + // Slow path, which must rely on hasOwnProperty + for (key in source) { + if (source.hasOwnProperty(key)) { + destination[key] = copyElement(source[key]); } - } else { - // Slowest path --- hasOwnProperty can't be called as a method - for (key in source) { - if (hasOwnProperty.call(source, key)) { - destination[key] = copy(source[key], null, stackSource, stackDest); - } + } + } else { + // Slowest path --- hasOwnProperty can't be called as a method + for (key in source) { + if (hasOwnProperty.call(source, key)) { + destination[key] = copyElement(source[key]); } } - setHashKey(destination,h); } + setHashKey(destination, h); + return destination; + } + + function copyElement(source) { + // Simple values + if (!isObject(source)) { + return source; + } + + // Already copied values + var index = stackSource.indexOf(source); + if (index !== -1) { + return stackDest[index]; + } + + if (isWindow(source) || isScope(source)) { + throw ngMinErr('cpws', + "Can't copy! Making copies of Window or Scope instances is not supported."); + } + + var needsRecurse = false; + var destination; + + if (isArray(source)) { + destination = []; + needsRecurse = true; + } else if (isTypedArray(source)) { + destination = new source.constructor(source); + } else if (isDate(source)) { + destination = new Date(source.getTime()); + } else if (isRegExp(source)) { + destination = new RegExp(source.source, source.toString().match(/[^\/]*$/)[0]); + destination.lastIndex = source.lastIndex; + } else if (isFunction(source.cloneNode)) { + destination = source.cloneNode(true); + } else { + destination = Object.create(getPrototypeOf(source)); + needsRecurse = true; + } + + stackSource.push(source); + stackDest.push(destination); + + return needsRecurse + ? copyRecurse(source, destination) + : destination; } - return destination; } /** diff --git a/test/AngularSpec.js b/test/AngularSpec.js index 0ffc93b3133e..22773cf9c7fd 100644 --- a/test/AngularSpec.js +++ b/test/AngularSpec.js @@ -313,11 +313,19 @@ describe('angular', function() { it('should throw an exception if a Scope is being copied', inject(function($rootScope) { expect(function() { copy($rootScope.$new()); }). toThrowMinErr("ng", "cpws", "Can't copy! Making copies of Window or Scope instances is not supported."); + expect(function() { copy({child: $rootScope.$new()}, {}); }). + toThrowMinErr("ng", "cpws", "Can't copy! Making copies of Window or Scope instances is not supported."); + expect(function() { copy([$rootScope.$new()]); }). + toThrowMinErr("ng", "cpws", "Can't copy! Making copies of Window or Scope instances is not supported."); })); it('should throw an exception if a Window is being copied', function() { expect(function() { copy(window); }). toThrowMinErr("ng", "cpws", "Can't copy! Making copies of Window or Scope instances is not supported."); + expect(function() { copy({child: window}); }). + toThrowMinErr("ng", "cpws", "Can't copy! Making copies of Window or Scope instances is not supported."); + expect(function() { copy([window], []); }). + toThrowMinErr("ng", "cpws", "Can't copy! Making copies of Window or Scope instances is not supported."); }); it('should throw an exception when source and destination are equivalent', function() { @@ -334,6 +342,11 @@ describe('angular', function() { hashKey(src); dst = copy(src); expect(hashKey(dst)).not.toEqual(hashKey(src)); + + src = {foo: {}}; + hashKey(src.foo); + dst = copy(src); + expect(hashKey(src.foo)).not.toEqual(hashKey(dst.foo)); }); it('should retain the previous $$hashKey when copying object with hashKey', function() {