Bug 1265796 - Use queueAsyncCalls in inspector
MozReview-Commit-ID: AiDqpr7cS0P
--- a/devtools/client/inspector/rules/models/element-style.js
+++ b/devtools/client/inspector/rules/models/element-style.js
@@ -7,16 +7,17 @@
"use strict";
const promise = require("promise");
const {Rule} = require("devtools/client/inspector/rules/models/rule");
const {promiseWarn} = require("devtools/client/inspector/shared/utils");
const {ELEMENT_STYLE} = require("devtools/shared/specs/styles");
const {getCssProperties} = require("devtools/shared/fronts/css-properties");
const {Task} = require("devtools/shared/task");
+const {queueAsyncCalls} = require("devtools/shared/DevToolsUtils");;
/**
* ElementStyle is responsible for the following:
* Keeps track of which properties are overridden.
* Maintains a list of Rule objects for a given element.
*
* @param {Element} element
* The element whose style we are viewing.
@@ -37,16 +38,20 @@ function ElementStyle(element, ruleView,
this.element = element;
this.ruleView = ruleView;
this.store = store || {};
this.pageStyle = pageStyle;
this.showUserAgentStyles = showUserAgentStyles;
this.rules = [];
this.cssProperties = getCssProperties(this.ruleView.inspector.toolbox);
+ // Make sure that the populate calls wait in a queue so they are processed in the order
+ // they were received.
+ this.populate = queueAsyncCalls(this.populate, this);
+
// We don't want to overwrite this.store.userProperties so we only create it
// if it doesn't already exist.
if (!("userProperties" in this.store)) {
this.store.userProperties = new UserProperties();
}
if (!("disabled" in this.store)) {
this.store.disabled = new WeakMap();
@@ -82,66 +87,62 @@ ElementStyle.prototype = {
/**
* Refresh the list of rules to be displayed for the active element.
* Upon completion, this.rules[] will hold a list of Rule objects.
*
* Returns a promise that will be resolved when the elementStyle is
* ready.
*/
- populate: function () {
- let self = this;
- let populated = this.pageStyle.getApplied(this.element, {
+ populate: Task.async(function* () {
+ let entries = yield this.pageStyle.getApplied(this.element, {
inherited: true,
matchedSelectors: true,
filter: this.showUserAgentStyles ? "ua" : undefined,
- }).then(Task.async(function* (entries) {
- if (self.destroyed) {
- return promise.resolve(undefined);
+ }).then(null, e => {
+ // Don't warn if the element style is already destroyed as populate is often
+ // called after a setTimeout and the connection may already be closed.
+ if (!this.destroyed) {
+ promiseWarn(e);
}
-
- if (self.populated !== populated) {
- // Don't care anymore.
- return promise.resolve(undefined);
- }
+ });
- // Store the current list of rules (if any) during the population
- // process. They will be reused if possible.
- let existingRules = self.rules;
+ if (!entries || this.destroyed) {
+ return;
+ }
- self.rules = [];
+ // Store the current list of rules (if any) during the population
+ // process. They will be reused if possible.
+ let existingRules = this.rules;
- for (let entry of entries) {
- yield self._maybeAddRule(entry, existingRules);
- }
+ this.rules = [];
- // Mark overridden computed styles.
- self.markOverriddenAll();
-
- self._sortRulesForPseudoElement();
+ for (let entry of entries) {
+ // Since this is async, check to make sure this component is still around before
+ // doing anything with it.
+ yield this._maybeAddRule(entry, existingRules);
+ if (this.destroyed) {
+ return;
+ }
+ }
- // We're done with the previous list of rules.
- for (let r of existingRules) {
- if (r && r.editor) {
- r.editor.destroy();
- }
- }
+ // Mark overridden computed styles.
+ this.markOverriddenAll();
- return undefined;
- })).then(null, e => {
- // populate is often called after a setTimeout,
- // the connection may already be closed.
- if (this.destroyed) {
- return promise.resolve(undefined);
+ this._sortRulesForPseudoElement();
+
+ // We're done with the previous list of rules.
+ for (let r of existingRules) {
+ if (r && r.editor) {
+ r.editor.destroy();
}
- return promiseWarn(e);
- });
- this.populated = populated;
- return this.populated;
- },
+ }
+
+ return;
+ }),
/**
* Put pseudo elements in front of others.
*/
_sortRulesForPseudoElement: function () {
this.rules = this.rules.sort((a, b) => {
return (a.pseudoElement || "z") > (b.pseudoElement || "z");
});
--- a/devtools/client/inspector/rules/models/rule.js
+++ b/devtools/client/inspector/rules/models/rule.js
@@ -10,16 +10,17 @@ const promise = require("promise");
const CssLogic = require("devtools/shared/inspector/css-logic");
const {ELEMENT_STYLE} = require("devtools/shared/specs/styles");
const {TextProperty} =
require("devtools/client/inspector/rules/models/text-property");
const {promiseWarn} = require("devtools/client/inspector/shared/utils");
const {parseDeclarations} = require("devtools/shared/css-parsing-utils");
const Services = require("Services");
const {Task} = require("devtools/shared/task");
+const {queueAsyncCalls} = require("devtools/shared/DevToolsUtils");;
/**
* Rule is responsible for the following:
* Manages a single style declaration or rule.
* Applies changes to the properties in a rule.
* Maintains a list of TextProperty objects.
*
* @param {ElementStyle} elementStyle
@@ -41,16 +42,19 @@ function Rule(elementStyle, options) {
this.pseudoElement = options.pseudoElement || "";
this.isSystem = options.isSystem;
this.isUnmatched = options.isUnmatched || false;
this.inherited = options.inherited || null;
this.keyframes = options.keyframes || null;
this._modificationDepth = 0;
+ // Make sure that the async refresh calls are queued and execute in order.
+ this.refresh = queueAsyncCalls(this.refresh, this);
+
if (this.domRule && this.domRule.mediaText) {
this.mediaText = this.domRule.mediaText;
}
this.cssProperties = this.elementStyle.ruleView.cssProperties;
// Populate the text properties with the style's current authoredText
// value, and add in any disabled properties from the store.
@@ -199,17 +203,16 @@ Rule.prototype = {
/**
* Helper function for applyProperties that is called when the actor
* does not support as-authored styles. Store disabled properties
* in the element style's store.
*/
_applyPropertiesNoAuthored: function (modifications) {
this.elementStyle.markOverriddenAll();
-
let disabledProps = [];
for (let prop of this.textProps) {
if (prop.invisible) {
continue;
}
if (!prop.enabled) {
disabledProps.push({
@@ -494,41 +497,40 @@ Rule.prototype = {
/**
* Reread the current state of the rules and rebuild text
* properties as needed.
*/
refresh: Task.async(function* (options) {
this.matchedSelectors = options.matchedSelectors || [];
let newTextProps = this._getTextProperties();
+ let visited = new Map();
- // Update current properties for each property present on the style.
- // This will mark any touched properties with _visited so we
- // can detect properties that weren't touched (because they were
- // removed from the style).
- // Also keep track of properties that didn't exist in the current set
- // of properties.
+ // Update current properties for each property present on the style. Any touched
+ // properties will be set to true in the `visited` map so we can detect properties
+ // that weren't touched (because they were removed from the style). Also keep track
+ // of properties that didn't exist in the current set of properties.
let brandNewProps = [];
for (let newProp of newTextProps) {
yield newProp.updateComputed();
- if (!this._updateTextProperty(newProp)) {
+ if (!this._updateTextProperty(newProp, visited)) {
brandNewProps.push(newProp);
}
}
// Refresh editors and disabled state for all the properties that
// were updated.
for (let prop of this.textProps) {
// Properties that weren't touched during the update
// process must no longer exist on the node. Mark them disabled.
- if (!prop._visited) {
+ if (!visited.get(prop)) {
prop.enabled = false;
prop.updateEditor();
} else {
- delete prop._visited;
+ visited.delete(prop);
}
}
// Add brand new properties.
this.textProps = this.textProps.concat(brandNewProps);
// Refresh the editor if one already exists.
if (this.editor) {
@@ -553,26 +555,26 @@ Rule.prototype = {
* If no existing properties match the property, nothing happens.
*
* @param {TextProperty} newProp
* The current version of the property, as parsed from the
* authoredText in Rule._getTextProperties().
* @return {Boolean} true if a property was updated, false if no properties
* were updated.
*/
- _updateTextProperty: function (newProp) {
+ _updateTextProperty: function (newProp, visited) {
let match = { rank: 0, prop: null };
for (let prop of this.textProps) {
if (prop.name !== newProp.name) {
continue;
}
// Mark this property visited.
- prop._visited = true;
+ visited.set(prop, true);
// Start at rank 1 for matching name.
let rank = 1;
// Value and Priority matches add 2 to the rank.
// Being enabled adds 1. This ranks better matches higher,
// with priority breaking ties.
if (prop.value === newProp.value) {
--- a/devtools/client/inspector/rules/models/text-property.js
+++ b/devtools/client/inspector/rules/models/text-property.js
@@ -54,16 +54,17 @@ function TextProperty(rule, name, value,
this.enabled = !!enabled;
this.invisible = invisible;
const toolbox = this.rule.elementStyle.ruleView.inspector.toolbox;
this.cssProperties = getCssProperties(toolbox);
this.pageStyle = this.rule.elementStyle.pageStyle;
}
TextProperty.prototype = {
+
/**
* Update the editor associated with this text property,
* if any.
*/
updateEditor: function () {
if (this.editor) {
this.editor.update();
}
--- a/devtools/client/inspector/rules/rules.js
+++ b/devtools/client/inspector/rules/rules.js
@@ -805,16 +805,21 @@ CssRuleView.prototype = {
this.element.scrollTop = 0;
}
this._stopSelectingElement();
this._elementStyle.onChanged = () => {
this._changed();
};
}
}).then(null, e => {
+ if (this.isDestroyed) {
+ // Selecting the element will fail if this element is destroyed midway through,
+ // which is expected.
+ return;
+ }
if (this._elementStyle === elementStyle) {
this._stopSelectingElement();
this._clearRules();
}
console.error(e);
});
},
@@ -1577,17 +1582,17 @@ RuleViewTool.prototype = {
this.view.selectElement(this.inspector.selection.nodeFront)
.then(done, done);
}
},
refresh: function () {
if (this.isSidebarActive()) {
this.view.refreshPanel().then(() => {
- if(this.view.element) {
+ if(this.view && this.view.element) {
// If there is an editor, make sure it has retained focus.
const el = this.view.element.querySelector('input, textarea');
if (el) {
el.focus();
}
}
});
}