SVG Sandboxing

packages/scratch-render/src/RenderWebGL.js

@@ -355,14 +355,14 @@ class RenderWebGL extends EventEmitter {
      * @param {!string} svgData - new SVG to use.
      * @param {?Array<number>} rotationCenter Optional: rotation center of the skin. If not supplied, the center of the
      * skin will be used
-     * @returns {!int} the ID for the new skin.
+     * @returns {Promise<number>} A promise that resolves with the skin ID once the skin's
+     *     dimensions are available via getSkinSize.
      */
     createSVGSkin (svgData, rotationCenter) {
         const skinId = this._nextSkinId++;
         const newSkin = new SVGSkin(skinId, this);
-        newSkin.setSVG(svgData, rotationCenter);
         this._allSkins[skinId] = newSkin;
-        return skinId;
+        return newSkin.setSVG(svgData, rotationCenter).then(() => skinId);
     }
 
     /**
@@ -398,16 +398,16 @@ class RenderWebGL extends EventEmitter {
      * @param {!string} svgData - new SVG to use.
      * @param {?Array<number>} rotationCenter Optional: rotation center of the skin. If not supplied, the center of the
      * skin will be used
+     * @returns {Promise<void>} A promise that resolves once the skin's dimensions are updated.
      */
     updateSVGSkin (skinId, svgData, rotationCenter) {
         if (this._allSkins[skinId] instanceof SVGSkin) {
-            this._allSkins[skinId].setSVG(svgData, rotationCenter);
-            return;
+            return this._allSkins[skinId].setSVG(svgData, rotationCenter);
         }
 
         const newSkin = new SVGSkin(skinId, this);
-        newSkin.setSVG(svgData, rotationCenter);
         this._reskin(skinId, newSkin);
+        return newSkin.setSVG(svgData, rotationCenter);
     }
 
     /**

packages/scratch-render/src/SVGSkin.js

@@ -54,6 +54,14 @@ class SVGSkin extends Skin {
          * @type {number}
          */
         this._maxTextureScale = 1;
+
+        /**
+         * Generation counter for cancelling stale setSVG loads.
+         * Incremented each time setSVG is called; async callbacks
+         * compare their captured generation to skip outdated results.
+         * @type {number}
+         */
+        this._svgGeneration = 0;
     }
 
     /**
@@ -192,46 +200,56 @@ class SVGSkin extends Skin {
      * @fires Skin.event:WasAltered
      */
     setSVG (svgData, rotationCenter) {
-        const svgTag = loadSvgString(svgData);
-        const svgText = serializeSvgToString(svgTag, true /* shouldInjectFonts */);
         this._svgImageLoaded = false;
 
-        const {x, y, width, height} = svgTag.viewBox.baseVal;
-        // While we're setting the size before the image is loaded, this doesn't cause the skin to appear with the wrong
-        // size for a few frames while the new image is loading, because we don't emit the `WasAltered` event, telling
-        // drawables using this skin to update, until the image is loaded.
-        // We need to do this because the VM reads the skin's `size` directly after calling `setSVG`.
-        // TODO: return a Promise so that the VM can read the skin's `size` after the image is loaded.
-        this._size[0] = width;
-        this._size[1] = height;
-
-        // If there is another load already in progress, replace the old onload to effectively cancel the old load
-        this._svgImage.onload = () => {
-            if (width === 0 || height === 0) {
-                super.setEmptyImageData();
-                return;
-            }
-
-            const maxDimension = Math.ceil(Math.max(width, height));
-            let testScale = 2;
-            for (testScale; maxDimension * testScale <= MAX_TEXTURE_DIMENSION; testScale *= 2) {
-                this._maxTextureScale = testScale;
-            }
-
-            this.resetMIPs();
-
-            if (typeof rotationCenter === 'undefined') rotationCenter = this.calculateRotationCenter();
-            // Compensate for viewbox offset.
-            // See https://github.com/LLK/scratch-render/pull/90.
-            this._rotationCenter[0] = rotationCenter[0] - x;
-            this._rotationCenter[1] = rotationCenter[1] - y;
-
-            this._svgImageLoaded = true;
-
-            this.emit(Skin.Events.WasAltered);
-        };
+        // Increment generation so that if setSVG is called again before
+        // the async pipeline finishes, the stale result is discarded.
+        const generation = ++this._svgGeneration;
+
+        // Full async pipeline: sanitize, normalize, serialize, render.
+        // Resolves after _size is updated; callers that need correct dimensions
+        // must await the returned Promise.
+        return loadSvgString(svgData).then(svgTag => {
+            // A newer setSVG call supersedes this one.
+            if (this._svgGeneration !== generation) return;
+
+            const svgText = serializeSvgToString(svgTag, true /* shouldInjectFonts */);
+            const {x, y, width, height} = svgTag.viewBox.baseVal;
+
+            // Update size from the normalized SVG (normalization may have
+            // changed dimensions, e.g. for Scratch 2 quirks-mode SVGs).
+            this._size[0] = width;
+            this._size[1] = height;
+
+            this._svgImage.onload = () => {
+                if (this._svgGeneration !== generation) return;
+
+                if (width === 0 || height === 0) {
+                    super.setEmptyImageData();
+                    return;
+                }
+
+                const maxDimension = Math.ceil(Math.max(width, height));
+                let testScale = 2;
+                for (testScale; maxDimension * testScale <= MAX_TEXTURE_DIMENSION; testScale *= 2) {
+                    this._maxTextureScale = testScale;
+                }
+
+                this.resetMIPs();
+
+                if (typeof rotationCenter === 'undefined') rotationCenter = this.calculateRotationCenter();
+                // Compensate for viewbox offset.
+                // See https://github.com/LLK/scratch-render/pull/90.
+                this._rotationCenter[0] = rotationCenter[0] - x;
+                this._rotationCenter[1] = rotationCenter[1] - y;
+
+                this._svgImageLoaded = true;
+
+                this.emit(Skin.Events.WasAltered);
+            };
 
-        this._svgImage.src = `data:image/svg+xml;utf8,${encodeURIComponent(svgText)}`;
+            this._svgImage.src = `data:image/svg+xml;utf8,${encodeURIComponent(svgText)}`;
+        });
     }
 
 }

packages/scratch-render/test/integration/skin-size-tests.js

@@ -14,8 +14,8 @@ const indexHTML = path.resolve(__dirname, 'index.html');
 
     await test('SVG skin size set properly', async t => {
         t.plan(1);
-        const skinSize = await page.evaluate(() => {
-            const skinID = render.createSVGSkin(`<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 50 100"></svg>`);
+        const skinSize = await page.evaluate(async () => {
+            const skinID = await render.createSVGSkin(`<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 50 100"></svg>`);
             return render.getSkinSize(skinID);
         });
         t.same(skinSize, [50, 100]);