1505895 - Add handler for changes to a recording/replaying child's state

Status: RESOLVED FIXED

(Core Graveyard :: Web Replay, enhancement, P1)

Description

[Brian Hackett [Laid off!]]

Attachment: patch

The attached patch adds a hook that can be installed on ReplayDebuggers and is called (in the server/middleman) when the child is paused and either just reached a checkpoint or just changed from recording to replaying or vice versa.  The patch also adds an interface for whether the active child is recording.  These can be used to update the UI or to keep track of the execution points for all checkpoints, providing a coarse-grained set of points to jump to on the timeline.

Comment 1

[Brian Hackett [Laid off!]]

Attachment: example

Example of using this handler to dump() whether the child is recording and how much progress it has made.

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/06b12567b865
Add handler for changes to a recording/replaying child's state, r=lsmyth.

Comment 3

[Daniel Varga [:dvarga]]

https://hg.mozilla.org/mozilla-central/rev/06b12567b865

Comment 4

[Jason Laster [:jlast]]

`replayingOnPositionChange` causes browser_dbg_rr_breakpoints-04 and other tests to fail

> diff --git a/devtools/server/actors/thread.js b/devtools/server/actors/thread.js
> --- a/devtools/server/actors/thread.js
> +++ b/devtools/server/actors/thread.js
> @@ -112,8 +112,9 @@ const ThreadActor = ActorClassWithSpec(t
>        this._dbg.onDebuggerStatement = this.onDebuggerStatement;
>        this._dbg.onNewScript = this.onNewScript;
>        if (this._dbg.replaying) {
>          this._dbg.replayingOnForcedPause = this.replayingOnForcedPause.bind(this);
> +        this._dbg.replayingOnPositionChange = () => {};
>        }
>        // Keep the debugger disabled until a client attaches.
>        this._dbg.enabled = this._state != "detached";
>      }
> @@ -1787,8 +1788,20 @@ const ThreadActor = ActorClassWithSpec(t
>      }
>    },

Comment 5

[Brian Hackett [Laid off!]]

Attachment: followup

This patch fixes a couple problems with the first one:

- After the position change handler runs, the process resumes implicitly if it hasn't been explicitly paused or given another resume/seek command.  This was causing things to resume in the wrong places during tests and break them.

- Hitting breakpoints for the position change handler is treated as an explicit pause, causing the recording to be flushed and replaying children to start saving all the checkpoints they encounter (not good for performance).

All web replay tests with this patch and a position change handler set.

This patch is also somewhat provisional in that it made me realize that the mechanisms for controlling resumes and the position of the active child need to be moved out of ParentIPC.cpp and into debugger.js; the ReplayDebugger can then have a lot more control over what the child process is doing and it will be easier to manage state with the various breakpoint kinds that are mainly relevant only to devtools JS.  That fix is more extensive though and it would be good to get this in for now.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/72bbcf253876
Part 2 - Avoid some default breakpoint behaviors for position changes, r=lsmyth.

Comment 7

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/72bbcf253876