Closed Bug 934487 Opened 7 years ago Closed 7 years ago
Error: this .markup is undefined: Inspector Panel _on New Root/</<@resource://gre/modules/commonjs/toolkit/loader .js -> resource:///modules/devtools/inspector/inspector-panel .js:361" in all mochitest-bc runs
Having stared at a LOT of mochitest-bc logs lately (as all of the above CCed also have), I've seen this staring at me and figured it should be filed. It affects 26/27/28. https://tbpl.mozilla.org/php/getParsedLog.php?id=30079315&tree=Mozilla-Inbound Windows 7 32-bit mozilla-inbound debug test mochitest-browser-chrome on 2013-11-04 06:10:37 PST for push fec854fceba6 slave: t-w732-ix-063 07:01:59 INFO - TEST-PASS | chrome://mochitests/content/browser/browser/devtools/inspector/test/browser_inspector_select_last_selected.js | Node re-selected after navigation 07:01:59 INFO - [Parent 3252] WARNING: No inner window available!: file e:/builds/moz2_slave/m-in-w32-d-0000000000000000000/build/dom/base/nsGlobalWindow.cpp, line 9281 07:01:59 INFO - TypeError: this.markup is undefined: InspectorPanel_onNewRoot/</<@resource://gre/modules/commonjs/toolkit/loader.js -> resource:///modules/devtools/inspector/inspector-panel.js:361 07:01:59 INFO - @resource://gre/modules/commonjs/toolkit/loader.js -> resource:///modules/devtools/shared/event-emitter.js:63 07:01:59 INFO - EventEmitter_emit@resource://gre/modules/commonjs/toolkit/loader.js -> resource:///modules/devtools/shared/event-emitter.js:110 07:01:59 INFO - InspectorPanel__onMarkupFrameLoad@resource://gre/modules/commonjs/toolkit/loader.js -> resource:///modules/devtools/inspector/inspector-panel.js:676 07:01:59 INFO - InspectorPanel_initMarkupPanel_onload@resource://gre/modules/commonjs/toolkit/loader.js -> resource:///modules/devtools/inspector/inspector-panel.js:658
I'm not sure if this message just began to reappear after fixes from Bug 932880 (specifically https://bugzilla.mozilla.org/attachment.cgi?id=825590&action=diff#a/browser/devtools/inspector/test/browser_inspector_select_last_selected.js_sec1), or if this has been around since before that. I know I've seen this message in the past, but I'm not sure if it was on every b-c run. I removed a bunch of executeSoons in that test to fix some of the leaks - I will take a look at it.
QA Contact: bgrinstead
Assignee: nobody → bgrinstead
QA Contact: bgrinstead
Small change to bail out if inspector panel has been destroyed, by checking for this._destroyPromise, as onNewRoot does. Pushed to try: https://tbpl.mozilla.org/?tree=Try&rev=6598a4c8d6c3
Attachment #826823 - Flags: review?(pbrosset)
There was something wrong with the last try push, here is another one: https://tbpl.mozilla.org/?tree=Try&rev=8ce73af51454
Comment on attachment 826823 [details] [diff] [review] markup-undefined.patch Review of attachment 826823 [details] [diff] [review]: ----------------------------------------------------------------- LGTM
Attachment #826823 - Flags: review?(pbrosset) → review+
I think we'll have to improve how we handle these things: - when a test ends abruptly without waiting for a server-side response for instance - when the toolbox is closed - when a page is navigated away. For now, we don't have that many nullchecks or early return statements as you did here, but we may have to add more and more as we fix bugs, and although it doesn't look very good, I don't see any other way, given that most of our code is async and relying on client-server communication. On the server-side inspector for instance, there are still many scenarios where, when a request for a given node comes, that node has already been removed from the DOM.
Status: NEW → ASSIGNED
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 28
You need to log in before you can comment on or make changes to this bug.