Bug 1481351 - Fix some issues around PBrowser::Show message handling, tidy up web replay child headers, r=mccr8.
authorBrian Hackett <bhackett1024@gmail.com>
Tue, 14 Aug 2018 00:40:15 +0000
changeset 431435 290b508c04ee871a1582f4ede10ed1d6ed2cc8dd
parent 431434 3352ce18917b2c7d210cc09aa1b610c2ca775580
child 431436 dc70bcd78ca84c628ae0077bc0f9084d3db609dc
push id34443
push usercsabou@mozilla.com
push dateWed, 15 Aug 2018 00:53:32 +0000
treeherdermozilla-central@b80906e2fbc9 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmccr8
bugs1481351
milestone63.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1481351 - Fix some issues around PBrowser::Show message handling, tidy up web replay child headers, r=mccr8.
dom/ipc/TabChild.cpp
toolkit/recordreplay/Callback.cpp
toolkit/recordreplay/ipc/ChildIPC.cpp
toolkit/recordreplay/ipc/ChildIPC.h
toolkit/recordreplay/ipc/ChildInternal.h
toolkit/recordreplay/ipc/DisabledIPC.cpp
toolkit/recordreplay/ipc/ParentForwarding.cpp
--- a/dom/ipc/TabChild.cpp
+++ b/dom/ipc/TabChild.cpp
@@ -1255,16 +1255,24 @@ TabChild::RecvShow(const ScreenIntSize& 
   }
 
   ApplyShowInfo(aInfo);
   RecvParentActivated(aParentIsActive);
 
   if (!res) {
     return IPC_FAIL_NO_REASON(this);
   }
