Bug 1499186 Part 3 - Don't show out of order graphics in the middleman, r=nical.
authorBrian Hackett <bhackett1024@gmail.com>
Wed, 17 Oct 2018 10:43:32 -0600
changeset 490287 7400b733b50f3a9d4fa8ca18363a9dbb52956398
parent 490286 b341213ecf95d1448abc0a38ef058138b41324ee
child 490338 72b97e539421a38d16e843783c2359381bc6ac33
push id247
push userfmarier@mozilla.com
push dateSat, 27 Oct 2018 01:06:44 +0000
reviewersnical
bugs1499186
milestone64.0a1
Bug 1499186 Part 3 - Don't show out of order graphics in the middleman, r=nical.
toolkit/recordreplay/ipc/ParentGraphics.cpp
toolkit/recordreplay/ipc/ParentIPC.cpp
toolkit/recordreplay/ipc/ParentInternal.h
--- a/toolkit/recordreplay/ipc/ParentGraphics.cpp
+++ b/toolkit/recordreplay/ipc/ParentGraphics.cpp
@@ -64,18 +64,16 @@ SendGraphicsMemoryToChild()
   MachSendMessage message(GraphicsMemoryMessageId);
   message.AddDescriptor(MachMsgPortDescriptor(gGraphicsPort, MACH_MSG_TYPE_COPY_SEND));
 
   MachPortSender sender(childPort);
   kr = sender.SendMessage(message, 1000);
   MOZ_RELEASE_ASSERT(kr == KERN_SUCCESS);
 }
 
-static Maybe<PaintMessage> gLastPaint;
-
 // Global object for the sandbox used to paint graphics data in this process.
 static JS::PersistentRootedObject* gGraphicsSandbox;
 
 static void
 InitGraphicsSandbox()
 {
   MOZ_RELEASE_ASSERT(!gGraphicsSandbox);
 
@@ -104,39 +102,67 @@ InitGraphicsSandbox()
   dom::ChromeUtils::Import(global, NS_LITERAL_STRING("resource://devtools/server/actors/replay/graphics.js"),
                            dom::Optional<HandleObject>(), &obj, er);
   MOZ_RELEASE_ASSERT(!er.Failed());
 }
 
 // Buffer used to transform graphics memory, if necessary.
 static void* gBufferMemory;
 
