1240771 - browser_rules_colorUnit.js and browser_rules_authored_color.js ends with pending requests

Status: RESOLVED DUPLICATE of bug 1240813

(DevTools :: Inspector: Rules, defect)

Description

[Alexandre Poirot [:ochameau]]

http://archive.mozilla.org/pub/firefox/try-builds/[e-mail]-5cc724ec7dd1ec23efd7e637ff1680eb1e52f9e4/try-linux64/try_ubuntu64_vm_test-mochitest-e10s-devtools-chrome-3-bm53-tests1-linux64-build94.txt.gz
06:08:33     INFO -  3930 INFO TEST-START | devtools/client/inspector/rules/test/browser_rules_authored_color.js
06:08:41     INFO -  console.error:
06:08:41     INFO -    Message: Error: Connection closed, pending request to server1.conn15.child1/domstylerule41, type setRuleText failed
06:08:41     INFO -  Request stack:

Comment 1

[Alexandre Poirot [:ochameau]]

Attachment: browser_rules_colorUnit.js fix

Comment 2

[Alexandre Poirot [:ochameau]]

Attachment: browser_styleinspector_csslogic-content-stylesheets.js fix

Comment 3

[Alexandre Poirot [:ochameau]]

Attachment: browser_rules_authored_color.js fix

Comment 4

[Alexandre Poirot [:ochameau]]

Comment on attachment 8709466 [details] [diff] [review]
browser_rules_colorUnit.js fix

Review of attachment 8709466 [details] [diff] [review]:
-----------------------------------------------------------------

We have to wait twice for ruleview-changed as setRuleText is called from these callsites:
Rule.prototype.previewPropertyValue@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/inspector/rules/models/rule.js:385:10
TextPropertyEditor.prototype._previewValue@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/inspector/rules/rules.js:2756:5
TextPropertyEditor.prototype._onSwatchPreview@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/inspector/rules/rules.js:2681:5
SwatchBasedEditorTooltip.prototype.preview@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/shared/widgets/Tooltip.js:1083:7
SwatchColorPickerTooltip.prototype<._selectColor@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/shared/widgets/Tooltip.js:1176:7
SwatchColorPickerTooltip.prototype<._onSpectrumColorChange@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/shared/widgets/Tooltip.js:1166:5

Rule.prototype.previewPropertyValue@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/inspector/rules/models/rule.js:385:10
TextPropertyEditor.prototype._onValueDone@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/inspector/rules/rules.js:2636:7
TextPropertyEditor.prototype._onSwatchCommit@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/inspector/rules/rules.js:2673:5
SwatchBasedEditorTooltip.prototype.commit@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/shared/widgets/Tooltip.js:1106:7
SwatchBasedEditorTooltip.prototype.show/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/shared/widgets/Tooltip.js:996:11

Rule.prototype._applyPropertiesAuthored@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/inspector/rules/models/rule.js:268:10
Rule.prototype.applyProperties/resultPromise<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/inspector/rules/models/rule.js:308:20

Comment 5

[Alexandre Poirot [:ochameau]]

Comment on attachment 8709469 [details] [diff] [review]
browser_rules_authored_color.js fix

Review of attachment 8709469 [details] [diff] [review]:
-----------------------------------------------------------------

Similar patch for another test

Comment 6

[Alexandre Poirot [:ochameau]]

Comment on attachment 8709468 [details] [diff] [review]
browser_styleinspector_csslogic-content-stylesheets.js fix

Review of attachment 8709468 [details] [diff] [review]:
-----------------------------------------------------------------

This one is less obvious, I had to introduce a test specific event.

Comment 7

[Patrick Brosset <:pbro>]

Comment on attachment 8709466 [details] [diff] [review]
browser_rules_colorUnit.js fix

Review of attachment 8709466 [details] [diff] [review]:
-----------------------------------------------------------------

You mentioned in bug 1240813 that I could take these patches and fold them in.
Let me know if you would like me to do this?

::: devtools/client/inspector/rules/test/browser_rules_colorUnit.js
@@ +32,5 @@
>      yield basicTest(view, name, result);
> +
> +    let target = TargetFactory.forTab(gBrowser.selectedTab);
> +    yield gDevTools.closeToolbox(target);
> +    gBrowser.removeCurrentTab();

This is already done in a registerCleanupFunction in head.js

@@ +55,5 @@
>  
>    let spectrum = yield cPicker.spectrum;
>    let onHidden = cPicker.tooltip.once("hidden");
> +  // Validating the color change ends up updating the rule view twice
> +  let onRuleViewChanged = new Promise(done => {

You can use waitForNEvents from head.js

Comment 8

[Patrick Brosset <:pbro>]

As discussed on IRC, I'll be merging these fixes into bug 1240813 and landing them there.