[BUGFIX lts] Fix double container destroy

[cibernox]

This code on the Engine Instance is not needed as it includes the
ContainerProxyMixin, which already takes care of destroying the
container.

This duplicated destroy step was causing ember-cp-validations to
attempt to destroy validators twice. This destroy process tries to
nullify some property with o.set(‘model’, null) on an object that is
already destroyed.

[cibernox]

This is the code in the ContainerProxyMixin that already takes care of destroying the container.
https://github.com/emberjs/ember.js/blob/master/packages/ember-runtime/lib/mixins/container_proxy.js#L128-L134

[rwjblue]

This seems like the correct change, but I would like to understand why it only started affecting things on Ember 2.14 beta before removing it...

[cibernox]

I cannot pinpoint the exact point where things go wrong, but I've noticed that on the test I'm using to trace this, on Ember 2.13 the list of keys to destroy contains three items (["controller:application", "controller:users", "controller:users/login"]) and in 2.14 it contains more controllers (["controller:application", "controller:users/login", "controller:users", "controller:users/signup/index", "controller:passwords/index"]).

It's in controller:passwords/index where validations are mixed:

const emailValidations = buildValidations({
  'model.email': email
});

export default Controller.extend(emailValidations, {

so I only see the error in 2.14.

I think that the different behaviour is because of the changes in the way transition.retry works. There is an open bug for that.

Either way, destroying the container twice seems accidental and the fix make sense still.

[rwjblue]

After chatting with @cibernox about this in slack a bit more, I believe that the reason for the additional controllers being instantiated is #15178 (removal of controller.proto() shenanigans). Now we owner.lookup the controllers needed for QPs whereas before we did a bunch of hoop jumping to avoid that .lookup.

[rwjblue]

@cibernox - Can you also remove https://github.com/emberjs/ember.js/blob/master/packages/ember-application/lib/system/engine-instance.js#L137?

Nevermind me, I just linked to the same thing that is being removed in this PR 😝 ...

[rwjblue]

Updated description to be [BUGFIX lts] as this bugfix will need to be backported to 2.12 (once released in a 2.13 point release).