Last Comment Bug 1249119 - Inspector doesn't load if I launch it right after hard-refresh
: Inspector doesn't load if I launch it right after hard-refresh
Status: RESOLVED FIXED
:
Product: Firefox
Classification: Client Software
Component: Developer Tools: Inspector (show other bugs)
: Trunk
: Unspecified Unspecified
P2 normal (vote)
: Firefox 52
Assigned To: Alexandre Poirot [:ochameau]
:
: Gabriel Luong [:gl][1 biz day review guarantee] (ΦωΦ)
Mentors:
Depends on:
Blocks: 1277348
  Show dependency treegraph
 
Reported: 2016-02-17 13:21 PST by arni2033
Modified: 2016-10-30 10:50 PDT (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard: [testday-20161028]
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
affected
fixed

MozReview Requests
Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:
Show discarded requests

Attachments
Bug 1249119 - Prevent trying to initialize highlighter on still-loading documents. (58 bytes, text/x-review-board-request)
2016-10-11 07:13 PDT, Alexandre Poirot [:ochameau]
pbrosset: review+
Details | Review
Bug 1249119 - Fix inspector startup when opening it on a loading document (58 bytes, text/x-review-board-request)
2016-10-11 07:13 PDT, Alexandre Poirot [:ochameau]
jdescottes: review+
Details | Review
Bug 1249119 - Test loading the inspector while the document is loading. (58 bytes, text/x-review-board-request)
2016-10-11 09:44 PDT, Alexandre Poirot [:ochameau]
no flags Details | Review
Bug 1249119 - Let shared-head helper cleanup tests instead of duplicating the effort in inspector head. (58 bytes, text/x-review-board-request)
2016-10-11 09:44 PDT, Alexandre Poirot [:ochameau]
pbrosset: review+
Details | Review
Bug 1249119 - Prevent exception when opening inspector on a loading document. (58 bytes, text/x-review-board-request)
2016-10-11 09:44 PDT, Alexandre Poirot [:ochameau]
pbrosset: review+
Details | Review

Description User image arni2033 2016-02-17 13:21:48 PST
>>>   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 User image Patrick Brosset <:pbro> 2016-02-18 02:26:20 PST
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
Comment 2 User image Alexandre Poirot [:ochameau] 2016-10-06 14:01:08 PDT
Just reproduced by running:
  ./mach run http://google.fr -devtools
Comment 3 User image Alexandre Poirot [:ochameau] 2016-10-10 08:23:49 PDT
https://treeherder.mozilla.org/#/jobs?repo=try&revision=8dbc4bc1f816
Comment 4 User image Alexandre Poirot [:ochameau] 2016-10-11 07:12:37 PDT
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.
Comment 5 User image Alexandre Poirot [:ochameau] 2016-10-11 07:13:36 PDT Comment hidden (mozreview-request)
Comment 6 User image Alexandre Poirot [:ochameau] 2016-10-11 07:13:36 PDT Comment hidden (mozreview-request)
Comment 7 User image Alexandre Poirot [:ochameau] 2016-10-11 09:44:27 PDT Comment hidden (mozreview-request)
Comment 8 User image Alexandre Poirot [:ochameau] 2016-10-11 09:44:27 PDT Comment hidden (mozreview-request)
Comment 9 User image Alexandre Poirot [:ochameau] 2016-10-11 09:44:27 PDT Comment hidden (mozreview-request)
Comment 10 User image Alexandre Poirot [:ochameau] 2016-10-11 09:44:27 PDT Comment hidden (mozreview-request)
Comment 11 User image Alexandre Poirot [:ochameau] 2016-10-11 09:44:27 PDT Comment hidden (mozreview-request)
Comment 12 User image Alexandre Poirot [:ochameau] 2016-10-11 13:00:41 PDT
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!
Comment 13 User image Alexandre Poirot [:ochameau] 2016-10-11 13:04:04 PDT
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...
Comment 14 User image Alexandre Poirot [:ochameau] 2016-10-13 07:35:16 PDT
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.
Comment 15 User image Alexandre Poirot [:ochameau] 2016-10-19 05:51:24 PDT Comment hidden (mozreview-request)
Comment 16 User image Alexandre Poirot [:ochameau] 2016-10-19 05:51:24 PDT Comment hidden (mozreview-request)
Comment 17 User image Alexandre Poirot [:ochameau] 2016-10-19 05:51:24 PDT Comment hidden (mozreview-request)
Comment 18 User image Alexandre Poirot [:ochameau] 2016-10-19 05:51:24 PDT Comment hidden (mozreview-request)
Comment 19 User image Alexandre Poirot [:ochameau] 2016-10-19 05:55:44 PDT
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.
Comment 20 User image Alexandre Poirot [:ochameau] 2016-10-19 06:05:26 PDT
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.
Comment 21 User image Alexandre Poirot [:ochameau] 2016-10-19 06:06:14 PDT
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
Comment 22 User image Alexandre Poirot [:ochameau] 2016-10-19 06:07:16 PDT
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 User image Patrick Brosset <:pbro> 2016-10-19 06:24:02 PDT
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.
Comment 24 User image Patrick Brosset <:pbro> 2016-10-19 06:54:21 PDT
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
Comment 25 User image Patrick Brosset <:pbro> 2016-10-19 06:55:36 PDT
Comment on attachment 8799855 [details]
Bug 1249119 - Prevent exception when opening inspector on a loading document.

https://reviewboard.mozilla.org/r/84944/#review85900
Comment 26 User image Patrick Brosset <:pbro> 2016-10-19 06:57:56 PDT
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 User image Patrick Brosset <:pbro> 2016-10-20 06:40:09 PDT
(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.
Comment 28 User image Alexandre Poirot [:ochameau] 2016-10-20 07:45:14 PDT
(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 User image Julian Descottes [:jdescottes] 2016-10-20 09:42:04 PDT
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).
Comment 30 User image Alexandre Poirot [:ochameau] 2016-10-20 14:12:20 PDT Comment hidden (mozreview-request)
Comment 31 User image Alexandre Poirot [:ochameau] 2016-10-20 14:12:20 PDT Comment hidden (mozreview-request)
Comment 32 User image Alexandre Poirot [:ochameau] 2016-10-20 14:12:20 PDT Comment hidden (mozreview-request)
Comment 33 User image Alexandre Poirot [:ochameau] 2016-10-20 14:33:30 PDT Comment hidden (mozreview-request)
Comment 34 User image Alexandre Poirot [:ochameau] 2016-10-20 14:33:30 PDT Comment hidden (mozreview-request)
Comment 35 User image Alexandre Poirot [:ochameau] 2016-10-20 14:33:30 PDT Comment hidden (mozreview-request)
Comment 36 User image Alexandre Poirot [:ochameau] 2016-10-20 14:33:30 PDT Comment hidden (mozreview-request)
Comment 37 User image Alexandre Poirot [:ochameau] 2016-10-21 03:45:35 PDT
(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.
Comment 38 User image Alexandre Poirot [:ochameau] 2016-10-21 03:47:35 PDT
Pushed again with just this bug's patches:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=9c75eeb7d683
Comment 39 User image Pulsebot 2016-10-24 02:13:57 PDT
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 41 User image Saheda Reza [:Antora] 2016-10-30 10:50:45 PDT
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

Note You need to log in before you can comment on or make changes to this bug.