Inspector doesn't load if I launch it right after hard-refresh

RESOLVED FIXED in Firefox 52

Status

()

Firefox
Developer Tools: Inspector
P2
normal
RESOLVED FIXED
a year ago
7 months ago

People

(Reporter: arni2033, Assigned: ochameau)

Tracking

(Blocks: 1 bug)

Trunk
Firefox 52
Points:
---

Firefox Tracking Flags

(firefox47 affected, firefox52 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(4 attachments, 1 obsolete attachment)

(Reporter)

Description

a year ago
>>>   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
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
Blocks: 1264624
(Assignee)

Comment 2

8 months ago
Just reproduced by running:
  ./mach run http://google.fr -devtools
(Assignee)

Comment 3

8 months ago
https://treeherder.mozilla.org/#/jobs?repo=try&revision=8dbc4bc1f816
(Assignee)

Comment 4

8 months 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 months 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 months 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

7 months 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

7 months ago
See Also: → bug 983386
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Updated

7 months ago
Attachment #8799853 - Attachment is obsolete: true
(Assignee)

Comment 19

7 months 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

7 months 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

7 months 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

7 months 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 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+
(Assignee)

Updated

7 months ago
Blocks: 1277348
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 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+
Attachment #8799763 - Flags: review?(pbrosset) → review?(jdescottes)
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.
(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

7 months 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

7 months 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

7 months 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

7 months ago
Pushed again with just this bug's patches:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=9c75eeb7d683

Comment 39

7 months 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

7 months 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
Last Resolved: 7 months ago
status-firefox52: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 52
No longer blocks: 1264624
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]
You need to log in before you can comment on or make changes to this bug.