From d04d6b2c6ad1c7a79b06f9eb727170cc3068c517 Mon Sep 17 00:00:00 2001
From: "Michael \"Z\" Goddard" <mzgoddard@gmail.com>
Date: Tue, 10 Apr 2018 16:29:51 -0400
Subject: [PATCH 1/9] Revert "Revert "Push reported""

This reverts commit 107adad647e63be9de90c8d8864d34609ef0701f.
---
 src/engine/blocks-execute-cache.js |  19 ++++
 src/engine/blocks.js               |  40 ++++++-
 src/engine/execute.js              | 173 ++++++++++++++++++++---------
 src/engine/runtime.js              |   4 +
 src/engine/sequencer.js            |   6 +-
 src/engine/thread.js               |  14 ++-
 test/unit/engine_sequencer.js      |   3 +-
 test/unit/engine_thread.js         |   4 +-
 8 files changed, 198 insertions(+), 65 deletions(-)
 create mode 100644 src/engine/blocks-execute-cache.js

diff --git a/src/engine/blocks-execute-cache.js b/src/engine/blocks-execute-cache.js
new file mode 100644
index 000000000..ffa8708b2
--- /dev/null
+++ b/src/engine/blocks-execute-cache.js
@@ -0,0 +1,19 @@
+/**
+ * @fileoverview
+ * Access point for private method shared between blocks.js and execute.js for
+ * caching execute information.
+ */
+
+/**
+ * A private method shared with execute to build an object containing the block
+ * information execute needs and that is reset when other cached Blocks info is
+ * reset.
+ * @param {Blocks} blocks Blocks containing the expected blockId
+ * @param {string} blockId blockId for the desired execute cache
+ */
+exports.getCached = function () {
+    throw new Error('blocks.js has not initialized BlocksExecuteCache');
+};
+
+// Call after the default throwing getCached is assigned for Blocks to replace.
+require('./blocks');
diff --git a/src/engine/blocks.js b/src/engine/blocks.js
index baad5e4c6..9af98db59 100644
--- a/src/engine/blocks.js
+++ b/src/engine/blocks.js
@@ -4,6 +4,7 @@ const xmlEscape = require('../util/xml-escape');
 const MonitorRecord = require('./monitor-record');
 const Clone = require('../util/clone');
 const {Map} = require('immutable');
+const BlocksExecuteCache = require('./blocks-execute-cache');
 
 /**
  * @fileoverview
@@ -47,7 +48,14 @@ class Blocks {
              * Cache procedure definitions by block id
              * @type {object.<string, ?string>}
              */
-            procedureDefinitions: {}
+            procedureDefinitions: {},
+
+            /**
+             * A cache for execute to use and store on. Only available to
+             * execute.
+             * @type {object.<string, object>}
+             */
+            _executeCached: {}
         };
 
     }
@@ -359,6 +367,7 @@ class Blocks {
         this._cache.inputs = {};
         this._cache.procedureParamNames = {};
         this._cache.procedureDefinitions = {};
+        this._cache._executeCached = {};
     }
 
     /**
@@ -435,7 +444,7 @@ class Blocks {
             const isSpriteSpecific = optRuntime.monitorBlockInfo.hasOwnProperty(block.opcode) &&
                 optRuntime.monitorBlockInfo[block.opcode].isSpriteSpecific;
             block.targetId = isSpriteSpecific ? optRuntime.getEditingTarget().id : null;
-            
+
             if (wasMonitored && !block.isMonitored) {
                 optRuntime.requestRemoveMonitor(block.id);
             } else if (!wasMonitored && block.isMonitored) {
@@ -859,4 +868,31 @@ class Blocks {
     }
 }
 
+/**
+ * A private method shared with execute to build an object containing the block
+ * information execute needs and that is reset when other cached Blocks info is
+ * reset.
+ * @param {Blocks} blocks Blocks containing the expected blockId
+ * @param {string} blockId blockId for the desired execute cache
+ * @return {object} execute cache object
+ */
+BlocksExecuteCache.getCached = function (blocks, blockId) {
+    const block = blocks.getBlock(blockId);
+    if (typeof block === 'undefined') return null;
+    let cached = blocks._cache._executeCached[blockId];
+    if (typeof cached !== 'undefined') {
+        return cached;
+    }
+
+    cached = {
+        _initialized: false,
+        opcode: blocks.getOpcode(block),
+        fields: blocks.getFields(block),
+        inputs: blocks.getInputs(block),
+        mutation: blocks.getMutation(block)
+    };
+    blocks._cache._executeCached[blockId] = cached;
+    return cached;
+};
+
 module.exports = Blocks;
diff --git a/src/engine/execute.js b/src/engine/execute.js
index 13742ec76..85af1b821 100644
--- a/src/engine/execute.js
+++ b/src/engine/execute.js
@@ -1,4 +1,5 @@
 const BlockUtility = require('./block-utility');
+const BlocksExecuteCache = require('./blocks-execute-cache');
 const log = require('../util/log');
 const Thread = require('./thread');
 const {Map} = require('immutable');
@@ -98,32 +99,27 @@ const handleReport = function (
     }
 };
 
+/**
+ * A convenienve constant to hide that the recursiveCall argument to execute is
+ * a boolean trap.
+ * @const {boolean}
+ */
+const RECURSIVE = true;
+
 /**
  * Execute a block.
  * @param {!Sequencer} sequencer Which sequencer is executing.
  * @param {!Thread} thread Thread which to read and execute.
+ * @param {boolean} recursiveCall is execute called from another execute call?
  */
