From 0e982433eb94391b3e9f6838d9b8fbf9bb31abf9 Mon Sep 17 00:00:00 2001
From: Jason Bedard <jason+github@jbedard.ca>
Date: Sat, 18 Jul 2015 11:27:11 -0700
Subject: [PATCH] Data: avoid using delete on DOM nodes

Closes gh-2479
---
 src/data/Data.js          | 18 +++++++++++++-----
 src/manipulation.js       | 10 ++++++++--
 test/unit/data.js         | 19 ++++++++++++++++++-
 test/unit/event.js        |  7 +++++--
 test/unit/manipulation.js |  4 ++--
 5 files changed, 46 insertions(+), 12 deletions(-)

diff --git a/src/data/Data.js b/src/data/Data.js
index e0d1eadcd..625d203cf 100644
--- a/src/data/Data.js
+++ b/src/data/Data.js
@@ -20,13 +20,12 @@ Data.prototype = {
 		if ( owner.nodeType ) {
 			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
+		// Otherwise secure it in a non-enumerable property
+		// configurable must be true to allow the property to be
+		// deleted when data is removed
 		} else {
 			Object.defineProperty( owner, this.expando, {
 				value: value,
-				writable: true,
 				configurable: true
 			} );
 		}
@@ -144,7 +143,16 @@ Data.prototype = {
 
 		// Remove the expando if there's no more data
 		if ( key === undefined || jQuery.isEmptyObject( cache ) ) {
-			delete owner[ this.expando ];
+
+			// Support: Chrome <= 35-45+
+			// Webkit & Blink performance suffers when deleting properties
+			// from DOM nodes, so set to undefined instead
+			// https://code.google.com/p/chromium/issues/detail?id=378607
+			if ( owner.nodeType ) {
+				owner[ this.expando ] = undefined;
+			} else {
+				delete owner[ this.expando ];
+			}
 		}
 	},
 	hasData: function( owner ) {
diff --git a/src/manipulation.js b/src/manipulation.js
index b85ef1824..ceb970cbd 100644
--- a/src/manipulation.js
+++ b/src/manipulation.js
@@ -290,10 +290,16 @@ jQuery.extend( {
 							}
 						}
 					}
-					delete elem[ dataPriv.expando ];
+
+					// Support: Chrome <= 35-45+
+					// Assign undefined instead of using delete, see Data#remove
+					elem[ dataPriv.expando ] = undefined;
 				}
 				if ( elem[ dataUser.expando ] ) {
-					delete elem[ dataUser.expando ];
+
+					// Support: Chrome <= 35-45+
+					// Assign undefined instead of using delete, see Data#remove
+					elem[ dataUser.expando ] = undefined;
 				}
 			}
 		}
diff --git a/test/unit/data.js b/test/unit/data.js
index 53cd3a4bb..9b0e2314c 100644
--- a/test/unit/data.js
+++ b/test/unit/data.js
@@ -867,7 +867,7 @@ testIframeWithCallback(
 );
 
 QUnit.test( "Check that the expando is removed when there's no more data", function( assert ) {
-	assert.expect( 1 );
+	assert.expect( 2 );
 
 	var key,
 		div = jQuery( "<div/>" );
@@ -877,6 +877,23 @@ QUnit.test( "Check that the expando is removed when there's no more data", funct
 
 	// Make sure the expando is gone
 	for ( key in div[ 0 ] ) {
+		if ( /^jQuery/.test( key ) ) {
+			assert.strictEqual( div[ 0 ][ key ], undefined, "Expando was not removed when there was no more data" );
+		}
+	}
+});
+
+QUnit.test( "Check that the expando is removed when there's no more data on non-nodes", function( assert ) {
+	assert.expect( 1 );
+
+	var key,
+		obj = jQuery( {key: 42} );
+	obj.data( "some", "data" );
+	assert.equal( obj.data( "some" ), "data", "Data is added" );
+	obj.removeData( "some" );
+
+	// Make sure the expando is gone
+	for ( key in obj[ 0 ] ) {
 		if ( /^jQuery/.test( key ) ) {
 			assert.ok( false, "Expando was not removed when there was no more data" );
 		}
diff --git a/test/unit/event.js b/test/unit/event.js
index f932317d2..0f0c9e6df 100644
--- a/test/unit/event.js
+++ b/test/unit/event.js
@@ -2702,7 +2702,7 @@ QUnit.test( "Inline event result is returned (#13993)", function( assert ) {
 } );
 
 QUnit.test( ".off() removes the expando when there's no more data", function( assert ) {
-	assert.expect( 1 );
+	assert.expect( 2 );
 
 	var key,
 		div = jQuery( "<div/>" ).appendTo( "#qunit-fixture" );
@@ -2717,7 +2717,10 @@ QUnit.test( ".off() removes the expando when there's no more data", function( as
 	// Make sure the expando is gone
 	for ( key in div[ 0 ] ) {
 		if ( /^jQuery/.test( key ) ) {
-			assert.ok( false, "Expando was not removed when there was no more data" );
+			assert.strictEqual(
+				div[ 0 ][ key ], undefined,
+				"Expando was not removed when there was no more data"
+			);
 		}
 	}
 } );
diff --git a/test/unit/manipulation.js b/test/unit/manipulation.js
index d9f1b0247..37da12062 100644
--- a/test/unit/manipulation.js
+++ b/test/unit/manipulation.js
@@ -2086,7 +2086,7 @@ QUnit.test( "jQuery.cleanData eliminates all private data (gh-2127)", function(
 } );
 
 QUnit.test( "jQuery.cleanData eliminates all public data", function( assert ) {
-	assert.expect( 2 );
+	assert.expect( 3 );
 
 	var key,
 		div = jQuery( "<div/>" );
@@ -2100,7 +2100,7 @@ QUnit.test( "jQuery.cleanData eliminates all public data", function( assert ) {
 	// Make sure the expando is gone
 	for ( key in div[ 0 ] ) {
 		if ( /^jQuery/.test( key ) ) {
-			assert.ok( false, "Expando was not removed when there was no more data" );
+			assert.strictEqual( div[ 0 ][ key ], undefined, "Expando was not removed when there was no more data" );
 		}
 	}
 } );
-- 
2.39.5