+// The dimensions of the data in the graphics shmem buffer.
+static size_t gLastPaintWidth, gLastPaintHeight;
+
+// Explicit Paint messages received from the child need to be handled with
+// care to make sure we show correct graphics. Each Paint message is for the
+// the process state at the most recent checkpoint in the past. When running
+// (forwards or backwards) between the checkpoint and the Paint message,
+// we could pause at a breakpoint and repaint the graphics at that point,
+// reflecting the process state at a point later than at the checkpoint.
+// In this case the Paint message's graphics will be stale. To avoid showing
+// its graphics, we wait until both the Paint and the checkpoint itself have
+// been hit, with no intervening repaint.
+
+// The last explicit paint message received from the child, if there has not
+// been an intervening repaint.
+static UniquePtr<PaintMessage> gLastExplicitPaint;
+
+// The last checkpoint the child reached, if there has not been an intervening
+// repaint.
+static size_t gLastCheckpoint;
+
 void
 UpdateGraphicsInUIProcess(const PaintMessage* aMsg)
 {
   MOZ_RELEASE_ASSERT(NS_IsMainThread());
 
   if (aMsg) {
-    gLastPaint = Some(*aMsg);
-  } else if (!gLastPaint.isSome()) {
+    gLastPaintWidth = aMsg->mWidth;
+    gLastPaintHeight = aMsg->mHeight;
+  }
+
+  if (!gLastPaintWidth || !gLastPaintHeight) {
     return;
   }
 
   bool hadFailure = !aMsg;
 
+  // Clear out the last explicit paint information. This may delete aMsg.
+  gLastExplicitPaint = nullptr;
+  gLastCheckpoint = CheckpointId::Invalid;
+
   // Make sure there is a sandbox which is running the graphics JS module.
   if (!gGraphicsSandbox) {
     InitGraphicsSandbox();
   }
 
   AutoSafeJSContext cx;
   JSAutoRealm ar(cx, *gGraphicsSandbox);
 
-  size_t width = gLastPaint.ref().mWidth;
-  size_t height = gLastPaint.ref().mHeight;
+  size_t width = gLastPaintWidth;
+  size_t height = gLastPaintHeight;
   size_t stride = layers::ImageDataSerializer::ComputeRGBStride(gSurfaceFormat, width);
 
   // Make sure the width and height are appropriately sized.
   CheckedInt<size_t> scaledWidth = CheckedInt<size_t>(width) * 4;
   CheckedInt<size_t> scaledHeight = CheckedInt<size_t>(height) * stride;
   MOZ_RELEASE_ASSERT(scaledWidth.isValid() && scaledWidth.value() <= stride);
   MOZ_RELEASE_ASSERT(scaledHeight.isValid() && scaledHeight.value() <= GraphicsMemorySize);
 
@@ -170,16 +196,38 @@ UpdateGraphicsInUIProcess(const PaintMes
 
   // Call into the graphics module to update the canvas it manages.
   RootedValue rval(cx);
   if (!JS_CallFunctionName(cx, *gGraphicsSandbox, "Update", args, &rval)) {
     MOZ_CRASH("UpdateGraphicsInUIProcess");
   }
 }
 
+static void
+MaybeTriggerExplicitPaint()
+{
+  if (gLastExplicitPaint && gLastExplicitPaint->mCheckpointId == gLastCheckpoint) {
+    UpdateGraphicsInUIProcess(gLastExplicitPaint.get());
+  }
+}
+
+void
+MaybeUpdateGraphicsAtPaint(const PaintMessage& aMsg)
+{
+  gLastExplicitPaint.reset(new PaintMessage(aMsg));
+  MaybeTriggerExplicitPaint();
+}
+
+void
+MaybeUpdateGraphicsAtCheckpoint(size_t aCheckpointId)
+{
+  gLastCheckpoint = aCheckpointId;
+  MaybeTriggerExplicitPaint();
+}
+
 bool
 InRepaintStressMode()
 {
   static bool checked = false;
   static bool rv;
   if (!checked) {
     AutoEnsurePassThroughThreadEvents pt;
     rv = TestEnv("MOZ_RECORD_REPLAY_REPAINT_STRESS");
--- a/toolkit/recordreplay/ipc/ParentIPC.cpp
+++ b/toolkit/recordreplay/ipc/ParentIPC.cpp
@@ -246,17 +246,17 @@ public:
     if (mProcess->LastCheckpoint() == CheckpointId::Invalid) {
       mProcess->SendMessage(ResumeMessage(/* aForward = */ true));
     }
   }
 
   void OnIncomingMessage(const Message& aMsg) override {
     switch (aMsg.mType) {
     case MessageType::Paint:
-      UpdateGraphicsInUIProcess((const PaintMessage*) &aMsg);
+      MaybeUpdateGraphicsAtPaint((const PaintMessage&) aMsg);
       break;
     case MessageType::HitCheckpoint:
       RecvHitCheckpoint((const HitCheckpointMessage&) aMsg);
       break;
     case MessageType::HitBreakpoint:
       RecvHitBreakpoint((const HitBreakpointMessage&) aMsg);
       break;
     case MessageType::DebuggerResponse:
@@ -997,17 +997,17 @@ MaybeSendRepaintMessage()
     AutoSafeJSContext cx;
     JS::RootedValue value(cx);
     if (JS_ParseJSON(cx, response.begin(), response.length(), &value)) {
       MOZ_RELEASE_ASSERT(value.isObject());
       JS::RootedObject obj(cx, &value.toObject());
       RootedValue width(cx), height(cx);
       if (JS_GetProperty(cx, obj, "width", &width) && width.isNumber() && width.toNumber() &&
           JS_GetProperty(cx, obj, "height", &height) && height.isNumber() && height.toNumber()) {
-        PaintMessage message(width.toNumber(), height.toNumber());
+        PaintMessage message(CheckpointId::Invalid, width.toNumber(), height.toNumber());
         UpdateGraphicsInUIProcess(&message);
       }
     }
   }
 }
 
 void
 Resume(bool aForward)
@@ -1161,16 +1161,17 @@ ResumeBeforeWaitingForIPDLReply()
     Resume(true);
   }
 }
 
 static void
 RecvHitCheckpoint(const HitCheckpointMessage& aMsg)
 {
   UpdateCheckpointTimes(aMsg);
+  MaybeUpdateGraphicsAtCheckpoint(aMsg.mCheckpointId);
 
   // Resume either forwards or backwards. Break the resume off into a separate
   // runnable, to avoid starving any code already on the stack and waiting for
   // the process to pause. Immediately resume if the main thread is blocked.
   if (MainThreadIsWaitingForIPDLReply()) {
     MOZ_RELEASE_ASSERT(gChildExecuteForward);
     Resume(true);
   } else if (!gResumeForwardOrBackward) {
--- a/toolkit/recordreplay/ipc/ParentInternal.h
+++ b/toolkit/recordreplay/ipc/ParentInternal.h
@@ -85,16 +85,21 @@ extern void* gGraphicsMemory;
 void InitializeGraphicsMemory();
 void SendGraphicsMemoryToChild();
 
 // Update the graphics painted in the UI process, per painting data received
 // from a child process, or null if a repaint was triggered and failed due to
 // an unhandled recording divergence.
 void UpdateGraphicsInUIProcess(const PaintMessage* aMsg);
 
+// If necessary, update graphics after the active child sends a paint message
+// or reaches a checkpoint.
+void MaybeUpdateGraphicsAtPaint(const PaintMessage& aMsg);
+void MaybeUpdateGraphicsAtCheckpoint(size_t aCheckpointId);
+
 // ID for the mach message sent from a child process to the middleman to
 // request a port for the graphics shmem.
 static const int32_t GraphicsHandshakeMessageId = 42;
 
 // ID for the mach message sent from the middleman to a child process with the
 // requested memory for.
 static const int32_t GraphicsMemoryMessageId = 43;