Skip to content
This repository has been archived by the owner on Mar 13, 2018. It is now read-only.

Commit

Permalink
ensure bindings and observers are not leaked
Browse files Browse the repository at this point in the history 
R=arv
BUG=

Review URL: https://codereview.appspot.com/9837045
  • Loading branch information
@rafaelw
rafaelw committed May 28, 2013
1 parent 425f436 commit 361fa3e
Show file tree
Hide file tree
Showing 5 changed files with 67 additions and 29 deletions.
22 changes: 16 additions & 6 deletions src/template_element.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,9 @@

Binding.prototype = {
dispose: function() {
if (this.model && typeof this.model.dispose == 'function')
this.model.dispose();

this.observer.close();
},

Expand Down Expand Up @@ -901,8 +904,8 @@
if (!templateIterator)
break;

// the template iterator will clear() and unobserve() if
// its resolveInputs() is called and its inputs.size is 0.
// the template iterator will remove its instances and
// abandon() itself if its inputs.size is 0.
templateIterator.inputs.unbind(name);
break;
default:
Expand Down Expand Up @@ -1213,13 +1216,16 @@
};

function TemplateIterator(templateElement) {
this.constructing = true;
this.templateElement_ = templateElement;
this.terminators = [];
this.iteratedValue = undefined;
this.arrayObserver = undefined;
this.boundHandleSplices = this.handleSplices.bind(this);
this.inputs = new CompoundBinding(this.resolveInputs.bind(this));
this.valueBinding = new Binding(this.inputs, 'value', this.valueChanged.bind(this));
this.valueBinding = new Binding(this.inputs, 'value',
this.valueChanged.bind(this));
this.constructing = false;
}

TemplateIterator.prototype = {
Expand Down Expand Up @@ -1249,10 +1255,14 @@
removed: Array.isArray(oldValue) ? oldValue : []
};

if (!splice.addedCount && !splice.removed.length)
return; // nothing to do.
if (splice.addedCount || splice.removed.length)
this.handleSplices([splice]);

this.handleSplices([splice]);
if (!this.constructing && !this.inputs.size) {
// End iteration
templateIteratorTable.delete(this);
this.abandon();
}
},

