Allow setting breakpoints in pretty-printed source
Categories
(Core Graveyard :: Web Replay, defect)
Tracking
(firefox66 fixed)
Tracking | Status | |
---|---|---|
firefox66 | --- | fixed |
People
(Reporter: bhackett1024, Assigned: bhackett1024)
References
(Blocks 1 open bug)
Details
Attachments
(1 file)
2.33 KB,
patch
|
loganfsmyth
:
review+
|
Details | Diff | Splinter Review |
A few parts of the Debugger interface are unimplemented by ReplayDebugger, which prevents breakpoints from being installed in sources that have been converted by the pretty printer (and probably other generated sources). The attached patch fixes these implementation gaps and the above problem.
Comment 1•5 years ago
|
||
Comment on attachment 9033575 [details] [diff] [review] patch Review of attachment 9033575 [details] [diff] [review]: ----------------------------------------------------------------- ::: devtools/server/actors/replay/debugger.js @@ +451,4 @@ > }, > > findScripts(query) { > + this._ensurePaused(); We recently landed https://bugzilla.mozilla.org/show_bug.cgi?id=1510463, so we aren't always paused when a breakpoint is added. Assuming this assertion is declaring an existing limitation, does this mean that users will need to manually pause before they can add a breakpoint?
Assignee | ||
Comment 2•5 years ago
|
||
(In reply to Logan Smyth [:loganfsmyth] from comment #1) > Comment on attachment 9033575 [details] [diff] [review] > patch > > Review of attachment 9033575 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: devtools/server/actors/replay/debugger.js > @@ +451,4 @@ > > }, > > > > findScripts(query) { > > + this._ensurePaused(); > > We recently landed https://bugzilla.mozilla.org/show_bug.cgi?id=1510463, so > we aren't always paused when a breakpoint is added. Assuming this assertion > is declaring an existing limitation, does this mean that users will need to > manually pause before they can add a breakpoint? No, users will not need to manually pause for this. This _ensurePaused call relates to whether the active child process is paused and is able to receive debugger messages --- web replay child processes have a restriction that messages can only be sent to them when they are paused at a checkpoint or breakpoint, so that we don't have to deal with situations such as the parent sending a message while the child is in the middle of rewinding. To allow the server code to interact with the debuggee regardless of whether it is paused, we use _ensurePaused to wait until the child process reaches the next point it can pause at, do the operation, and then resume executing in the same direction it was traveling previously. The latter bit is handled at the end of _performPause, which will be called soon after the pause happens via the event loop.
Comment 3•5 years ago
|
||
Comment on attachment 9033575 [details] [diff] [review] patch Review of attachment 9033575 [details] [diff] [review]: ----------------------------------------------------------------- Ahh perfect, looks good then.
Pushed by bhackett@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/ec9a7aa1898c Implement a couple ReplayDebugger interfaces, r=lsmyth.
Comment 5•5 years ago
•
|
||
Backed out 12 changesets (bug 1516578, bug 1513118, bug 1516694) for failing at browser_toolbox_remoteness_change.js on a CLOSED TREE
Backout link: https://hg.mozilla.org/integration/mozilla-inbound/rev/9e3564442734c89fa1b735ff8662588576cf0115
Log snippet:
[task 2019-01-12T21:42:48.171Z] 21:42:48 INFO - GECKO(1484) | nsStringStats
[task 2019-01-12T21:42:48.171Z] 21:42:48 INFO - GECKO(1484) | => mAllocCount: 1939881
[task 2019-01-12T21:42:48.173Z] 21:42:48 INFO - GECKO(1484) | => mReallocCount: 1
[task 2019-01-12T21:42:48.173Z] 21:42:48 INFO - GECKO(1484) | => mFreeCount: 1939881
[task 2019-01-12T21:42:48.174Z] 21:42:48 INFO - GECKO(1484) | => mShareCount: 1889903
[task 2019-01-12T21:42:48.175Z] 21:42:48 INFO - GECKO(1484) | => mAdoptCount: 67026
[task 2019-01-12T21:42:48.176Z] 21:42:48 INFO - GECKO(1484) | => mAdoptFreeCount: 67986
[task 2019-01-12T21:42:48.177Z] 21:42:48 INFO - GECKO(1484) | => Process ID: 1484, Thread ID: 139783454422848
[task 2019-01-12T21:42:48.210Z] 21:42:48 INFO - TEST-INFO | Main app process: exit 0
[task 2019-01-12T21:42:48.211Z] 21:42:48 ERROR - TEST-UNEXPECTED-FAIL | devtools/client/framework/test/browser_toolbox_remoteness_change.js | leaked 4 window(s) until shutdown [url = about:blank]
[task 2019-01-12T21:42:48.212Z] 21:42:48 ERROR - TEST-UNEXPECTED-FAIL | devtools/client/framework/test/browser_toolbox_remoteness_change.js | leaked 1 window(s) until shutdown [url = chrome://devtools/content/webconsole/index.html]
[task 2019-01-12T21:42:48.222Z] 21:42:48 ERROR - TEST-UNEXPECTED-FAIL | devtools/client/framework/test/browser_toolbox_remoteness_change.js | leaked 1 window(s) until shutdown [url = about:devtools-toolbox]
[task 2019-01-12T21:42:48.223Z] 21:42:48 INFO - TEST-INFO | devtools/client/framework/test/browser_toolbox_remoteness_change.js | windows(s) leaked: [pid = 1484] [serial = 492], [pid = 1484] [serial = 490], [pid = 1484] [serial = 489], [pid = 1484] [serial = 491], [pid = 1484] [serial = 488], [pid = 1484] [serial = 487]
[task 2019-01-12T21:42:48.224Z] 21:42:48 ERROR - TEST-UNEXPECTED-FAIL | devtools/client/framework/test/browser_toolbox_remoteness_change.js | leaked 2 docShell(s) until shutdown
[task 2019-01-12T21:42:48.224Z] 21:42:48 INFO - TEST-INFO | devtools/client/framework/test/browser_toolbox_remoteness_change.js | docShell(s) leaked: [pid = 1484] [id = {2478efcb-416b-4465-ac17-c720bf9ea414}], [pid = 1484] [id = {4df99fc4-7efa-48f0-b186-723fe21e3008}]
[task 2019-01-12T21:42:48.226Z] 21:42:48 INFO - runtests.py | Application ran for: 0:15:58.112473
[task 2019-01-12T21:42:48.228Z] 21:42:48 INFO - zombiecheck | Reading PID log: /tmp/tmpCS3TLopidlog
Pushed by bhackett@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/25b80743d650 Implement a couple ReplayDebugger interfaces, r=lsmyth.
Assignee | ||
Comment 7•5 years ago
|
||
I don't think anything in these patches has anything to do with the browser_toolbox_remoteness_change.js failure: when I test locally, browser_toolbox_remoteness_change.js leaks windows both with and without these changes. The try push below contains two out of three of the bugs whose patches were in the earlier push, and doesn't have any browser_toolbox_remoteness_change.js failures.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=191ee7f02a12
Comment 8•5 years ago
|
||
bugherder |
Updated•4 years ago
|
Description
•