Data: move element cache to element[expando]

rwaldron · timmywil · commit d702b7637a61 · 2015-03-04T17:26:47.000-05:00
- avoid explicit data.discard() cleanup calls - explicitly remove the data.events property, only when private data exists - reduces code footprint Fixes gh-1734 Close gh-1428
diff --git a/src/data/Data.js b/src/data/Data.js
@@ -5,61 +5,67 @@ define([
 ], function( jQuery, rnotwhite ) {
 
 function Data() {
-	// Support: Android<4,
-	// Old WebKit does not have Object.preventExtensions/freeze method,
-	// return new empty object instead with no [[set]] accessor
-	Object.defineProperty( this.cache = {}, 0, {
-		get: function() {
-			return {};
-		}
-	});
-
 	this.expando = jQuery.expando + Data.uid++;
 }
 
 Data.uid = 1;
 Data.accepts = jQuery.acceptData;
 
 Data.prototype = {
-	key: function( owner ) {
-		// We can accept data for non-element nodes in modern browsers,
-		// but we should not, see #8335.
-		// Always return the key for a frozen object.
-		if ( !Data.accepts( owner ) ) {
-			return 0;
-		}
-
-		// Check if the owner object already has a cache key
-		var unlock = owner[ this.expando ];
 
-		// If not, create one
-		if ( !unlock ) {
-			unlock = Data.uid++;
+	register: function( owner, initial ) {
+		var descriptor = {},
+			value = initial || {};
 
+		try {
 			// If it is a node unlikely to be stringify-ed or looped over
 			// use plain assignment
 			if ( owner.nodeType ) {
-				owner[ this.expando ] = unlock;
+				owner[ this.expando ] = value;
+
 			// Otherwise secure it in a non-enumerable, non-writable property
+			// configurability must be true to allow the property to be
+			// deleted with the delete operator
 			} else {
-				Object.defineProperty( owner, this.expando, { value: unlock } );
+				descriptor[ this.expando ] = {
+					value: value,
+					writable: true,
+					configurable: true
+				};
+				Object.defineProperties( owner, descriptor );
 			}
+
+		// Support: Android < 4
+		// Fallback to a less secure definition
+		} catch ( e ) {
+			descriptor[ this.expando ] = value;
+			jQuery.extend( owner, descriptor );
 		}
 
-		// Ensure the cache object
-		if ( !this.cache[ unlock ] ) {
-			this.cache[ unlock ] = {};
+		return owner[ this.expando ];
+	},
+	cache: function( owner, initial ) {
+		// We can accept data for non-element nodes in modern browsers,
+		// but we should not, see #8335.
+		// Always return an empty object.
+		if ( !Data.accepts( owner ) ) {
+			return {};
 		}
 
-		return unlock;
+		// Check if the owner object already has a cache
+		var cache = owner[ this.expando ];
+
+		// If so, return it
+		if ( cache ) {
+			return cache;
+		}
+
+		// If not, register one
+		return this.register( owner, initial );
 	},
 	set: function( owner, data, value ) {
 		var prop,
-			// There may be an unlock assigned to this node,
-			// if there is no entry for this "owner", create one inline
-			// and set the unlock as though an owner entry had always existed
-			unlock = this.key( owner ),
-			cache = this.cache[ unlock ];
+			cache = this.cache( owner );
 
 		// Handle: [ owner, key, value ] args
 		if ( typeof data === "string" ) {
@@ -69,7 +75,8 @@ Data.prototype = {
 		} else {
 			// Fresh assignments by object are shallow copied
 			if ( jQuery.isEmptyObject( cache ) ) {
-				jQuery.extend( this.cache[ unlock ], data );
+
+				jQuery.extend( cache, data );
 			// Otherwise, copy the properties one-by-one to the cache object
 			} else {
 				for ( prop in data ) {
@@ -80,11 +87,7 @@ Data.prototype = {
 		return cache;
 	},
 	get: function( owner, key ) {
-		// Either a valid cache is found, or will be created.
-		// New caches will be created and the unlock returned,
-		// allowing direct access to the newly created
-		// empty data object. A valid owner object must be provided.
-		var cache = this.cache[ this.key( owner ) ];
+		var cache = this.cache( owner );
 
 		return key === undefined ?
 			cache : cache[ key ];
@@ -125,11 +128,10 @@ Data.prototype = {
 	},
 	remove: function( owner, key ) {
 		var i, name, camel,
-			unlock = this.key( owner ),
-			cache = this.cache[ unlock ];
+			cache = this.cache( owner );
 
 		if ( key === undefined ) {
-			this.cache[ unlock ] = {};
+			this.register( owner );
 
 		} else {
 			// Support array or space separated string of keys
@@ -156,19 +158,19 @@ Data.prototype = {
 			}
 
 			i = name.length;
+
 			while ( i-- ) {
 				delete cache[ name[ i ] ];
 			}
 		}
 	},
 	hasData: function( owner ) {
-		return !jQuery.isEmptyObject(
-			this.cache[ owner[ this.expando ] ] || {}
-		);
+		var cache = owner[ this.expando ];
+		return cache !== undefined && !jQuery.isEmptyObject( cache );
 	},
 	discard: function( owner ) {
 		if ( owner[ this.expando ] ) {
-			delete this.cache[ owner[ this.expando ] ];
+			delete owner[ this.expando ];
 		}
 	}
 };
diff --git a/src/event.js b/src/event.js
@@ -216,8 +216,12 @@ jQuery.event = {
 
 		// Remove the expando if it's no longer used
 		if ( jQuery.isEmptyObject( events ) ) {
+			// Normally this should go through the data api
+			// but since event.js owns these properties,
+			// this exception is made for the sake of optimizing
+			// the operation.
 			delete elemData.handle;
-			dataPriv.remove( elem, "events" );
+			delete elemData.events;
 		}
 	},
 
diff --git a/src/manipulation.js b/src/manipulation.js
@@ -288,34 +288,25 @@ jQuery.extend({
 	},
 
 	cleanData: function( elems ) {
-		var data, elem, type, key,
+		var data, elem, type,
 			special = jQuery.event.special,
 			i = 0;
 
 		for ( ; (elem = elems[ i ]) !== undefined; i++ ) {
-			if ( jQuery.acceptData( elem ) ) {
-				key = elem[ dataPriv.expando ];
-
-				if ( key && (data = dataPriv.cache[ key ]) ) {
-					if ( data.events ) {
-						for ( type in data.events ) {
-							if ( special[ type ] ) {
-								jQuery.event.remove( elem, type );
-
-							// This is a shortcut to avoid jQuery.event.remove's overhead
-							} else {
-								jQuery.removeEvent( elem, type, data.handle );
-							}
+			if ( jQuery.acceptData( elem ) && (data = elem[ dataPriv.expando ])) {
+				if ( data.events ) {
+					for ( type in data.events ) {
+						if ( special[ type ] ) {
+							jQuery.event.remove( elem, type );
+
+						// This is a shortcut to avoid jQuery.event.remove's overhead
+						} else {
+							jQuery.removeEvent( elem, type, data.handle );
 						}
 					}
-					if ( dataPriv.cache[ key ] ) {
-						// Discard any remaining `private` data
-						delete dataPriv.cache[ key ];
-					}
 				}
+				delete data.events;
 			}
-			// Discard any remaining `user` data
-			delete dataUser.cache[ elem[ dataUser.expando ] ];
 		}
 	}
 });
diff --git a/test/unit/manipulation.js b/test/unit/manipulation.js
@@ -64,7 +64,7 @@ test( "text(undefined)", function() {
 
 function testText( valueObj ) {
 
-	expect( 7 );
+	expect( 6 );
 
 	var val, j, expected, $multipleElements, $parentDiv, $childDiv;
 
@@ -94,8 +94,6 @@ function testText( valueObj ) {
 	$parentDiv = jQuery( "<div/>" );
 	$parentDiv.append( $childDiv );
 	$parentDiv.text("Dry off");
-
-	equal( $childDiv.data("leak"), undefined, "Check for leaks (#11809)" );
 }
 
 test( "text(String)", function() {
@@ -1814,7 +1812,7 @@ test( "clone()/html() don't expose jQuery/Sizzle expandos (#12858)", function()
 
 test( "remove() no filters", function() {
 
-  expect( 3 );
+  expect( 2 );
 
 	var first = jQuery("#ap").children().first();
 
@@ -1823,9 +1821,6 @@ test( "remove() no filters", function() {
 	jQuery("#ap").children().remove();
 	ok( jQuery("#ap").text().length > 10, "Check text is not removed" );
 	equal( jQuery("#ap").children().length, 0, "Check remove" );
-
-	equal( first.data("foo"), null, "first data" );
-
 });
 
 test( "remove() with filters", function() {
