Skip to content

Commit

Permalink
[BUGFIX release-1-13] backports V8 memory leak 🚱
Browse files Browse the repository at this point in the history
original pr emberjs#12666
fixes emberjs#12618
  • Loading branch information
calderas committed Mar 30, 2016
1 parent 50005ad commit 2cba382
Show file tree
Hide file tree
Showing 9 changed files with 234 additions and 113 deletions.
53 changes: 46 additions & 7 deletions packages/container/lib/registry.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,13 @@ if (Ember.FEATURES.isEnabled('ember-application-instance-initializers')) {
function Registry(options) {
this.fallback = options && options.fallback ? options.fallback : null;

this.resolver = options && options.resolver ? options.resolver : function() {};
if (options && options.resolver) {
this.resolver = options.resolver;
if (typeof this.resolver === 'function') {
deprecateResolverFunction(this);
}

}

this.registrations = dictionary(options && options.registrations ? options.registrations : null);

Expand Down Expand Up @@ -58,7 +64,7 @@ Registry.prototype = {
/**
@private
@property resolver
@type function
@type Resolver
*/
resolver: null,

Expand Down Expand Up @@ -327,7 +333,13 @@ Registry.prototype = {
@return {string} described fullName
*/
describe(fullName) {
return fullName;
if (this.resolver && this.resolver.lookupDescription) {
return this.resolver.lookupDescription(fullName);
} else if (this.fallback) {
return this.fallback.describe(fullName);
} else {
return fullName;
}
},

/**
Expand All @@ -339,7 +351,13 @@ Registry.prototype = {
@return {string} normalized fullName
*/
normalizeFullName(fullName) {
return fullName;
if (this.resolver && this.resolver.normalize) {
return this.resolver.normalize(fullName);
} else if (this.fallback) {
return this.fallback.normalizeFullName(fullName);
} else {
return fullName;
}
},

/**
Expand All @@ -365,7 +383,13 @@ Registry.prototype = {
@return {function} toString function
*/
makeToString(factory, fullName) {
return factory.toString();
if (this.resolver && this.resolver.makeToString) {
return this.resolver.makeToString(factory, fullName);
} else if (this.fallback) {
return this.fallback.makeToString(factory, fullName);
} else {
return factory.toString();
}
},

/**
Expand Down Expand Up @@ -719,7 +743,7 @@ Registry.prototype = {
fallbackKnown = this.fallback.knownForType(type);
}

if (this.resolver.knownForType) {
if (this.resolver && this.resolver.knownForType) {
resolverKnown = this.resolver.knownForType(type);
}

Expand Down Expand Up @@ -797,12 +821,27 @@ Registry.prototype = {
}
};

function deprecateResolverFunction(registry) {
Ember.deprecate('Passing a `resolver` function into a Registry is deprecated. Please pass in a Resolver object with a `resolve` method.',
false,
{ id: 'ember-application.registry-resolver-as-function', until: '3.0.0', url: 'http://emberjs.com/deprecations/v2.x#toc_registry-resolver-as-function' });
registry.resolver = {
resolve: registry.resolver
};
}

function resolve(registry, normalizedName) {
var cached = registry._resolveCache[normalizedName];
if (cached) { return cached; }
if (registry._failCache[normalizedName]) { return; }

var resolved = registry.resolver(normalizedName) || registry.registrations[normalizedName];
var resolved;

if (registry.resolver) {
resolved = registry.resolver.resolve(normalizedName);
}

resolved = resolved || registry.registrations[normalizedName];

if (resolved) {
registry._resolveCache[normalizedName] = resolved;
Expand Down
24 changes: 15 additions & 9 deletions packages/container/tests/container_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -320,9 +320,11 @@ QUnit.test("The container can use a registry hook to resolve factories lazily",
var container = registry.container();
var PostController = factory();

registry.resolver = function(fullName) {
if (fullName === 'controller:post') {
return PostController;
registry.resolver = {
resolve(fullName) {
if (fullName === 'controller:post') {
return PostController;
}
}
};

Expand Down Expand Up @@ -366,9 +368,11 @@ QUnit.test("The container can get options that should be applied to a given fact
var container = registry.container();
var PostView = factory();

registry.resolver = function(fullName) {
if (fullName === 'view:post') {
return PostView;
registry.resolver = {
resolve(fullName) {
if (fullName === 'view:post') {
return PostView;
}
}
};

Expand All @@ -388,9 +392,11 @@ QUnit.test("The container can get options that should be applied to all factorie
var container = registry.container();
var PostView = factory();

registry.resolver = function(fullName) {
if (fullName === 'view:post') {
return PostView;
registry.resolver = {
resolve(fullName) {
if (fullName === 'view:post') {
return PostView;
}
}
};

Expand Down
163 changes: 135 additions & 28 deletions packages/container/tests/registry_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -60,27 +60,29 @@ QUnit.test("Throw exception when trying to inject `type:thing` on all type(s)",
});

QUnit.test("The registry can take a hook to resolve factories lazily", function() {
var registry = new Registry();
var PostController = factory();

registry.resolver = function(fullName) {
if (fullName === 'controller:post') {
return PostController;
var resolver = {
resolve(fullName) {
if (fullName === 'controller:post') {
return PostController;
}
}
};
var registry = new Registry({ resolver });

strictEqual(registry.resolve('controller:post'), PostController, "The correct factory was provided");
});

QUnit.test("The registry respects the resolver hook for `has`", function() {
var registry = new Registry();
var PostController = factory();

registry.resolver = function(fullName) {
if (fullName === 'controller:post') {
return PostController;
var resolver = {
resolve(fullName) {
if (fullName === 'controller:post') {
return PostController;
}
}
};
var registry = new Registry({ resolver });

ok(registry.has('controller:post'), "the `has` method uses the resolver hook");
});
Expand Down Expand Up @@ -200,14 +202,18 @@ QUnit.test('once resolved, always return the same result', function() {

var registry = new Registry();

registry.resolver = function() {
return 'bar';
registry.resolver = {
resolve() {
return 'bar';
}
};

var Bar = registry.resolve('models:bar');

registry.resolver = function() {
return 'not bar';
registry.resolver = {
resolve() {
return 'not bar';
}
};

equal(registry.resolve('models:bar'), Bar);
Expand All @@ -217,9 +223,11 @@ QUnit.test("factory resolves are cached", function() {
var registry = new Registry();
var PostController = factory();
var resolveWasCalled = [];
registry.resolver = function(fullName) {
resolveWasCalled.push(fullName);
return PostController;
registry.resolver = {
resolve(fullName) {
resolveWasCalled.push(fullName);
return PostController;
}
};

deepEqual(resolveWasCalled, []);
Expand All @@ -234,9 +242,11 @@ QUnit.test("factory for non extendables (MODEL) resolves are cached", function()
var registry = new Registry();
var PostController = factory();
var resolveWasCalled = [];
registry.resolver = function(fullName) {
resolveWasCalled.push(fullName);
return PostController;
registry.resolver = {
resolve(fullName) {
resolveWasCalled.push(fullName);
return PostController;
}
};

deepEqual(resolveWasCalled, []);
Expand All @@ -251,9 +261,11 @@ QUnit.test("factory for non extendables resolves are cached", function() {
var registry = new Registry();
var PostController = {};
var resolveWasCalled = [];
registry.resolver = function(fullName) {
resolveWasCalled.push(fullName);
return PostController;
registry.resolver = {
resolve(fullName) {
resolveWasCalled.push(fullName);
return PostController;
}
};

deepEqual(resolveWasCalled, []);
Expand All @@ -276,6 +288,84 @@ QUnit.test("registry.container creates an associated container", function() {
strictEqual(registry._defaultContainer, container, "_defaultContainer is set to the first created container and used for Ember 1.x Container compatibility");
});

QUnit.test('`describe` will be handled by the resolver, then by the fallback registry, if available', function() {
var fallback = {
describe(fullName) {
return `${fullName}-fallback`;
}
};

var resolver = {
lookupDescription(fullName) {
return `${fullName}-resolver`;
}
};

var registry = new Registry({ fallback, resolver });

equal(registry.describe('controller:post'), 'controller:post-resolver', '`describe` handled by the resolver first.');

registry.resolver = null;

equal(registry.describe('controller:post'), 'controller:post-fallback', '`describe` handled by fallback registry next.');

registry.fallback = null;

equal(registry.describe('controller:post'), 'controller:post', '`describe` by default returns argument.');
});

QUnit.test('`normalizeFullName` will be handled by the resolver, then by the fallback registry, if available', function() {
var fallback = {
normalizeFullName(fullName) {
return `${fullName}-fallback`;
}
};

var resolver = {
normalize(fullName) {
return `${fullName}-resolver`;
}
};

var registry = new Registry({ fallback, resolver });

equal(registry.normalizeFullName('controller:post'), 'controller:post-resolver', '`normalizeFullName` handled by the resolver first.');

registry.resolver = null;

equal(registry.normalizeFullName('controller:post'), 'controller:post-fallback', '`normalizeFullName` handled by fallback registry next.');

registry.fallback = null;

equal(registry.normalizeFullName('controller:post'), 'controller:post', '`normalizeFullName` by default returns argument.');
});

QUnit.test('`makeToString` will be handled by the resolver, then by the fallback registry, if available', function() {
var fallback = {
makeToString(fullName) {
return `${fullName}-fallback`;
}
};

var resolver = {
makeToString(fullName) {
return `${fullName}-resolver`;
}
};

var registry = new Registry({ fallback, resolver });

equal(registry.makeToString('controller:post'), 'controller:post-resolver', '`makeToString` handled by the resolver first.');

registry.resolver = null;

equal(registry.makeToString('controller:post'), 'controller:post-fallback', '`makeToString` handled by fallback registry next.');

registry.fallback = null;

equal(registry.makeToString('controller:post'), 'controller:post', '`makeToString` by default returns argument.');
});

QUnit.test("`resolve` can be handled by a fallback registry", function() {
var fallback = new Registry();

Expand Down Expand Up @@ -380,12 +470,13 @@ QUnit.test("`knownForType` includes fallback registry results", function() {
QUnit.test("`knownForType` is called on the resolver if present", function() {
expect(3);

function resolver() { }
resolver.knownForType = function(type) {
ok(true, 'knownForType called on the resolver');
equal(type, 'foo', 'the type was passed through');
var resolver = {
knownForType(type) {
ok(true, 'knownForType called on the resolver');
equal(type, 'foo', 'the type was passed through');

return { 'foo:yorp': true };
return { 'foo:yorp': true };
}
};

var registry = new Registry({
Expand All @@ -400,3 +491,19 @@ QUnit.test("`knownForType` is called on the resolver if present", function() {
'foo:bar-baz': true
});
});

QUnit.test('A registry can be created with a deprecated `resolver` function instead of an object', function() {
expect(2);

let registry;

expectDeprecation(function() {
registry = new Registry({
resolver(fullName) {
return `${fullName}-resolved`;
}
});
}, 'Passing a `resolver` function into a Registry is deprecated. Please pass in a Resolver object with a `resolve` method.');

equal(registry.resolve('foo:bar'), 'foo:bar-resolved', '`resolve` still calls the deprecated function');
});
2 changes: 0 additions & 2 deletions packages/ember-application/lib/system/application-instance.js
Original file line number Diff line number Diff line change
Expand Up @@ -94,8 +94,6 @@ export default EmberObject.extend({
fallback: this.applicationRegistry,
resolver: this.applicationRegistry.resolver
});
this.registry.normalizeFullName = this.applicationRegistry.normalizeFullName;
this.registry.makeToString = this.applicationRegistry.makeToString;

// Create a per-instance container from the instance's registry
this.container = this.registry.container();
Expand Down
Loading

0 comments on commit 2cba382

Please sign in to comment.