From 64bde25d22083e1f057974b605512792d0f8a297 Mon Sep 17 00:00:00 2001
From: Christopher Willis-Ford <cwillisf@media.mit.edu>
Date: Tue, 22 Aug 2017 14:28:38 -0700
Subject: [PATCH] Offer a promise for extension worker init

---
 src/extension-support/extension-manager.js | 62 ++++++++++++++++------
 src/extension-support/extension-worker.js  | 20 +++++--
 2 files changed, 62 insertions(+), 20 deletions(-)

diff --git a/src/extension-support/extension-manager.js b/src/extension-support/extension-manager.js
index a453fb3a8..e02234031 100644
--- a/src/extension-support/extension-manager.js
+++ b/src/extension-support/extension-manager.js
@@ -28,16 +28,18 @@ const BlockType = require('./block-type');
  * @property {string} color1 - the primary color for this category, in '#rrggbb' format
  * @property {string} color2 - the secondary color for this category, in '#rrggbb' format
  * @property {string} color3 - the tertiary color for this category, in '#rrggbb' format
+ * @property {Array.<BlockInfo> block - the blocks in this category
+ */
+
+/**
+ * @typedef {object} PendingExtensionWorker - Information about an extension worker still initializing
+ * @property {string} extensionURL - the URL of the extension to be loaded by this worker
+ * @property {Function} resolve - function to call on successful worker startup
+ * @property {Function} reject - function to call on failed worker startup
  */
 
 class ExtensionManager {
     constructor () {
-        /**
-         * The list of current active extension workers.
-         * @type {Array.<ExtensionWorker>}
-         */
-        this.workers = [];
-
         /**
          * The ID number to provide to the next extension worker.
          * @type {int}
@@ -45,10 +47,18 @@ class ExtensionManager {
         this.nextExtensionWorker = 0;
 
         /**
-         * The list of extension URLs which have been requested but not yet loaded in a worker.
-         * @type {Array}
+         * FIFO queue of extensions which have been requested but not yet loaded in a worker,
+         * along with promise resolution functions to call once the worker is ready or failed.
+         *
+         * @type {Array.<PendingExtensionWorker>}
          */
-        this.pendingExtensionURLs = [];
+        this.pendingExtensions = [];
+
+        /**
+         * Map of worker ID to workers which have been allocated but have not yet finished initialization.
+         * @type {Array.<PendingExtensionWorker>}
+         */
+        this.pendingWorkers = [];
 
         dispatch.setService('extensions', this).catch(e => {
             log.error(`ExtensionManager was unable to register extension service: ${JSON.stringify(e)}`);
@@ -56,21 +66,29 @@ class ExtensionManager {
     }
 
     foo () {
-        this.loadExtensionURL('extensions/example-extension.js');
+        return this.loadExtensionURL('extensions/example-extension.js');
     }
 
+    /**
+     * Load an extension by URL
+     * @param {string} extensionURL - the URL for the extension to load
+     * @returns {Promise} resolved once the extension is loaded and initialized or rejected on failure
+     */
     loadExtensionURL (extensionURL) {
-        // If we `require` this at the global level it breaks non-webpack targets, including tests
-        const ExtensionWorker = require('worker-loader!./extension-worker');
+        return new Promise((resolve, reject) => {
+            // If we `require` this at the global level it breaks non-webpack targets, including tests
+            const ExtensionWorker = require('worker-loader!./extension-worker');
 
-        this.pendingExtensionURLs.push(extensionURL);
-        dispatch.addWorker(new ExtensionWorker());
+            this.pendingExtensions.push({extensionURL, resolve, reject});
+            dispatch.addWorker(new ExtensionWorker());
+        });
     }
 
     allocateWorker () {
         const id = this.nextExtensionWorker++;
-        const extFile = this.pendingExtensionURLs.shift();
-        return [id, extFile];
+        const workerInfo = this.pendingExtensions.shift();
+        this.pendingWorkers[id] = workerInfo;
+        return [id, workerInfo.extensionURL];
     }
 
     registerExtensionService (serviceName) {
@@ -79,6 +97,16 @@ class ExtensionManager {
         });
     }
 
+    onWorkerInit (id, e) {
+        const workerInfo = this.pendingWorkers[id];
+        delete this.pendingWorkers[id];
+        if (e) {
+            workerInfo.reject(e);
+        } else {
+            workerInfo.resolve(id);
+        }
+    }
+
     _registerExtensionInfo (serviceName, extensionInfo) {
         extensionInfo = this._prepareExtensionInfo(serviceName, extensionInfo);
         dispatch.call('runtime', '_registerExtensionPrimitives', extensionInfo).catch(e => {
@@ -115,7 +143,7 @@ class ExtensionManager {
                 result.push(this._prepareBlockInfo(serviceName, blockInfo));
             } catch (e) {
                 // TODO: more meaningful error reporting
-                log.error(`Skipping malformed block: ${e}`);
+                log.error(`Skipping malformed block: ${JSON.stringify(e)}`);
             }
             return result;
         }, []);
diff --git a/src/extension-support/extension-worker.js b/src/extension-support/extension-worker.js
index 8c4a57410..02406e08b 100644
--- a/src/extension-support/extension-worker.js
+++ b/src/extension-support/extension-worker.js
@@ -8,13 +8,23 @@ class ExtensionWorker {
     constructor () {
         this.nextExtensionId = 0;
 
+        this.initialRegistrations = [];
+
         dispatch.waitForConnection.then(() => {
             dispatch.call('extensions', 'allocateWorker').then(x => {
                 const [id, extension] = x;
                 this.workerId = id;
 
-                // TODO: catch and report any exceptions here
-                importScripts(extension);
+                try {
+                    importScripts(extension);
+
+                    const initialRegistrations = this.initialRegistrations;
+                    this.initialRegistrations = null;
+
+                    Promise.all(initialRegistrations).then(() => dispatch.call('extensions', 'onWorkerInit', id));
+                } catch (e) {
+                    dispatch.call('extensions', 'onWorkerInit', id, e);
+                }
             });
         });
 
@@ -25,8 +35,12 @@ class ExtensionWorker {
         const extensionId = this.nextExtensionId++;
         this.extensions.push(extensionObject);
         const serviceName = `extension.${this.workerId}.${extensionId}`;
-        return dispatch.setService(serviceName, extensionObject)
+        const promise = dispatch.setService(serviceName, extensionObject)
             .then(() => dispatch.call('extensions', 'registerExtensionService', serviceName));
+        if (this.initialRegistrations) {
+            this.initialRegistrations.push(promise);
+        }
+        return promise;
     }
 }