Merge pull request #2060 from kchadha/load-core-extension

Load core extensions synchronously

src/blocks/scratch3_core_example.js

@@ -0,0 +1,45 @@
+const BlockType = require('../extension-support/block-type');
+
+/**
+ * An example core block implemented using the extension spec.
+ * This is not loaded as part of the core blocks in the VM but it is provided
+ * and used as part of tests.
+ */
+class Scratch3CoreExample {
+    constructor (runtime) {
+        /**
+         * The runtime instantiating this block package.
+         * @type {Runtime}
+         */
+        this.runtime = runtime;
+    }
+
+    /**
+     * @returns {object} metadata for this extension and its blocks.
+     */
+    getInfo () {
+        return {
+            id: 'coreExample',
+            name: 'CoreEx', // This string does not need to be translated as this extension is only used as an example.
+            blocks: [
+                {
+                    opcode: 'exampleOpcode',
+                    blockType: BlockType.REPORTER,
+                    text: 'example block'
+                }
+            ]
+        };
+    }
+
+    /**
+     * Example opcode just returns the name of the stage target.
+     * @returns {string} The name of the first target in the project.
+     */
+    exampleOpcode () {
+        const stage = this.runtime.getTargetForStage();
+        return stage ? stage.getName() : 'no stage yet';
+    }
+
+}
+
+module.exports = Scratch3CoreExample;

src/dispatch/central-dispatch.js

@@ -35,6 +35,39 @@ class CentralDispatch extends SharedDispatch {
         this.workers = [];
     }
 
