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 500509 7400b733b50f3a9d4fa8ca18363a9dbb52956398
parent 500508 b341213ecf95d1448abc0a38ef058138b41324ee
child 500510 72b97e539421a38d16e843783c2359381bc6ac33
push id1864
push userffxbld-merge
push dateMon, 03 Dec 2018 15:51:40 +0000
treeherdermozilla-release@f040763d99ad [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnical
bugs1499186
milestone64.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 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;