Refactor destruction (Ember.Application and Ember.ApplicationInstance)

* Collapse `createEmberApp` into `buildApp` (since we are no longer
  separating the concepts of `Ember.Application` vs
  `Ember.ApplicationInstance`)
* Stash the `Ember.Application` instance (along with the
  `Ember.ApplicationInstance` instance) after creation, for destruction
  later
* Ensure destruction of both `Ember.Application` and
  `Ember.ApplicationInstance` occur when the `visit` has completed.

Author: Robert Jackson

src/ember-app.js

@@ -180,29 +180,6 @@ class EmberApp {
     };
   }
 
-  /**
-   * @private
-   *
-   * Create the ember application in the sandbox.
-   *
-   */
-  createEmberApp(sandbox) {
-    // Retrieve the application factory from within the sandbox
-    let AppFactory = sandbox.run(function(ctx) {
-      return ctx.require('~fastboot/app-factory');
-    });
-
-    // If the application factory couldn't be found, throw an error
-    if (!AppFactory || typeof AppFactory['default'] !== 'function') {
-      throw new Error(
-        'Failed to load Ember app from app.js, make sure it was built for FastBoot with the `ember fastboot:build` command.'
-      );
-    }
-
-    // Otherwise, return a new `Ember.Application` instance
-    return AppFactory['default']();
-  }
-
   /**
    * Perform any cleanup that is needed
    */
@@ -211,11 +188,11 @@ class EmberApp {
   /**
    * @private
    *
-   * Creates a new `ApplicationInstance` from the sandboxed `Application`.
+   * Creates a new `Application`
    *
-   * @returns {Promise<Ember.ApplicationInstance>} instance
+   * @returns {Ember.Application} instance
    */
-  async buildAppInstance() {
+  async buildApp() {
     let sandbox = this.buildSandbox();
 
     debug('adding files to sandbox');
@@ -227,10 +204,26 @@ class EmberApp {
 
     debug('files evaluated');
 
-    let app = await this.createEmberApp(sandbox).boot();
+    // Retrieve the application factory from within the sandbox
+    let AppFactory = sandbox.run(function(ctx) {
+      return ctx.require('~fastboot/app-factory');
+    });
 
-    debug('building instance');
-    return app.buildInstance();
+    // If the application factory couldn't be found, throw an error
+    if (!AppFactory || typeof AppFactory['default'] !== 'function') {
+      throw new Error(
+        'Failed to load Ember app from app.js, make sure it was built for FastBoot with the `ember fastboot:build` command.'
+      );
+    }
+
+    debug('creating application');
+
+    // Otherwise, return a new `Ember.Application` instance
+    let app = AppFactory['default']();
+
+    await app.boot();
+
+    return app;
   }
 
   /**
@@ -252,9 +245,12 @@ class EmberApp {
    * @return {Promise<instance>} instance
    */
   async visitRoute(path, fastbootInfo, bootOptions, result) {
-    let instance = await this.buildAppInstance();
+    let app = await this.buildApp();
+    result.applicationInstance = app;
+
+    let instance = await app.buildInstance();
+    result.applicationInstanceInstance = instance;
 
-    result.instance = instance;
     registerFastBootInfo(fastbootInfo, instance);
 
     await instance.boot(bootOptions);
@@ -307,7 +303,7 @@ class EmberApp {
       // start a timer to destroy the appInstance forcefully in the given ms.
       // This is a failure mechanism so that node process doesn't get wedged if the `visit` never completes.
       destroyAppInstanceTimer = setTimeout(function() {
-        if (result._destroyAppInstance()) {
+        if (result._destroy()) {
           result.error = new Error(
             'App instance was forcefully destroyed in ' + destroyAppInstanceInMs + 'ms'
           );
@@ -328,11 +324,12 @@ class EmberApp {
       // eslint-disable-next-line require-atomic-updates
       result.error = error;
     } finally {
-      if (result._destroyAppInstance()) {
-        if (destroyAppInstanceTimer) {
-          clearTimeout(destroyAppInstanceTimer);
-        }
-      }
+      // ensure we invoke `Ember.Application.destroy()` and
+      // `Ember.ApplicationInstance.destroy()`, but use `result._destroy()` so
+      // that the `result` object's internal `this.isDestroyed` flag is correct
+      result._destroy();
+
+      clearTimeout(destroyAppInstanceTimer);
     }
 
     return result;

src/result.js

@@ -13,12 +13,13 @@ const HTML_HEAD_REGEX = /^([\s\S]*<\/head>)([\s\S]*)/;
  */
 class Result {
   constructor(doc, html, fastbootInfo) {
-    this._instanceDestroyed = false;
+    this.isDestroyed = false;
 
     this._doc = doc;
     this._html = html;
     this._fastbootInfo = fastbootInfo;
-    this.instance = undefined;
+    this.applicationInstance = undefined;
+    this.applicationInstanceInstance = undefined;
   }
 
   /**
@@ -113,10 +114,10 @@ class Result {
   /**
    * @private
    *
-   * Called once the Result has finished being constructed and the application
-   * instance has finished rendering. Once `finalize()` is called, state is
-   * gathered from the completed application instance and statically copied
-   * to this Result instance.
+   * Called once the Result has finished being constructed and the
+   * ApplicationInstance instance has finished rendering. Once `finalize()` is
+   * called, state is gathered from the completed ApplicationInstance instance
+   * and statically copied to this Result instance.
    */
   _finalize() {
     if (this.finalized) {
@@ -125,9 +126,9 @@ class Result {
 
     // Grab some metadata from the sandboxed application instance
     // and copy it to this Result object.
-    let { instance } = this;
-    if (instance) {
-      this._finalizeMetadata(instance);
+    let { applicationInstanceInstance } = this;
+    if (applicationInstanceInstance) {
+      this._finalizeMetadata(applicationInstanceInstance);
     }
 
     this._finalizeHTML();
@@ -136,9 +137,9 @@ class Result {
     return this;
   }
 
-  _finalizeMetadata(instance) {
-    if (instance._booted) {
-      this.url = instance.getURL();
+  _finalizeMetadata(applicationInstanceInstance) {
+    if (applicationInstanceInstance._booted) {
+      this.url = applicationInstanceInstance.getURL();
     }
 
     let { response } = this._fastbootInfo;
@@ -149,15 +150,20 @@ class Result {
     }
   }
 
-  _destroyAppInstance() {
-    if (this.instance && !this._instanceDestroyed) {
-      this._instanceDestroyed = true;
-      this.instance.destroy();
-
+  _destroy() {
+    if (this.isDestroyed) {
       return true;
     }
 
-    return false;
+    this.isDestroyed = true;
+
+    if (this.applicationInstanceInstance !== undefined) {
+      this.applicationInstanceInstance.destroy();
+    }
+
+    if (this.applicationInstance !== undefined) {
+      this.applicationInstance.destroy();
+    }
   }
 
   _finalizeHTML() {

test/fastboot-test.js

@@ -387,17 +387,6 @@ describe('FastBoot', function() {
     return fastboot.visit('/').catch(e => expect(e).to.be.an('error'));
   });
 
-  it("matches app's fastboot-info and result's fastboot-info", function() {
-    var fastboot = new FastBoot({
-      distPath: fixture('basic-app'),
-    });
-
-    return fastboot.visit('/').then(r => {
-      let lookupFastboot = r.instance.lookup('info:-fastboot');
-      expect(r._fastbootInfo).to.deep.equal(lookupFastboot);
-    });
-  });
-
   it('can read multiple configs', function() {
     var fastboot = new FastBoot({
       distPath: fixture('app-with-multiple-config'),