Closed Bug 1499186 Opened Last year Closed Last year

Show graphics updates in order when hitting breakpoints during replay

Categories

(Core :: Web Replay, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla64
Tracking Status
firefox64 --- fixed

People

(Reporter: bhackett1024, Assigned: bhackett1024)

References

Details

Attachments

(4 files)

Attached patch patchSplinter Review
Newly painted bitmaps sent from the child process as Paint messages are used by the middleman to immediately update the graphics shown by the UI process.  Before bug 1488808 this was fine, as the child blocked the main thread to wait for the paint to complete, ensuring that the paint shows the state of the DOM at the exact point where the painting data is sent to the middleman.  After bug 1488808, the paint shows the state where the checkpoint was created, but the paint is sent to the middleman at a later point in time: after the compositor finishes the update and a runnable executes on the main thread to send the data to the middleman. Between the time when the checkpoint was created and the painting data was posted, the main thread can run JS, update the DOM, etc.

This is a problem when a breakpoint is hit somewhere between the time the checkpoint is created and the time the associated paint completes.  When rewinding, the Paint message will be received before the breakpoint is hit, with the result being that earlier graphics (from the Paint message, reflecting the checkpoint) are shown before the later graphics that are expected when the process pauses at the breakpoint and repaints.  A similar problem happens when running forward: we show the later graphics first when hitting the breakpoint and repainting, then clobber that with the earlier graphics when the Paint message is received afterwards.

The solution to this is pretty simple: the graphics from a Paint message are only shown when both the Paint and the associated checkpoint hit have both been received, with no interim repainting due to pausing at a breakpoint.  This patch implements this fix, and a supporting fix that ensures we can't get into weird situations like having multiple in flight paints at a time that would complicate the solution here.  The latter is done by ignoring vsyncs (which might trigger new paints) that are received between the time a paint is triggered and the time it completes.
Allow recording/replaying processes to ignore vsyncs that are received before the last paint completes, and simplify the calls needed when a paint is triggered.
Attachment #9017299 - Flags: review?(nical.bugzilla)
Make sure there is only one paint happening at a time when recording/replaying, so that each paint is associated with the most recent checkpoint.
Attachment #9017300 - Flags: review?(nical.bugzilla)
Wait until both a Paint message and its associated checkpoint have been reached before updating the graphics in the UI process.
Attachment #9017301 - Flags: review?(nical.bugzilla)
Attachment #9017299 - Flags: review?(nical.bugzilla) → review+
Comment on attachment 9017300 [details] [diff] [review]
Part 2 - Only allow one in flight paint at a time.

Review of attachment 9017300 [details] [diff] [review]:
-----------------------------------------------------------------

::: toolkit/recordreplay/ipc/ChildIPC.cpp
@@ +452,5 @@
>  static void* gDrawTargetBuffer;
>  static size_t gDrawTargetBufferSize;
>  
> +// Message for the last paint which the compositor generated.
> +static Maybe<PaintMessage> gPaintMessage;

nit: Obligatory warning about static initializers that add up and show startup down. Maybe a static PaintMessage* pointer would do here (I also see we have a ot of static UniquePtr in the code base so I suppose they are fine as well)?
Attachment #9017300 - Flags: review?(nical.bugzilla) → review+
Attachment #9017301 - Flags: review?(nical.bugzilla) → review+
Pushed by bhackett@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/0fba5665bc65
Part 1 - Allow recording/replaying processes to ignore vsyncs, r=nical.
https://hg.mozilla.org/integration/mozilla-inbound/rev/b341213ecf95
Part 2 - Only allow one in flight paint at a time, r=nical.
https://hg.mozilla.org/integration/mozilla-inbound/rev/7400b733b50f
Part 3 - Don't show out of order graphics in the middleman, r=nical.
https://hg.mozilla.org/projects/larch/rev/0fba5665bc65c55a4d1f27905a2a60396dbe2bb8
Bug 1499186 Part 1 - Allow recording/replaying processes to ignore vsyncs, r=nical.

https://hg.mozilla.org/projects/larch/rev/b341213ecf95d1448abc0a38ef058138b41324ee
Bug 1499186 Part 2 - Only allow one in flight paint at a time, r=nical.

https://hg.mozilla.org/projects/larch/rev/7400b733b50f3a9d4fa8ca18363a9dbb52956398
Bug 1499186 Part 3 - Don't show out of order graphics in the middleman, r=nical.
You need to log in before you can comment on or make changes to this bug.