+
+  // We have now done enough initialization for the record/replay system to
+  // create checkpoints. Try to create the initial checkpoint now, in case this
+  // process never paints later on (the usual place where checkpoints occur).
+  if (recordreplay::IsRecordingOrReplaying()) {
+    recordreplay::child::MaybeCreateInitialCheckpoint();
+  }
+
   return IPC_OK();
 }
 
 mozilla::ipc::IPCResult
 TabChild::RecvInitRendering(const TextureFactoryIdentifier& aTextureFactoryIdentifier,
                             const layers::LayersId& aLayersId,
                             const CompositorOptions& aCompositorOptions,
                             const bool& aLayersConnected,
--- a/toolkit/recordreplay/Callback.cpp
+++ b/toolkit/recordreplay/Callback.cpp
@@ -1,17 +1,17 @@
 /* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
 /* vim: set ts=8 sts=2 et sw=2 tw=80: */
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "Callback.h"
 
-#include "ipc/ChildIPC.h"
+#include "ipc/ChildInternal.h"
 #include "mozilla/Assertions.h"
 #include "mozilla/RecordReplay.h"
 #include "mozilla/StaticMutex.h"
 #include "ProcessRewind.h"
 #include "Thread.h"
 #include "ValueIndex.h"
 
 namespace mozilla {
--- a/toolkit/recordreplay/ipc/ChildIPC.cpp
+++ b/toolkit/recordreplay/ipc/ChildIPC.cpp
@@ -304,16 +304,22 @@ MiddlemanProcessId()
 
 base::ProcessId
 ParentProcessId()
 {
   return gParentPid;
 }
 
 void
+MaybeCreateInitialCheckpoint()
+{
+  NewCheckpoint(/* aTemporary = */ false);
+}
+
+void
 ReportFatalError(const Maybe<MinidumpInfo>& aMinidump, const char* aFormat, ...)
 {
   // Unprotect any memory which might be written while producing the minidump.
   UnrecoverableSnapshotFailure();
 
   AutoEnsurePassThroughThreadEvents pt;
 
 #ifdef MOZ_CRASHREPORTER
--- a/toolkit/recordreplay/ipc/ChildIPC.h
+++ b/toolkit/recordreplay/ipc/ChildIPC.h
@@ -13,59 +13,60 @@
 
 namespace mozilla {
 
 class VsyncObserver;
 
 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
-// the parent, function redirections are invoked that simply replay the values
-// which those calls produced in the original recording.
-//
-// The replayed process needs to be able to communicate with the parent in some
-// ways, however. IPDL messages need to be sent to the compositor in the parent
-// to render graphics, and the parent needs to send messages to the client to
-// control and debug the replay.
-//
-// This file manages the real IPC which occurs in a replaying process. New
-// threads --- which did not existing while recording --- are spawned to manage
-// IPC with the middleman process, and IPDL actors are created up front for use
-// in communicating with the middleman using the PReplay protocol.
-
-///////////////////////////////////////////////////////////////////////////////
-// Public API
-///////////////////////////////////////////////////////////////////////////////
+// This file has the public API for definitions used in facilitating IPC
+// between a child recording/replaying process and the middleman process.
 
 // Initialize replaying IPC state. This is called once during process startup,
 // and is a no-op if the process is not recording/replaying.
 void InitRecordingOrReplayingProcess(int* aArgc, char*** aArgv);
 
+// Get the IDs of the middleman and parent processes.
 base::ProcessId MiddlemanProcessId();
 base::ProcessId ParentProcessId();
 
+// Create a normal checkpoint, if no such checkpoint has been created yet.
+void MaybeCreateInitialCheckpoint();
+
+///////////////////////////////////////////////////////////////////////////////
+// Painting Coordination
+///////////////////////////////////////////////////////////////////////////////
+
+// In child processes, paints do not occur in response to vsyncs from the UI
+// process: when a page is updating rapidly these events occur sporadically and
+// cause the tab's graphics to not accurately reflect the tab's state at that
+// 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 artificial vsyncs whenever the page is updated.
+// SetVsyncObserver is used to tell the child code about any singleton vsync
+// observer that currently exists, and NotifyVsyncObserver is used to trigger
+// a vsync on that observer at predictable points, e.g. the top of the main
+// thread's event loop.
 void SetVsyncObserver(VsyncObserver* aObserver);
 void NotifyVsyncObserver();
 
-void NotifyPaint();
+// Similarly to the vsync handling above, in order to ensure that the tab's
+// graphics accurately reflect its state, we want to perform paints
+// synchronously after a vsync has occurred. When a paint is about to happen,
+// the main thread calls NotifyPaintStart, and after the compositor thread has
+// been informed about the update the main thread calls WaitForPaintToComplete
+// to block until the compositor thread has finished painting and called
+// NotifyPaintComplete.
 void NotifyPaintStart();
+void WaitForPaintToComplete();
 void NotifyPaintComplete();
-void WaitForPaintToComplete();
-
-already_AddRefed<gfx::DrawTarget> DrawTargetForRemoteDrawing(LayoutDeviceIntSize aSize);
 
-// Notify the middleman that the recording was flushed.
-void NotifyFlushedRecording();
-
-// Notify the middleman about an AlwaysMarkMajorCheckpoints directive.
-void NotifyAlwaysMarkMajorCheckpoints();
-
-// Mark a time span when the main thread is idle.
-void BeginIdleTime();
-void EndIdleTime();
+// Get a draw target which the compositor thread can paint to.
+already_AddRefed<gfx::DrawTarget> DrawTargetForRemoteDrawing(LayoutDeviceIntSize aSize);
 
 } // namespace child
 } // namespace recordreplay
 } // namespace mozilla
 
 #endif // mozilla_recordreplay_ChildIPC_h
--- a/toolkit/recordreplay/ipc/ChildInternal.h
+++ b/toolkit/recordreplay/ipc/ChildInternal.h
@@ -9,16 +9,19 @@
 
 #include "ChildIPC.h"
 #include "JSControl.h"
 #include "Monitor.h"
 
 namespace mozilla {
 namespace recordreplay {
 
+// This file has internal definitions for communication between the main
+// record/replay infrastructure and child side IPC code.
+
 // The navigation namespace has definitions for managing breakpoints and all
 // other state that persists across rewinds, and for keeping track of the
 // precise execution position of the child process. The middleman will send the
 // child process Resume messages to travel forward and backward, but it is up
 // to the child process to keep track of the rewinding and resuming necessary
 // to find the next or previous point where a breakpoint or checkpoint is hit.
 namespace navigation {
 
@@ -64,23 +67,21 @@ js::ExecutionPoint TimeWarpTargetExecuti
 void BeforeCheckpoint();
 
 // Called immediately after hitting a normal or temporary checkpoint, either
 // when running forward or immediately after rewinding.
 void AfterCheckpoint(const CheckpointId& aCheckpoint);
 
 } // namespace navigation
 
-// IPC activity that can be triggered by navigation.
 namespace child {
 
+// IPC activity that can be triggered by navigation.
 void RespondToRequest(const js::CharBuffer& aBuffer);
-
 void HitCheckpoint(size_t aId, bool aRecordingEndpoint);
-
 void HitBreakpoint(bool aRecordingEndpoint, const uint32_t* aBreakpoints, size_t aNumBreakpoints);
 
 // Optional information about a crash that occurred. If not provided to
 // ReportFatalError, the current thread will be treated as crashed.
 struct MinidumpInfo
 {
   int mExceptionType;
   int mCode;
@@ -94,14 +95,27 @@ struct MinidumpInfo
 
 // Generate a minidump and report a fatal error to the middleman process.
 void ReportFatalError(const Maybe<MinidumpInfo>& aMinidumpInfo,
                       const char* aFormat, ...);
 
 // Monitor used for various synchronization tasks.
 extern Monitor* gMonitor;
 
+// Notify the middleman that the recording was flushed.
+void NotifyFlushedRecording();
+
+// Notify the middleman about an AlwaysMarkMajorCheckpoints directive.
+void NotifyAlwaysMarkMajorCheckpoints();
+
+// Report a fatal error to the middleman process.
+void ReportFatalError(const char* aFormat, ...);
+
+// Mark a time span when the main thread is idle.
+void BeginIdleTime();
+void EndIdleTime();
+
 } // namespace child
 
 } // namespace recordreplay
 } // namespace mozilla
 
 #endif // mozilla_recordreplay_ChildInternal_h
--- a/toolkit/recordreplay/ipc/DisabledIPC.cpp
+++ b/toolkit/recordreplay/ipc/DisabledIPC.cpp
@@ -13,16 +13,18 @@
 namespace mozilla {
 namespace recordreplay {
 
 namespace child {
 
 void
 InitRecordingOrReplayingProcess(int* aArgc, char*** aArgv)
 {
+  // This is called from all child processes, and is a no-op if not
+  // recording or replaying.
 }
 
 char*
 PrefsShmemContents(size_t aPrefsLen)
 {
   MOZ_CRASH();
 }
 
@@ -33,35 +35,34 @@ MiddlemanProcessId()
 }
 
 base::ProcessId
 ParentProcessId()
 {
   MOZ_CRASH();
 }
 
+void MaybeCreateInitialCheckpoint()
+{
+  MOZ_CRASH();
+}
+
 void
 SetVsyncObserver(VsyncObserver* aObserver)
 {
   MOZ_CRASH();
 }
 
 void
 NotifyVsyncObserver()
 {
   MOZ_CRASH();
 }
 
 void
-NotifyPaint()
-{
-  MOZ_CRASH();
-}
-
-void
 NotifyPaintStart()
 {
   MOZ_CRASH();
 }
 
 void
 NotifyPaintComplete()
 {
@@ -75,52 +76,32 @@ WaitForPaintToComplete()
 }
 
 already_AddRefed<gfx::DrawTarget>
 DrawTargetForRemoteDrawing(LayoutDeviceIntSize aSize)
 {
   MOZ_CRASH();
 }
 
-void
-NotifyFlushedRecording()
-{
-  MOZ_CRASH();
-}
-
-void
-NotifyAlwaysMarkMajorCheckpoints()
-{
-  MOZ_CRASH();
-}
-
-void
-BeginIdleTime()
-{
-  MOZ_CRASH();
-}
-
-void
-EndIdleTime()
-{
-  MOZ_CRASH();
-}
-
 } // namespace child
 
 namespace parent {
 
 void
 InitializeUIProcess(int aArgc, char** aArgv)
 {
+  // This is called from UI processes, and has no state to initialize if
+  // recording/replaying is disabled on this platform.
 }
 
 const char*
 SaveAllRecordingsDirectory()
 {
+  // This is called from UI processes, and recordings are never saved if
+  // recording/replaying is disabled on this platform.
   return nullptr;
 }
 
 void
 SaveRecording(const ipc::FileDescriptor& aFile)
 {
   MOZ_CRASH();
 }
--- a/toolkit/recordreplay/ipc/ParentForwarding.cpp
+++ b/toolkit/recordreplay/ipc/ParentForwarding.cpp
@@ -102,18 +102,19 @@ HandleMessageInMiddleman(ipc::Side aSide
       type == dom::PContent::Msg_SetXPCOMProcessAttributes__ID ||
       type == dom::PContent::Msg_SetProcessSandbox__ID ||
       // Graphics messages that affect both processes.
       type == dom::PBrowser::Msg_InitRendering__ID ||
       type == dom::PBrowser::Msg_SetDocShellIsActive__ID ||
       type == dom::PBrowser::Msg_PRenderFrameConstructor__ID ||
       type == dom::PBrowser::Msg_RenderLayers__ID ||
       type == dom::PBrowser::Msg_UpdateDimensions__ID ||
-      // This message performs some graphics related initialization.
+      // These messages perform some graphics related initialization.
       type == dom::PBrowser::Msg_LoadURL__ID ||
+      type == dom::PBrowser::Msg_Show__ID ||
       // May be loading devtools code that runs in the middleman process.
       type == dom::PBrowser::Msg_LoadRemoteScript__ID ||
       // May be sending a message for receipt by devtools code.
       type == dom::PBrowser::Msg_AsyncMessage__ID ||
       // Teardown that must be performed in both processes.
       type == dom::PBrowser::Msg_Destroy__ID) {
     ipc::IProtocol::Result r =
       dom::ContentChild::GetSingleton()->PContentChild::OnMessageReceived(aMessage);