fix: cereal backwards compatibility (google#5421)

* fix: remove duplicate serialization hook implementations

* feat: add backwards compatibility to field serialization

* feat: add support for serializing old mutator hooks

* fix: build

* fix: refactor field changes into helpers

* fix: typo

* fix: removing xmlns

* tests: add tests for serialization and deserialization of mutator hooks

* fix: switch to early returns

core/field.js

@@ -45,11 +45,13 @@ const Tooltip = goog.require('Blockly.Tooltip');
 const WidgetDiv = goog.require('Blockly.WidgetDiv');
 /* eslint-disable-next-line no-unused-vars */
 const WorkspaceSvg = goog.requireType('Blockly.WorkspaceSvg');
+const Xml = goog.require('Blockly.Xml');
 const dom = goog.require('Blockly.utils.dom');
 const browserEvents = goog.require('Blockly.browserEvents');
 const style = goog.require('Blockly.utils.style');
 const userAgent = goog.require('Blockly.utils.userAgent');
 const utils = goog.require('Blockly.utils');
+const utilsXml = goog.require('Blockly.utils.xml');
 /** @suppress {extraRequire} */
 goog.require('Blockly.Events.BlockChange');
 /** @suppress {extraRequire} */
@@ -439,6 +441,10 @@ Field.prototype.toXml = function(fieldElement) {
  * @package
  */
 Field.prototype.saveState = function() {
+  const legacyState = this.saveLegacyState(Field);
+  if (legacyState !== null) {
+    return legacyState;
+  }
   return this.getValue();
 };
 
@@ -449,9 +455,56 @@ Field.prototype.saveState = function() {
  * @package
  */
 Field.prototype.loadState = function(state) {
+  if (this.loadLegacyState(Field, state)) {
+    return;
+  }
   this.setValue(state);
 };
 
+// eslint-disable-next-line valid-jsdoc
+/**
+ * Returns a stringified version of the XML state, if it should be used.
+ * Otherwise this returns null, to signal the field should use its own
+ * serialization.
+ * @param {?} callingClass The class calling this method.
+ *     Used to see if `this` has overridden any relevant hooks.
+ * @return {?string} The stringified version of the XML state, or null.
+ * @protected
+ */
+Field.prototype.saveLegacyState = function(callingClass) {
+  if (callingClass.prototype.saveState === this.saveState &&
+      callingClass.prototype.toXml !== this.toXml) {
+    const elem = utilsXml.createElement("field");
+    elem.setAttribute("name", this.name || '');
+    const text = Xml.domToText(this.toXml(elem));
+    return text.replace(
+        ' xmlns="https://developers.google.com/blockly/xml"', '');
+  }
+  // Either they called this on purpose from their saveState, or they have
+  // no implementations of either hook. Just do our thing.
+  return null;
+};
+
+// eslint-disable-next-line valid-jsdoc
+/**
+ * Loads the given state using either the old XML hoooks, if they should be
+ * used. Returns true to indicate loading has been handled, false otherwise.
+ * @param {?} callingClass The class calling this method.
+ *     Used to see if `this` has overridden any relevant hooks.
+ * @param {*} state The state to apply to the field.
+ * @return {boolean} Whether the state was applied or not.
+ */
+Field.prototype.loadLegacyState = function(callingClass, state) {
+  if (callingClass.prototype.loadState === this.loadState &&
+      callingClass.prototype.fromXml !== this.fromXml) {
+    this.fromXml(Xml.textToDom(/** @type {string} */ (state)));
+    return true;
+  }
+  // Either they called this on purpose from their loadState, or they have
+  // no implementations of either hook. Just do our thing.
+  return false;
+};
+
 /**
  * Dispose of all DOM objects and events belonging to this editable field.
  * @package

core/field_angle.js

@@ -254,26 +254,6 @@ Blockly.FieldAngle.prototype.initView = function() {
   this.textElement_.appendChild(this.symbol_);
 };
 
-/**
- * Saves this field's value.
- * @return {number} The angle value held by this field.
- * @override
- * @package
- */
-Blockly.FieldAngle.prototype.saveState = function() {
-  return /** @type {number} */ (this.getValue());
-};
-
-/**
- * Sets the field's value based on the given state.
- * @param {*} state The state to apply to the angle field.
- * @override
- * @package
- */
-Blockly.FieldAngle.prototype.loadState = function(state) {
-  this.setValue(state);
-};
-
 /**
  * Updates the graph when the field rerenders.
  * @protected

core/field_checkbox.js

@@ -104,22 +104,16 @@ FieldCheckbox.prototype.configure_ = function(config) {
 
 /**
  * Saves this field's value.
- * @return {boolean} The boolean value held by this field.
+ * @return {*} The boolean value held by this field.
  * @override
  * @package
  */
 FieldCheckbox.prototype.saveState = function() {
-  return /** @type {boolean} */ (this.getValueBoolean());
-};
-
-/**
- * Sets the field's value based on the given state.
- * @param {*} state The state to apply to the checkbox field.
- * @override
- * @package
- */
-FieldCheckbox.prototype.loadState = function(state) {
-  this.setValue(state);
+  const legacyState = this.saveLegacyState(FieldCheckbox);
+  if (legacyState !== null) {
+    return legacyState;
+  }
+  return this.getValueBoolean();
 };
 
 /**

core/field_colour.js

@@ -188,26 +188,6 @@ FieldColour.prototype.initView = function() {
   }
 };
 
-/**
- * Saves this field's value.
- * @return {string} The colour value held by this field.
- * @override
- * @package
- */
-FieldColour.prototype.saveState = function() {
-  return /** @type {string} */ (this.getValue());
-};
-
-/**
- * Sets the field's value based on the given state.
- * @param {*} state The state to apply to the colour field.
- * @override
- * @package
- */
-FieldColour.prototype.loadState = function(state) {
-  this.setValue(state);
-};
-
 /**
  * @override
  */

core/field_dropdown.js

@@ -170,23 +170,16 @@ FieldDropdown.prototype.fromXml = function(fieldElement) {
   this.setValue(fieldElement.textContent);
 };
 
-/**
- * Saves this field's value.
- * @return {*} The dropdown value held by this field.
- * @override
- * @package
- */
-FieldDropdown.prototype.saveState = function() {
-  return this.getValue();
-};
-
 /**
  * Sets the field's value based on the given state.
  * @param {*} state The state to apply to the dropdown field.
  * @override
  * @package
  */
 FieldDropdown.prototype.loadState = function(state) {
+  if (this.loadLegacyState(FieldDropdown, state)) {
+    return;
+  }
   if (this.isOptionListDynamic()) {
     this.getOptions(false);
   }

core/field_label_serializable.js

@@ -68,26 +68,6 @@ FieldLabelSerializable.prototype.EDITABLE = false;
  */
 FieldLabelSerializable.prototype.SERIALIZABLE = true;
 
-/**
- * Saves this field's value.
- * @return {string} The text value held by this field.
- * @override
- * @package
- */
-FieldLabelSerializable.prototype.saveState = function() {
-  return /** @type {string} */ (this.getValue());
-};
-
-/**
- * Sets the field's value based on the given state.
- * @param {*} state The state to apply to the label field.
- * @override
- * @package
- */
-FieldLabelSerializable.prototype.loadState = function(state) {
-  this.setValue(state);
-};
-
 fieldRegistry.register('field_label_serializable', FieldLabelSerializable);
 
 exports = FieldLabelSerializable;

core/field_multilineinput.js

@@ -124,21 +124,27 @@ FieldMultilineInput.prototype.fromXml = function(fieldElement) {
 
 /**
  * Saves this field's value.
- * @return {string} The text value held by this field.
- * @override
+ * @return {*} The state of this field.
  * @package
  */
 FieldMultilineInput.prototype.saveState = function() {
-  return /** @type {string} */ (this.getValue());
+  const legacyState = this.saveLegacyState(FieldMultilineInput);
+  if (legacyState !== null) {
+    return legacyState;
+  }
+  return this.getValue();
 };
 
 /**
  * Sets the field's value based on the given state.
- * @param {*} state The state to apply to the multiline input field.
+ * @param {*} state The state of the variable to assign to this variable field.
  * @override
  * @package
  */
 FieldMultilineInput.prototype.loadState = function(state) {
+  if (this.loadLegacyState(Field, state)) {
+    return;
+  }
   this.setValue(state);
 };
 

core/field_number.js

@@ -118,26 +118,6 @@ FieldNumber.prototype.configure_ = function(config) {
   this.setPrecisionInternal_(config['precision']);
 };
 
-/**
- * Saves this field's value.
- * @return {number} The number value held by this field.
- * @override
- * @package
- */
-FieldNumber.prototype.saveState = function() {
-  return /** @type {number} */ (this.getValue());
-};
-
-/**
- * Sets the field's value based on the given state.
- * @param {*} state The state to apply to the nuber field.
- * @override
- * @package
- */
-FieldNumber.prototype.loadState = function(state) {
-  this.setValue(state);
-};
-
 /**
  * Set the maximum, minimum and precision constraints on this field.
  * Any of these properties may be undefined or NaN to be disabled.

core/field_textinput.js

@@ -182,26 +182,6 @@ FieldTextInput.prototype.initView = function() {
   this.createTextElement_();
 };
 
-/**
- * Saves this field's value.
- * @return {*} The text value held by this field.
- * @override
- * @package
- */
-FieldTextInput.prototype.saveState = function() {
-  return this.getValue();
-};
-
-/**
- * Sets the field's value based on the given state.
- * @param {*} state The state to apply to the text input field.
- * @override
- * @package
- */
-FieldTextInput.prototype.loadState = function(state) {
-  this.setValue(state);
-};
-
 /**
  * Ensure that the input value casts to a valid string.
  * @param {*=} opt_newValue The input value.

core/field_variable.js

@@ -199,11 +199,15 @@ FieldVariable.prototype.toXml = function(fieldElement) {
 
 /**
  * Saves this field's value.
- * @return {{id: string}} The ID of the variable referenced by this field.
+ * @return {*} The ID of the variable referenced by this field.
  * @override
  * @package
  */
 FieldVariable.prototype.saveState = function() {
+  const legacyState = this.saveLegacyState(FieldVariable);
+  if (legacyState !== null) {
+    return legacyState;
+  }
   // Make sure the variable is initialized.
   this.initModel();
   return {
@@ -218,6 +222,9 @@ FieldVariable.prototype.saveState = function() {
  * @package
  */
 FieldVariable.prototype.loadState = function(state) {
+  if (this.loadLegacyState(FieldVariable, state)) {
+    return;
+  }
   // This is necessary so that blocks in the flyout can have custom var names.
   const variable = Variables.getOrCreateVariablePackage(
       this.sourceBlock_.workspace,

core/serialization/blocks.js

@@ -24,6 +24,7 @@ const {ISerializer} = goog.requireType('Blockly.serialization.ISerializer');
 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');
 const priorities = goog.require('Blockly.serialization.priorities');
 const serializationRegistry = goog.require('Blockly.serialization.registry');
@@ -162,6 +163,12 @@ const saveExtraState = function(block, state) {
     if (extraState !== null) {
       state['extraState'] = extraState;
     }
+  } else if (block.mutationToDom) {
+    const extraState = block.mutationToDom();
+    if (extraState !== null) {
+      state['extraState'] = Xml.domToText(extraState).replace(
+          ' xmlns="https://developers.google.com/blockly/xml"', '');
+    }
   }
 };
 
@@ -427,7 +434,11 @@ const loadExtraState = function(block, state) {
   if (!state['extraState']) {
     return;
   }
-  block.loadExtraState(state['extraState']);
+  if (block.loadExtraState) {
+    block.loadExtraState(state['extraState']);
+  } else {
+    block.domToMutation(Xml.textToDom(state['extraState']));
+  }
 };
 
 /**