Closed
Bug 1299602
Opened 9 years ago
Closed 9 years ago
Debugger fails to emit tabNavigated event when paused
Categories
(DevTools :: Debugger, defect, P2)
DevTools
Debugger
Tracking
(firefox51 fixed)
RESOLVED
FIXED
Firefox 51
Tracking | Status | |
---|---|---|
firefox51 | --- | fixed |
People
(Reporter: jlast, Unassigned)
References
Details
Attachments
(1 file)
When the debugger is paused, refreshing the debuggee does not emit a tabNavigated event.
It looks like it's due to an error in request.suspend() that prevents the event from being emitted.
https://www.irccloud.com/pastebin/CaJRoQ5q/
Reporter | ||
Updated•9 years ago
|
Priority: -- → P2
Reporter | ||
Updated•9 years ago
|
Flags: needinfo?(poirot.alex)
My STR to produce the linked stack was:
1. Open Debugger
2. Set a breakpoint in your favorite script and trigger it to hit
3. Reload the page (Cmd-R) while paused at the breakpoint
Comment 2•9 years ago
|
||
Whaaa you are hitting a old piece of code that predates me:
https://dxr.mozilla.org/mozilla-central/source/devtools/server/actors/webbrowser.js#1951
Looks like we are trying to:
1) pause the request <=== but we throw here!
2) somewhat synchronously resume js execution
3) on event loop wake up, resume the request
May be it was still working in non-e10s but it now throws in e10s?
Flags: needinfo?(poirot.alex)
Comment 3•9 years ago
|
||
I can surely make a patch that remove this complexity and stop messing up with the request,
but it would be better to know why we had to do that in the past and this is very related to the debugger guts, which I don't know that much about.
Comment 4•9 years ago
|
||
Looking at blame. It began in bug 751949, because of an assertion and things being broken.
It is much different now in e10s as the request ends up happening in the parent.
May be we could let the request go and "just" resume the debugger?
Depends on: 751949
Comment 5•9 years ago
|
||
Comment hidden (mozreview-request) |
Comment 7•9 years ago
|
||
More like a feedback request than review as I expect that to break things on non-e10s.
And I don't really know the consequences of not suspending the request...
Comment 8•9 years ago
|
||
For what it is worth, try looks green on linux.
Comment 9•9 years ago
|
||
(In reply to Alexandre Poirot [:ochameau] from comment #3)
> I can surely make a patch that remove this complexity and stop messing up
> with the request,
> but it would be better to know why we had to do that in the past and this is
> very related to the debugger guts, which I don't know that much about.
I agree, I have no idea why it was trying to suspend it before and unfortunately there are no comments. I guess the only path forward is to stop doing it, as per your patch, and we'll learn the reason why it was doing it before. It may have been a hack before that we don't need to do anymore.
Comment 10•9 years ago
|
||
mozreview-review |
Comment on attachment 8787130 [details]
Bug 1299602 - Do not suspend navigation when navigating with a breakpoint.
https://reviewboard.mozilla.org/r/75994/#review74026
What kinds of thing to do you expect to break in non-e10s? I'm assuming that you ran this locally and it does fix the problem? You could turn of e10s and at least make sure it's not totally broken. If try is green, that seems good enough...
Attachment #8787130 -
Flags: review?(jlong) → review+
Comment 11•9 years ago
|
||
(In reply to James Long (:jlongster) from comment #10)
> Comment on attachment 8787130 [details]
> Bug 1299602 - Do not suspend navigation when navigating with a breakpoint.
>
> https://reviewboard.mozilla.org/r/75994/#review74026
>
> What kinds of thing to do you expect to break in non-e10s?
I was expecting to resurect bug 751949. But that's only an assertion, not an exception.
I'll try to build a debug build to see if it revives the assertion.
Testing with this doc:
data:text/html,<script>onclick=function () {%0Aconsole.log("plop");%0A}</script>
Setting a breakpoint at line two, on the console.log(),
reloading after hitting the breakpoint doesn't seem to introduce any breakage nor exception no matter if e10s is turned on or off.
Comment 12•9 years ago
|
||
I do not see any assertion on debug build either.
Let's proceed, but keep this patch in mind if you see anything unexpected happening on reload.
Comment 13•9 years ago
|
||
Pushed by apoirot@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/42ada9dc0ab3
Do not suspend navigation when navigating with a breakpoint. r=jlongster
Comment 14•9 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 9 years ago
status-firefox51:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 51
Comment 15•9 years ago
|
||
Thanks so much Alex!
Updated•7 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•