+    /**
+     * Synchronously call a particular method on a particular service provided locally.
+     * Calling this function on a remote service will fail.
+     * @param {string} service - the name of the service.
+     * @param {string} method - the name of the method.
+     * @param {*} [args] - the arguments to be copied to the method, if any.
+     * @returns {*} - the return value of the service method.
+     */
+    callSync (service, method, ...args) {
+        const {provider, isRemote} = this._getServiceProvider(service);
+        if (provider) {
+            if (isRemote) {
+                throw new Error(`Cannot use 'callSync' on remote provider for service ${service}.`);
+            }
+
+            return provider[method].apply(provider, args);
+        }
+        throw new Error(`Provider not found for service: ${service}`);
+    }
+
+    /**
+     * Synchronously set a local object as the global provider of the specified service.
+     * WARNING: Any method on the provider can be called from any worker within the dispatch system.
+     * @param {string} service - a globally unique string identifying this service. Examples: 'vm', 'gui', 'extension9'.
+     * @param {object} provider - a local object which provides this service.
+     */
+    setServiceSync (service, provider) {
+        if (this.services.hasOwnProperty(service)) {
+            log.warn(`Central dispatch replacing existing service provider for ${service}`);
+        }
+        this.services[service] = provider;
+    }
+
     /**
      * Set a local object as the global provider of the specified service.
      * WARNING: Any method on the provider can be called from any worker within the dispatch system.
@@ -45,10 +78,7 @@ class CentralDispatch extends SharedDispatch {
     setService (service, provider) {
         /** Return a promise for consistency with {@link WorkerDispatch#setService} */
         try {
-            if (this.services.hasOwnProperty(service)) {
+            this.setServiceSync(service, provider);
-                log.warn(`Central dispatch replacing existing service provider for ${service}`);
-            }
-            this.services[service] = provider;
             return Promise.resolve();
         } catch (e) {
             return Promise.reject(e);

src/extension-support/extension-manager.js

@@ -9,6 +9,10 @@ const BlockType = require('./block-type');
 // TODO: change extension spec so that library info, including extension ID, can be collected through static methods
 
 const builtinExtensions = {
+    // This is an example that isn't loaded with the other core blocks,
+    // but serves as a reference for loading core blocks as extensions.
+    coreExample: () => require('../blocks/scratch3_core_example'),
+    // These are the non-core built-in extensions.
     pen: () => require('../extensions/scratch3_pen'),
     wedo2: () => require('../extensions/scratch3_wedo2'),
     music: () => require('../extensions/scratch3_music'),
@@ -106,6 +110,30 @@ class ExtensionManager {
         return this._loadedExtensions.has(extensionID);
     }
 
+    /**
+     * Synchronously load an internal extension (core or non-core) by ID. This call will
+     * fail if the provided id is not does not match an internal extension.
+     * @param {string} extensionId - the ID of an internal extension
+     */
+    loadExtensionIdSync (extensionId) {
+        if (!builtinExtensions.hasOwnProperty(extensionId)) {
+            log.warn(`Could not find extension ${extensionId} in the built in extensions.`);
+            return;
+        }
+
+        /** @TODO dupe handling for non-builtin extensions. See commit 670e51d33580e8a2e852b3b038bb3afc282f81b9 */
+        if (this.isExtensionLoaded(extensionId)) {
+            const message = `Rejecting attempt to load a second extension with ID ${extensionId}`;
+            log.warn(message);
+            return;
+        }
+
+        const extension = builtinExtensions[extensionId]();
+        const extensionInstance = new extension(this.runtime);
+        const serviceName = this._registerInternalExtension(extensionInstance);
+        this._loadedExtensions.set(extensionId, serviceName);
+    }
+
     /**
      * Load an extension by URL or internal extension ID
      * @param {string} extensionURL - the URL for the extension to load OR the ID of an internal extension
@@ -117,14 +145,14 @@ class ExtensionManager {
             if (this.isExtensionLoaded(extensionURL)) {
                 const message = `Rejecting attempt to load a second extension with ID ${extensionURL}`;
                 log.warn(message);
-                return Promise.reject(new Error(message));
+                return Promise.resolve();
             }
 
             const extension = builtinExtensions[extensionURL]();
             const extensionInstance = new extension(this.runtime);
-            return this._registerInternalExtension(extensionInstance).then(serviceName => {
+            const serviceName = this._registerInternalExtension(extensionInstance);
-                this._loadedExtensions.set(extensionURL, serviceName);
+            this._loadedExtensions.set(extensionURL, serviceName);
-            });
+            return Promise.resolve();
         }
 
         return new Promise((resolve, reject) => {
@@ -148,7 +176,7 @@ class ExtensionManager {
                     dispatch.call('runtime', '_refreshExtensionPrimitives', info);
                 })
                 .catch(e => {
-                    log.error(`Failed to refresh buildtin extension primitives: ${JSON.stringify(e)}`);
+                    log.error(`Failed to refresh built-in extension primitives: ${JSON.stringify(e)}`);
                 })
         );
         return Promise.all(allPromises);
@@ -161,6 +189,15 @@ class ExtensionManager {
         return [id, workerInfo.extensionURL];
     }
 
+    /**
+     * Synchronously collect extension metadata from the specified service and begin the extension registration process.
+     * @param {string} serviceName - the name of the service hosting the extension.
+     */
+    registerExtensionServiceSync (serviceName) {
+        const info = dispatch.callSync(serviceName, 'getInfo');
+        this._registerExtensionInfo(serviceName, info);
+    }
+
     /**
      * Collect extension metadata from the specified service and begin the extension registration process.
      * @param {string} serviceName - the name of the service hosting the extension.
@@ -189,17 +226,15 @@ class ExtensionManager {
     /**
      * Register an internal (non-Worker) extension object
      * @param {object} extensionObject - the extension object to register
-     * @returns {Promise} resolved once the extension is fully registered or rejected on failure
+     * @returns {string} The name of the registered extension service
      */
     _registerInternalExtension (extensionObject) {
         const extensionInfo = extensionObject.getInfo();
         const fakeWorkerId = this.nextExtensionWorker++;
         const serviceName = `extension_${fakeWorkerId}_${extensionInfo.id}`;
-        return dispatch.setService(serviceName, extensionObject)
+        dispatch.setServiceSync(serviceName, extensionObject);
-            .then(() => {
+        dispatch.callSync('extensions', 'registerExtensionServiceSync', serviceName);
-                dispatch.call('extensions', 'registerExtensionService', serviceName);
+        return serviceName;
-                return serviceName;
-            });
     }
 
     /**

src/virtual-machine.js

@@ -151,6 +151,11 @@ class VirtualMachine extends EventEmitter {
 
         this.extensionManager = new ExtensionManager(this.runtime);
 
+        // Load core extensions
+        for (const id of CORE_EXTENSIONS) {
+            this.extensionManager.loadExtensionIdSync(id);
+        }
+
         this.blockListener = this.blockListener.bind(this);
         this.flyoutBlockListener = this.flyoutBlockListener.bind(this);
         this.monitorBlockListener = this.monitorBlockListener.bind(this);
@@ -493,14 +498,6 @@ class VirtualMachine extends EventEmitter {
     installTargets (targets, extensions, wholeProject) {
         const extensionPromises = [];
 
-        if (wholeProject) {
-            CORE_EXTENSIONS.forEach(extensionID => {
-                if (!this.extensionManager.isExtensionLoaded(extensionID)) {
-                    extensionPromises.push(this.extensionManager.loadExtensionURL(extensionID));
-                }
-            });
-        }
-
         extensions.extensionIDs.forEach(extensionID => {
             if (!this.extensionManager.isExtensionLoaded(extensionID)) {
                 const extensionURL = extensions.extensionURLs.get(extensionID) || extensionID;

test/integration/internal-extension.js

@@ -4,6 +4,9 @@ const Worker = require('tiny-worker');
 const dispatch = require('../../src/dispatch/central-dispatch');
 const VirtualMachine = require('../../src/virtual-machine');
 
+const Sprite = require('../../src/sprites/sprite');
+const RenderedTarget = require('../../src/sprites/rendered-target');
+
 // By default Central Dispatch works with the Worker class built into the browser. Tell it to use TinyWorker instead.
 dispatch.workerClass = Worker;
 
@@ -52,23 +55,49 @@ test('internal extension', t => {
     t.ok(extension.status.constructorCalled);
 
     t.notOk(extension.status.getInfoCalled);
-    return vm.extensionManager._registerInternalExtension(extension).then(() => {
+    vm.extensionManager._registerInternalExtension(extension);
-        t.ok(extension.status.getInfoCalled);
+    t.ok(extension.status.getInfoCalled);
 
-        const func = vm.runtime.getOpcodeFunction('testInternalExtension_go');
+    const func = vm.runtime.getOpcodeFunction('testInternalExtension_go');
-        t.type(func, 'function');
+    t.type(func, 'function');
 
-        t.notOk(extension.status.goCalled);
+    t.notOk(extension.status.goCalled);
-        func();
+    func();
-        t.ok(extension.status.goCalled);
+    t.ok(extension.status.goCalled);
 
-        // There should be 2 menus - one is an array, one is the function to call.
+    // There should be 2 menus - one is an array, one is the function to call.
-        t.equal(vm.runtime._blockInfo[0].menus.length, 2);
+    t.equal(vm.runtime._blockInfo[0].menus.length, 2);
-        // First menu has 3 items.
+    // First menu has 3 items.
-        t.equal(
+    t.equal(
-            vm.runtime._blockInfo[0].menus[0].json.args0[0].options.length, 3);
+        vm.runtime._blockInfo[0].menus[0].json.args0[0].options.length, 3);
-        // Second menu is a dynamic menu and therefore should be a function.
+    // Second menu is a dynamic menu and therefore should be a function.
-        t.type(
+    t.type(
-            vm.runtime._blockInfo[0].menus[1].json.args0[0].options, 'function');
+        vm.runtime._blockInfo[0].menus[1].json.args0[0].options, 'function');
-    });
+
+    t.end();
+});
+
+test('load sync', t => {
+    const vm = new VirtualMachine();
+    vm.extensionManager.loadExtensionIdSync('coreExample');
+    t.ok(vm.extensionManager.isExtensionLoaded('coreExample'));
+
+    t.equal(vm.runtime._blockInfo.length, 1);
+    t.equal(vm.runtime._blockInfo[0].blocks.length, 1);
+    t.type(vm.runtime._blockInfo[0].blocks[0].info, 'object');
+    t.equal(vm.runtime._blockInfo[0].blocks[0].info.opcode, 'exampleOpcode');
+    t.equal(vm.runtime._blockInfo[0].blocks[0].info.blockType, 'reporter');
+
+    // Test the opcode function
+    t.equal(vm.runtime._blockInfo[0].blocks[0].info.func(), 'no stage yet');
+
+    const sprite = new Sprite(null, vm.runtime);
+    sprite.name = 'Stage';
+    const stage = new RenderedTarget(sprite, vm.runtime);
+    stage.isStage = true;
+    vm.runtime.targets = [stage];
+
+    t.equal(vm.runtime._blockInfo[0].blocks[0].info.func(), 'Stage');
+
+    t.end();
 });

test/unit/dispatch.js

@@ -62,3 +62,21 @@ test('remote', t => {
         .then(() => runServiceTest('RemoteDispatchTest', t), e => t.fail(e))
         .then(() => dispatch._remoteCall(worker, 'dispatch', 'terminate'), e => t.fail(e));
 });
+
+test('local, sync', t => {
+    dispatch.setServiceSync('SyncDispatchTest', new DispatchTestService());
+
+    const a = dispatch.callSync('SyncDispatchTest', 'returnFortyTwo');
+    t.equal(a, 42);
+
+    const b = dispatch.callSync('SyncDispatchTest', 'doubleArgument', 9);
+    t.equal(b, 18);
+
+    const c = dispatch.callSync('SyncDispatchTest', 'doubleArgument', 123);
+    t.equal(c, 246);
+
+    t.throws(() => dispatch.callSync('SyncDispatchTest', 'throwException'),
+        new Error('This is a test exception thrown by DispatchTest'));
+
+    t.end();
+});