Closed Bug 1481351 Opened Last year Closed Last year

Fix some issues around PBrowser::Show message handling

Categories

(Core :: Web Replay, enhancement)

enhancement
Not set

Tracking

()

RESOLVED FIXED
mozilla63
Tracking Status
firefox63 --- fixed

People

(Reporter: bhackett, Assigned: bhackett)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

Attached patch patchSplinter Review
In some cases a Show message is sent to a content child instead of a LoadURL.  Both of these methods perform some graphics related initialization (LoadURL seems to implicitly contain a Show) and Show needs to be handled in the middleman alongside LoadURL.

Additionally, some child processes on mochitests receive a Show but never actually perform any painting, which causes problems for the record/replay system since we expect a paint to eventually occur, which creates the first checkpoint and finishes initialization of various components.  If this doesn't happen then the middleman will not be able to communicate with the child process, since the middleman cannot force a recording process to create a checkpoint, pause, and permit interaction before the first normal checkpoint spot has been reached.  With this patch, receiving a Show message will cause the recording process to create a checkpoint and avoid this situation.

Finally, this patch also cleans up and pares down the interfaces in ChildIPC.h, which includes several methods that are only used internally.
Attachment #8998073 - Flags: review?(continuation)
Comment on attachment 8998073 [details] [diff] [review]
patch

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

::: toolkit/recordreplay/ipc/ChildIPC.h
@@ -18,5 @@
>  namespace recordreplay {
>  namespace child {
>  
> -// Naively replaying a child process execution will not perform any IPC. When
> -// the replaying process attempts to make system calls that communicate with

What's the deal with the removal of this comment?

@@ +42,5 @@
> +// point in time. When viewing a normal tab this is no problem because the tab
> +// will be painted with the correct graphics momentarily, but when the tab can
> +// be rewound and paused this behavior is visible.
> +//
> +// This API is used to trigger artifical vsyncs whenever the page is updated.

nit: "artificial".
Attachment #8998073 - Flags: review?(continuation) → review+
(In reply to Andrew McCreight [:mccr8] from comment #1)
> Comment on attachment 8998073 [details] [diff] [review]
> patch
> 
> Review of attachment 8998073 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: toolkit/recordreplay/ipc/ChildIPC.h
> @@ -18,5 @@
> >  namespace recordreplay {
> >  namespace child {
> >  
> > -// Naively replaying a child process execution will not perform any IPC. When
> > -// the replaying process attempts to make system calls that communicate with
> 
> What's the deal with the removal of this comment?

This comment is out of date: the design it describes of communicating with the middleman via a PReplay IPDL protocol is no longer in use.  There is a longer and up to date comment in Channel.h that describes how recording/replaying processes perform IPC.
Pushed by bhackett@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/290b508c04ee
Fix some issues around PBrowser::Show message handling, tidy up web replay child headers, r=mccr8.
https://hg.mozilla.org/mozilla-central/rev/290b508c04ee
Status: NEW → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
You need to log in before you can comment on or make changes to this bug.