Skip to content

Commit

Permalink
fix: Avoid flatTree memory leak
Browse files Browse the repository at this point in the history
  • Loading branch information
WilcoFiers committed Mar 10, 2018
1 parent cc66eb2 commit a902e80
Show file tree
Hide file tree
Showing 11 changed files with 308 additions and 283 deletions.
10 changes: 5 additions & 5 deletions lib/core/base/context.js
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ function parseSelectorArray(context, type) {
nodeList = Array.from(document.querySelectorAll(item));
//eslint no-loop-func:0
result = result.concat(nodeList.map((node) => {
return axe.utils.getNodeFromTree(axe._tree[0], node);
return axe.utils.getNodeFromTree(context.flatTree[0], node);
}));
break;
} else if (item && item.length && !(item instanceof Node)) {
Expand All @@ -140,14 +140,14 @@ function parseSelectorArray(context, type) {
nodeList = Array.from(document.querySelectorAll(item[0]));
//eslint no-loop-func:0
result = result.concat(nodeList.map((node) => {
return axe.utils.getNodeFromTree(axe._tree[0], node);
return axe.utils.getNodeFromTree(context.flatTree[0], node);
}));
}
} else if (item instanceof Node) {
if (item.documentElement instanceof Node) {
result.push(axe._tree[0]);
result.push(context.flatTree[0]);
} else {
result.push(axe.utils.getNodeFromTree(axe._tree[0], item));
result.push(axe.utils.getNodeFromTree(context.flatTree[0], item));
}
}
}
Expand Down Expand Up @@ -232,7 +232,7 @@ function Context(spec) {
spec = normalizeContext(spec);

//cache the flattened tree
axe._tree = axe.utils.getFlattenedTree(getRootNode(spec));
this.flatTree = axe.utils.getFlattenedTree(getRootNode(spec));
this.exclude = spec.exclude;
this.include = spec.include;

Expand Down
27 changes: 18 additions & 9 deletions lib/core/public/run-rules.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,13 @@
/*global Context */
/*exported runRules */

function complete (cb, ...args) {
cb(...args);
// Clean up after resolve / reject
axe._tree = undefined;
axe._selectorData = undefined;
}

/**
* Starts analysis on the current document and its subframes
* @private
Expand All @@ -12,8 +19,10 @@ function runRules(context, options, resolve, reject) {
'use strict';
try {
context = new Context(context);
axe._tree = context.flatTree;
axe._selectorData = axe.utils.getSelectorData(context.flatTree);
} catch (e) {
return reject(e);
return complete(reject, e);
}

var q = axe.utils.queue();
Expand Down Expand Up @@ -45,10 +54,8 @@ function runRules(context, options, resolve, reject) {
}

// Add wrapper object so that we may use the same "merge" function for results from inside and outside frames
var results = axe.utils.mergeResults(data.map(function (d) {
return {
results: d
};
var results = axe.utils.mergeResults(data.map(function (results) {
return { results };
}));

// after should only run once, so ensure we are in the top level window
Expand All @@ -59,14 +66,16 @@ function runRules(context, options, resolve, reject) {
results = results.map(axe.utils.finalizeRuleResult);
}
try {
resolve(results);
complete(resolve, results);
} catch(e) {
axe.log(e);
complete(axe.log, e);
}
} catch (e) {
reject(e);
complete(reject, e);
}
}).catch(reject);
}).catch((e) => {
complete(reject, e);
});
}

axe._runRules = runRules;
2 changes: 0 additions & 2 deletions lib/core/public/run.js
Original file line number Diff line number Diff line change
Expand Up @@ -128,8 +128,6 @@ axe.run = function (context, options, callback) {
try {
let reporter = axe.getReporter(options.reporter);
let results = reporter(rawResults, options, respond);
axe._selectorData = undefined;
axe._tree = undefined;
if (results !== undefined) {
respond(results);
}
Expand Down
6 changes: 2 additions & 4 deletions lib/core/utils/get-selector.js
Original file line number Diff line number Diff line change
Expand Up @@ -307,11 +307,9 @@ function getThreeLeastCommonFeatures(elm, selectorData) {
function generateSelector (elm, options, doc) {
/*eslint max-statements:["error", 22], no-loop-func:0*/
if (!axe._selectorData) {
axe._selectorData = axe.utils.getSelectorData(axe._tree);
throw new Error('Expect axe._selectorData to be set up');
}
const {
toRoot = false
} = options;
const { toRoot = false } = options;
let selector;
let similar;

Expand Down
21 changes: 11 additions & 10 deletions test/core/base/context.js
Original file line number Diff line number Diff line change
Expand Up @@ -327,11 +327,10 @@ describe('Context', function() {
});
});

