Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

Commit d129354

Browse files
jbedardpetebacondarwin
authored andcommitted
perf(copy): only validate/clear user specified destination
Closes #12068
1 parent 22f6602 commit d129354

File tree

2 files changed

+103
-79
lines changed

2 files changed

+103
-79
lines changed

src/Angular.js

+90-79
Original file line numberDiff line numberDiff line change
@@ -802,100 +802,111 @@ function arrayRemove(array, value) {
802802
</file>
803803
</example>
804804
*/
805-
function copy(source, destination, stackSource, stackDest) {
806-
if (isWindow(source) || isScope(source)) {
807-
throw ngMinErr('cpws',
808-
"Can't copy! Making copies of Window or Scope instances is not supported.");
809-
}
810-
if (isTypedArray(destination)) {
811-
throw ngMinErr('cpta',
812-
"Can't copy! TypedArray destination cannot be mutated.");
813-
}
814-
815-
if (!destination) {
816-
destination = source;
817-
if (isObject(source)) {
818-
var index;
819-
if (stackSource && (index = stackSource.indexOf(source)) !== -1) {
820-
return stackDest[index];
821-
}
805+
function copy(source, destination) {
806+
var stackSource = [];
807+
var stackDest = [];
822808

823-
// TypedArray, Date and RegExp have specific copy functionality and must be
824-
// pushed onto the stack before returning.
825-
// Array and other objects create the base object and recurse to copy child
826-
// objects. The array/object will be pushed onto the stack when recursed.
827-
if (isArray(source)) {
828-
return copy(source, [], stackSource, stackDest);
829-
} else if (isTypedArray(source)) {
830-
destination = new source.constructor(source);
831-
} else if (isDate(source)) {
832-
destination = new Date(source.getTime());
833-
} else if (isRegExp(source)) {
834-
destination = new RegExp(source.source, source.toString().match(/[^\/]*$/)[0]);
835-
destination.lastIndex = source.lastIndex;
836-
} else if (isFunction(source.cloneNode)) {
837-
destination = source.cloneNode(true);
838-
} else {
839-
var emptyObject = Object.create(getPrototypeOf(source));
840-
return copy(source, emptyObject, stackSource, stackDest);
841-
}
809+
if (destination) {
810+
if (isTypedArray(destination)) {
811+
throw ngMinErr('cpta', "Can't copy! TypedArray destination cannot be mutated.");
812+
}
813+
if (source === destination) {
814+
throw ngMinErr('cpi', "Can't copy! Source and destination are identical.");
815+
}
842816

843-
if (stackDest) {
844-
stackSource.push(source);
845-
stackDest.push(destination);
846-
}
817+
// Empty the destination object
818+
if (isArray(destination)) {
819+
destination.length = 0;
820+
} else {
821+
forEach(destination, function(value, key) {
822+
if (key !== '$$hashKey') {
823+
delete destination[key];
824+
}
825+
});
847826
}
848-
} else {
849-
if (source === destination) throw ngMinErr('cpi',
850-
"Can't copy! Source and destination are identical.");
851827

852-
stackSource = stackSource || [];
853-
stackDest = stackDest || [];
828+
stackSource.push(source);
829+
stackDest.push(destination);
830+
return copyRecurse(source, destination);
831+
}
854832

855-
if (isObject(source)) {
856-
stackSource.push(source);
857-
stackDest.push(destination);
858-
}
833+
return copyElement(source);
859834

835+
function copyRecurse(source, destination) {
836+
var h = destination.$$hashKey;
860837
var result, key;
861838
if (isArray(source)) {
862-
destination.length = 0;
863-
for (var i = 0; i < source.length; i++) {
864-
destination.push(copy(source[i], null, stackSource, stackDest));
839+
for (var i = 0, ii = source.length; i < ii; i++) {
840+
destination.push(copyElement(source[i]));
865841
}
866-
} else {
867-
var h = destination.$$hashKey;
868-
if (isArray(destination)) {
869-
destination.length = 0;
870-
} else {
871-
forEach(destination, function(value, key) {
872-
delete destination[key];
873-
});
842+
} else if (isBlankObject(source)) {
843+
// createMap() fast path --- Safe to avoid hasOwnProperty check because prototype chain is empty
844+
for (key in source) {
845+
destination[key] = copyElement(source[key]);
874846
}
875-
if (isBlankObject(source)) {
876-
// createMap() fast path --- Safe to avoid hasOwnProperty check because prototype chain is empty
877-
for (key in source) {
878-
destination[key] = copy(source[key], null, stackSource, stackDest);
879-
}
880-
} else if (source && typeof source.hasOwnProperty === 'function') {
881-
// Slow path, which must rely on hasOwnProperty
882-
for (key in source) {
883-
if (source.hasOwnProperty(key)) {
884-
destination[key] = copy(source[key], null, stackSource, stackDest);
885-
}
847+
} else if (source && typeof source.hasOwnProperty === 'function') {
848+
// Slow path, which must rely on hasOwnProperty
849+
for (key in source) {
850+
if (source.hasOwnProperty(key)) {
851+
destination[key] = copyElement(source[key]);
886852
}
887-
} else {
888-
// Slowest path --- hasOwnProperty can't be called as a method
889-
for (key in source) {
890-
if (hasOwnProperty.call(source, key)) {
891-
destination[key] = copy(source[key], null, stackSource, stackDest);
892-
}
853+
}
854+
} else {
855+
// Slowest path --- hasOwnProperty can't be called as a method
856+
for (key in source) {
857+
if (hasOwnProperty.call(source, key)) {
858+
destination[key] = copyElement(source[key]);
893859
}
894860
}
895-
setHashKey(destination,h);
896861
}
862+
setHashKey(destination, h);
863+
return destination;
864+
}
865+
866+
function copyElement(source) {
867+
// Simple values
868+
if (!isObject(source)) {
869+
return source;
870+
}
871+
872+
// Already copied values
873+
var index = stackSource.indexOf(source);
874+
if (index !== -1) {
875+
return stackDest[index];
876+
}
877+
878+
if (isWindow(source) || isScope(source)) {
879+
throw ngMinErr('cpws',
880+
"Can't copy! Making copies of Window or Scope instances is not supported.");
881+
}
882+
883+
var needsRecurse = false;
884+
var destination;
885+
886+
if (isArray(source)) {
887+
destination = [];
888+
needsRecurse = true;
889+
} else if (isTypedArray(source)) {
890+
destination = new source.constructor(source);
891+
} else if (isDate(source)) {
892+
destination = new Date(source.getTime());
893+
} else if (isRegExp(source)) {
894+
destination = new RegExp(source.source, source.toString().match(/[^\/]*$/)[0]);
895+
destination.lastIndex = source.lastIndex;
896+
} else if (isFunction(source.cloneNode)) {
897+
destination = source.cloneNode(true);
898+
} else {
899+
destination = Object.create(getPrototypeOf(source));
900+
needsRecurse = true;
901+
}
902+
903+
stackSource.push(source);
904+
stackDest.push(destination);
905+
906+
return needsRecurse
907+
? copyRecurse(source, destination)
908+
: destination;
897909
}
898-
return destination;
899910
}
900911

901912
/**

test/AngularSpec.js

+13
Original file line numberDiff line numberDiff line change
@@ -313,11 +313,19 @@ describe('angular', function() {
313313
it('should throw an exception if a Scope is being copied', inject(function($rootScope) {
314314
expect(function() { copy($rootScope.$new()); }).
315315
toThrowMinErr("ng", "cpws", "Can't copy! Making copies of Window or Scope instances is not supported.");
316+
expect(function() { copy({child: $rootScope.$new()}, {}); }).
317+
toThrowMinErr("ng", "cpws", "Can't copy! Making copies of Window or Scope instances is not supported.");
318+
expect(function() { copy([$rootScope.$new()]); }).
319+
toThrowMinErr("ng", "cpws", "Can't copy! Making copies of Window or Scope instances is not supported.");
316320
}));
317321

318322
it('should throw an exception if a Window is being copied', function() {
319323
expect(function() { copy(window); }).
320324
toThrowMinErr("ng", "cpws", "Can't copy! Making copies of Window or Scope instances is not supported.");
325+
expect(function() { copy({child: window}); }).
326+
toThrowMinErr("ng", "cpws", "Can't copy! Making copies of Window or Scope instances is not supported.");
327+
expect(function() { copy([window], []); }).
328+
toThrowMinErr("ng", "cpws", "Can't copy! Making copies of Window or Scope instances is not supported.");
321329
});
322330

323331
it('should throw an exception when source and destination are equivalent', function() {
@@ -334,6 +342,11 @@ describe('angular', function() {
334342
hashKey(src);
335343
dst = copy(src);
336344
expect(hashKey(dst)).not.toEqual(hashKey(src));
345+
346+
src = {foo: {}};
347+
hashKey(src.foo);
348+
dst = copy(src);
349+
expect(hashKey(src.foo)).not.toEqual(hashKey(dst.foo));
337350
});
338351

339352
it('should retain the previous $$hashKey when copying object with hashKey', function() {

0 commit comments

Comments
 (0)