-const execute = function (sequencer, thread) {
+const execute = function (sequencer, thread, recursiveCall) {
     const runtime = sequencer.runtime;
-    const target = thread.target;
-
-    // Stop if block or target no longer exists.
-    if (target === null) {
-        // No block found: stop the thread; script no longer exists.
-        sequencer.retireThread(thread);
-        return;
-    }
 
     // Current block to execute is the one on the top of the stack.
     const currentBlockId = thread.peekStack();
     const currentStackFrame = thread.peekStackFrame();
 
-    let blockContainer;
-    if (thread.updateMonitor) {
-        blockContainer = runtime.monitorBlocks;
-    } else {
-        blockContainer = target.blocks;
-    }
+    let blockContainer = thread.blockContainer;
     let block = blockContainer.getBlock(currentBlockId);
     if (typeof block === 'undefined') {
         blockContainer = runtime.flyoutBlocks;
@@ -136,33 +132,83 @@ const execute = function (sequencer, thread) {
         }
     }
 
-    const opcode = blockContainer.getOpcode(block);
-    const fields = blockContainer.getFields(block);
-    const inputs = blockContainer.getInputs(block);
-    const blockFunction = runtime.getOpcodeFunction(opcode);
-    const isHat = runtime.getIsHat(opcode);
+    const blockCached = BlocksExecuteCache.getCached(blockContainer, currentBlockId);
+    if (blockCached._initialized !== true) {
+        const {opcode, fields, inputs} = blockCached;
 
+        // Assign opcode isHat and blockFunction data to avoid dynamic lookups.
+        blockCached._isHat = runtime.getIsHat(opcode);
+        blockCached._blockFunction = runtime.getOpcodeFunction(opcode);
+        blockCached._definedBlockFunction = typeof blockCached._blockFunction !== 'undefined';
 
-    if (!opcode) {
-        log.warn(`Could not get opcode for block: ${currentBlockId}`);
-        return;
+        const fieldKeys = Object.keys(fields);
+
+        // Store the current shadow value if there is a shadow value.
+        blockCached._isShadowBlock = fieldKeys.length === 1 && Object.keys(inputs).length === 0;
+        blockCached._shadowValue = fieldKeys.length === 1 && fields[fieldKeys[0]].value;
+
+        // Store a fields copy. If fields is a VARIABLE, LIST, or
+        // BROADCAST_OPTION, store the created values so fields assignment to
+        // argValues does not iterate over fields.
+        blockCached._fields = Object.assign({}, blockCached.fields);
+        blockCached._isFieldVariable = fieldKeys.length === 1 && fieldKeys.includes('VARIABLE');
+        blockCached._fieldVariable = blockCached._isFieldVariable ?
+            {
+                id: fields.VARIABLE.id,
+                name: fields.VARIABLE.value
+            } :
+            null;
+        blockCached._isFieldList = fieldKeys.length === 1 && fieldKeys.includes('LIST');
+        blockCached._fieldList = blockCached._isFieldList ?
+            {
+                id: fields.LIST.id,
+                name: fields.LIST.value
+            } :
+            null;
+        blockCached._isFieldBroadcastOption = fieldKeys.length === 1 && fieldKeys.includes('BROADCAST_OPTION');
+        blockCached._fieldBroadcastOption = blockCached._isFieldBroadcastOption ?
+            {
+                id: fields.BROADCAST_OPTION.id,
+                name: fields.BROADCAST_OPTION.value
+            } :
+            null;
+        blockCached._isFieldKnown = blockCached._isFieldVariable ||
+            blockCached._isFieldList || blockCached._isFieldBroadcastOption;
+
+        // Store a modified inputs. This assures the keys are its own properties
+        // and that custom_block will not be evaluated.
+        blockCached._inputs = Object.assign({}, blockCached.inputs);
+        delete blockCached._inputs.custom_block;
+
+        blockCached._initialized = true;
     }
 
+    const opcode = blockCached.opcode;
+    const fields = blockCached._fields;
+    const inputs = blockCached._inputs;
+    const mutation = blockCached.mutation;
+    const blockFunction = blockCached._blockFunction;
+    const isHat = blockCached._isHat;
+
     // Hats and single-field shadows are implemented slightly differently
     // from regular blocks.
     // For hats: if they have an associated block function,
     // it's treated as a predicate; if not, execution will proceed as a no-op.
     // For single-field shadows: If the block has a single field, and no inputs,
     // immediately return the value of the field.
-    if (typeof blockFunction === 'undefined') {
-        if (isHat) {
-            // Skip through the block (hat with no predicate).
+    if (!blockCached._definedBlockFunction) {
+        if (!opcode) {
+            log.warn(`Could not get opcode for block: ${currentBlockId}`);
             return;
         }
-        const keys = Object.keys(fields);
-        if (keys.length === 1 && Object.keys(inputs).length === 0) {
+
+        if (recursiveCall === RECURSIVE && blockCached._isShadowBlock) {
             // One field and no inputs - treat as arg.
-            handleReport(fields[keys[0]].value, sequencer, thread, currentBlockId, opcode, isHat);
+            thread.pushReportedValue(blockCached._shadowValue);
+            thread.status = Thread.STATUS_RUNNING;
+        } else if (isHat) {
+            // Skip through the block (hat with no predicate).
+            return;
         } else {
             log.warn(`Could not get implementation for opcode: ${opcode}`);
         }
@@ -174,24 +220,22 @@ const execute = function (sequencer, thread) {
     const argValues = {};
 
     // Add all fields on this block to the argValues.
-    for (const fieldName in fields) {
-        if (!fields.hasOwnProperty(fieldName)) continue;
-        if (fieldName === 'VARIABLE' || fieldName === 'LIST' ||
-            fieldName === 'BROADCAST_OPTION') {
-            argValues[fieldName] = {
-                id: fields[fieldName].id,
-                name: fields[fieldName].value
-            };
-        } else {
+    if (blockCached._isFieldKnown) {
+        if (blockCached._isFieldVariable) {
+            argValues.VARIABLE = blockCached._fieldVariable;
+        } else if (blockCached._isFieldList) {
+            argValues.LIST = blockCached._fieldList;
+        } else if (blockCached._isFieldBroadcastOption) {
+            argValues.BROADCAST_OPTION = blockCached._fieldBroadcastOption;
+        }
+    } else {
+        for (const fieldName in fields) {
             argValues[fieldName] = fields[fieldName].value;
         }
     }
 
     // Recursively evaluate input blocks.
     for (const inputName in inputs) {
-        if (!inputs.hasOwnProperty(inputName)) continue;
-        // Do not evaluate the internal custom command block within definition
-        if (inputName === 'custom_block') continue;
         const input = inputs[inputName];
         const inputBlockId = input.block;
         // Is there no value for this input waiting in the stack frame?
@@ -202,8 +246,16 @@ const execute = function (sequencer, thread) {
             // Save name of input for `Thread.pushReportedValue`.
             currentStackFrame.waitingReporter = inputName;
             // Actually execute the block.
-            execute(sequencer, thread);
+            execute(sequencer, thread, RECURSIVE);
             if (thread.status === Thread.STATUS_PROMISE_WAIT) {
+                for (const _inputName in inputs) {
+                    if (_inputName === inputName) break;
+                    if (_inputName === 'BROADCAST_INPUT') {
+                        currentStackFrame.reported[_inputName] = argValues[_inputName].name;
+                    } else {
+                        currentStackFrame.reported[_inputName] = argValues[_inputName];
+                    }
+                }
                 return;
             }
 
@@ -212,7 +264,23 @@ const execute = function (sequencer, thread) {
             currentStackFrame.waitingReporter = null;
             thread.popStack();
         }
-        const inputValue = currentStackFrame.reported[inputName];
+        let inputValue;
+        if (
+            currentStackFrame.waitingReporter === null
+        ) {
+            inputValue = currentStackFrame.justReported;
+        } else if (currentStackFrame.waitingReporter === inputName) {
+            inputValue = currentStackFrame.justReported;
+            currentStackFrame.waitingReporter = null;
+            // If we've gotten this far, all of the input blocks are evaluated,
+            // and `argValues` is fully populated. So, execute the block
+            // primitive. First, clear `currentStackFrame.reported`, so any
+            // subsequent execution (e.g., on return from a branch) gets fresh
+            // inputs.
+            currentStackFrame.reported = {};
+        } else if (typeof currentStackFrame.reported[inputName] !== 'undefined') {
+            inputValue = currentStackFrame.reported[inputName];
+        }
         if (inputName === 'BROADCAST_INPUT') {
             const broadcastInput = inputs[inputName];
             // Check if something is plugged into the broadcast block, or
@@ -239,16 +307,7 @@ const execute = function (sequencer, thread) {
     }
 
     // Add any mutation to args (e.g., for procedures).
-    const mutation = blockContainer.getMutation(block);
-    if (mutation !== null) {
-        argValues.mutation = mutation;
-    }
-
-    // If we've gotten this far, all of the input blocks are evaluated,
-    // and `argValues` is fully populated. So, execute the block primitive.
-    // First, clear `currentStackFrame.reported`, so any subsequent execution
-    // (e.g., on return from a branch) gets fresh inputs.
-    currentStackFrame.reported = {};
+    argValues.mutation = mutation;
 
     let primitiveReportedValue = null;
     blockUtility.sequencer = sequencer;
@@ -271,7 +330,7 @@ const execute = function (sequencer, thread) {
         runtime.profiler.records.push(runtime.profiler.STOP, performance.now());
     }
 
-    if (typeof primitiveReportedValue === 'undefined') {
+    if (recursiveCall !== RECURSIVE && typeof primitiveReportedValue === 'undefined') {
         // No value reported - potentially a command block.
         // Edge-activated hats don't request a glow; all commands do.
         thread.requestScriptGlowInFrame = true;
@@ -318,7 +377,11 @@ const execute = function (sequencer, thread) {
             thread.popStack();
         });
     } else if (thread.status === Thread.STATUS_RUNNING) {
-        handleReport(primitiveReportedValue, sequencer, thread, currentBlockId, opcode, isHat);
+        if (recursiveCall === RECURSIVE) {
+            thread.pushReportedValue(primitiveReportedValue);
+        } else {
+            handleReport(primitiveReportedValue, sequencer, thread, currentBlockId, opcode, isHat);
+        }
     }
 };
 
diff --git a/src/engine/runtime.js b/src/engine/runtime.js
index 7c400e0e4..f0500061b 100644
--- a/src/engine/runtime.js
+++ b/src/engine/runtime.js
@@ -947,6 +947,9 @@ class Runtime extends EventEmitter {
         thread.target = target;
         thread.stackClick = opts.stackClick;
         thread.updateMonitor = opts.updateMonitor;
+        thread.blockContainer = opts.updateMonitor ?
+            this.monitorBlocks :
+            target.blocks;
 
         thread.pushStack(id);
         this.threads.push(thread);
@@ -976,6 +979,7 @@ class Runtime extends EventEmitter {
         newThread.target = thread.target;
         newThread.stackClick = thread.stackClick;
         newThread.updateMonitor = thread.updateMonitor;
+        newThread.blockContainer = thread.blockContainer;
         newThread.pushStack(thread.topBlock);
         const i = this.threads.indexOf(thread);
         if (i > -1) {
diff --git a/src/engine/sequencer.js b/src/engine/sequencer.js
index ce65163d4..7cdb47247 100644
--- a/src/engine/sequencer.js
+++ b/src/engine/sequencer.js
@@ -196,7 +196,11 @@ class Sequencer {
                 this.runtime.profiler.records.push(
                     this.runtime.profiler.START, executeProfilerId, null, performance.now());
             }
-            execute(this, thread);
+            if (thread.target === null) {
+                this.retireThread(thread);
+            } else {
+                execute(this, thread);
+            }
             if (this.runtime.profiler !== null) {
                 // this.runtime.profiler.stop();
                 this.runtime.profiler.records.push(this.runtime.profiler.STOP, performance.now());
diff --git a/src/engine/thread.js b/src/engine/thread.js
index def1fc1ef..a618b460b 100644
--- a/src/engine/thread.js
+++ b/src/engine/thread.js
@@ -42,6 +42,12 @@ class Thread {
          */
         this.target = null;
 
+        /**
+         * The Blocks this thread will execute.
+         * @type {Blocks}
+         */
+        this.blockContainer = null;
+
         /**
          * Whether the thread requests its script to glow during this frame.
          * @type {boolean}
@@ -124,7 +130,8 @@ class Thread {
             this.stackFrames.push({
                 isLoop: false, // Whether this level of the stack is a loop.
                 warpMode: warpMode, // Whether this level is in warp mode.
-                reported: {}, // Collects reported input values.
+                justReported: null, // Reported value from just executed block.
+                reported: {}, // Persists reported inputs during async block.
                 waitingReporter: null, // Name of waiting reporter.
                 params: {}, // Procedure parameters.
                 executionContext: {} // A context passed to block implementations.
@@ -209,9 +216,8 @@ class Thread {
      */
     pushReportedValue (value) {
         const parentStackFrame = this.peekParentStackFrame();
-        if (parentStackFrame) {
-            const waitingReporter = parentStackFrame.waitingReporter;
-            parentStackFrame.reported[waitingReporter] = value;
+        if (parentStackFrame !== null) {
+            parentStackFrame.justReported = value;
         }
     }
 
diff --git a/test/unit/engine_sequencer.js b/test/unit/engine_sequencer.js
index 751e9bb64..55faeb69e 100644
--- a/test/unit/engine_sequencer.js
+++ b/test/unit/engine_sequencer.js
@@ -91,7 +91,8 @@ const generateThread = function (runtime) {
     rt.blocks.createBlock(generateBlock(next));
     th.pushStack(next);
     th.target = rt;
-    
+    th.blockContainer = rt.blocks;
+
     runtime.threads.push(th);
 
     return th;
diff --git a/test/unit/engine_thread.js b/test/unit/engine_thread.js
index 15fab7c02..7667a1b2e 100644
--- a/test/unit/engine_thread.js
+++ b/test/unit/engine_thread.js
@@ -89,8 +89,8 @@ test('pushReportedValue', t => {
     th.pushStack('arbitraryString');
     th.pushStack('secondString');
     th.pushReportedValue('value');
-    t.strictEquals(th.peekParentStackFrame().reported.null, 'value');
-    
+    t.strictEquals(th.peekParentStackFrame().justReported, 'value');
+
     t.end();
 });
 

From 02dd9a8c22b1284afa265fe2cafe565092836ff2 Mon Sep 17 00:00:00 2001
From: "Michael \"Z\" Goddard" <mzgoddard@gmail.com>
Date: Mon, 16 Apr 2018 17:35:56 -0400
Subject: [PATCH 2/9] Unset justReported after reading its value

Avoid justReported values leaking to other argValues by setting back to the
default null value.
---
 src/engine/execute.js | 79 +++++++++++++++++++++++++------------------
 src/engine/thread.js  |  2 +-
 2 files changed, 47 insertions(+), 34 deletions(-)

diff --git a/src/engine/execute.js b/src/engine/execute.js
index 85af1b821..2169ede95 100644
--- a/src/engine/execute.js
+++ b/src/engine/execute.js
@@ -100,12 +100,24 @@ const handleReport = function (
 };
 
 /**
- * A convenienve constant to hide that the recursiveCall argument to execute is
- * a boolean trap.
+ * A convenience constant to help make use of the recursiveCall argument easier
+ * to read.
  * @const {boolean}
  */
 const RECURSIVE = true;
 
+/**
+ * A simple description of the kind of information held in the fields of a block.
+ * @enum {string}
+ */
+const FieldKind = {
+    NONE: 'NONE',
+    VARIABLE: 'VARIABLE',
+    LIST: 'LIST',
+    BROADCAST_OPTION: 'BROADCAST_OPTION',
+    DYNAMIC: 'DYNAMIC'
+};
+
 /**
  * Execute a block.
  * @param {!Sequencer} sequencer Which sequencer is executing.
@@ -145,35 +157,32 @@ const execute = function (sequencer, thread, recursiveCall) {
 
         // Store the current shadow value if there is a shadow value.
         blockCached._isShadowBlock = fieldKeys.length === 1 && Object.keys(inputs).length === 0;
-        blockCached._shadowValue = fieldKeys.length === 1 && fields[fieldKeys[0]].value;
+        blockCached._shadowValue = blockCached._isShadowBlock && fields[fieldKeys[0]].value;
 
         // Store a fields copy. If fields is a VARIABLE, LIST, or
         // BROADCAST_OPTION, store the created values so fields assignment to
         // argValues does not iterate over fields.
         blockCached._fields = Object.assign({}, blockCached.fields);
-        blockCached._isFieldVariable = fieldKeys.length === 1 && fieldKeys.includes('VARIABLE');
-        blockCached._fieldVariable = blockCached._isFieldVariable ?
-            {
+        blockCached._fieldKind = fieldKeys.length > 0 ? FieldKind.DYNAMIC : FieldKind.NONE;
+        if (fieldKeys.length === 1 && fieldKeys.includes('VARIABLE')) {
+            blockCached._fieldKind = FieldKind.VARIABLE;
+            blockCached._fieldVariable = {
                 id: fields.VARIABLE.id,
                 name: fields.VARIABLE.value
-            } :
-            null;
-        blockCached._isFieldList = fieldKeys.length === 1 && fieldKeys.includes('LIST');
-        blockCached._fieldList = blockCached._isFieldList ?
-            {
+            };
+        } else if (fieldKeys.length === 1 && fieldKeys.includes('LIST')) {
+            blockCached._fieldKind = FieldKind.LIST;
+            blockCached._fieldList = {
                 id: fields.LIST.id,
                 name: fields.LIST.value
-            } :
-            null;
-        blockCached._isFieldBroadcastOption = fieldKeys.length === 1 && fieldKeys.includes('BROADCAST_OPTION');
-        blockCached._fieldBroadcastOption = blockCached._isFieldBroadcastOption ?
-            {
+            };
+        } else if (fieldKeys.length === 1 && fieldKeys.includes('BROADCAST_OPTION')) {
+            blockCached._fieldKind = FieldKind.BROADCAST_OPTION;
+            blockCached._fieldBroadcastOption = {
                 id: fields.BROADCAST_OPTION.id,
                 name: fields.BROADCAST_OPTION.value
-            } :
-            null;
-        blockCached._isFieldKnown = blockCached._isFieldVariable ||
-            blockCached._isFieldList || blockCached._isFieldBroadcastOption;
+            };
+        }
 
         // Store a modified inputs. This assures the keys are its own properties
         // and that custom_block will not be evaluated.
@@ -220,17 +229,21 @@ const execute = function (sequencer, thread, recursiveCall) {
     const argValues = {};
 
     // Add all fields on this block to the argValues.
-    if (blockCached._isFieldKnown) {
-        if (blockCached._isFieldVariable) {
+    if (blockCached._fieldKind !== FieldKind.NONE) {
+        switch (blockCached._fieldKind) {
+        case FieldKind.VARIABLE:
             argValues.VARIABLE = blockCached._fieldVariable;
-        } else if (blockCached._isFieldList) {
+            break;
+        case FieldKind.LIST:
             argValues.LIST = blockCached._fieldList;
-        } else if (blockCached._isFieldBroadcastOption) {
+            break;
+        case FieldKind.BROADCAST_OPTION:
             argValues.BROADCAST_OPTION = blockCached._fieldBroadcastOption;
-        }
-    } else {
-        for (const fieldName in fields) {
-            argValues[fieldName] = fields[fieldName].value;
+            break;
+        default:
+            for (const fieldName in fields) {
+                argValues[fieldName] = fields[fieldName].value;
+            }
         }
     }
 
@@ -239,7 +252,7 @@ const execute = function (sequencer, thread, recursiveCall) {
         const input = inputs[inputName];
         const inputBlockId = input.block;
         // Is there no value for this input waiting in the stack frame?
-        if (inputBlockId !== null && typeof currentStackFrame.reported[inputName] === 'undefined') {
+        if (inputBlockId !== null && currentStackFrame.waitingReporter === null) {
             // If there's not, we need to evaluate the block.
             // Push to the stack to evaluate the reporter block.
             thread.pushStack(inputBlockId);
@@ -251,7 +264,7 @@ const execute = function (sequencer, thread, recursiveCall) {
                 for (const _inputName in inputs) {
                     if (_inputName === inputName) break;
                     if (_inputName === 'BROADCAST_INPUT') {
-                        currentStackFrame.reported[_inputName] = argValues[_inputName].name;
+                        currentStackFrame.reported[_inputName] = argValues.BROADCAST_OPTION.name;
                     } else {
                         currentStackFrame.reported[_inputName] = argValues[_inputName];
                     }
@@ -265,13 +278,13 @@ const execute = function (sequencer, thread, recursiveCall) {
             thread.popStack();
         }
         let inputValue;
-        if (
-            currentStackFrame.waitingReporter === null
-        ) {
+        if (currentStackFrame.waitingReporter === null) {
             inputValue = currentStackFrame.justReported;
+            currentStackFrame.justReported = null;
         } else if (currentStackFrame.waitingReporter === inputName) {
             inputValue = currentStackFrame.justReported;
             currentStackFrame.waitingReporter = null;
+            currentStackFrame.justReported = null;
             // If we've gotten this far, all of the input blocks are evaluated,
             // and `argValues` is fully populated. So, execute the block
             // primitive. First, clear `currentStackFrame.reported`, so any
diff --git a/src/engine/thread.js b/src/engine/thread.js
index a618b460b..77444af94 100644
--- a/src/engine/thread.js
+++ b/src/engine/thread.js
@@ -217,7 +217,7 @@ class Thread {
     pushReportedValue (value) {
         const parentStackFrame = this.peekParentStackFrame();
         if (parentStackFrame !== null) {
-            parentStackFrame.justReported = value;
+            parentStackFrame.justReported = typeof value === 'undefined' ? null : value;
         }
     }
 

From 1c68b543dfa726e2c199bf6c4955b85afcc94e6f Mon Sep 17 00:00:00 2001
From: "Michael \"Z\" Goddard" <mzgoddard@gmail.com>
Date: Wed, 18 Apr 2018 12:24:39 -0400
Subject: [PATCH 3/9] Comment execute's BlocksExecuteCache usage

Explain how BlocksExecuteCache helps speed up execute by storing values from
Blocks and one-time derived values on an object that is released when Blocks is
edited in the editor. The one time created cache object for a block then
simplifies the complexity of later javascript operations to use these stored
values.
---
 src/engine/execute.js | 45 +++++++++++++++++++++++++++++++++----------
 1 file changed, 35 insertions(+), 10 deletions(-)

diff --git a/src/engine/execute.js b/src/engine/execute.js
index 2169ede95..f819b6df2 100644
--- a/src/engine/execute.js
+++ b/src/engine/execute.js
@@ -144,6 +144,20 @@ const execute = function (sequencer, thread, recursiveCall) {
         }
     }
 
+    // With the help of the Blocks class create a cached copy of values from
+    // Blocks and the derived values execute needs. These values can be produced
+    // one time during the first execution of a block so that later executions
+    // are faster by using these cached values. This helps turn most costly
+    // javascript operations like testing if the fields for a block has a
+    // certain key like VARIABLE into a test that done once is saved on the
+    // cache object to _isFieldVariable. This reduces the cost later in execute
+    // when that will determine how execute creates the argValues for the
+    // blockFunction.
+    //
+    // With Blocks providing this private function for execute to use, any time
+    // Blocks is modified in the editor these cached objects will be cleaned up
+    // and new cached copies can be created. This lets us optimize this critical
+    // path while keeping up to date with editor changes to a project.
     const blockCached = BlocksExecuteCache.getCached(blockContainer, currentBlockId);
     if (blockCached._initialized !== true) {
         const {opcode, fields, inputs} = blockCached;
@@ -201,10 +215,10 @@ const execute = function (sequencer, thread, recursiveCall) {
 
     // Hats and single-field shadows are implemented slightly differently
     // from regular blocks.
-    // For hats: if they have an associated block function,
-    // it's treated as a predicate; if not, execution will proceed as a no-op.
-    // For single-field shadows: If the block has a single field, and no inputs,
-    // immediately return the value of the field.
+    // For hats: if they have an associated block function, it's treated as a
+    // predicate; if not, execution will proceed as a no-op. For single-field
+    // shadows: If the block has a single field, and no inputs, immediately
+    // return the value of the field.
     if (!blockCached._definedBlockFunction) {
         if (!opcode) {
             log.warn(`Could not get opcode for block: ${currentBlockId}`);
@@ -228,7 +242,9 @@ const execute = function (sequencer, thread, recursiveCall) {
     // Generate values for arguments (inputs).
     const argValues = {};
 
-    // Add all fields on this block to the argValues.
+    // Add all fields on this block to the argValues. Some known fields may
+    // appear by themselves and can be set to argValues quicker by setting them
+    // explicitly.
     if (blockCached._fieldKind !== FieldKind.NONE) {
         switch (blockCached._fieldKind) {
         case FieldKind.VARIABLE:
@@ -261,7 +277,13 @@ const execute = function (sequencer, thread, recursiveCall) {
             // Actually execute the block.
             execute(sequencer, thread, RECURSIVE);
             if (thread.status === Thread.STATUS_PROMISE_WAIT) {
+                // Waiting for the block to resolve, store the current argValues
+                // onto a member of the currentStackFrame that can be used once
+                // the nested block resolves to rebuild argValues up to this
+                // point.
                 for (const _inputName in inputs) {
+                    // We are waiting on the nested block at inputName so we
+                    // don't need to store any more inputs.
                     if (_inputName === inputName) break;
                     if (_inputName === 'BROADCAST_INPUT') {
                         currentStackFrame.reported[_inputName] = argValues.BROADCAST_OPTION.name;
@@ -277,6 +299,7 @@ const execute = function (sequencer, thread, recursiveCall) {
             currentStackFrame.waitingReporter = null;
             thread.popStack();
         }
+
         let inputValue;
         if (currentStackFrame.waitingReporter === null) {
             inputValue = currentStackFrame.justReported;
@@ -285,11 +308,11 @@ const execute = function (sequencer, thread, recursiveCall) {
             inputValue = currentStackFrame.justReported;
             currentStackFrame.waitingReporter = null;
             currentStackFrame.justReported = null;
-            // If we've gotten this far, all of the input blocks are evaluated,
-            // and `argValues` is fully populated. So, execute the block
-            // primitive. First, clear `currentStackFrame.reported`, so any
-            // subsequent execution (e.g., on return from a branch) gets fresh
-            // inputs.
+            // We have rebuilt argValues with all the stored values in the
+            // currentStackFrame from the nested block's promise resolving.
+            // Using the reported value from the block we waited on, reset the
+            // storage member of currentStackFrame so the next execute call at
+            // this level can use it in a clean state.
             currentStackFrame.reported = {};
         } else if (typeof currentStackFrame.reported[inputName] !== 'undefined') {
             inputValue = currentStackFrame.reported[inputName];
@@ -391,6 +414,8 @@ const execute = function (sequencer, thread, recursiveCall) {
         });
     } else if (thread.status === Thread.STATUS_RUNNING) {
         if (recursiveCall === RECURSIVE) {
+            // In recursive calls (where execute calls execute) handleReport
+            // simplifies to just calling thread.pushReportedValue.
             thread.pushReportedValue(primitiveReportedValue);
         } else {
             handleReport(primitiveReportedValue, sequencer, thread, currentBlockId, opcode, isHat);

From 218725d7714cae85be408add1ee316b016bc43a9 Mon Sep 17 00:00:00 2001
From: Andrew Sliwinski <andrewsliwinski@acm.org>
Date: Thu, 26 Apr 2018 10:29:29 -0400
Subject: [PATCH 4/9] Cast 'create clone' argument to string. Resolves GH-974

---
 src/blocks/scratch3_control.js | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/src/blocks/scratch3_control.js b/src/blocks/scratch3_control.js
index 3f7a8e79b..4636d282d 100644
--- a/src/blocks/scratch3_control.js
+++ b/src/blocks/scratch3_control.js
@@ -144,15 +144,21 @@ class Scratch3ControlBlocks {
     }
 
     createClone (args, util) {
+        // Cast argument to string
+        args.CLONE_OPTION = Cast.toString(args.CLONE_OPTION);
+
+        // Set clone target
         let cloneTarget;
         if (args.CLONE_OPTION === '_myself_') {
             cloneTarget = util.target;
         } else {
             cloneTarget = this.runtime.getSpriteTargetByName(args.CLONE_OPTION);
         }
-        if (!cloneTarget) {
-            return;
-        }
+
+        // If clone target is not found, return
+        if (!cloneTarget) return;
+
+        // Create clone
         const newClone = cloneTarget.makeClone();
         if (newClone) {
             this.runtime.targets.push(newClone);

From 5f4139cbe4cc1fadafc4df102522d813b6682da4 Mon Sep 17 00:00:00 2001
From: Karishma Chadha <kchadha@scratch.mit.edu>
Date: Thu, 26 Apr 2018 13:24:31 -0400
Subject: [PATCH 5/9] Use video provider instead of making call to getUserMedia
 directly.

---
 src/io/video.js        | 325 ++++-------------------------------------
 src/virtual-machine.js |   4 +
 2 files changed, 32 insertions(+), 297 deletions(-)

diff --git a/src/io/video.js b/src/io/video.js
index ba05ff244..70b753908 100644
--- a/src/io/video.js
+++ b/src/io/video.js
@@ -1,67 +1,7 @@
-const log = require('../util/log');
-
 class Video {
     constructor (runtime) {
-        /**
-         * Reference to the owning Runtime.
-         * @type{!Runtime}
-         */
         this.runtime = runtime;
-
-        /**
-         * Default value for mirrored frames.
-         * @type boolean
-         */
-        this.mirror = true;
-
-        /**
-         * Cache frames for this many ms.
-         * @type number
-         */
-        this._frameCacheTimeout = 16;
-
-        /**
-         * DOM Video element
-         * @private
-         */
-        this._video = null;
-
-        /**
-         * Usermedia stream track
-         * @private
-         */
-        this._track = null;
-
-        /**
-         * Stores some canvas/frame data per resolution/mirror states
-         */
-        this._workspace = [];
-
-        /**
-         * Id representing a Scratch Renderer skin the video is rendered to for
-         * previewing.
-         * @type {number}
-         */
-        this._skinId = -1;
-
-        /**
-         * The Scratch Renderer Skin object.
-         * @type {Skin}
-         */
-        this._skin = null;
-
-        /**
-         * Id for a drawable using the video's skin that will render as a video
-         * preview.
-         * @type {Drawable}
-         */
-        this._drawable = -1;
-
-        /**
-         * Store the last state of the video transparency ghost effect
-         * @type {number}
-         */
-        this._ghost = 0;
+        this.provider = null;
     }
 
     static get FORMAT_IMAGE_DATA () {
@@ -97,39 +37,17 @@ class Video {
      * @return {Promise.<Video>} resolves a promise to this IO device when video is ready.
      */
     enableVideo () {
-        this.enabled = true;
-        return this._setupVideo();
+        if (this.provider) return this.provider.enableVideo();
+        return null;
     }
 
     /**
      * Disable video stream (turn video off)
+     * @return {void}
      */
     disableVideo () {
-        this.enabled = false;
-        // If we have begun a setup process, call _teardown after it completes
-        if (this._singleSetup) {
-            this._singleSetup
-                .then(this._teardown.bind(this))
-                .catch(err => this.onError(err));
-        }
-    }
-
-    /**
-     * async part of disableVideo
-     * @private
-     */
-    _teardown () {
-        // we might be asked to re-enable before _teardown is called, just ignore it.
-        if (this.enabled === false) {
-            this._disablePreview();
-            this._singleSetup = null;
-            // by clearing refs to video and track, we should lose our hold over the camera
-            this._video = null;
-            if (this._track) {
-                this._track.stop();
-            }
-            this._track = null;
-        }
+        if (this.provider) return this.provider.disableVideo();
+        return null;
     }
 
     /**
@@ -151,233 +69,46 @@ class Video {
         format = Video.FORMAT_IMAGE_DATA,
         cacheTimeout = this._frameCacheTimeout
     }) {
+        if (this.provider) return this.provider.getFrame({dimensions, mirror, format, cacheTimeout});
         if (!this.videoReady) {
             return null;
         }
-        const [width, height] = dimensions;
-        const workspace = this._getWorkspace({dimensions, mirror: Boolean(mirror)});
-        const {videoWidth, videoHeight} = this._video;
-        const {canvas, context, lastUpdate, cacheData} = workspace;
-        const now = Date.now();
-
-        // if the canvas hasn't been updated...
-        if (lastUpdate + cacheTimeout < now) {
-
-            if (mirror) {
-                context.scale(-1, 1);
-                context.translate(width * -1, 0);
-            }
-
-            context.drawImage(this._video,
-                // source x, y, width, height
-                0, 0, videoWidth, videoHeight,
-                // dest x, y, width, height
-                0, 0, width, height
-            );
-
-            context.resetTransform();
-            workspace.lastUpdate = now;
-        }
-
-        // each data type has it's own data cache, but the canvas is the same
-        if (!cacheData[format]) {
-            cacheData[format] = {lastUpdate: 0};
-        }
-        const formatCache = cacheData[format];
-
-        if (formatCache.lastUpdate + cacheTimeout < now) {
-            if (format === Video.FORMAT_IMAGE_DATA) {
-                formatCache.lastData = context.getImageData(0, 0, width, height);
-            } else if (format === Video.FORMAT_CANVAS) {
-                // this will never change
-                formatCache.lastUpdate = Infinity;
-                formatCache.lastData = canvas;
-            } else {
-                log.error(`video io error - unimplemented format ${format}`);
-                // cache the null result forever, don't log about it again..
-                formatCache.lastUpdate = Infinity;
-                formatCache.lastData = null;
-            }
-
-            // rather than set to now, this data is as stale as it's canvas is
-            formatCache.lastUpdate = Math.max(workspace.lastUpdate, formatCache.lastUpdate);
-        }
-
-        return formatCache.lastData;
+        return null;
     }
 
     /**
      * Set the preview ghost effect
      * @param {number} ghost from 0 (visible) to 100 (invisible) - ghost effect
+     * @return {void}
      */
     setPreviewGhost (ghost) {
-        this._ghost = ghost;
-        if (this._drawable) {
-            this.runtime.renderer.updateDrawableProperties(this._drawable, {ghost});
-        }
-    }
-
-    /**
-     * Method called when an error happens.  Default implementation is just to log error.
-     *
-     * @abstract
-     * @param {Error} error An error object from getUserMedia or other source of error.
-     */
-    onError (error) {
-        log.error('Unhandled video io device error', error);
-    }
-
-    /**
-     * Create a video stream.
-     * Should probably be moved to -render or somewhere similar later
-     * @private
-     * @return {Promise} When video has been received, rejected if video is not received
-     */
-    _setupVideo () {
-        // We cache the result of this setup so that we can only ever have a single
-        // video/getUserMedia request happen at a time.
-        if (this._singleSetup) {
-            return this._singleSetup;
-        }
-
-        this._singleSetup = navigator.mediaDevices.getUserMedia({
-            audio: false,
-            video: {
-                width: {min: 480, ideal: 640},
-                height: {min: 360, ideal: 480}
-            }
-        })
-            .then(stream => {
-                this._video = document.createElement('video');
-                // Use the new srcObject API, falling back to createObjectURL
-                try {
-                    this._video.srcObject = stream;
-                } catch (error) {
-                    this._video.src = window.URL.createObjectURL(stream);
-                }
-                // Hint to the stream that it should load. A standard way to do this
-                // is add the video tag to the DOM. Since this extension wants to
-                // hide the video tag and instead render a sample of the stream into
-                // the webgl rendered Scratch canvas, another hint like this one is
-                // needed.
-                this._video.play(); // Needed for Safari/Firefox, Chrome auto-plays.
-                this._track = stream.getTracks()[0];
-                this._setupPreview();
-                return this;
-            })
-            .catch(error => {
-                this._singleSetup = null;
-                this.onError(error);
-            });
-
-        return this._singleSetup;
-    }
-
-    _disablePreview () {
-        if (this._skin) {
-            this._skin.clear();
-            this.runtime.renderer.updateDrawableProperties(this._drawable, {visible: false});
-        }
-        this._renderPreviewFrame = null;
-    }
-
-    _setupPreview () {
-        const {renderer} = this.runtime;
-        if (!renderer) return;
-
-        if (this._skinId === -1 && this._skin === null && this._drawable === -1) {
-            this._skinId = renderer.createPenSkin();
-            this._skin = renderer._allSkins[this._skinId];
-            this._drawable = renderer.createDrawable();
-            renderer.setDrawableOrder(
-                this._drawable,
-                Video.ORDER
-            );
-            renderer.updateDrawableProperties(this._drawable, {
-                skinId: this._skinId
-            });
-        }
-
-        // if we haven't already created and started a preview frame render loop, do so
-        if (!this._renderPreviewFrame) {
-            renderer.updateDrawableProperties(this._drawable, {
-                ghost: this._ghost,
-                visible: true
-            });
-
-            this._renderPreviewFrame = () => {
-                clearTimeout(this._renderPreviewTimeout);
-                if (!this._renderPreviewFrame) {
-                    return;
-                }
-
-                this._renderPreviewTimeout = setTimeout(this._renderPreviewFrame, this.runtime.currentStepTime);
-
-                const canvas = this.getFrame({format: Video.FORMAT_CANVAS});
-
-                if (!canvas) {
-                    this._skin.clear();
-                    return;
-                }
-
-                const xOffset = Video.DIMENSIONS[0] / -2;
-                const yOffset = Video.DIMENSIONS[1] / 2;
-                this._skin.drawStamp(canvas, xOffset, yOffset);
-                this.runtime.requestRedraw();
-            };
-
-            this._renderPreviewFrame();
-        }
+        if (this.provider) return this.provider.setPreviewGhost(ghost);
+        return null;
     }
 
     get videoReady () {
-        if (!this.enabled) {
-            return false;
-        }
-        if (!this._video) {
-            return false;
-        }
-        if (!this._track) {
-            return false;
-        }
-        const {videoWidth, videoHeight} = this._video;
-        if (typeof videoWidth !== 'number' || typeof videoHeight !== 'number') {
-            return false;
-        }
-        if (videoWidth === 0 || videoHeight === 0) {
-            return false;
-        }
-        return true;
+        if (this.provider) return this.provider.videoReady();
+        return false;
     }
 
     /**
-     * get an internal workspace for canvas/context/caches
-     * this uses some document stuff to create a canvas and what not, probably needs abstraction
-     * into the renderer layer?
-     * @private
-     * @return {object} A workspace for canvas/data storage.  Internal format not documented intentionally
+     * @typedef VideoProvider
+     * @property {Function} enableVideo - Requests camera access from the user, and upon success,
+     * enables the video feed
+     * @property {Function} disableVideo - Turns off the video feed
+     * @property {Function} setGhostPreview - Controls the transparency of a visual layer
+     * over the video feed
+     * @property {Function} getFrame - Return frame data from the video feed in
+     * specified dimensions, format, and mirroring.
      */
-    _getWorkspace ({dimensions, mirror}) {
-        let workspace = this._workspace.find(space => (
-            space.dimensions.join('-') === dimensions.join('-') &&
-            space.mirror === mirror
-        ));
-        if (!workspace) {
-            workspace = {
-                dimensions,
-                mirror,
-                canvas: document.createElement('canvas'),
-                lastUpdate: 0,
-                cacheData: {}
-            };
-            workspace.canvas.width = dimensions[0];
-            workspace.canvas.height = dimensions[1];
-            workspace.context = workspace.canvas.getContext('2d');
-            this._workspace.push(workspace);
-        }
-        return workspace;
-    }
 
+    /**
+     * Set a video provider for this device.
+     * @param {VideoProvider} provider - Video provider to use
+     */
+    setProvider (provider) {
+        this.provider = provider;
+    }
 }
 
 
diff --git a/src/virtual-machine.js b/src/virtual-machine.js
index 1e7a0b9e1..118c11d2d 100644
--- a/src/virtual-machine.js
+++ b/src/virtual-machine.js
@@ -188,6 +188,10 @@ class VirtualMachine extends EventEmitter {
         }
     }
 
+    setVideoProvider (videoProvider) {
+        this.runtime.ioDevices.video.setProvider(videoProvider);
+    }
+
     /**
      * Load a Scratch project from a .sb, .sb2, .sb3 or json string.
      * @param {string | object} input A json string, object, or ArrayBuffer representing the project to load.

From fc36ea5208b46295908ae37f957f4330929ea40c Mon Sep 17 00:00:00 2001
From: DD <liudi08@gmail.com>
Date: Thu, 26 Apr 2018 13:36:51 -0400
Subject: [PATCH 6/9] Rename getCostumeSvg to getCostume

---
 src/virtual-machine.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/virtual-machine.js b/src/virtual-machine.js
index 1e7a0b9e1..8d0445598 100644
--- a/src/virtual-machine.js
+++ b/src/virtual-machine.js
@@ -545,7 +545,7 @@ class VirtualMachine extends EventEmitter {
      * @param {int} costumeIndex - the index of the costume to be got.
      * @return {string} the costume's SVG string, or null if it's not an SVG costume.
      */
-    getCostumeSvg (costumeIndex) {
+    getCostume (costumeIndex) {
         const id = this.editingTarget.getCostumes()[costumeIndex].assetId;
         if (id && this.runtime && this.runtime.storage &&
                 this.runtime.storage.get(id).dataFormat === 'svg') {

From 271a19c05dd686f4fc0852d35cf4ad5c6c62d1b3 Mon Sep 17 00:00:00 2001
From: Karishma Chadha <kchadha@scratch.mit.edu>
Date: Thu, 26 Apr 2018 19:17:41 -0400
Subject: [PATCH 7/9] Add runtime/render related functionality back to VM.

---
 src/io/video.js | 137 ++++++++++++++++++++++++++++++++++++++----------
 1 file changed, 110 insertions(+), 27 deletions(-)

diff --git a/src/io/video.js b/src/io/video.js
index 70b753908..e65edbe65 100644
--- a/src/io/video.js
+++ b/src/io/video.js
@@ -1,7 +1,42 @@
 class Video {
     constructor (runtime) {
         this.runtime = runtime;
+
+        /**
+         * @typedef VideoProvider
+         * @property {Function} enableVideo - Requests camera access from the user, and upon success,
+         * enables the video feed
+         * @property {Function} disableVideo - Turns off the video feed
+         * @property {Function} getFrame - Return frame data from the video feed in
+         * specified dimensions, format, and mirroring.
+         */
         this.provider = null;
+
+        /**
+         * Id representing a Scratch Renderer skin the video is rendered to for
+         * previewing.
+         * @type {number}
+         */
+        this._skinId = -1;
+
+        /**
+         * The Scratch Renderer Skin object.
+         * @type {Skin}
+         */
+        this._skin = null;
+
+        /**
+         * Id for a drawable using the video's skin that will render as a video
+         * preview.
+         * @type {Drawable}
+         */
+        this._drawable = -1;
+
+        /**
+         * Store the last state of the video transparency ghost effect
+         * @type {number}
+         */
+        this._ghost = 0;
     }
 
     static get FORMAT_IMAGE_DATA () {
@@ -29,6 +64,14 @@ class Video {
         return 1;
     }
 
+    /**
+     * Set a video provider for this device.
+     * @param {VideoProvider} provider - Video provider to use
+     */
+    setProvider (provider) {
+        this.provider = provider;
+    }
+
     /**
      * Request video be enabled.  Sets up video, creates video skin and enables preview.
      *
@@ -37,8 +80,8 @@ class Video {
      * @return {Promise.<Video>} resolves a promise to this IO device when video is ready.
      */
     enableVideo () {
-        if (this.provider) return this.provider.enableVideo();
-        return null;
+        if (!this.provider) return null;
+        return this.provider.enableVideo().then(() => this._setupPreview());
     }
 
     /**
@@ -46,8 +89,9 @@ class Video {
      * @return {void}
      */
     disableVideo () {
-        if (this.provider) return this.provider.disableVideo();
-        return null;
+        if (!this.provider) return null;
+        this._disablePreview();
+        this.provider.disableVideo();
     }
 
     /**
@@ -79,36 +123,75 @@ class Video {
     /**
      * Set the preview ghost effect
      * @param {number} ghost from 0 (visible) to 100 (invisible) - ghost effect
-     * @return {void}
      */
     setPreviewGhost (ghost) {
-        if (this.provider) return this.provider.setPreviewGhost(ghost);
-        return null;
+        this._ghost = ghost;
+        if (this._drawable) {
+            this.runtime.renderer.updateDrawableProperties(this._drawable, {ghost});
+        }
+    }
+
+    _disablePreview () {
+        if (this._skin) {
+            this._skin.clear();
+            this.runtime.renderer.updateDrawableProperties(this._drawable, {visible: false});
+        }
+        this._renderPreviewFrame = null;
+    }
+
+    _setupPreview () {
+        const {renderer} = this.runtime;
+        if (!renderer) return;
+
+        if (this._skinId === -1 && this._skin === null && this._drawable === -1) {
+            this._skinId = renderer.createPenSkin();
+            this._skin = renderer._allSkins[this._skinId];
+            this._drawable = renderer.createDrawable();
+            renderer.setDrawableOrder(
+                this._drawable,
+                Video.ORDER
+            );
+            renderer.updateDrawableProperties(this._drawable, {
+                skinId: this._skinId
+            });
+        }
+
+        // if we haven't already created and started a preview frame render loop, do so
+        if (!this._renderPreviewFrame) {
+            renderer.updateDrawableProperties(this._drawable, {
+                ghost: this._ghost,
+                visible: true
+            });
+
+            this._renderPreviewFrame = () => {
+                clearTimeout(this._renderPreviewTimeout);
+                if (!this._renderPreviewFrame) {
+                    return;
+                }
+
+                this._renderPreviewTimeout = setTimeout(this._renderPreviewFrame, this.runtime.currentStepTime);
+
+                const canvas = this.getFrame({format: Video.FORMAT_CANVAS});
+
+                if (!canvas) {
+                    this._skin.clear();
+                    return;
+                }
+
+                const xOffset = Video.DIMENSIONS[0] / -2;
+                const yOffset = Video.DIMENSIONS[1] / 2;
+                this._skin.drawStamp(canvas, xOffset, yOffset);
+                this.runtime.requestRedraw();
+            };
+
+            this._renderPreviewFrame();
+        }
     }
 
     get videoReady () {
-        if (this.provider) return this.provider.videoReady();
+        if (this.provider) return this.provider.videoReady;
         return false;
     }
-
-    /**
-     * @typedef VideoProvider
-     * @property {Function} enableVideo - Requests camera access from the user, and upon success,
-     * enables the video feed
-     * @property {Function} disableVideo - Turns off the video feed
-     * @property {Function} setGhostPreview - Controls the transparency of a visual layer
-     * over the video feed
-     * @property {Function} getFrame - Return frame data from the video feed in
-     * specified dimensions, format, and mirroring.
-     */
-
-    /**
-     * Set a video provider for this device.
-     * @param {VideoProvider} provider - Video provider to use
-     */
-    setProvider (provider) {
-        this.provider = provider;
-    }
 }
 
 

From c2bb1e3123966ae3daa4bf43f4a7b108fb99ec57 Mon Sep 17 00:00:00 2001
From: Paul Kaplan <pkaplan@media.mit.edu>
Date: Fri, 27 Apr 2018 09:21:38 -0400
Subject: [PATCH 8/9] Add comment to jsdoc typedef where it belongs.

---
 src/blocks/scratch3_looks.js | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/blocks/scratch3_looks.js b/src/blocks/scratch3_looks.js
index b561682be..c676aaa5c 100644
--- a/src/blocks/scratch3_looks.js
+++ b/src/blocks/scratch3_looks.js
@@ -11,6 +11,8 @@ const uid = require('../util/uid');
  *      See _renderBubble for explanation of this optimization.
  * @property {string} text - the text of the bubble.
  * @property {string} type - the type of the bubble, "say" or "think"
+ * @property {?string} usageId - ID indicating the most recent usage of the say/think bubble.
+ *      Used for comparison when determining whether to clear a say/think bubble.
  */
 
 class Scratch3LooksBlocks {
@@ -46,7 +48,7 @@ class Scratch3LooksBlocks {
             skinId: null,
             text: '',
             type: 'say',
-            usageId: null // ID for hiding a timed say/think block.
+            usageId: null
         };
     }
 

From 857f77c5117e8513c29e50062735c5934953a2f2 Mon Sep 17 00:00:00 2001
From: Karishma Chadha <kchadha@scratch.mit.edu>
Date: Sun, 29 Apr 2018 19:52:21 -0400
Subject: [PATCH 9/9] Addressing PR comments.

---
 src/io/video.js | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/src/io/video.js b/src/io/video.js
index e65edbe65..c01141c1e 100644
--- a/src/io/video.js
+++ b/src/io/video.js
@@ -65,7 +65,8 @@ class Video {
     }
 
     /**
-     * Set a video provider for this device.
+     * Set a video provider for this device. A default implementation of
+     * a video provider can be found in scratch-gui/src/lib/video/video-provider
      * @param {VideoProvider} provider - Video provider to use
      */
     setProvider (provider) {
@@ -89,8 +90,8 @@ class Video {
      * @return {void}
      */
     disableVideo () {
-        if (!this.provider) return null;
         this._disablePreview();
+        if (!this.provider) return null;
         this.provider.disableVideo();
     }
 
@@ -114,9 +115,6 @@ class Video {
         cacheTimeout = this._frameCacheTimeout
     }) {
         if (this.provider) return this.provider.getFrame({dimensions, mirror, format, cacheTimeout});
-        if (!this.videoReady) {
-            return null;
-        }
         return null;
     }