Bug 1556847 - Fix various record/replay bugs, r=loganfsmyth.

--HG--
extra : rebase_source : c4746f6baefb9cbdcbb89397807aba0060a2b369

devtools/server/actors/replay/control.js

@@ -367,15 +367,9 @@ function timeSinceCheckpoint(id) {
 // The checkpoint up to which the recording runs.
 let gLastFlushCheckpoint = InvalidCheckpointId;
 
-// The last saved checkpoint.
-let gLastSavedCheckpoint = FirstCheckpointId;
-
 // How often we want to flush the recording.
 const FlushMs = 0.5 * 1000;
 
-// How often we want to save a checkpoint.
-const SavedCheckpointMs = 0.25 * 1000;
-
 function addSavedCheckpoint(checkpoint) {
   if (getCheckpointInfo(checkpoint).owner) {
     return;
@@ -384,25 +378,14 @@ function addSavedCheckpoint(checkpoint) {
   const owner = pickReplayingChild();
   getCheckpointInfo(checkpoint).owner = owner;
   owner.addSavedCheckpoint(checkpoint);
-  gLastSavedCheckpoint = checkpoint;
 }
 
 function addCheckpoint(checkpoint, duration) {
   assert(!getCheckpointInfo(checkpoint).duration);
   getCheckpointInfo(checkpoint).duration = duration;
-
-  // Mark saved checkpoints as required, unless we haven't spawned any replaying
-  // children yet.
-  if (
-    timeSinceCheckpoint(gLastSavedCheckpoint) >= SavedCheckpointMs &&
-    gReplayingChildren.length > 0
-  ) {
-    addSavedCheckpoint(checkpoint + 1);
-  }
 }
 
 function ownerChild(checkpoint) {
-  assert(checkpoint <= gLastSavedCheckpoint);
   while (!getCheckpointInfo(checkpoint).owner) {
     checkpoint--;
   }
@@ -1006,8 +989,8 @@ function handleResumeManifestResponse({
     consoleMessages.forEach(msg => gDebugger.onConsoleMessage(msg));
   }
 
-  if (gDebugger && gDebugger.onNewScript) {
-    scripts.forEach(script => gDebugger.onNewScript(script));
+  if (gDebugger) {
+    scripts.forEach(script => gDebugger._onNewScript(script));
   }
 }
 
@@ -1071,8 +1054,8 @@ function ensureFlushed() {
     spawnReplayingChildren();
   }
 
-  // Checkpoints where the recording was flushed to disk are always saved.
-  // This allows the recording to be scanned as soon as it has been flushed.
+  // Checkpoints where the recording was flushed to disk are saved. This allows
+  // the recording to be scanned as soon as it has been flushed.
   addSavedCheckpoint(gLastFlushCheckpoint);
 
   // Flushing creates a new region of the recording for replaying children

devtools/server/actors/replay/debugger.js

@@ -324,7 +324,7 @@ ReplayDebugger.prototype = {
   replayPushThreadPause() {
     // The thread has paused so that the user can interact with it. The child
     // will stay paused until this thread-wide pause has been popped.
-    assert(this._paused);
+    this._ensurePaused();
     assert(!this._resumeCallback);
     if (++this._threadPauseCount == 1) {
       // There is no preferred direction of travel after an explicit pause.
@@ -363,7 +363,8 @@ ReplayDebugger.prototype = {
   },
 
   _performResume() {
-    assert(this._paused && !this._threadPauseCount);
+    this._ensurePaused();
+    assert(!this._threadPauseCount);
     if (this._resumeCallback && !this._threadPauseCount) {
       const callback = this._resumeCallback;
       this._invalidateAfterUnpause();
@@ -528,11 +529,11 @@ ReplayDebugger.prototype = {
     return data.map(script => this._addScript(script));
   },
 
-  findAllConsoleMessages() {
-    const messages = this._sendRequestMainChild({
-      type: "findConsoleMessages",
-    });
-    return messages.map(this._convertConsoleMessage.bind(this));
+  _onNewScript(data) {
+    if (this.onNewScript) {
+      const script = this._addScript(data);
+      this.onNewScript(script);
+    }
   },
 
   /////////////////////////////////////////////////////////
@@ -544,7 +545,9 @@ ReplayDebugger.prototype = {
     if (source) {
       return source;
     }
-    return this._addSource(this._sendRequest({ type: "getSource", id }));
+    return this._addSource(
+      this._sendRequestMainChild({ type: "getSource", id })
+    );
   },
 
   _addSource(data) {
@@ -692,6 +695,13 @@ ReplayDebugger.prototype = {
     return message;
   },
 
+  findAllConsoleMessages() {
+    const messages = this._sendRequestMainChild({
+      type: "findConsoleMessages",
+    });
+    return messages.map(this._convertConsoleMessage.bind(this));
+  },
+
   /////////////////////////////////////////////////////////
   // Handlers
   /////////////////////////////////////////////////////////

devtools/server/actors/replay/replay.js

@@ -810,6 +810,7 @@ const gManifestFinishedAfterCheckpointHandlers = {
   },
 
   runToPoint({ endpoint }, point) {
+    assert(endpoint.checkpoint >= point.checkpoint);
     if (!endpoint.position && point.checkpoint == endpoint.checkpoint) {
       RecordReplayControl.manifestFinished({ point });
     }
@@ -892,6 +893,7 @@ let gFrameStepsFrameIndex = 0;
 // This must be specified for any manifest that uses ensurePositionHandler.
 const gManifestPositionHandlers = {
   resume(manifest, point) {
+    clearPositionHandlers();
     RecordReplayControl.manifestFinished({
       point,
       consoleMessages: gNewConsoleMessages,
@@ -1249,6 +1251,7 @@ function getPauseData() {
     const names = getEnvironmentNames(env);
     rv.environments[id] = { data, names };
 
+    addObject(data.callee);
     addEnvironment(data.parent);
   }
 

toolkit/recordreplay/ProcessRedirectDarwin.cpp

@@ -340,10 +340,30 @@ static void MM_CFTypeOutputArg(MiddlemanCallContext& aCx) {
   MM_CFTypeOutput(aCx, arg, /* aOwnsReference = */ false);
 }
 
+static void SendMessageToObject(const void* aObject, const char* aMessage) {
+  CallArguments arguments;
+  arguments.Arg<0, const void*>() = aObject;
+  arguments.Arg<1, SEL>() = sel_registerName(aMessage);
+  RecordReplayInvokeCall(gOriginal_objc_msgSend, &arguments);
+}
+
 // For APIs whose result will be released by the middleman's autorelease pool.
 static void MM_AutoreleaseCFTypeRval(MiddlemanCallContext& aCx) {
   auto& rval = aCx.mArguments->Rval<const void*>();
   MM_SystemOutput(aCx, &rval);
+
+  if (rval) {
+    switch (aCx.mPhase) {
+      case MiddlemanCallPhase::MiddlemanOutput:
+        SendMessageToObject(rval, "retain");
+        break;
+      case MiddlemanCallPhase::MiddlemanRelease:
+        SendMessageToObject(rval, "autorelease");
+        break;
+      default:
+        break;
+    }
+  }
 }
 
 // For functions which have an input CFType value and also have side effects on

toolkit/recordreplay/ipc/ChildProcess.cpp

@@ -264,13 +264,17 @@ void ChildProcessInfo::WaitUntilPaused() {
 
   bool sentTerminateMessage = false;
   while (true) {
-    MonitorAutoLock lock(*gMonitor);
+    Maybe<MonitorAutoLock> lock;
+    lock.emplace(*gMonitor);
+
+    MaybeHandlePendingSyncMessage();
 
     // Search for the first message received from this process.
     ChildProcessInfo* process = this;
     Message::UniquePtr msg = ExtractChildMessage(&process);
 
     if (msg) {
+      lock.reset();
       OnIncomingMessage(*msg);
       if (IsPaused()) {
         return;

toolkit/recordreplay/ipc/ParentForwarding.cpp

@@ -260,89 +260,83 @@ class MiddlemanProtocol : public ipc::IToplevelProtocol {
     return MsgProcessed;
   }
 
-  static void ForwardMessageSync(MiddlemanProtocol* aProtocol,
-                                 Message* aMessage, Message** aReply) {
-    PrintSpew("ForwardSyncMsg %s\n",
-              IPC::StringFromIPCMessageType(aMessage->type()));
+  Message* mSyncMessage = nullptr;
+  Message* mSyncMessageReply = nullptr;
+  bool mSyncMessageIsCall = false;
 
-    MOZ_RELEASE_ASSERT(!*aReply);
-    Message* nReply = new Message();
-    if (!aProtocol->GetIPCChannel()->Send(aMessage, nReply)) {
-      MOZ_RELEASE_ASSERT(aProtocol->mSide == ipc::ParentSide);
+  void MaybeSendSyncMessage(bool aLockHeld) {
+    Maybe<MonitorAutoLock> lock;
+    if (!aLockHeld) {
+      lock.emplace(*gMonitor);
+    }
+
+    if (!mSyncMessage) {
+      return;
+    }
+
+    PrintSpew("ForwardSyncMsg %s\n",
+              IPC::StringFromIPCMessageType(mSyncMessage->type()));
+
+    MOZ_RELEASE_ASSERT(!mSyncMessageReply);
+    mSyncMessageReply = new Message();
+    if (mSyncMessageIsCall
+        ? !mOpposite->GetIPCChannel()->Call(mSyncMessage, mSyncMessageReply)
+        : !mOpposite->GetIPCChannel()->Send(mSyncMessage, mSyncMessageReply)) {
+      MOZ_RELEASE_ASSERT(mSide == ipc::ChildSide);
       BeginShutdown();
     }
 
-    MonitorAutoLock lock(*gMonitor);
-    *aReply = nReply;
-    gMonitor->Notify();
+    mSyncMessage = nullptr;
+
+    gMonitor->NotifyAll();
+  }
+
+  static void StaticMaybeSendSyncMessage(MiddlemanProtocol* aProtocol) {
+    aProtocol->MaybeSendSyncMessage(false);
+  }
+
+  void HandleSyncMessage(const Message& aMessage, Message*& aReply, bool aCall) {
+    MOZ_RELEASE_ASSERT(mOppositeMessageLoop);
+
+    mSyncMessage = new Message();
+    mSyncMessage->CopyFrom(aMessage);
+    mSyncMessageIsCall = aCall;
+
+    mOppositeMessageLoop->PostTask(
+        NewRunnableFunction("StaticMaybeSendSyncMessage", StaticMaybeSendSyncMessage, this));
+
+    if (mSide == ipc::ChildSide) {
+      AutoMarkMainThreadWaitingForIPDLReply blocked;
+      while (!mSyncMessageReply) {
+        MOZ_CRASH("NYI");
+      }
+    } else {
+      MonitorAutoLock lock(*gMonitor);
+
+      // If the main thread is blocked waiting for the recording child to pause,
+      // wake it up so it can call MaybeHandlePendingSyncMessage().
+      gMonitor->NotifyAll();
+
+      while (!mSyncMessageReply) {
+        gMonitor->Wait();
+      }
+    }
+
+    aReply = mSyncMessageReply;
+    mSyncMessageReply = nullptr;
+
+    PrintSpew("SyncMsgDone\n");
   }
 
   virtual Result OnMessageReceived(const Message& aMessage,
                                    Message*& aReply) override {
-    MOZ_RELEASE_ASSERT(mOppositeMessageLoop);
-
-    Message* nMessage = new Message();
-    nMessage->CopyFrom(aMessage);
-    mOppositeMessageLoop->PostTask(
-        NewRunnableFunction("ForwardMessageSync", ForwardMessageSync, mOpposite,
-                            nMessage, &aReply));
-
-    if (mSide == ipc::ChildSide) {
-      AutoMarkMainThreadWaitingForIPDLReply blocked;
-      while (!aReply) {
-        MOZ_CRASH("NYI");
-      }
-    } else {
-      MonitorAutoLock lock(*gMonitor);
-      while (!aReply) {
-        gMonitor->Wait();
-      }
-    }
-
-    PrintSpew("SyncMsgDone\n");
+    HandleSyncMessage(aMessage, aReply, false);
     return MsgProcessed;
   }
 
-  static void ForwardCallMessage(MiddlemanProtocol* aProtocol,
-                                 Message* aMessage, Message** aReply) {
-    PrintSpew("ForwardSyncCall %s\n",
-              IPC::StringFromIPCMessageType(aMessage->type()));
-
-    MOZ_RELEASE_ASSERT(!*aReply);
-    Message* nReply = new Message();
-    if (!aProtocol->GetIPCChannel()->Call(aMessage, nReply)) {
-      MOZ_RELEASE_ASSERT(aProtocol->mSide == ipc::ParentSide);
-      BeginShutdown();
-    }
-
-    MonitorAutoLock lock(*gMonitor);
-    *aReply = nReply;
-    gMonitor->Notify();
-  }
-
   virtual Result OnCallReceived(const Message& aMessage,
                                 Message*& aReply) override {
-    MOZ_RELEASE_ASSERT(mOppositeMessageLoop);
-
-    Message* nMessage = new Message();
-    nMessage->CopyFrom(aMessage);
-    mOppositeMessageLoop->PostTask(
-        NewRunnableFunction("ForwardCallMessage", ForwardCallMessage, mOpposite,
-                            nMessage, &aReply));
-
-    if (mSide == ipc::ChildSide) {
-      AutoMarkMainThreadWaitingForIPDLReply blocked;
-      while (!aReply) {
-        MOZ_CRASH("NYI");
-      }
-    } else {
-      MonitorAutoLock lock(*gMonitor);
-      while (!aReply) {
-        gMonitor->Wait();
-      }
-    }
-
-    PrintSpew("SyncCallDone\n");
+    HandleSyncMessage(aMessage, aReply, true);
     return MsgProcessed;
   }
 
@@ -357,6 +351,12 @@ class MiddlemanProtocol : public ipc::IToplevelProtocol {
 static MiddlemanProtocol* gChildProtocol;
 static MiddlemanProtocol* gParentProtocol;
 
+void MaybeHandlePendingSyncMessage() {
+  if (gParentProtocol) {
+    gParentProtocol->MaybeSendSyncMessage(true);
+  }
+}
+
 ipc::MessageChannel* ChannelToUIProcess() {
   return gChildProtocol->GetIPCChannel();
 }

toolkit/recordreplay/ipc/ParentInternal.h

@@ -51,6 +51,11 @@ bool MainThreadIsWaitingForIPDLReply();
 // to block while waiting on an IPDL reply from the child.
 void ResumeBeforeWaitingForIPDLReply();
 
+// Immediately forward any sync child->parent IPDL message. These are sent on
+// the main thread, which might be blocked waiting for a response from the
+// recording child and unable to run an event loop.
+void MaybeHandlePendingSyncMessage();
+
 // Initialize state which handles incoming IPDL messages from the UI and
 // recording child processes.
 void InitializeForwarding();