Closed
Bug 1249119
Opened 9 years ago
Closed 8 years ago
Inspector doesn't load if I launch it right after hard-refresh
Categories
(DevTools :: Inspector, defect, P2)
DevTools
Inspector
Tracking
(firefox47 affected, firefox52 fixed)
RESOLVED
FIXED
Firefox 52
People
(Reporter: arni2033, Assigned: ochameau)
References
(Blocks 1 open bug)
Details
Attachments
(4 files, 1 obsolete file)
>>> My Info: Win7_64, Nightly 47, 32bit, ID 20160212030242
STR:
1. Open https://www.mozilla.org/en-US/firefox/nightly/firstrun/ , wait until it's loaded
2. Open devtools -> Inspector. Close devtools.
3. Press Ctrl+Shift+R , then immediately, with no pause, press Ctrl+Shift+I
4. Wait until the page is loaded
AR: Inspector frame is empty. Sometimes empty toolbar is displayed at the top of Inspector frame.
ER: Inspector should load normally, at least when the page finishes loading.
Screenshot:
> https://dl.dropboxusercontent.com/s/o49fwlikc63oxb5/screenshot%201%20-%20Inspector%20doesn%27t%20load%20if%20I%20launch%20it%20right%20after%20hard-refresh.png?dl=0
Comment 1•9 years ago
|
||
Hard to reproduce consistently, but I did manage a few times.
I think I've seen several errors in the console when it happens. One of them was:
0x8000ffff (NS_ERROR_UNEXPECTED)" location: "JS frame :: resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/actors/highlighters/utils/markup.js :: CanvasFrameAnonymousContentHelper.prototype._insert :: line 236" data: no]
But also this one:
Protocol error (unknownError): TypeError: window.document.documentElement is null
And also:
Protocol error (unknownError): TypeError: can't access dead object Promise-backend.js:940
Full Message: TypeError: panel is undefined
Full Stack: Toolbox.prototype.loadTool/onLoad/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/framework/toolbox.js:1274:13
Handler.prototype.process@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/Promise-backend.js:937:23
this.PromiseWalker.walkerLoop@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/Promise-backend.js:816:7
Promise*this.PromiseWalker.scheduleWalkerLoop@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/Promise-backend.js:747:11
this.PromiseWalker.schedulePromise@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/Promise-backend.js:779:7
this.PromiseWalker.completePromise@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/Promise-backend.js:714:7
Front<.onPacket/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/protocol.js:1234:9
DevTools RDP*Front<.request@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/protocol.js:1178:7
frontProto/</proto[name]@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/protocol.js:1338:14
InspectorPanel.prototype._getDefaultNodeForSelection/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/inspector/inspector-panel.js:280:14
Handler.prototype.process@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/Promise-backend.js:937:23
this.PromiseWalker.walkerLoop@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/Promise-backend.js:816:7
Promise*this.PromiseWalker.scheduleWalkerLoop@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/Promise-backend.js:747:11
this.PromiseWalker.schedulePromise@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/Promise-backend.js:779:7
this.PromiseWalker.completePromise@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/Promise-backend.js:714:7
Front<.onPacket/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/protocol.js:1236:9
DevTools RDP*Front<.request@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/protocol.js:1178:7
frontProto/</proto[name]@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/protocol.js:1338:14
exports.WalkerFront<.querySelector<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/actors/inspector.js:3407:12
InspectorPanel.prototype._getDefaultNodeForSelection/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/inspector/inspector-panel.js:271:14
Handler.prototype.process@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/Promise-backend.js:937:23
this.PromiseWalker.walkerLoop@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/Promise-backend.js:816:7
Promise*this.PromiseWalker.scheduleWalkerLoop@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/Promise-backend.js:747:11
this.PromiseWalker.schedulePromise@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/Promise-backend.js:779:7
this.PromiseWalker.completePromise@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/Promise-backend.js:714:7
Front<.onPacket/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/protocol.js:1236:9
DevTools RDP*Front<.request@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/protocol.js:1178:7
frontProto/</proto[name]@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/protocol.js:1338:14
exports.InspectorFront<.getPageStyle<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/actors/inspector.js:3980:12
InspectorPanel.prototype._getPageStyle@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/inspector/inspector-panel.js:232:12
InspectorPanel.prototype.open/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/inspector/inspector-panel.js:111:14
Handler.prototype.process@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/Promise-backend.js:937:23
this.PromiseWalker.walkerLoop@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/Promise-backend.js:816:7
Promise*this.PromiseWalker.scheduleWalkerLoop@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/Promise-backend.js:747:11
this.PromiseWalker.schedulePromise@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/Promise-backend.js:779:7
Promise.prototype.then@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/Promise-backend.js:454:5
InspectorPanel.prototype.open@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/inspector/inspector-panel.js:110:12
Toolbox.prototype.loadTool/onLoad@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/framework/toolbox.js:1258:19
Priority: -- → P2
Updated•9 years ago
|
Blocks: top-inspector-bugs
Assignee | ||
Comment 2•8 years ago
|
||
Just reproduced by running:
./mach run http://google.fr -devtools
Assignee | ||
Comment 3•8 years ago
|
||
Assignee | ||
Comment 4•8 years ago
|
||
Actually, I'll start with this one. It is a good start for fixing inspector startup codepath.
I may put these patches on hold for a bit to ensure it won't be completely replaced by the patches needed for bug 983386.
Assignee: nobody → poirot.alex
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Comment 12•8 years ago
|
||
mozreview-review |
Comment on attachment 8799763 [details]
Bug 1249119 - Fix inspector startup when opening it on a loading document
https://reviewboard.mozilla.org/r/84886/#review83556
::: devtools/shared/fronts/inspector.js:758
(Diff revision 2)
> if (change.type === "newRoot") {
> + // We may receive a new root without receiving any documentUnload
> + // beforehand. Like when opening tools in middle of a document load.
> + if (this.rootNode) {
> + this._createRootNodePromise();
> + }
This is a critical tweak. It allows to recover various inspector crash, like this STR related to bug 983386:
- open data:text/html,foo
- open http://localhost:32423 (port with no listener)
The inspector is in a broken state, it still inspects the data URL. But, with this patch, if you now open a valid URL, it will inspect the new document correctly!
Assignee | ||
Comment 13•8 years ago
|
||
Comment on attachment 8799853 [details]
Bug 1249119 - Test loading the inspector while the document is loading.
I would really like to introduce tests for these edge cases,
but this one looks very hard as it is a race during inspector actor initialization. Also, I don't know how to end up with an uninitialized document.
This test works great locally but seems to fail on try...
Assignee | ||
Comment 14•8 years ago
|
||
Comment on attachment 8799853 [details]
Bug 1249119 - Test loading the inspector while the document is loading.
I tried really hard to come up with a test, but I'm unable to make another one less brutal which would pass on try.
The best solution I found, but which doesn't trigger this error was to listen for inspector-init, dispatched by toolbox.js. This event should fire right after the inspector actors are set and registered to the current document, but it should also be sent before we call InspectorPanel.open and start setting up inspector UI.
I have the feeling that my attempt to reload the document is asynchronous as we are on e10s now, and it happens to reload it too late on the child side. Somehow the inspector is able to set itself up correctly before.
Assignee | ||
Updated•8 years ago
|
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Updated•8 years ago
|
Attachment #8799853 -
Attachment is obsolete: true
Assignee | ||
Comment 19•8 years ago
|
||
Comment on attachment 8799762 [details]
Bug 1249119 - Prevent trying to initialize highlighter on still-loading documents.
Ok. So here is a first serie of patches that do not depend on any other one.
This very first patch is just here to prevent exception, it doesn't fix much.
The main issue behind this bug is that the actors are still refering to what looks like a dead wrapper for an about:blank document loaded very early during firefox startup. That ends up breaking various actors. Here the highlighter get back on its feets as it listen for navigate event and restore itself.
Assignee | ||
Comment 20•8 years ago
|
||
Comment on attachment 8799763 [details]
Bug 1249119 - Fix inspector startup when opening it on a loading document
Here is the patch containing the critical fix, in inspector front:
if (this.rootNode)
this._createRootNodePromise();
When we hit this race we end up in the middle of a document unload and document load, so that we do not receive the will-navigate, so the front doesn't receive documentUnload and do not recreated a "new root node promise".
So here we just ensure that newRoot works correctly even if we happen to missed a related documentUnload event before.
The rest of the patch is useful but less critical.
I moved Breadcrumbs instanciation before setting the event listeners as these events eventually used the breadcrumb.
Assignee | ||
Comment 21•8 years ago
|
||
Comment on attachment 8799854 [details]
Bug 1249119 - Let shared-head helper cleanup tests instead of duplicating the effort in inspector head.
Nothing much to say, this is just code being duplicated between inspector/test/head.js and framework/test/shared-head.js
Assignee | ||
Comment 22•8 years ago
|
||
Comment on attachment 8799855 [details]
Bug 1249119 - Prevent exception when opening inspector on a loading document.
Even with previous patches, we still have other codepaths were the actor code is still refering to a dead about:blank document, so just prevent them.
Comment 23•8 years ago
|
||
mozreview-review |
Comment on attachment 8799762 [details]
Bug 1249119 - Prevent trying to initialize highlighter on still-loading documents.
https://reviewboard.mozilla.org/r/84884/#review85892
Looks good! Thanks. Very excited to see patches on this bug. Thanks a lot Alex.
Attachment #8799762 -
Flags: review?(pbrosset) → review+
Comment 24•8 years ago
|
||
mozreview-review |
Comment on attachment 8799854 [details]
Bug 1249119 - Let shared-head helper cleanup tests instead of duplicating the effort in inspector head.
https://reviewboard.mozilla.org/r/84942/#review85898
Attachment #8799854 -
Flags: review?(pbrosset) → review+
Comment 25•8 years ago
|
||
mozreview-review |
Comment on attachment 8799855 [details]
Bug 1249119 - Prevent exception when opening inspector on a loading document.
https://reviewboard.mozilla.org/r/84944/#review85900
Attachment #8799855 -
Flags: review?(pbrosset) → review+
Updated•8 years ago
|
Attachment #8799763 -
Flags: review?(pbrosset) → review?(jdescottes)
Comment 26•8 years ago
|
||
One comment got lost on mozreview: I've reassigned the review to Julian for the part2 because I don't have the time to test this manually just now. The code changes look sane to me, but I'd like someone else to try this locally and finalize the review.
Comment 27•8 years ago
|
||
(In reply to Patrick Brosset <:pbro> from comment #26)
> One comment got lost on mozreview: I've reassigned the review to Julian for
> the part2 because I don't have the time to test this manually just now. The
> code changes look sane to me, but I'd like someone else to try this locally
> and finalize the review.
I didn't realize that Julian was on PTO when I assigned the review request. But he and I discussed and he said he would do the review anyway.
Julian: please shout if you'd rather me do it after all. I have a bit of time.
Having said this, I've tested the patches locally, and they appear to work pretty well.
I was able to reproduce a similar issue though. My approach was to basically keep doing the STRs of comment 0 many times over until something went wrong.
At some stage I got to a state where no node was selected and an error appeared in the console
Handler function LocalDebuggerTransport instance's this.other.hooks.onPacket threw an exception: TypeError: to is null
Stack: onPacket@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/main.js:1717:11
LocalDebuggerTransport.prototype.send/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/transport/transport.js:570:13
exports.makeInfallible/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/ThreadSafeDevToolsUtils.js:101:14
DevToolsUtils.executeSoon*executeSoon@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/DevToolsUtils.js:36:19
LocalDebuggerTransport.prototype.send@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/transport/transport.js:563:9
Front<.send/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/protocol.js:1209:9
Handler.prototype.process@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:937:23
this.PromiseWalker.walkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:816:7
Promise*this.PromiseWalker.scheduleWalkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:747:11
this.PromiseWalker.schedulePromise@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:779:7
Promise.prototype.then@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:454:5
Front<.send@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/protocol.js:1207:7
Front<.request@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/protocol.js:1227:5
generateRequestMethods/</frontProto[name]@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/protocol.js:1363:14
MarkupView.prototype._getVisibleChildren@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/inspector/markup/markup.js:1708:12
MarkupView.prototype._updateChildren@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/inspector/markup/markup.js:1637:7
MarkupView.prototype._expandContainer@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/inspector/markup/markup.js:1146:12
MarkupView.prototype.expandNode@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/inspector/markup/markup.js:1161:5
MarkupView.prototype.showNode@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/inspector/markup/markup.js:1129:7
MarkupView.prototype._onNewSelection@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/inspector/markup/markup.js:592:14
MarkupView@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/inspector/markup/markup.js:134:3
Inspector.prototype._onMarkupFrameLoad@chrome://devtools/content/inspector/inspector.js:1353:19
EventListener.handleEvent*Inspector.prototype._initMarkup@chrome://devtools/content/inspector/inspector.js:1337:5
Inspector.prototype.onNewRoot/onNodeSelected@chrome://devtools/content/inspector/inspector.js:697:7
Handler.prototype.process@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:937:23
Promise*this.PromiseWalker.scheduleWalkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:747:11
this.PromiseWalker.schedulePromise@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:779:7
this.PromiseWalker.completePromise@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:714:7
Front<.onPacket/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/protocol.js:1283:9
DevTools RDP*Front<.request@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/protocol.js:1225:14
generateRequestMethods/</frontProto[name]@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/protocol.js:1363:14
WalkerFront<.querySelector<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/fronts/inspector.js:620:12
Inspector.prototype._getDefaultNodeForSelection/<@chrome://devtools/content/inspector/inspector.js:334:14
Handler.prototype.process@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:937:23
this.PromiseWalker.walkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:816:7
Promise*this.PromiseWalker.scheduleWalkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:747:11
this.PromiseWalker.schedulePromise@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:779:7
this.PromiseWalker.completePromise@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:714:7
Front<.onPacket/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/protocol.js:1283:9
DevTools RDP*Front<.request@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/protocol.js:1225:14
generateRequestMethods/</frontProto[name]@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/protocol.js:1363:14
WalkerFront<.getMutations<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/fronts/inspector.js:746:12
WalkerFront<.onMutations<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/fronts/inspector.js:892:5
Front<.onPacket/results<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/protocol.js:1248:44
Front<.onPacket@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/protocol.js:1248:23
DebuggerClient.prototype.onPacket@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/client/main.js:947:7
LocalDebuggerTransport.prototype.send/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/transport/transport.js:570:13
exports.makeInfallible/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/ThreadSafeDevToolsUtils.js:101:14
DevToolsUtils.executeSoon*executeSoon@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/DevToolsUtils.js:36:19
LocalDebuggerTransport.prototype.send@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/transport/transport.js:563:9
send@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/main.js:1465:5
ChildDebuggerTransport.prototype.receiveMessage@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/transport/transport.js:761:7
Line: 1717, column: 11
When that happened, the inspector was still visible and usable. It wasn't completely empty, so this is most probably another problem.
Assignee | ||
Comment 28•8 years ago
|
||
(In reply to Patrick Brosset <:pbro> from comment #27)
> (In reply to Patrick Brosset <:pbro> from comment #26)
> > One comment got lost on mozreview: I've reassigned the review to Julian for
> > the part2 because I don't have the time to test this manually just now. The
> > code changes look sane to me, but I'd like someone else to try this locally
> > and finalize the review.
> I didn't realize that Julian was on PTO when I assigned the review request.
> But he and I discussed and he said he would do the review anyway.
> Julian: please shout if you'd rather me do it after all. I have a bit of
> time.
>
> Having said this, I've tested the patches locally, and they appear to work
> pretty well.
> I was able to reproduce a similar issue though. My approach was to basically
> keep doing the STRs of comment 0 many times over until something went wrong.
> At some stage I got to a state where no node was selected and an error
> appeared in the console
You can get many exception when toggle tools on/off.
I tracked this particular one. It is related to the client side racing its own initialization.
It call getUniqueSelector from onNewSelection on a destroyed node.
We end up receiving a new-root-node before deferredOpen finishes, which ends up calling onNewSelection twice, one with reason=navigateaway (related to on-new-root) and another with reason=inspector-open (related to deferredOpen). And it looks like the later one, inspector-open ends up calling selector.getUniqueSelector on a dead front.
Comment 29•8 years ago
|
||
mozreview-review |
Comment on attachment 8799763 [details]
Bug 1249119 - Fix inspector startup when opening it on a loading document
https://reviewboard.mozilla.org/r/84886/#review86282
I could verify that the STRs from comment #12 are fixed.
Had quite a hard time reproducing the original STRs, but managed to get it a few times without your patch and 0 times with your patch.
Really great to see fixes for this kind of issue coming through, thanks!
::: devtools/client/inspector/inspector.js:137
(Diff revision 3)
> - let defaultSelection = yield this._getDefaultNodeForSelection();
> +
> + let defaultSelection;
> + try {
> + defaultSelection = yield this._getDefaultNodeForSelection();
> + } catch (e) {
> + // This may throw if the document is still loading and we are
in onNewRoot, we call _handleRejectionIfNotDestroyed
in case _getDefaultNodeForSelection throws. Maybe do the same here for consistency (unless we really don't want to log anything here).
Attachment #8799763 -
Flags: review?(jdescottes) → review+
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Comment 37•8 years ago
|
||
(In reply to Julian Descottes [:jdescottes] (PTO until 04-Nov) from comment #29)
> Comment on attachment 8799763 [details]
> Bug 1249119 - Fix inspector startup when opening it on a loading document
>
> https://reviewboard.mozilla.org/r/84886/#review86282
>
> I could verify that the STRs from comment #12 are fixed.
> Had quite a hard time reproducing the original STRs, but managed to get it a
> few times without your patch and 0 times with your patch.
>
> Really great to see fixes for this kind of issue coming through, thanks!
>
> ::: devtools/client/inspector/inspector.js:137
> (Diff revision 3)
> > - let defaultSelection = yield this._getDefaultNodeForSelection();
> > +
> > + let defaultSelection;
> > + try {
> > + defaultSelection = yield this._getDefaultNodeForSelection();
> > + } catch (e) {
> > + // This may throw if the document is still loading and we are
>
> in onNewRoot, we call _handleRejectionIfNotDestroyed
> in case _getDefaultNodeForSelection throws. Maybe do the same here for
> consistency (unless we really don't want to log anything here).
I switched to use _handleRejectionIfNotDestroyed. I didn't realize this helper was specific to these kind of exceptions.
I got a somewhat green try:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=b852d0833472&selectedJob=29610972
There is some oranges but they look related to the other inspector bugs.
Assignee | ||
Comment 38•8 years ago
|
||
Pushed again with just this bug's patches:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=9c75eeb7d683
Comment 39•8 years ago
|
||
Pushed by apoirot@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/7664d47e1f14
Prevent trying to initialize highlighter on still-loading documents. r=pbro
https://hg.mozilla.org/integration/autoland/rev/f08cf9dc4b92
Fix inspector startup when opening it on a loading document r=jdescottes
https://hg.mozilla.org/integration/autoland/rev/46b32222e682
Let shared-head helper cleanup tests instead of duplicating the effort in inspector head. r=pbro
https://hg.mozilla.org/integration/autoland/rev/9ebb57bb1d76
Prevent exception when opening inspector on a loading document. r=pbro
Comment 40•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/7664d47e1f14
https://hg.mozilla.org/mozilla-central/rev/f08cf9dc4b92
https://hg.mozilla.org/mozilla-central/rev/46b32222e682
https://hg.mozilla.org/mozilla-central/rev/9ebb57bb1d76
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox52:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 52
Updated•8 years ago
|
No longer blocks: top-inspector-bugs
Comment 41•8 years ago
|
||
I have reproduced this bug on firefox nightly according to(2016-02-17)
Fixing bug is verified on Latest Nightly--Build ID:(20161030030204),User Agent: Mozilla/5.0 (Windows NT 10.0; rv:52.0) Gecko/20100101 Firefox/52.0
tested OS-- Windows10 32bit
QA Whiteboard: [testday-20161028]
Updated•6 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•