Closed Bug 702861 Opened 8 years ago Closed 8 years ago

browser chrome mochitests trigger uncaught JS exception in CssHtmlTree.jsm

Categories

(DevTools :: General, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 11

People

(Reporter: heycam, Assigned: miker)

References

(Blocks 1 open bug)

Details

(Whiteboard: [styleinspector][has-patch][fixed-in-fx-team])

Attachments

(1 file, 2 obsolete files)

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.)
Attached patch window.onerror error fix (obsolete) — Splinter Review
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.
Attachment #574851 - Flags: review?(dcamp)
I tested this patch (with my local changes also) and it fixes the test failures.
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.
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.
Attached patch Stop on destroy (obsolete) — Splinter Review
Attachment #574851 - Attachment is obsolete: true
Attachment #577583 - Flags: review?(dcamp)
(As part of bug 703176, rather.)
Why isn't the update cancelled in the same place the propertyContainer is destroyed?
Whiteboard: [styleinspector]
Attached patch Stop on destroySplinter Review
(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.
Attachment #577583 - Attachment is obsolete: true
Attachment #577583 - Flags: review?(dcamp)
Attachment #579687 - Flags: review?(dcamp)
Whiteboard: [styleinspector] → [styleinspector][has-patch]
Attachment #579687 - Flags: review?(dcamp) → review+
Shouldn't this be [land-in-fx-team]? The patch has r+.

(and assigned to mike?)
Yes, it should ... done.
Assignee: nobody → mratcliffe
Status: NEW → ASSIGNED
Whiteboard: [styleinspector][has-patch] → [styleinspector][has-patch][land-in-fx-team]
https://hg.mozilla.org/integration/fx-team/rev/18d3b3a1f605
Whiteboard: [styleinspector][has-patch][land-in-fx-team] → [styleinspector][has-patch][fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/18d3b3a1f605
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 11
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.