702861 - browser chrome mochitests trigger uncaught JS exception in CssHtmlTree.jsm

Status: RESOLVED FIXED

(DevTools :: General, defect)

Description

[Cameron McCormack (:heycam)]

browser_bug683672.js, browser_styleinspector.js and browser_styleinspector_webconsole.js trigger an exception in CssHtmlTree.jsm:

TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/styleinspector/test/browser/browser_bug683672.js | an unexpected uncaught JS exception reported through window.onerror - this.propertyContainer is undefined at resource:///modules/devtools/CssHtmlTree.jsm:299
TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/styleinspector/test/browser/browser_styleinspector.js | an unexpected uncaught JS exception reported through window.onerror - this.propertyContainer is undefined at resource:///modules/devtools/CssHtmlTree.jsm:299
TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/styleinspector/test/browser/browser_styleinspector_webconsole.js | an unexpected uncaught JS exception reported through window.onerror - this.propertyContainer is undefined at resource:///modules/devtools/CssHtmlTree.jsm:299

(These are not reported as TEST-UNEXPECTED-FAILs yet, just with my local patches.)

Comment 1

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Attachment: window.onerror error fix

propertyContainer is defined as:
<div id="propertyContainer">
</div>

We get it when CssHtmlTree is instantiated: 
this.propertyContainer = this.styleDocument.getElementById("propertyContainer");

In the highlight method the following will always work unless the window is in the process of closing:
this.propertyContainer.appendChild(fragment);

This is most likely window.onerror catching errors from JS running when the window is in the process of closing. If this is the problem here then this patch should fix it.

Comment 2

[Cameron McCormack (:heycam)]

I tested this patch (with my local changes also) and it fixes the test failures.

Comment 3

[Dave Camp (:dcamp)]

Is the load being triggered after destroy() is called, or is an in-progress refresh completing?

Seems like destroy() should properly cancel an in-progress refresh and prevent new refreshes from being scheduled, or we'll see other similar bugs crop up.

Comment 4

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

It seems that on my windows box this error is breaking tests. We need to cancel the panel updates etc. if the panel is destroyed.

Comment 5

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Attachment: Stop on destroy

Comment 6

[Cameron McCormack (:heycam)]

As part of this patch can you please remove these two calls to ignoreAllUncaughtExceptions(), which I just added as part of bug 670857?

http://hg.mozilla.org/mozilla-central/file/tip/browser/devtools/styleinspector/test/browser/browser_bug683672.js#l17

http://hg.mozilla.org/mozilla-central/file/tip/browser/devtools/styleinspector/test/browser/browser_styleinspector.js#l82

Comment 7

[Cameron McCormack (:heycam)]

(As part of bug 703176, rather.)

Comment 8

[Dave Camp (:dcamp)]

Why isn't the update cancelled in the same place the propertyContainer is destroyed?

Comment 9

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Attachment: Stop on destroy

(In reply to Cameron McCormack (:heycam) from comment #6)
> As part of this patch can you please remove these two calls to
> ignoreAllUncaughtExceptions(), which I just added as part of bug 670857?
> 

Done

(In reply to Dave Camp (:dcamp) from comment #8)
> Why isn't the update cancelled in the same place the propertyContainer is
> destroyed?

Yup, that is a better place ... done.

Comment 10

[Mihai Sucan [:msucan]]

Shouldn't this be [land-in-fx-team]? The patch has r+.

(and assigned to mike?)

Comment 11

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Yes, it should ... done.

Comment 12

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

https://hg.mozilla.org/integration/fx-team/rev/18d3b3a1f605

Comment 13

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

https://hg.mozilla.org/mozilla-central/rev/18d3b3a1f605