Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Y.Node._instances breaks GC #1637

Closed
wants to merge 7 commits into from
Closed

Y.Node._instances breaks GC #1637

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
798ea49
switch to storing instances on HTMLElement instead of global map
jafl Feb 12, 2014
a741ae5
don't use _instances map in IE10 and up
jafl Feb 12, 2014
cd63b12
consistently use UID; fix comments
jafl Feb 12, 2014
1543690
only compute uid if needed
jafl Feb 12, 2014
672f651
clean up terminology in code
jafl Feb 12, 2014
6303377
update Y.Plugin.Host to work even if loaded after Y.Node's have been …
jafl Feb 12, 2014
7665d37
fix unit tests to work with and without Y.Node._instances
jafl Feb 12, 2014
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions build/node-core/node-core-coverage.js
View file
Open in desktop

Large diffs are not rendered by default.

84 changes: 69 additions & 15 deletions build/node-core/node-core-debug.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ var DOT = '.',
UID = '_yuid',
EMPTY_OBJ = {},

use_instance_map = 0 < Y.UA.ie && Y.UA.ie < 10, // define flag, in case other browsers need it, too

_slice = Array.prototype.slice,

Y_DOM = Y.DOM,
Expand All @@ -48,7 +50,7 @@ var DOT = '.',

var uid = (node.nodeType !== 9) ? node.uniqueID : node[UID];

if (uid && Y_Node._instances[uid] && Y_Node._instances[uid]._node !== node) {
if (use_instance_map && uid && Y_Node._instances[uid] && Y_Node._instances[uid]._node !== node) {
node[UID] = null; // unset existing uid to prevent collision (via clone or hack)
}

Expand Down Expand Up @@ -125,14 +127,17 @@ Y_Node.SHOW_TRANSITION = 'fadeIn';
Y_Node.HIDE_TRANSITION = 'fadeOut';

/**
* A list of Node instances that have been created
* A list of Node instances that have been created. Only defined in browsers
* that already have broken GC, since this global map also breaks GC.
* @private
* @type Object
* @property _instances
* @static
*
*/
Y_Node._instances = {};
if (use_instance_map) {
Y_Node._instances = {};
}

/**
* Retrieves the DOM node bound to a Node instance
Expand Down Expand Up @@ -289,13 +294,24 @@ Y_Node.one = function(node) {
}

if (node.nodeType || Y.DOM.isWindow(node)) { // avoid bad input (numbers, boolean, etc)
uid = (node.uniqueID && node.nodeType !== 9) ? node.uniqueID : node._yuid;
instance = Y_Node._instances[uid]; // reuse exising instances
if (use_instance_map) {
uid = (node.uniqueID && node.nodeType !== 9) ? node.uniqueID : node[UID];
instance = Y_Node._instances[uid]; // reuse exising instances
} else {
instance = node._yui_instances && node._yui_instances[Y._yuid]; // reuse exising instances
}
cachedNode = instance ? instance._node : null;
if (!instance || (cachedNode && node !== cachedNode)) { // new Node when nodes don't match
instance = new Y_Node(node);
if (node.nodeType != 11) { // dont cache document fragment
Y_Node._instances[instance[UID]] = instance; // cache node
if (node.nodeType != 11) { // don't cache document fragment
if (use_instance_map) {
Y_Node._instances[instance[UID]] = instance; // cache node
} else {
if (!node._yui_instances) {
node._yui_instances = {};
}
node._yui_instances[Y._yuid] = instance; // cache node
}
}
}
}
Expand Down Expand Up @@ -377,7 +393,6 @@ Y.mix(Y_Node.prototype, {
str += '.' + className.replace(' ', '.');
}

// TODO: add yuid?
str += ' ' + this[UID];
}
return str;
Expand Down Expand Up @@ -747,7 +762,11 @@ Y.mix(Y_Node.prototype, {

if (recursive) {
Y.NodeList.each(this.all('*'), function(node) {
instance = Y_Node._instances[node[UID]];
if (use_instance_map) {
instance = Y_Node._instances[node[UID]];
} else {
instance = node._yui_instances && node._yui_instances[Y._yuid];
}
if (instance) {
instance.destroy();
} else { // purge in case added by other means
Expand All @@ -756,10 +775,16 @@ Y.mix(Y_Node.prototype, {
});
}

if (this._node._yui_instances) {
delete this._node._yui_instances[Y._yuid];
}

this._node = null;
this._stateProxy = null;

delete Y_Node._instances[this._yuid];
if (use_instance_map) {
delete Y_Node._instances[this[UID]];
}
},

/**
Expand Down Expand Up @@ -928,11 +953,18 @@ NodeList.addMethod = function(name, fn, context) {
args = arguments;

Y.Array.each(this._nodes, function(node) {
var UID = (node.uniqueID && node.nodeType !== 9 ) ? 'uniqueID' : '_yuid',
instance = Y.Node._instances[node[UID]],
var uid,
instance,
ctx,
result;

if (Y.Node._instances) {
uid = (node.uniqueID && node.nodeType !== 9 ) ? node.uniqueID : node[UID];
instance = Y.Node._instances[uid];
} else {
instance = node._yui_instances && node._yui_instances[Y._yuid];
}

if (!instance) {
instance = NodeList._getTempNode(node);
}
Expand Down Expand Up @@ -1021,7 +1053,16 @@ Y.mix(NodeList.prototype, {
var nodelist = this;

Y.Array.each(this._nodes, function(node, index) {
var instance = Y.Node._instances[node[UID]];
var uid,
instance;

if (Y.Node._instances) {
uid = (node.uniqueID && node.nodeType !== 9 ) ? node.uniqueID : node[UID];
instance = Y.Node._instances[uid];
} else {
instance = node._yui_instances && node._yui_instances[Y._yuid];
}

if (!instance) {
instance = NodeList._getTempNode(node);
}
Expand Down Expand Up @@ -1259,19 +1300,32 @@ NodeList.prototype.get = function(attr) {
nodes = this._nodes,
isNodeList = false,
getTemp = NodeList._getTempNode,
uid,
instance,
val;

if (nodes[0]) {
instance = Y.Node._instances[nodes[0]._yuid] || getTemp(nodes[0]);
if (Y.Node._instances) {
uid = (nodes[0].uniqueID && nodes[0].nodeType !== 9 ) ? nodes[0].uniqueID : nodes[0][UID];
instance = Y.Node._instances[uid];
} else {
instance = nodes[0]._yui_instances && nodes[0]._yui_instances[Y._yuid];
}
instance = instance || getTemp(nodes[0]);

val = instance._get(attr);
if (val && val.nodeType) {
isNodeList = true;
}
}

Y.Array.each(nodes, function(node) {
instance = Y.Node._instances[node._yuid];
if (Y.Node._instances) {
uid = (node.uniqueID && node.nodeType !== 9 ) ? node.uniqueID : node[UID];
instance = Y.Node._instances[uid];
} else {
instance = node._yui_instances && node._yui_instances[Y._yuid];
}

if (!instance) {
instance = getTemp(node);
Expand Down
Loading