Bug 1475782 - Return promises from Rule mutation methods and catch errors in font editor. r=pbro draft
authorRazvan Caliman <rcaliman@mozilla.com>
Wed, 18 Jul 2018 14:48:31 +0200
changeset 823407 773672745f6890a064d5d6a862c46e5246b0cfef
parent 823406 349b62c2da79c5ec58bd5465d396e907a52646b8
push id117676
push userbmo:rcaliman@mozilla.com
push dateFri, 27 Jul 2018 09:57:14 +0000
reviewerspbro
bugs1475782
milestone63.0a1
Bug 1475782 - Return promises from Rule mutation methods and catch errors in font editor. r=pbro MozReview-Commit-ID: 12ByNb6rrWp
devtools/client/inspector/fonts/fonts.js
devtools/client/inspector/rules/models/rule.js
devtools/client/inspector/rules/models/text-property.js
--- a/devtools/client/inspector/fonts/fonts.js
+++ b/devtools/client/inspector/fonts/fonts.js
@@ -655,21 +655,23 @@ class FontInspector {
    *         CSS property value
    */
   syncChanges(name, value) {
     const textProperty = this.getTextProperty(name, value);
     if (textProperty) {
       // This method may be called after the connection to the page style actor is closed.
       // For example, during teardown of automated tests. Here, we catch any failure that
       // may occur because of that. We're not interested in handling the error.
-      try {
-        textProperty.setValue(value);
-      } catch (e) {
-        // Silent error.
-      }
+      textProperty.setValue(value).catch(error => {
+        if (!this.document) {
+          return;
+        }
+
+        throw error;
+      });
     }
 
     this.ruleView.on("property-value-updated", this.onRulePropertyUpdated);
   }
 
   /**
    * Handler for changes of a font axis value coming from the FontEditor.
    *
@@ -1067,16 +1069,16 @@ class FontInspector {
 
     if (textProperty.value === value) {
       return;
     }
 
     // Prevent reacting to changes we caused.
     this.ruleView.off("property-value-updated", this.onRulePropertyUpdated);
     // Live preview font property changes on the page.
-    textProperty.rule.previewPropertyValue(textProperty, value, "");
+    textProperty.rule.previewPropertyValue(textProperty, value, "").catch(console.error);
 
     // Sync Rule view with changes reflected on the page (debounced).
     this.syncChanges(name, value);
   }
 }
 
 module.exports = FontInspector;
--- a/devtools/client/inspector/rules/models/rule.js
+++ b/devtools/client/inspector/rules/models/rule.js
@@ -325,47 +325,49 @@ Rule.prototype = {
    * Sets the value and priority of a property, then reapply all properties.
    *
    * @param {TextProperty} property
    *        The property to manipulate.
    * @param {String} value
    *        The property's value (not including priority).
    * @param {String} priority
    *        The property's priority (either "important" or an empty string).
+   * @return {Promise}
    */
   setPropertyValue: function(property, value, priority) {
     if (value === property.value && priority === property.priority) {
-      return;
+      return Promise.resolve();
     }
 
     property.value = value;
     property.priority = priority;
 
     const index = this.textProps.indexOf(property);
-    this.applyProperties((modifications) => {
+    return this.applyProperties((modifications) => {
       modifications.setProperty(index, property.name, value, priority);
     });
   },
 
   /**
    * Just sets the value and priority of a property, in order to preview its
    * effect on the content document.
    *
    * @param {TextProperty} property
    *        The property which value will be previewed
    * @param {String} value
    *        The value to be used for the preview
    * @param {String} priority
    *        The property's priority (either "important" or an empty string).
+   **@return {Promise}
    */
   previewPropertyValue: function(property, value, priority) {
     const modifications = this.domRule.startModifyingProperties(this.cssProperties);
     modifications.setProperty(this.textProps.indexOf(property),
                               property.name, value, priority);
-    modifications.apply().then(() => {
+    return modifications.apply().then(() => {
       // Ensure dispatching a ruleview-changed event
       // also for previews
       this.elementStyle._changed();
     });
   },
 
   /**
    * Disables or enables given TextProperty.
--- a/devtools/client/inspector/rules/models/text-property.js
+++ b/devtools/client/inspector/rules/models/text-property.js
@@ -117,18 +117,18 @@ TextProperty.prototype = {
 
   setValue: function(value, priority, force = false) {
     const store = this.rule.elementStyle.store;
 
     if (this.editor && value !== this.editor.committed.value || force) {
       store.userProperties.setProperty(this.rule.domRule, this.name, value);
     }
 
-    this.rule.setPropertyValue(this, value, priority);
-    this.updateEditor();
+    return this.rule.setPropertyValue(this, value, priority)
+      .then(() => this.updateEditor());
   },
 
   /**
    * Called when the property's value has been updated externally, and
    * the property and editor should update to reflect that value.
    *
    * @param {String} value
    *        Property value