getTerminatorAt: function(index) {
Expand Down
48 changes: 27 additions & 21 deletions tests/node_bindings.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,22 @@
// webkit.
var testDiv;

function unbindAll(node) {
node.unbindAll();
for (var child = node.firstChild; child; child = child.nextSibling)
unbindAll(child);
}

function doSetup() {
testDiv = document.body.appendChild(document.createElement('div'));
}

function doTeardown() {
assert.isFalse(!!Observer._errorThrownDuringCallback);
document.body.removeChild(testDiv);
unbindAll(testDiv);
Platform.performMicrotaskCheckpoint();
assert.strictEqual(2, Observer._allObserversCount);
}

function dispatchEvent(type, target) {
Expand Down Expand Up @@ -56,14 +66,14 @@ suite('Text bindings', function() {
});

test('Path unreachable', function() {
var text = document.createTextNode('hi');
var text = testDiv.appendChild(document.createTextNode('hi'));
var model = {};
text.bind('textContent', model, 'a');
assert.strictEqual(text.data, '');
});

test('Compound Binding', function() {
var text = document.createTextNode('hi');
var text = testDiv.appendChild(document.createTextNode('hi'));
var model = {a: 1, b: 2};
text.textContent = '{{a}} and {{b}}';
HTMLTemplateElement.bindAllMustachesFrom_(text, model);
Expand All @@ -83,7 +93,7 @@ suite('Element attribute bindings', function() {
teardown(doTeardown);

test('Basic', function() {
var el = document.createElement('div');
var el = testDiv.appendChild(document.createElement('div'));
var model = {a: '1'};
el.bind('foo', model, 'a');
Platform.performMicrotaskCheckpoint();
Expand Down Expand Up @@ -111,7 +121,7 @@ suite('Element attribute bindings', function() {
});

test('Dashes', function() {
var el = document.createElement('div');
var el = testDiv.appendChild(document.createElement('div'));
var model = {a: '1'};
el.bind('foo-bar', model, 'a');
Platform.performMicrotaskCheckpoint();
Expand All @@ -123,7 +133,7 @@ suite('Element attribute bindings', function() {
});

test('Element.id, Element.hidden?', function() {
var element = document.createElement('div');
var element = testDiv.appendChild(document.createElement('div'));
var model = {a: 1, b: 2};
element.bind('hidden?', model, 'a');
element.bind('id', model, 'b');
Expand All @@ -145,14 +155,14 @@ suite('Element attribute bindings', function() {
});

test('Element.id - path unreachable', function() {
var element = document.createElement('div');
var element = testDiv.appendChild(document.createElement('div'));
var model = {};
element.bind('id', model, 'a');
assert.strictEqual(element.id, '');
});

test('MultipleReferences', function() {
var el = document.createElement('div');
var el = testDiv.appendChild(document.createElement('div'));
var model = {foo: 'bar'};
el.setAttribute('foo', '{{foo}} {{foo}}');
HTMLTemplateElement.bindAllMustachesFrom_(el, model);
Expand All @@ -167,7 +177,7 @@ suite('Form Element Bindings', function() {
teardown(doTeardown);

test('Input.value', function() {
var input = document.createElement('input');
var input = testDiv.appendChild(document.createElement('input'));
var model = {x: 42};
input.bind('value', model, 'x');
assert.strictEqual('42', input.value);
Expand Down Expand Up @@ -196,7 +206,7 @@ suite('Form Element Bindings', function() {
test('Input.value - user value rejected', function() {
var model = {val: 'ping'};

var el = document.createElement('input');
var el = testDiv.appendChild(document.createElement('input'));
el.bind('value', model, 'val');
Platform.performMicrotaskCheckpoint();
assert.strictEqual('ping', el.value);
Expand Down Expand Up @@ -243,7 +253,7 @@ suite('Form Element Bindings', function() {
});

test('(Checkbox)Input.checked', function() {
var input = document.createElement('input');
var input = testDiv.appendChild(document.createElement('input'));
testDiv.appendChild(input);
input.type = 'checkbox';
var model = {x: true};
Expand All @@ -266,7 +276,7 @@ suite('Form Element Bindings', function() {
test('(Checkbox)Input.checked 2', function() {
var model = {val: true};

var el = document.createElement('input');
var el = testDiv.appendChild(document.createElement('input'));
testDiv.appendChild(el);
el.type = 'checkbox';
el.bind('checked', model, 'val');
Expand Down Expand Up @@ -299,7 +309,7 @@ suite('Form Element Bindings', function() {
test('(Checkbox)Input.checked - binding updated on click', function() {
var model = {val: true};

var el = document.createElement('input');
var el = testDiv.appendChild(document.createElement('input'));
testDiv.appendChild(el);
el.type = 'checkbox';
el.bind('checked', model, 'val');
Expand All @@ -319,7 +329,7 @@ suite('Form Element Bindings', function() {
test('(Checkbox)Input.checked - binding updated on change', function() {
var model = {val: true};

var el = document.createElement('input');
var el = testDiv.appendChild(document.createElement('input'));
testDiv.appendChild(el);
el.type = 'checkbox';
el.bind('checked', model, 'val');
Expand All @@ -337,7 +347,7 @@ suite('Form Element Bindings', function() {
});

test('(Radio)Input.checked', function() {
var input = document.createElement('input');
var input = testDiv.appendChild(document.createElement('input'));
input.type = 'radio';
var model = {x: true};
input.bind('checked', model, 'x');
Expand All @@ -363,7 +373,7 @@ suite('Form Element Bindings', function() {
var model = {val1: true, val2: false, val3: false, val4: true};
var RADIO_GROUP_NAME = 'test';

var container = document.body.appendChild(document.createElement('div'));
var container = testDiv.appendChild(document.createElement('div'));

var el1 = container.appendChild(document.createElement('input'));
el1.type = 'radio';
Expand Down Expand Up @@ -412,15 +422,13 @@ suite('Form Element Bindings', function() {
assert.strictEqual(false, model.val2);
assert.strictEqual(true, model.val3);
assert.strictEqual(true, model.val4);

document.body.removeChild(container);
});

test('(Radio)Input.checked - multiple forms', function() {
var model = {val1: true, val2: false, val3: false, val4: true};
var RADIO_GROUP_NAME = 'test';

var container = document.body.appendChild(document.createElement('div'));
var container = testDiv.appendChild(document.createElement('div'));
var form1 = container.appendChild(document.createElement('form'));
var form2 = container.appendChild(document.createElement('form'));

Expand Down Expand Up @@ -467,12 +475,10 @@ suite('Form Element Bindings', function() {
// Radio buttons in form1 should be unaffected
assert.strictEqual(false, model.val1);
assert.strictEqual(true, model.val2);

document.body.removeChild(container);
});

test('Select.selectedIndex', function() {
var select = document.createElement('select');
var select = testDiv.appendChild(document.createElement('select'));
testDiv.appendChild(select);
var option0 = select.appendChild(document.createElement('option'));
var option1 = select.appendChild(document.createElement('option'));
Expand Down
11 changes: 11 additions & 0 deletions tests/syntax.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,23 @@

suite('Syntax', function() {

function unbindAll(node) {
node.unbindAll();
for (var child = node.firstChild; child; child = child.nextSibling)
unbindAll(child);
}

setup(function() {
testDiv = document.body.appendChild(document.createElement('div'));
Observer._errorThrownDuringCallback = false;
})

teardown(function() {
assert.isFalse(!!Observer._errorThrownDuringCallback);
document.body.removeChild(testDiv);
unbindAll(testDiv);
Platform.performMicrotaskCheckpoint();
assert.strictEqual(2, Observer._allObserversCount);
});

function createTestHtml(s) {
Expand Down
13 changes: 12 additions & 1 deletion tests/template_element.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,12 @@ suite('Template Element', function() {

var testDiv;

function unbindAll(node) {
node.unbindAll();
for (var child = node.firstChild; child; child = child.nextSibling)
unbindAll(child);
}

setup(function() {
testDiv = document.body.appendChild(document.createElement('div'));
Observer._errorThrownDuringCallback = false;
Expand All @@ -24,6 +30,9 @@ suite('Template Element', function() {
teardown(function() {
assert.isFalse(!!Observer._errorThrownDuringCallback);
document.body.removeChild(testDiv);
unbindAll(testDiv);
Platform.performMicrotaskCheckpoint();
assert.strictEqual(2, Observer._allObserversCount);
});

function createTestHtml(s) {
Expand Down Expand Up @@ -1476,6 +1485,7 @@ suite('Template Element', function() {
recursivelySetTemplateModel(root, model);
Platform.performMicrotaskCheckpoint();
assert.strictEqual('Hi Leela', root.childNodes[1].textContent);
unbindAll(root);
}
});

Expand All @@ -1486,6 +1496,7 @@ suite('Template Element', function() {
recursivelySetTemplateModel(root, {});
Platform.performMicrotaskCheckpoint();
assert.strictEqual(3, root.childNodes.length);
unbindAll(root);
}
});

Expand Down Expand Up @@ -1571,7 +1582,7 @@ suite('Template Element', function() {
}
};

var host = document.createElement('div');
var host = testDiv.appendChild(document.createElement('div'));
var instance = outer.createInstance(model, 'Test');
assert.strictEqual(instance.firstChild.ref, outer.content.firstChild);
assert.strictEqual('Test', instance.firstChild.getAttribute('syntax'));
Expand Down
2 changes: 1 addition & 1 deletion third_party/ChangeSummary
Submodule ChangeSummary updated from a6f8a7 to b774be

0 comments on commit 361fa3e

Please sign in to comment.