689160 - Investigate and Fix leaked csshtmltree.xhtml windows in Highlighter+Style Inspector

Status: RESOLVED FIXED

(DevTools :: General, defect)

Description

[Rob Campbell [:rc] (:robcee)]

During the landing of bug 663831 and associates, it was found to leak csshtmltree.xhtml tree during the course of the inspector tests. We should investigate and fix this leak as well as provide a proper test of the Style Inspector from the Highlighter toolbar.

Comment 1

[Rob Campbell [:rc] (:robcee)]

Attachment: csshtmltree leak fix

A somewhat hacky patch that does manage to clear out the references to csshtmltree.xhtml on inspector close. All tests passed locally with no additional leaked windows.

This should be made better when the work for bug 685893 is completed but should at least let us turn on the style inspector in the interim.

Comment 2

[Mihai Sucan [:msucan]]

Comment on attachment 565015 [details] [diff] [review]
csshtmltree leak fix

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

Patch looks good and the memleaks are gone. Thank you!

As previously discussed, this patch should have a mochitest for opening and closing the Style Inspector from within the Highlighter.

Comment 3

[Rob Campbell [:rc] (:robcee)]

Attachment: [in-fx-team] csshtmltree leak fix + test

added a simple test to browser_highlighter_initialization.js.

Comment 4

[Mihai Sucan [:msucan]]

Comment on attachment 565521 [details] [diff] [review]
[in-fx-team] csshtmltree leak fix + test

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

Patch looks fine! All tests pass.

::: browser/devtools/styleinspector/CssHtmlTree.jsm
@@ +328,5 @@
>      CssHtmlTree.propertyNames.push.apply(CssHtmlTree.propertyNames,
>        mozProps.sort());
>    },
> +
> +  destroy: function CssHtmlTree_destroy()

Maybe a jsdoc comment here?

Comment 5

[Rob Campbell [:rc] (:robcee)]

Comment on attachment 565521 [details] [diff] [review]
[in-fx-team] csshtmltree leak fix + test

https://hg.mozilla.org/integration/fx-team/rev/294ed82810e1

Comment 6

[Rob Campbell [:rc] (:robcee)]

https://hg.mozilla.org/mozilla-central/rev/294ed82810e1