it('should assign the result of getFlattenedTree to axe._tree', function () {
/* eslint no-new:0 */
new Context({ include: [document] });
it('should create a flatTree property', function () {
var context = new Context({ include: [document] });
// WARNING: This only works because there is now Shadow DOM on this page
assert.deepEqual(axe._tree, axe.utils.getFlattenedTree(document));
assert.deepEqual(context.flatTree, axe.utils.getFlattenedTree(document));
});

it('should throw when frame could not be found', function (done) {
Expand All @@ -350,27 +349,29 @@ describe('Context', function() {

describe('object definition', function() {
it('should assign include/exclude', function() {

var flatTree = axe.utils.getFlattenedTree(document);
assert.deepEqual(new Context({
include: ['#fixture'],
exclude: ['#mocha']
}), {
include: [axe.utils.getFlattenedTree(document.getElementById('fixture'))[0]],
exclude: [axe.utils.getFlattenedTree(document.getElementById('mocha'))[0]],
include: axe.utils.querySelectorAll(flatTree, '#fixture'),
exclude: axe.utils.querySelectorAll(flatTree, '#mocha'),
flatTree: flatTree,
initiator: true,
page: false,
frames: []
});

});
it('should disregard bad input, non-matching selectors', function() {

it('should disregard bad input, non-matching selectors', function() {
var flatTree = axe.utils.getFlattenedTree(document);
assert.deepEqual(new Context({
include: ['#fixture', '#monkeys'],
exclude: ['#bananas']
}), {
include: [axe.utils.getFlattenedTree(document.getElementById('fixture'))[0]],
include: axe.utils.querySelectorAll(flatTree, '#fixture'),
exclude: [],
flatTree: flatTree,
initiator: true,
page: false,
frames: []
Expand Down
47 changes: 47 additions & 0 deletions test/core/public/run-rules.js
Original file line number Diff line number Diff line change
Expand Up @@ -650,4 +650,51 @@ describe('runRules', function () {
done();
}, isNotCalled);
});

it('should clear up axe._tree / axe._selectorData after resolving', function (done) {
axe._load({ rules: [{
id: 'html',
selector: 'html',
any: ['html']
}], checks: [{
id: 'html',
evaluate: function () {
return true;
}
}], messages: {}});

runRules(document, {}, function resolve() {
assert.isDefined(axe._tree);
assert.isDefined(axe._selectorData);
setTimeout(function () {
assert.isUndefined(axe._tree);
assert.isUndefined(axe._selectorData);
done();
}, 10);
}, isNotCalled);
});

it('should clear up axe._tree / axe._selectorData after an error', function (done) {
axe._load({ rules: [{
id: 'invalidRule'
}], checks: [], messages: {}});

createFrames(function () {
setTimeout(function () {
runRules(document, {}, function () {
assert.ok(false, 'You shall not pass!');
done();
},
function () {
assert.isDefined(axe._tree);
assert.isDefined(axe._selectorData);
setTimeout(function () {
assert.isUndefined(axe._tree);
assert.isUndefined(axe._selectorData);
done();
}, 10);
});
}, 100);
});
});
});
25 changes: 14 additions & 11 deletions test/core/utils/collect-results-from-frames.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,14 @@ describe('axe.utils.collectResultsFromFrames', function () {
var fixture = document.getElementById('fixture');
var noop = function () {};

function contextSetup (scope) {
var context = new Context(scope);
axe._tree = context.flatTree;
axe._selectorData = axe.utils.getSelectorData(context.flatTree);

return context
}

afterEach(function () {
fixture.innerHTML = '';
axe._tree = undefined;
Expand All @@ -25,8 +33,7 @@ describe('axe.utils.collectResultsFromFrames', function () {

var frame = document.createElement('iframe');
frame.addEventListener('load', function () {
var context = new Context(document);
axe._tree = axe.utils.getFlattenedTree(document.documentElement);
var context = contextSetup(document);
axe.utils.collectResultsFromFrames(context, {}, 'stuff', 'morestuff', noop,
function (err) {
assert.instanceOf(err, Error);
Expand All @@ -40,7 +47,6 @@ describe('axe.utils.collectResultsFromFrames', function () {
frame.id = 'level0';
frame.src = '../mock/frames/results-timeout.html';
fixture.appendChild(frame);

});

it('should override the timeout with `options.frameWaitTime`, if provided', function (done) {
Expand All @@ -57,9 +63,9 @@ describe('axe.utils.collectResultsFromFrames', function () {

var frame = document.createElement('iframe');
frame.addEventListener('load', function () {
var context = new Context(document);
var context = contextSetup(document);
var params = { frameWaitTime: 90000 };
axe._tree = axe.utils.getFlattenedTree(document.documentElement);

axe.utils.collectResultsFromFrames(context, params, 'stuff', 'morestuff', noop,
function (err) {
assert.instanceOf(err, Error);
Expand All @@ -73,7 +79,6 @@ describe('axe.utils.collectResultsFromFrames', function () {
frame.id = 'level0';
frame.src = '../mock/frames/results-timeout.html';
fixture.appendChild(frame);

});

it('should not throw given a recursive iframe', function (done) {
Expand All @@ -93,8 +98,8 @@ describe('axe.utils.collectResultsFromFrames', function () {

var frame = document.createElement('iframe');
frame.addEventListener('load', function () {
var context = new Context(document);
axe._tree = axe.utils.getFlattenedTree(document.documentElement);
var context = contextSetup(document);

axe.utils.collectResultsFromFrames(context, {}, 'rules', 'morestuff', function () {
done();
}, function (e) {
Expand All @@ -106,14 +111,12 @@ describe('axe.utils.collectResultsFromFrames', function () {
frame.id = 'level0';
frame.src = '../mock/frames/nested0.html';
fixture.appendChild(frame);

});

it('returns errors send from the frame', function (done) {
var frame = document.createElement('iframe');
frame.addEventListener('load', function () {
var context = new Context(document);
axe._tree = axe.utils.getFlattenedTree(document.documentElement);
var context = contextSetup(document);
axe.utils.collectResultsFromFrames(context, {}, 'command', 'params', noop,
function (err) {
assert.instanceOf(err, Error);
Expand Down
8 changes: 5 additions & 3 deletions test/core/utils/dq-element.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ describe('DqElement', function () {
'use strict';

var fixture = document.getElementById('fixture');
var fixtureSetup = axe.testUtils.fixtureSetup;

afterEach(function () {
fixture.innerHTML = '';
Expand Down Expand Up @@ -33,7 +34,8 @@ describe('DqElement', function () {

it('should not be present in stringified version', function () {
var div = document.createElement('div');
axe._tree = axe.utils.getFlattenedTree(document.documentElement);
fixtureSetup();

var dqEl = new DqElement(div);

assert.isUndefined(JSON.parse(JSON.stringify(dqEl)).element);
Expand Down Expand Up @@ -141,8 +143,8 @@ describe('DqElement', function () {

describe('absolutePaths', function () {
it('creates a path all the way to root', function () {
fixture.innerHTML = '<div id="foo" class="bar">Hello!</div>';
axe._tree = axe.utils.getFlattenedTree(document.documentElement);
fixtureSetup('<div id="foo" class="bar">Hello!</div>');

var result = new DqElement(fixture.firstChild, {
absolutePaths: true
});
Expand Down
Loading

0 comments on commit a902e80

Please sign in to comment.