Add serializing shadows as JSOs for the JSO system (#5246)

* Move existing tests into new suite

* Add tests for setShadowState

* Add assertions for serialization

* Unskip serialization tests

* Add logic to handle shadows in both systems

* Uncomment tests

* fix: add access modifiers to new comment funcs

* fix: fixup types

* fix: remove addNextBlocks = true

* feat: add real child of shadow errors

* fix: types

core/connection.js

@@ -21,6 +21,7 @@ goog.require('Blockly.Events.BlockMove');
 goog.require('Blockly.IASTNodeLocationWithBlock');
 goog.require('Blockly.utils.deprecation');
 goog.require('Blockly.Xml');
+goog.require('Blockly.serialization.blocks');
 
 goog.requireType('Blockly.Block');
 goog.requireType('Blockly.IConnectionChecker');
@@ -117,8 +118,7 @@ Blockly.Connection.prototype.connect_ = function(childConnection) {
   // Make sure the parentConnection is available.
   var orphan;
   if (parentConnection.isConnected()) {
-    var shadowDom = parentConnection.getShadowDom(true);
-    parentConnection.shadowDom_ = null;  // Set to null so it doesn't respawn.
+    let shadowState = parentConnection.stashShadowState_();
     var target = parentConnection.targetBlock();
     if (target.isShadow()) {
       target.dispose(false);
@@ -126,7 +126,7 @@ Blockly.Connection.prototype.connect_ = function(childConnection) {
       parentConnection.disconnect();
       orphan = target;
     }
-    parentConnection.shadowDom_ = shadowDom;
+    parentConnection.applyShadowState_(shadowState);
   }
 
   // Connect the new connection to the parent.
@@ -161,11 +161,10 @@ Blockly.Connection.prototype.connect_ = function(childConnection) {
  * @package
  */
 Blockly.Connection.prototype.dispose = function() {
-
   // isConnected returns true for shadows and non-shadows.
   if (this.isConnected()) {
     // Destroy the attached shadow block & its children (if it exists).
-    this.setShadowDom(null);
+    this.setShadowStateInternal_();
 
     var targetBlock = this.targetBlock();
     if (targetBlock) {
@@ -472,18 +471,8 @@ Blockly.Connection.prototype.disconnectInternal_ = function(parentBlock,
  * @protected
  */
 Blockly.Connection.prototype.respawnShadow_ = function() {
-  var parentBlock = this.getSourceBlock();
-  var shadow = this.getShadowDom();
-  if (parentBlock.workspace && shadow) {
-    var blockShadow = Blockly.Xml.domToBlock(shadow, parentBlock.workspace);
-    if (blockShadow.outputConnection) {
-      this.connect(blockShadow.outputConnection);
-    } else if (blockShadow.previousConnection) {
-      this.connect(blockShadow.previousConnection);
-    } else {
-      throw Error('Child block does not have output or previous statement.');
-    }
-  }
+  // Have to keep respawnShadow_ for backwards compatibility.
+  this.createShadowBlock_(true);
 };
 
 /**
@@ -581,18 +570,10 @@ Blockly.Connection.prototype.getCheck = function() {
 
 /**
  * Changes the connection's shadow block.
- * @param {?Element} shadow DOM representation of a block or null.
+ * @param {?Element} shadowDom DOM representation of a block or null.
  */
-Blockly.Connection.prototype.setShadowDom = function(shadow) {
-  this.shadowDom_ = shadow;
-  var target = this.targetBlock();
-  if (!target) {
-    this.respawnShadow_();
-  } else if (target.isShadow()) {
-    // The disconnect from dispose will automatically generate the new shadow.
-    target.dispose(false);
-    this.respawnShadow_();
-  }
+Blockly.Connection.prototype.setShadowDom = function(shadowDom) {
+  this.setShadowStateInternal_({shadowDom: shadowDom});
 };
 
 /**
@@ -610,6 +591,33 @@ Blockly.Connection.prototype.getShadowDom = function(returnCurrent) {
       this.shadowDom_;
 };
 
+/**
+ * Changes the connection's shadow block.
+ * @param {?Blockly.serialization.blocks.State} shadowState An state
+ *     represetation of the block or null.
+ */
+Blockly.Connection.prototype.setShadowState = function(shadowState) {
+  this.setShadowStateInternal_({shadowState: shadowState});
+};
+
+/**
+ * Returns the serialized object representation of the connection's shadow
+ * block.
+ * @param {boolean=} returnCurrent If true, and the shadow block is currently
+ *     attached to this connection, this serializes the state of that block
+ *     and returns it (so that field values are correct). Otherwise the saved
+ *     state is just returned.
+ * @return {?Blockly.serialization.blocks.State} Serialized object
+ *     representation of the block, or null.
+ */
+Blockly.Connection.prototype.getShadowState = function(returnCurrent) {
+  if (returnCurrent && this.targetBlock() && this.targetBlock().isShadow()) {
+    return Blockly.serialization.blocks.save(
+        /** @type {!Blockly.Block} */ (this.targetBlock()));
+  }
+  return this.shadowState_;
+};
+
 /**
  * Find all nearby compatible connections to this connection.
  * Type checking does not apply, since this function is used for bumping.
@@ -678,3 +686,124 @@ Blockly.Connection.prototype.toString = function() {
   }
   return msg + block.toDevString();
 };
+
+/**
+ * Returns the state of the shadowDom_ and shadowState_ properties, then
+ * temporarily sets those properties to null so no shadow respawns.
+ * @return {{shadowDom: ?Element,
+ *     shadowState: ?Blockly.serialization.blocks.State}} The state of both the
+ *     shadowDom_ and shadowState_ properties.
+ * @private
+ */
+Blockly.Connection.prototype.stashShadowState_ = function() {
+  const shadowDom = this.getShadowDom(true);
+  const shadowState = this.getShadowState(true);
+  // Set to null so it doesn't respawn.
+  this.shadowDom_ = null;
+  this.shadowState_ = null;
+  return {shadowDom, shadowState};
+};
+
+/**
+ * Reapplies the stashed state of the shadowDom_ and shadowState_ properties.
+ * @param {{shadowDom: ?Element,
+ *     shadowState: ?Blockly.serialization.blocks.State}} param0 The state to
+ *     reapply to the shadowDom_ and shadowState_ properties.
+ * @private
+ */
+Blockly.Connection.prototype.applyShadowState_ =
+    function({shadowDom, shadowState}) {
+      this.shadowDom_ = shadowDom;
+      this.shadowState_ = shadowState;
+    };
+
+/**
+ * Sets the state of the shadow of this connection.
+ * @param {{shadowDom: (?Element|undefined),
+ *     shadowState: (?Blockly.serialization.blocks.State|undefined)}=} param0
+ *     The state to set the shadow of this connection to.
+ * @private
+ */
+Blockly.Connection.prototype.setShadowStateInternal_ =
+    function({shadowDom = null, shadowState = null} = {}) {
+      // One or both of these should always be null.
+      // If neither is null, the shadowState will get priority.
+      this.shadowDom_ = shadowDom;
+      this.shadowState_ = shadowState;
+
+      var target = this.targetBlock();
+      if (!target) {
+        this.respawnShadow_();
+        if (this.targetBlock() && this.targetBlock().isShadow()) {
+          this.serializeShadow_(this.targetBlock());
+        }
+      } else if (target.isShadow()) {
+        target.dispose(false);
+        this.respawnShadow_();
+        if (this.targetBlock() && this.targetBlock().isShadow()) {
+          this.serializeShadow_(this.targetBlock());
+        }
+      } else {
+        var shadow = this.createShadowBlock_(false);
+        this.serializeShadow_(shadow);
+        if (shadow) {
+          shadow.dispose(false);
+        }
+      }
+    };
+
+/**
+ * Creates a shadow block based on the current shadowState_ or shadowDom_.
+ * shadowState_ gets priority.
+ * @param {boolean} attemptToConnect Whether to try to connect the shadow block
+ *     to this connection or not.
+ * @return {?Blockly.Block} The shadow block that was created, or null if both
+ *     the shadowState_ and shadowDom_ are null.
+ * @private
+ */
+Blockly.Connection.prototype.createShadowBlock_ = function(attemptToConnect) {
+  var parentBlock = this.getSourceBlock();
+  var shadowState = this.getShadowState();
+  var shadowDom = this.getShadowDom();
+  if (!parentBlock.workspace || (!shadowState && !shadowDom)) {
+    return null;
+  }
+
+  if (shadowState) {
+    var blockShadow = Blockly.serialization.blocks.loadInternal(
+        shadowState,
+        parentBlock.workspace,
+        attemptToConnect ? this : undefined,
+        true);
+    return blockShadow;
+  }
+  
+  if (shadowDom) {
+    blockShadow = Blockly.Xml.domToBlock(shadowDom, parentBlock.workspace);
+    if (attemptToConnect) {
+      if (blockShadow.outputConnection) {
+        this.connect(blockShadow.outputConnection);
+      } else if (blockShadow.previousConnection) {
+        this.connect(blockShadow.previousConnection);
+      } else {
+        throw Error('Shadow block does not have output or previous statement.');
+      }
+    }
+    return blockShadow;
+  }
+  return null;
+};
+
+/**
+ * Saves the given shadow block to both the shadowDom_ and shadowState_
+ * properties, in their respective serialized forms.
+ * @param {?Blockly.Block} shadow The shadow to serialize, or null.
+ * @private
+ */
+Blockly.Connection.prototype.serializeShadow_ = function(shadow) {
+  if (!shadow) {
+    return;
+  }
+  this.shadowDom_ = /** @type {!Element} */ (Blockly.Xml.blockToDom(shadow));
+  this.shadowState_ = Blockly.serialization.blocks.save(shadow);
+};

core/rendered_connection.js

@@ -504,7 +504,6 @@ Blockly.RenderedConnection.prototype.respawnShadow_ = function() {
   Blockly.RenderedConnection.superClass_.respawnShadow_.call(this);
   var blockShadow = this.targetBlock();
   if (!blockShadow) {
-    // This connection must not have a shadowDom_.
     return;
   }
   blockShadow.initSvg();

core/serialization/blocks.js

@@ -13,17 +13,15 @@
 goog.module('Blockly.serialization.blocks');
 goog.module.declareLegacyNamespace();
 
+const {BadConnectionCheck, MissingBlockType, MissingConnection, RealChildOfShadow} = goog.require('Blockly.serialization.exceptions');
 // eslint-disable-next-line no-unused-vars
 const Block = goog.requireType('Blockly.Block');
 // eslint-disable-next-line no-unused-vars
 const Connection = goog.requireType('Blockly.Connection');
 const Events = goog.require('Blockly.Events');
-const {MissingBlockType, MissingConnection, BadConnectionCheck} =
-    goog.require('Blockly.serialization.exceptions');
 const Size = goog.require('Blockly.utils.Size');
 // eslint-disable-next-line no-unused-vars
 const Workspace = goog.requireType('Blockly.Workspace');
-const Xml = goog.require('Blockly.Xml');
 const inputTypes = goog.require('Blockly.inputTypes');
 
 
@@ -266,15 +264,14 @@ const saveNextBlocks = function(block, state) {
  *     shadow block, or any connected real block.
  */
 const saveConnection = function(connection) {
-  const shadow = connection.getShadowDom();
+  const shadow = connection.getShadowState();
   const child = connection.targetBlock();
   if (!shadow && !child) {
     return null;
   }
   var state = Object.create(null);
   if (shadow) {
-    state['shadow'] = Xml.domToText(shadow)
-        .replace('xmlns="https://developers.google.com/blockly/xml"', '');
+    state['shadow'] = shadow;
   }
   if (child && !child.isShadow()) {
     state['block'] = save(child);
@@ -332,14 +329,18 @@ exports.load = load;
  * @param {!Workspace} workspace The workspace to add the block to.
  * @param {!Connection=} parentConnection The optional parent connection to
  *     attach the block to.
+ * @param {boolean} isShadow Whether the block we are loading is a shadow block
+ *     or not.
  * @return {!Block} The block that was just loaded.
  */
-const loadInternal = function(state, workspace, parentConnection = undefined) {
+const loadInternal = function(
+    state, workspace, parentConnection = undefined, isShadow = false) {
   if (!state['type']) {
     throw new MissingBlockType(state);
   }
 
   const block = workspace.newBlock(state['type'], state['id']);
+  block.setShadow(isShadow);
   loadCoords(block, state);
   loadAttributes(block, state);
   loadExtraState(block, state);
@@ -351,6 +352,8 @@ const loadInternal = function(state, workspace, parentConnection = undefined) {
   initBlock(block, workspace.rendered);
   return block;
 };
+/** @package */
+exports.loadInternal = loadInternal;
 
 /**
  * Applies any coordinate information available on the state object to the
@@ -417,6 +420,10 @@ const tryToConnectParent = function(parentConnection, child, state) {
   if (!parentConnection) {
     return;
   }
+
+  if (parentConnection.getSourceBlock().isShadow() && !child.isShadow()) {
+    throw new RealChildOfShadow(state);
+  }
   
   let connected = false;
   let childConnection;
@@ -542,7 +549,7 @@ const loadNextBlocks = function(block, state) {
  */
 const loadConnection = function(connection, connectionState) {
   if (connectionState['shadow']) {
-    connection.setShadowDom(Blockly.Xml.textToDom(connectionState['shadow']));
+    connection.setShadowState(connectionState['shadow']);
   }
   if (connectionState['block']) {
     loadInternal(

tests/mocha/jso_deserialization_test.js

@@ -622,7 +622,7 @@ suite('JSO Deserialization', function() {
       });
     });
 
-    suite.skip('Real child of shadow', function() {
+    suite('Real child of shadow', function() {
       test('Input', function() {
         const state = {
           'blocks': {
@@ -659,10 +659,10 @@ suite('JSO Deserialization', function() {
                 'type': 'text_print',
                 'next': {
                   'shadow': {
-                    'type': 'text_input',
+                    'type': 'text_print',
                     'next': {
                       'block': {
-                        'type': 'text_input',
+                        'type': 'text_print',
                       }
                     }
                   }

tests/mocha/jso_serialization_test.js

@@ -419,7 +419,7 @@ suite('JSO Serialization', function() {
           const block = this.workspace.newBlock(blockType);
           block.getInput(inputName).connection.setShadowDom(
               Blockly.Xml.textToDom(
-                  '<block type="' + blockType + '" id="test"></block>'));
+                  '<shadow type="' + blockType + '" id="test"></shadow>'));
           return block;
         };
 
@@ -430,7 +430,7 @@ suite('JSO Serialization', function() {
               childBlock.outputConnection || childBlock.previousConnection);
           block.getInput(inputName).connection.setShadowDom(
               Blockly.Xml.textToDom(
-                  '<block type="' + blockType + '" id="test"></block>'));
+                  '<shadow type="' + blockType + '" id="test"></shadow>'));
           return block;
         };
   
@@ -496,11 +496,11 @@ suite('JSO Serialization', function() {
             this.assertChild('row_block', 'INPUT');
           });
   
-          test.skip('Shadow', function() {
+          test('Shadow', function() {
             this.assertShadow('row_block', 'INPUT');
           });
   
-          test.skip('Overwritten shadow', function() {
+          test('Overwritten shadow', function() {
             this.assertOverwrittenShadow('row_block', 'INPUT');
           });
         });
@@ -526,11 +526,11 @@ suite('JSO Serialization', function() {
             this.assertChild('statement_block', 'NAME');
           });
   
-          test.skip('Shadow', function() {
+          test('Shadow', function() {
             this.assertShadow('statement_block', 'NAME');
           });
   
-          test.skip('Overwritten shadow', function() {
+          test('Overwritten shadow', function() {
             this.assertOverwrittenShadow('statement_block', 'NAME');
           });
 
@@ -590,7 +590,7 @@ suite('JSO Serialization', function() {
             const block = this.workspace.newBlock('stack_block');
             block.nextConnection.setShadowDom(
                 Blockly.Xml.textToDom(
-                    '<block type="stack_block" id="test"></block>'));
+                    '<shadow type="stack_block" id="test"></shadow>'));
             return block;
           };
 
@@ -600,7 +600,7 @@ suite('JSO Serialization', function() {
             block.nextConnection.connect(childBlock.previousConnection);
             block.nextConnection.setShadowDom(
                 Blockly.Xml.textToDom(
-                    '<block type="stack_block" id="test"></block>'));
+                    '<shadow type="stack_block" id="test"></shadow>'));
             return block;
           };
         });
@@ -614,14 +614,14 @@ suite('JSO Serialization', function() {
                 jso['next'], {'block': { 'type': 'stack_block', 'id': 'id2'}});
           });
   
-          test.skip('Shadow', function() {
+          test('Shadow', function() {
             const block = this.createNextWithShadow();
             const jso = Blockly.serialization.blocks.save(block);
             chai.assert.deepInclude(
                 jso['next'], {'shadow': { 'type': 'stack_block', 'id': 'test'}});
           });
   
-          test.skip('Overwritten shadow', function() {
+          test('Overwritten shadow', function() {
             const block = this.createNextWithShadowAndChild();
             const jso = Blockly.serialization.blocks.save(block);
             chai.assert.deepInclude(