Bug 1718333 - Remove unnecessary AssertLinkThread assertions, r=handyman a=pascalc

After looking through the methods which have this assertion, I couldn't
find any examples of places where not having a specific "link thread"
sequence would cause any issues. I think these assertions can and should
be removed.

The main change required by this was to remove the `!NS_IsMainThread()`
assertion from the SchedulerGroup listener. Due to how callbacks work,
it would be possible for a vsync message to be detected by a
MessageChannel from the main thread if it was sent before the channel
was bound. I don't believe that this change should cause any issues.

Differential Revision: https://phabricator.services.mozilla.com/D119348

ipc/glue/MessageChannel.cpp

@@ -709,8 +709,7 @@ bool MessageChannel::CanSend() const {
 }
 
 void MessageChannel::Clear() {
-  // Don't clear mWorkerThread; we use it in AssertLinkThread() and
+  // Don't clear mWorkerThread; we use it in AssertWorkerThread().
-  // AssertWorkerThread().
   //
   // Also don't clear mListener.  If we clear it, then sending a message
   // through this channel after it's Clear()'ed can cause this process to
@@ -1042,7 +1041,6 @@ class CancelMessage : public IPC::Message {
 };
 
 bool MessageChannel::MaybeInterceptSpecialIOMessage(const Message& aMsg) {
-  AssertLinkThread();
   mMonitor->AssertCurrentThreadOwns();
 
   if (MSG_ROUTING_NONE == aMsg.routing_id()) {
@@ -1124,10 +1122,11 @@ bool MessageChannel::ShouldDeferMessage(const Message& aMsg) {
 }
 
 void MessageChannel::OnMessageReceivedFromLink(Message&& aMsg) {
-  AssertLinkThread();
   mMonitor->AssertCurrentThreadOwns();
 
-  if (MaybeInterceptSpecialIOMessage(aMsg)) return;
+  if (MaybeInterceptSpecialIOMessage(aMsg)) {
+    return;
+  }
 
   mListener->OnChannelReceivedMessage(aMsg);
 
@@ -1204,8 +1203,8 @@ void MessageChannel::OnMessageReceivedFromLink(Message&& aMsg) {
   // before returning.
   bool shouldPostTask = !shouldWakeUp || wakeUpSyncSend;
 
-  IPC_LOG("Receive on link thread; seqno=%d, xid=%d, shouldWakeUp=%d",
+  IPC_LOG("Receive from link; seqno=%d, xid=%d, shouldWakeUp=%d", aMsg.seqno(),
-          aMsg.seqno(), aMsg.transaction_id(), shouldWakeUp);
+          aMsg.transaction_id(), shouldWakeUp);
 
   if (reuseTask) {
     return;
@@ -2257,7 +2256,7 @@ bool MessageChannel::WaitResponse(bool aWaitTimedOut) {
 #ifndef OS_WIN
 bool MessageChannel::WaitForSyncNotify(bool /* aHandleWindowsMessages */) {
 #  ifdef DEBUG
-  // WARNING: We don't release the lock here. We can't because the link thread
+  // WARNING: We don't release the lock here. We can't because the link
   // could signal at this time and we would miss it. Instead we require
   // ArtificialTimeout() to be extremely simple.
   if (mListener->ArtificialTimeout()) {
@@ -2425,7 +2424,6 @@ bool MessageChannel::MaybeHandleError(Result code, const Message& aMsg,
 }
 
 void MessageChannel::OnChannelErrorFromLink() {
-  AssertLinkThread();
   mMonitor->AssertCurrentThreadOwns();
 
   IPC_LOG("OnChannelErrorFromLink");

ipc/glue/MessageChannel.h

@@ -553,24 +553,6 @@ class MessageChannel : HasResultCodes {
                        "not on worker thread!");
   }
 
-  // The "link" thread is either the I/O thread (ProcessLink), the other
-  // actor's work thread (ThreadLink), or the worker thread (same-thread
-  // channels).
-  void AssertLinkThread() const {
-    if (mIsSameThreadChannel) {
-      // If we're a same-thread channel, we have to be on our worker
-      // thread.
-      AssertWorkerThread();
-      return;
-    }
-
-    // If we aren't a same-thread channel, our "link" thread is _not_ our
-    // worker thread!
-    MOZ_ASSERT(mWorkerThread, "Channel hasn't been opened yet");
-    MOZ_RELEASE_ASSERT(mWorkerThread && !mWorkerThread->IsOnCurrentThread(),
-                       "on worker thread but should not be!");
-  }
-
  private:
   class MessageTask : public CancelableRunnable,
                       public LinkedListElement<RefPtr<MessageTask>>,

xpcom/threads/SchedulerGroup.cpp

@@ -38,20 +38,21 @@ nsresult SchedulerGroup::UnlabeledDispatch(
 
 /* static */
 void SchedulerGroup::MarkVsyncReceived() {
-  if (gEarliestUnprocessedVsync) {
+  // May be called on any thread when a vsync is received and scheduled to be
-    // If we've seen a vsync already, but haven't handled it, keep the
+  // processed. This may occur on the main thread due to queued messages when
-    // older one.
+  // the channel is connected.
-    return;
-  }
-
-  MOZ_ASSERT(!NS_IsMainThread());
   bool inconsistent = false;
   TimeStamp creation = TimeStamp::ProcessCreation(&inconsistent);
   if (inconsistent) {
     return;
   }
 
-  gEarliestUnprocessedVsync = (TimeStamp::Now() - creation).ToMicroseconds();
+  // Attempt to set gEarliestUnprocessedVsync to our new value. If we've seen a
+  // vsync already, but haven't handled it, the `compareExchange` will fail and
+  // the static won't be updated.
+  uint64_t unprocessedVsync =
+      uint64_t((TimeStamp::Now() - creation).ToMicroseconds());
+  gEarliestUnprocessedVsync.compareExchange(0, unprocessedVsync);
 }
 
 /* static */