Backed out 10 changesets (bug 1242097, bug 1240985) for near-permafailing in test_plugin_focus.html
authorWes Kocher <wkocher@mozilla.com>
Tue, 26 Jan 2016 09:55:59 -0800
changeset 281762 928befa33041ff093b817b912dfcff26215bece6
parent 281761 f8be949a739785785a1b09a69ac12c1b6ece43db
child 281763 f920df4d107f5e4933c5e4025e1033aa0d1617b0
push id29948
push usercbook@mozilla.com
push dateWed, 27 Jan 2016 11:00:24 +0000
treeherdermozilla-central@211a4c710fb6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1242097, 1240985
milestone47.0a1
backs out30f1acd9387f270279d71da436bba1d0aedcc617
6d7f80a057f0b19d1c050300b8d3f254b54b1ef3
c3dfc2c674ff15da1041f9769a3c51867ed9a9bc
b7f2ce03b34ea7cc87fc37b22e23f90071b01a28
2c0c0ed35656c130f1a99270e1d60cbcbc437bda
85c8cb422bada79f0caf5c881225f3919e073f6d
06b4cb890ab0c8902cdca225a8e028e960d47f61
f10c0445c4506f1efb41b729fcc3edb2b903076d
62eaf89ab82fbd509041026ac34b6a7e572bb176
b94c1a472d4654e801c2de90e3119ebdf776b50a
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
Backed out 10 changesets (bug 1242097, bug 1240985) for near-permafailing in test_plugin_focus.html Backed out changeset 30f1acd9387f (bug 1240985) Backed out changeset 6d7f80a057f0 (bug 1240985) Backed out changeset c3dfc2c674ff (bug 1240985) Backed out changeset b7f2ce03b34e (bug 1240985) Backed out changeset 2c0c0ed35656 (bug 1240985) Backed out changeset 85c8cb422bad (bug 1240985) Backed out changeset 06b4cb890ab0 (bug 1240985) Backed out changeset f10c0445c450 (bug 1240985) Backed out changeset 62eaf89ab82f (bug 1242097) Backed out changeset b94c1a472d46 (bug 1240985)
ipc/glue/MessageChannel.cpp
ipc/glue/MessageChannel.h
ipc/glue/MessageLink.h
xpcom/base/Logging.cpp
--- a/ipc/glue/MessageChannel.cpp
+++ b/ipc/glue/MessageChannel.cpp
@@ -311,25 +311,23 @@ MessageChannel::MessageChannel(MessageLi
     mSide(UnknownSide),
     mLink(nullptr),
     mWorkerLoop(nullptr),
     mChannelErrorTask(nullptr),
     mWorkerLoopID(-1),
     mTimeoutMs(kNoTimeout),
     mInTimeoutSecondHalf(false),
     mNextSeqno(0),
-    mLastSendError(SyncSendError::SendSuccess),
     mAwaitingSyncReply(false),
     mAwaitingSyncReplyPriority(0),
     mDispatchingSyncMessage(false),
     mDispatchingSyncMessagePriority(0),
     mDispatchingAsyncMessage(false),
     mDispatchingAsyncMessagePriority(0),
     mCurrentTransaction(0),
-    mPendingSendPriorities(0),
     mTimedOutMessageSeqno(0),
     mTimedOutMessagePriority(0),
     mRecvdErrors(0),
     mRemoteStackDepthGuess(false),
     mSawInterruptOutMsg(false),
     mIsWaitingForIncoming(false),
     mAbortOnError(false),
     mBlockScripts(false),
@@ -567,20 +565,19 @@ MessageChannel::Send(Message* aMsg)
     }
     mLink->SendMessage(msg.forget());
     return true;
 }
 
 class CancelMessage : public IPC::Message
 {
 public:
-    CancelMessage(int transaction) :
+    CancelMessage() :
         IPC::Message(MSG_ROUTING_NONE, CANCEL_MESSAGE_TYPE, PRIORITY_NORMAL)
     {
-        set_transaction_id(transaction);
     }
     static bool Read(const Message* msg) {
         return true;
     }
     void Log(const std::string& aPrefix, FILE* aOutf) const {
         fputs("(special `Cancel' message)", aOutf);
     }
 };
@@ -598,19 +595,30 @@ MessageChannel::MaybeInterceptSpecialIOM
             mChannelState = ChannelClosing;
             if (LoggingEnabled()) {
                 printf("NOTE: %s process received `Goodbye', closing down\n",
                        (mSide == ChildSide) ? "child" : "parent");
             }
             return true;
         } else if (CANCEL_MESSAGE_TYPE == aMsg.type()) {
             IPC_LOG("Cancel from message");
-            CancelTransaction(aMsg.transaction_id());
-            NotifyWorkerThread();
-            return true;
+
+            if (aMsg.transaction_id() == mTimedOutMessageSeqno) {
+                // An unusual case: We timed out a transaction which the other
+                // side then cancelled. In this case we just leave the timedout
+                // state and try to forget this ever happened.
+                mTimedOutMessageSeqno = 0;
+                return true;
+            } else {
+                MOZ_RELEASE_ASSERT(mCurrentTransaction == aMsg.transaction_id());
+                CancelCurrentTransactionInternal();
+                NotifyWorkerThread();
+                IPC_LOG("Notified");
+                return true;
+            }
         }
     }
     return false;
 }
 
 bool
 MessageChannel::ShouldDeferMessage(const Message& aMsg)
 {
@@ -675,17 +683,17 @@ MessageChannel::OnMessageReceivedFromLin
     // Regardless of the Interrupt stack, if we're awaiting a sync reply,
     // we know that it needs to be immediately handled to unblock us.
     if (aMsg.is_sync() && aMsg.is_reply()) {
         IPC_LOG("Received reply seqno=%d xid=%d", aMsg.seqno(), aMsg.transaction_id());
 
         if (aMsg.seqno() == mTimedOutMessageSeqno) {
             // Drop the message, but allow future sync messages to be sent.
             IPC_LOG("Received reply to timedout message; igoring; xid=%d", mTimedOutMessageSeqno);
-            EndTimeout();
+            mTimedOutMessageSeqno = 0;
             return;
         }
 
         MOZ_ASSERT(aMsg.transaction_id() == mCurrentTransaction);
         MOZ_ASSERT(AwaitingSyncReply());
         MOZ_ASSERT(!mRecvd);
         MOZ_ASSERT(!mTimedOutMessageSeqno);
 
@@ -768,33 +776,32 @@ MessageChannel::OnMessageReceivedFromLin
     // Note that, we may notify the main thread even though the monitor is not
     // blocked. This is okay, since we always check for pending events before
     // blocking again.
 
     mPending.push_back(aMsg);
 
     if (shouldWakeUp) {
         NotifyWorkerThread();
-    }
-
-    // It's possible that a Send or Call call on the stack will process the
-    // message. However, it's difficult to ensure that Send always processes all
-    // messages in mPending before exiting, so it's safer to enqueue a task for
-    // each one. If the queue is empty, the task does nothing.
-    if (!compress) {
-        // If we compressed away the previous message, we'll re-use
-        // its pending task.
-        mWorkerLoop->PostTask(FROM_HERE, new DequeueTask(mDequeueOneTask));
+    } else {
+        // Worker thread is either not blocked on a reply, or this is an
+        // incoming Interrupt that raced with outgoing sync, and needs to be
+        // deferred to a later event-loop iteration.
+        if (!compress) {
+            // If we compressed away the previous message, we'll re-use
+            // its pending task.
+            mWorkerLoop->PostTask(FROM_HERE, new DequeueTask(mDequeueOneTask));
+        }
     }
 }
 
 void
-MessageChannel::ProcessPendingRequests(int seqno, int transaction)
+MessageChannel::ProcessPendingRequests(int transaction, int prio)
 {
-    IPC_LOG("ProcessPendingRequests for seqno=%d, xid=%d", seqno, transaction);
+    IPC_LOG("ProcessPendingRequests");
 
     // Loop until there aren't any more priority messages to process.
     for (;;) {
         mozilla::Vector<Message> toProcess;
 
         for (MessageQueue::iterator it = mPending.begin(); it != mPending.end(); ) {
             Message &msg = *it;
 
@@ -823,41 +830,60 @@ MessageChannel::ProcessPendingRequests(i
         for (auto it = toProcess.begin(); it != toProcess.end(); it++)
             ProcessPendingRequest(*it);
 
         // If we canceled during ProcessPendingRequest, then we need to leave
         // immediately because the results of ShouldDeferMessage will be
         // operating with weird state (as if no Send is in progress). That could
         // cause even normal priority sync messages to be processed (but not
         // normal priority async messages), which would break message ordering.
-        if (WasTransactionCanceled(transaction)) {
+        if (WasTransactionCanceled(transaction, prio)) {
             return;
         }
     }
 }
 
 bool
-MessageChannel::WasTransactionCanceled(int transaction)
+MessageChannel::WasTransactionCanceled(int transaction, int prio)
 {
-    if (transaction != mCurrentTransaction) {
-        // Imagine this scenario:
-        // 1. Child sends high prio sync message H1.
-        // 2. Parent sends reply to H1.
-        // 3. Parent sends high prio sync message H2.
-        // 4. Child's link thread receives H1 reply and H2 before worker wakes up.
-        // 5. Child dispatches H2 while still waiting for H1 reply.
-        // 6. Child cancels H2.
-        //
-        // In this case H1 will also be considered cancelled. However, its
-        // reply is still sitting in mRecvd, which can trip up later Sends. So
-        // we null it out here.
-        mRecvd = nullptr;
-        return true;
+    if (transaction == mCurrentTransaction) {
+        return false;
     }
-    return false;
+
+    // This isn't an assert so much as an intentional crash because we're in a
+    // situation that we don't know how to recover from: The child is awaiting
+    // a reply to a normal-priority sync message. The transaction that this
+    // message initiated has now been canceled. That could only happen if a CPOW
+    // raced with the sync message and was dispatched by the child while the
+    // child was awaiting the sync reply; at some point while dispatching the
+    // CPOW, the transaction was canceled.
+    //
+    // Notes:
+    //
+    // 1. We don't want to cancel the normal-priority sync message along with
+    // the CPOWs because the browser relies on these messages working
+    // reliably.
+    //
+    // 2. Ideally we would like to avoid dispatching CPOWs while awaiting a sync
+    // response. This isn't possible though. To avoid deadlock, the parent would
+    // have to dispatch the sync message while waiting for the CPOW
+    // response. However, it wouldn't have dispatched async messages at that
+    // time, so we would have a message ordering bug. Dispatching the async
+    // messages first causes other hard-to-handle situations (what if they send
+    // CPOWs?).
+    //
+    // 3. We would like to be able to cancel the CPOWs but not the sync
+    // message. However, that would leave both the parent and the child running
+    // code at the same time, all while the sync message is still
+    // outstanding. That can cause a problem where message replies are received
+    // out of order.
+    IPC_ASSERT(prio != IPC::Message::PRIORITY_NORMAL,
+               "Intentional crash: We canceled a CPOW that was racing with a sync message.");
+
+    return true;
 }
 
 bool
 MessageChannel::Send(Message* aMsg, Message* aReply)
 {
     nsAutoPtr<Message> msg(aMsg);
 
     // See comment in DispatchSyncMessage.
@@ -877,53 +903,50 @@ MessageChannel::Send(Message* aMsg, Mess
     MonitorAutoLock lock(*mMonitor);
 
     if (mTimedOutMessageSeqno) {
         // Don't bother sending another sync message if a previous one timed out
         // and we haven't received a reply for it. Once the original timed-out
         // message receives a reply, we'll be able to send more sync messages
         // again.
         IPC_LOG("Send() failed due to previous timeout");
-        mLastSendError = SyncSendError::PreviousTimeout;
         return false;
     }
 
     if (mCurrentTransaction &&
         DispatchingSyncMessagePriority() == IPC::Message::PRIORITY_NORMAL &&
         msg->priority() > IPC::Message::PRIORITY_NORMAL)
     {
         // Don't allow sending CPOWs while we're dispatching a sync message.
         // If you want to do that, use sendRpcMessage instead.
         IPC_LOG("Prio forbids send");
-        mLastSendError = SyncSendError::SendingCPOWWhileDispatchingSync;
         return false;
     }
 
     if (mCurrentTransaction &&
         (DispatchingSyncMessagePriority() == IPC::Message::PRIORITY_URGENT ||
          DispatchingAsyncMessagePriority() == IPC::Message::PRIORITY_URGENT))
     {
         // Generally only the parent dispatches urgent messages. And the only
         // sync messages it can send are high-priority. Mainly we want to ensure
         // here that we don't return false for non-CPOW messages.
         MOZ_ASSERT(msg->priority() == IPC::Message::PRIORITY_HIGH);
-        IPC_LOG("Sending while dispatching urgent message");
-        mLastSendError = SyncSendError::SendingCPOWWhileDispatchingUrgent;
         return false;
     }
 
     if (mCurrentTransaction &&
         (msg->priority() < DispatchingSyncMessagePriority() ||
          msg->priority() < AwaitingSyncReplyPriority()))
     {
         MOZ_ASSERT(DispatchingSyncMessage() || DispatchingAsyncMessage());
         IPC_LOG("Cancel from Send");
-        CancelMessage *cancel = new CancelMessage(mCurrentTransaction);
-        CancelTransaction(mCurrentTransaction);
+        CancelMessage *cancel = new CancelMessage();
+        cancel->set_transaction_id(mCurrentTransaction);
         mLink->SendMessage(cancel);
+        CancelCurrentTransactionInternal();
     }
 
     IPC_ASSERT(msg->is_sync(), "can only Send() sync messages here");
 
     if (mCurrentTransaction) {
         IPC_ASSERT(msg->priority() >= DispatchingSyncMessagePriority(),
                    "can't send sync message of a lesser priority than what's being dispatched");
         IPC_ASSERT(AwaitingSyncReplyPriority() <= msg->priority(),
@@ -932,125 +955,110 @@ MessageChannel::Send(Message* aMsg, Mess
                    "not allowed to send messages while dispatching urgent messages");
     }
 
     IPC_ASSERT(DispatchingAsyncMessagePriority() != IPC::Message::PRIORITY_URGENT,
                "not allowed to send messages while dispatching urgent messages");
 
     if (!Connected()) {
         ReportConnectionError("MessageChannel::SendAndWait", msg);
-        mLastSendError = SyncSendError::NotConnectedBeforeSend;
         return false;
     }
 
     msg->set_seqno(NextSeqno());
 
     int32_t seqno = msg->seqno();
     int prio = msg->priority();
     DebugOnly<msgid_t> replyType = msg->type() + 1;
 
     AutoSetValue<bool> replies(mAwaitingSyncReply, true);
     AutoSetValue<int> prioSet(mAwaitingSyncReplyPriority, prio);
     AutoEnterTransaction transact(this, seqno);
 
-    int prios = mPendingSendPriorities | (1 << prio);
-    AutoSetValue<int> priosSet(mPendingSendPriorities, prios);
-
     int32_t transaction = mCurrentTransaction;
     msg->set_transaction_id(transaction);
 
-    IPC_LOG("Send seqno=%d, xid=%d, pending=%d", seqno, transaction, prios);
+    IPC_LOG("Send seqno=%d, xid=%d", seqno, transaction);
+
+    ProcessPendingRequests(transaction, prio);
+    if (WasTransactionCanceled(transaction, prio)) {
+        IPC_LOG("Other side canceled seqno=%d, xid=%d", seqno, transaction);
+        return false;
+    }
 
     bool handleWindowsMessages = mListener->HandleWindowsMessages(*aMsg);
     mLink->SendMessage(msg.forget());
 
     while (true) {
-        ProcessPendingRequests(seqno, transaction);
-        if (WasTransactionCanceled(transaction)) {
+        ProcessPendingRequests(transaction, prio);
+        if (WasTransactionCanceled(transaction, prio)) {
             IPC_LOG("Other side canceled seqno=%d, xid=%d", seqno, transaction);
-            mLastSendError = SyncSendError::CancelledAfterSend;
-            return false;
-        }
-        if (!Connected()) {
-            ReportConnectionError("MessageChannel::Send");
-            mLastSendError = SyncSendError::DisconnectedDuringSend;
             return false;
         }
 
         // See if we've received a reply.
         if (mRecvdErrors) {
             IPC_LOG("Error: seqno=%d, xid=%d", seqno, transaction);
             mRecvdErrors--;
-            mLastSendError = SyncSendError::ReplyError;
             return false;
         }
 
         if (mRecvd) {
             IPC_LOG("Got reply: seqno=%d, xid=%d", seqno, transaction);
             break;
         }
 
         MOZ_ASSERT(!mTimedOutMessageSeqno);
 
         MOZ_ASSERT(mCurrentTransaction == transaction);
         bool maybeTimedOut = !WaitForSyncNotify(handleWindowsMessages);
 
         if (!Connected()) {
             ReportConnectionError("MessageChannel::SendAndWait");
-            mLastSendError = SyncSendError::DisconnectedDuringSend;
             return false;
         }
 
-        if (WasTransactionCanceled(transaction)) {
+        if (WasTransactionCanceled(transaction, prio)) {
             IPC_LOG("Other side canceled seqno=%d, xid=%d", seqno, transaction);
-            mLastSendError = SyncSendError::CancelledAfterSend;
             return false;
         }
 
         // We only time out a message if it initiated a new transaction (i.e.,
         // if neither side has any other message Sends on the stack).
         bool canTimeOut = transaction == seqno;
         if (maybeTimedOut && canTimeOut && !ShouldContinueFromTimeout()) {
-            // Since ShouldContinueFromTimeout drops the lock, we need to
-            // re-check all our conditions here. We shouldn't time out if any of
-            // these things happen because there won't be a reply to the timed
-            // out message in these cases.
-            if (WasTransactionCanceled(transaction)) {
-                IPC_LOG("Other side canceled seqno=%d, xid=%d", seqno, transaction);
-                mLastSendError = SyncSendError::CancelledAfterSend;
-                return false;
-            }
+            IPC_LOG("Timing out Send: xid=%d", transaction);
+
+            // We might have received a reply during WaitForSyncNotify or inside
+            // ShouldContinueFromTimeout (which drops the lock). We need to make
+            // sure not to set mTimedOutMessageSeqno if that happens, since then
+            // there would be no way to unset it.
             if (mRecvdErrors) {
                 mRecvdErrors--;
-                mLastSendError = SyncSendError::ReplyError;
                 return false;
             }
             if (mRecvd) {
                 break;
             }
 
-            IPC_LOG("Timing out Send: xid=%d", transaction);
-
             mTimedOutMessageSeqno = seqno;
             mTimedOutMessagePriority = prio;
-            mLastSendError = SyncSendError::TimedOut;
             return false;
         }
     }
 
     MOZ_ASSERT(mRecvd);
     MOZ_ASSERT(mRecvd->is_reply(), "expected reply");
     MOZ_ASSERT(!mRecvd->is_reply_error());
     MOZ_ASSERT(mRecvd->type() == replyType, "wrong reply type");
     MOZ_ASSERT(mRecvd->seqno() == seqno);
     MOZ_ASSERT(mRecvd->is_sync());
 
     *aReply = Move(*mRecvd);
     mRecvd = nullptr;
-    mLastSendError = SyncSendError::SendSuccess;
     return true;
 }
 
 bool
 MessageChannel::Call(Message* aMsg, Message* aReply)
 {
     nsAutoPtr<Message> msg(aMsg);
     AssertWorkerThread();
@@ -1305,47 +1313,16 @@ MessageChannel::DequeueOne(Message *recv
     if (!Connected()) {
         ReportConnectionError("OnMaybeDequeueOne");
         return false;
     }
 
     if (!mDeferred.empty())
         MaybeUndeferIncall();
 
-    // If we've timed out a message and we're awaiting the reply to the timed
-    // out message, we have to be careful what messages we process. Here's what
-    // can go wrong:
-    // 1. child sends a normal priority sync message S
-    // 2. parent sends a high priority sync message H at the same time
-    // 3. parent times out H
-    // 4. child starts processing H and sends a high priority message H' nested
-    //    within the same transaction
-    // 5. parent dispatches S and sends reply
-    // 6. child asserts because it instead expected a reply to H'.
-    //
-    // To solve this, we refuse to process S in the parent until we get a reply
-    // to H. More generally, let the timed out message be M. We don't process a
-    // message unless the child would need the response to that message in order
-    // to process M. Those messages are the ones that have a higher priority
-    // than M or that are part of the same transaction as M.
-    if (mTimedOutMessageSeqno) {
-        for (MessageQueue::iterator it = mPending.begin(); it != mPending.end(); it++) {
-            Message &msg = *it;
-            if (msg.priority() > mTimedOutMessagePriority ||
-                (msg.priority() == mTimedOutMessagePriority
-                 && msg.transaction_id() == mTimedOutMessageSeqno))
-            {
-                *recvd = Move(msg);
-                mPending.erase(it);
-                return true;
-            }
-        }
-        return false;
-    }
-
     if (mPending.empty())
         return false;
 
     *recvd = Move(mPending.front());
     mPending.pop_front();
     return true;
 }
 
@@ -1428,17 +1405,32 @@ MessageChannel::DispatchSyncMessage(cons
     // no longer blocked.
     MOZ_ASSERT_IF(prio > IPC::Message::PRIORITY_NORMAL, NS_IsMainThread());
     MaybeScriptBlocker scriptBlocker(this, prio > IPC::Message::PRIORITY_NORMAL);
 
     MessageChannel* dummy;
     MessageChannel*& blockingVar = ShouldBlockScripts() ? gParentProcessBlocker : dummy;
 
     Result rv;
-    {
+    if (mTimedOutMessageSeqno && mTimedOutMessagePriority >= prio) {
+        // If the other side sends a message in response to one of our messages
+        // that we've timed out, then we reply with an error.
+        //
+        // We do this because want to avoid a situation where we process an
+        // incoming message from the child here while it simultaneously starts
+        // processing our timed-out CPOW. It's very bad for both sides to
+        // be processing sync messages concurrently.
+        //
+        // The only exception is if the incoming message has urgent priority and
+        // our timed-out message had only high priority. In that case it's safe
+        // to process the incoming message because we know that the child won't
+        // process anything (the child will defer incoming messages when waiting
+        // for a response to its urgent message).
+        rv = MsgNotAllowed;
+    } else {
         AutoSetValue<MessageChannel*> blocked(blockingVar, this);
         AutoSetValue<bool> sync(mDispatchingSyncMessage, true);
         AutoSetValue<int> prioSet(mDispatchingSyncMessagePriority, prio);
         rv = mListener->OnMessageReceived(aMsg, aReply);
     }
 
     if (!MaybeHandleError(rv, aMsg, "DispatchSyncMessage")) {
         aReply = new Message();
@@ -1831,18 +1823,16 @@ MessageChannel::MaybeHandleError(Result 
 }
 
 void
 MessageChannel::OnChannelErrorFromLink()
 {
     AssertLinkThread();
     mMonitor->AssertCurrentThreadOwns();
 
-    IPC_LOG("OnChannelErrorFromLink");
-
     if (InterruptStackDepth() > 0)
         NotifyWorkerThread();
 
     if (AwaitingSyncReply() || AwaitingIncomingMessage())
         NotifyWorkerThread();
 
     if (ChannelClosing != mChannelState) {
         if (mAbortOnError) {
@@ -2098,157 +2088,61 @@ MessageChannel::GetTopmostMessageRouting
     if (mCxxStackFrames.empty()) {
         return MSG_ROUTING_NONE;
     }
     const InterruptFrame& frame = mCxxStackFrames.back();
     return frame.GetRoutingId();
 }
 
 void
-MessageChannel::EndTimeout()
-{
-    mMonitor->AssertCurrentThreadOwns();
-
-    IPC_LOG("Ending timeout of seqno=%d", mTimedOutMessageSeqno);
-    mTimedOutMessageSeqno = 0;
-    mTimedOutMessagePriority = 0;
-
-    for (size_t i = 0; i < mPending.size(); i++) {
-        // There may be messages in the queue that we expected to process from
-        // OnMaybeDequeueOne. But during the timeout, that function will skip
-        // some messages. Now they're ready to be processed, so we enqueue more
-        // tasks.
-        mWorkerLoop->PostTask(FROM_HERE, new DequeueTask(mDequeueOneTask));
-    }
-}
-
-void
-MessageChannel::CancelTransaction(int transaction)
+MessageChannel::CancelCurrentTransactionInternal()
 {
     mMonitor->AssertCurrentThreadOwns();
 
     // When we cancel a transaction, we need to behave as if there's no longer
     // any IPC on the stack. Anything we were dispatching or sending will get
     // canceled. Consequently, we have to update the state variables below.
     //
     // We also need to ensure that when any IPC functions on the stack return,
     // they don't reset these values using an RAII class like AutoSetValue. To
     // avoid that, these RAII classes check if the variable they set has been
     // tampered with (by us). If so, they don't reset the variable to the old
     // value.
 
-    IPC_LOG("CancelTransaction: xid=%d prios=%d", transaction, mPendingSendPriorities);
+    IPC_LOG("CancelInternal: current xid=%d", mCurrentTransaction);
 
-    if (mPendingSendPriorities & (1 << IPC::Message::PRIORITY_NORMAL)) {
-        // This isn't an assert so much as an intentional crash because we're in
-        // a situation that we don't know how to recover from: The child is
-        // awaiting a reply to a normal-priority sync message. The transaction
-        // that this message initiated has now been canceled. That could only
-        // happen if a CPOW raced with the sync message and was dispatched by
-        // the child while the child was awaiting the sync reply; at some point
-        // while dispatching the CPOW, the transaction was canceled.
-        //
-        // Notes:
-        //
-        // 1. We don't want to cancel the normal-priority sync message along
-        // with the CPOWs because the browser relies on these messages working
-        // reliably.
-        //
-        // 2. Ideally we would like to avoid dispatching CPOWs while awaiting a
-        // sync response. This isn't possible though. To avoid deadlock, the
-        // parent would have to dispatch the sync message while waiting for the
-        // CPOW response. However, it wouldn't have dispatched async messages at
-        // that time, so we would have a message ordering bug. Dispatching the
-        // async messages first causes other hard-to-handle situations (what if
-        // they send CPOWs?).
-        //
-        // 3. We would like to be able to cancel the CPOWs but not the sync
-        // message. However, that would leave both the parent and the child
-        // running code at the same time, all while the sync message is still
-        // outstanding. That can cause a problem where message replies are
-        // received out of order.
-        mListener->IntentionalCrash();
-    }
+    MOZ_ASSERT(mCurrentTransaction);
+    mCurrentTransaction = 0;
 
-    // An unusual case: We timed out a transaction which the other side then
-    // cancelled. In this case we just leave the timedout state and try to
-    // forget this ever happened.
-    if (transaction == mTimedOutMessageSeqno) {
-        IPC_LOG("Cancelled timed out message %d", mTimedOutMessageSeqno);
-        EndTimeout();
-
-        // Normally mCurrentTransaction == 0 here. But it can be non-zero if:
-        // 1. Parent sends hi prio message H.
-        // 2. Parent times out H.
-        // 3. Child dispatches H and sends nested message H' (same transaction).
-        // 4. Parent dispatches H' and cancels.
-        MOZ_ASSERT_IF(mCurrentTransaction, mCurrentTransaction == transaction);
-        mCurrentTransaction = 0;
-
-        // During a timeout Send should always fail.
-        MOZ_ASSERT(!mAwaitingSyncReply);
-    } else {
-        MOZ_ASSERT(mCurrentTransaction == transaction);
-        mCurrentTransaction = 0;
+    mAwaitingSyncReply = false;
+    mAwaitingSyncReplyPriority = 0;
 
-        mAwaitingSyncReply = false;
-        mAwaitingSyncReplyPriority = 0;
-    }
-
-    DebugOnly<bool> foundSync = false;
-    for (MessageQueue::iterator it = mPending.begin(); it != mPending.end(); ) {
-        Message &msg = *it;
-
-        // If there was a race between the parent and the child, then we may
-        // have a queued sync message. We want to drop this message from the
-        // queue since it will get cancelled along with the transaction being
-        // cancelled. We don't bother doing this for normal priority messages
-        // because the child is just going to crash in that case, and we want to
-        // avoid processing messages out of order in the short time before it
-        // crashes.
-        if (msg.is_sync() && msg.priority() != IPC::Message::PRIORITY_NORMAL) {
-            MOZ_ASSERT(!foundSync);
-            MOZ_ASSERT(msg.transaction_id() != transaction);
-            IPC_LOG("Removing msg from queue seqno=%d xid=%d", msg.seqno(), msg.transaction_id());
-            foundSync = true;
-            it = mPending.erase(it);
-            continue;
-        }
-
+    for (size_t i = 0; i < mPending.size(); i++) {
         // There may be messages in the queue that we expected to process from
         // ProcessPendingRequests. However, Send will no longer call that
         // function once it's been canceled. So we may need to process these
         // messages in the normal event loop instead.
         mWorkerLoop->PostTask(FROM_HERE, new DequeueTask(mDequeueOneTask));
-
-        it++;
     }
 
     // We could also zero out mDispatchingSyncMessage here. However, that would
     // cause a race because mDispatchingSyncMessage is a worker-thread-only
     // field and we can be called on the I/O thread. Luckily, we can check to
     // see if mCurrentTransaction is 0 before examining DispatchSyncMessage.
 }
 
 void
 MessageChannel::CancelCurrentTransaction()
 {
     MonitorAutoLock lock(*mMonitor);
     if (mCurrentTransaction) {
-        if (DispatchingSyncMessagePriority() == IPC::Message::PRIORITY_URGENT ||
-            DispatchingAsyncMessagePriority() == IPC::Message::PRIORITY_URGENT)
-        {
-            mListener->IntentionalCrash();
-        }
-
-        IPC_LOG("Cancel requested: current xid=%d", mCurrentTransaction);
-        MOZ_ASSERT(DispatchingSyncMessage());
-        CancelMessage *cancel = new CancelMessage(mCurrentTransaction);
-        CancelTransaction(mCurrentTransaction);
+        CancelMessage *cancel = new CancelMessage();
+        cancel->set_transaction_id(mCurrentTransaction);
         mLink->SendMessage(cancel);
+        CancelCurrentTransactionInternal();
     }
 }
 
 void
 CancelCPOWs()
 {
     if (gParentProcessBlocker) {
         mozilla::Telemetry::Accumulate(mozilla::Telemetry::IPC_TRANSACTION_CANCEL, true);
--- a/ipc/glue/MessageChannel.h
+++ b/ipc/glue/MessageChannel.h
@@ -39,29 +39,16 @@ class RefCountedMonitor : public Monitor
     {}
 
     NS_INLINE_DECL_THREADSAFE_REFCOUNTING(RefCountedMonitor)
 
   private:
     ~RefCountedMonitor() {}
 };
 
-enum class SyncSendError {
-    SendSuccess,
-    PreviousTimeout,
-    SendingCPOWWhileDispatchingSync,
-    SendingCPOWWhileDispatchingUrgent,
-    NotConnectedBeforeSend,
-    DisconnectedDuringSend,
-    CancelledBeforeSend,
-    CancelledAfterSend,
-    TimedOut,
-    ReplyError,
-};
-
 class MessageChannel : HasResultCodes
 {
     friend class ProcessLink;
     friend class ThreadLink;
 
     class CxxStackFrame;
     class InterruptFrame;
 
@@ -140,23 +127,16 @@ class MessageChannel : HasResultCodes
     // Make an Interrupt call to the other side of the channel
     bool Call(Message* aMsg, Message* aReply);
 
     // Wait until a message is received
     bool WaitForIncomingMessage();
 
     bool CanSend() const;
 
-    // If sending a sync message returns an error, this function gives a more
-    // descriptive error message.
-    SyncSendError LastSendError() const {
-        AssertWorkerThread();
-        return mLastSendError;
-    }
-
     // Currently only for debugging purposes, doesn't aquire mMonitor.
     ChannelState GetChannelState__TotallyRacy() const {
         return mChannelState;
     }
 
     void SetReplyTimeoutMs(int32_t aTimeoutMs);
 
     bool IsOnCxxStack() const {
@@ -265,17 +245,17 @@ class MessageChannel : HasResultCodes
     void Clear();
 
     // Send OnChannelConnected notification to listeners.
     void DispatchOnChannelConnected();
 
     bool InterruptEventOccurred();
     bool HasPendingEvents();
 
-    void ProcessPendingRequests(int seqno, int transaction);
+    void ProcessPendingRequests(int transaction, int prio);
     bool ProcessPendingRequest(const Message &aUrgent);
 
     void MaybeUndeferIncall();
     void EnqueuePendingMessages();
 
     // Executed on the worker thread. Dequeues one pending message.
     bool OnMaybeDequeueOne();
     bool DequeueOne(Message *recvd);
@@ -303,18 +283,17 @@ class MessageChannel : HasResultCodes
     // necessarily.
     bool WaitForSyncNotify(bool aHandleWindowsMessages);
     bool WaitForInterruptNotify();
 
     bool WaitResponse(bool aWaitTimedOut);
 
     bool ShouldContinueFromTimeout();
 
-    void EndTimeout();
-    void CancelTransaction(int transaction);
+    void CancelCurrentTransactionInternal();
 
     // The "remote view of stack depth" can be different than the
     // actual stack depth when there are out-of-turn replies.  When we
     // receive one, our actual Interrupt stack depth doesn't decrease, but
     // the other side (that sent the reply) thinks it has.  So, the
     // "view" returned here is |stackDepth| minus the number of
     // out-of-turn replies.
     //
@@ -442,17 +421,17 @@ class MessageChannel : HasResultCodes
     // thread, in which case it shouldn't be delivered to the worker.
     bool MaybeInterceptSpecialIOMessage(const Message& aMsg);
 
     void OnChannelConnected(int32_t peer_id);
 
     // Tell the IO thread to close the channel and wait for it to ACK.
     void SynchronouslyClose();
 
-    bool WasTransactionCanceled(int transaction);
+    bool WasTransactionCanceled(int transaction, int prio);
     bool ShouldDeferMessage(const Message& aMsg);
     void OnMessageReceivedFromLink(const Message& aMsg);
     void OnChannelErrorFromLink();
 
   private:
     // Run on the not current thread.
     void NotifyChannelClosed();
     void NotifyMaybeChannelError();
@@ -537,19 +516,16 @@ class MessageChannel : HasResultCodes
     bool mInTimeoutSecondHalf;
 
     // Worker-thread only; sequence numbers for messages that require
     // synchronous replies.
     int32_t mNextSeqno;
 
     static bool sIsPumpingMessages;
 
-    // If ::Send returns false, this gives a more descriptive error.
-    SyncSendError mLastSendError;
-
     template<class T>
     class AutoSetValue {
       public:
         explicit AutoSetValue(T &var, const T &newValue)
           : mVar(var), mPrev(var), mNew(newValue)
         {
             mVar = newValue;
         }
@@ -595,23 +571,16 @@ class MessageChannel : HasResultCodes
     // transaction, the initiating transaction ID is used.
     //
     // To ensure IDs are unique, we use sequence numbers for transaction IDs,
     // which grow in opposite directions from child to parent.
 
     // The current transaction ID.
     int32_t mCurrentTransaction;
 
-    // This field describes the priorities of the sync Send calls that are
-    // currently on stack. If a Send call for a message with priority P is on
-    // the C stack, then mPendingSendPriorities & (1 << P) will be
-    // non-zero. Note that cancelled Send calls are not removed from this
-    // bitfield (until they return).
-    int mPendingSendPriorities;
-
     class AutoEnterTransaction
     {
      public:
        explicit AutoEnterTransaction(MessageChannel *aChan, int32_t aMsgSeqno)
         : mChan(aChan),
           mNewTransaction(INT32_MAX),
           mOldTransaction(mChan->mCurrentTransaction)
        {
--- a/ipc/glue/MessageLink.h
+++ b/ipc/glue/MessageLink.h
@@ -71,21 +71,16 @@ class MessageListener
     virtual Result OnMessageReceived(const Message& aMessage, Message *& aReply) = 0;
     virtual Result OnCallReceived(const Message& aMessage, Message *& aReply) = 0;
     virtual void OnProcessingError(Result aError, const char* aMsgName) = 0;
     virtual void OnChannelConnected(int32_t peer_pid) {}
     virtual bool OnReplyTimeout() {
         return false;
     }
 
-    // WARNING: This function is called with the MessageChannel monitor held.
-    virtual void IntentionalCrash() {
-        MOZ_CRASH("Intentional IPDL crash");
-    }
-
     virtual void OnEnteredCxxStack() {
         NS_RUNTIMEABORT("default impl shouldn't be invoked");
     }
     virtual void OnExitedCxxStack() {
         NS_RUNTIMEABORT("default impl shouldn't be invoked");
     }
     virtual void OnEnteredCall() {
         NS_RUNTIMEABORT("default impl shouldn't be invoked");
--- a/xpcom/base/Logging.cpp
+++ b/xpcom/base/Logging.cpp
@@ -82,51 +82,46 @@ ToLogStr(LogLevel aLevel) {
 class LogModuleManager
 {
 public:
   LogModuleManager()
     : mModulesLock("logmodules")
     , mModules(kInitialModuleCount)
     , mOutFile(nullptr)
     , mAddTimestamp(false)
-    , mIsSync(false)
   {
   }
 
   ~LogModuleManager()
   {
     // NB: mModules owns all of the log modules, they will get destroyed by
     //     its destructor.
   }
 
   /**
    * Loads config from env vars if present.
    */
   void Init()
   {
     bool shouldAppend = false;
     bool addTimestamp = false;
-    bool isSync = false;
     const char* modules = PR_GetEnv("NSPR_LOG_MODULES");
     NSPRLogModulesParser(modules,
-        [&shouldAppend, &addTimestamp, &isSync]
+        [&shouldAppend, &addTimestamp]
             (const char* aName, LogLevel aLevel) mutable {
           if (strcmp(aName, "append") == 0) {
             shouldAppend = true;
           } else if (strcmp(aName, "timestamp") == 0) {
             addTimestamp = true;
-          } else if (strcmp(aName, "sync") == 0) {
-            isSync = true;
           } else {
             LogModule::Get(aName)->SetLevel(aLevel);
           }
     });
 
     mAddTimestamp = addTimestamp;
-    mIsSync = isSync;
 
     const char* logFile = PR_GetEnv("NSPR_LOG_FILE");
     if (logFile && logFile[0]) {
       mOutFile = fopen(logFile, shouldAppend ? "a" : "w");
     }
   }
 
   LogModule* CreateOrGetModule(const char* aName)
@@ -188,31 +183,26 @@ public:
           out,
           "%04d-%02d-%02d %02d:%02d:%02d.%06d UTC - [%p]: %s/%s %s%s",
           now.tm_year, now.tm_month + 1, now.tm_mday,
           now.tm_hour, now.tm_min, now.tm_sec, now.tm_usec,
           PR_GetCurrentThread(), ToLogStr(aLevel),
           aName, buffToWrite, newline);
     }
 
-    if (mIsSync) {
-      fflush(out);
-    }
-
     if (buffToWrite != buff) {
       PR_smprintf_free(buffToWrite);
     }
   }
 
 private:
   OffTheBooksMutex mModulesLock;
   nsClassHashtable<nsCharPtrHashKey, LogModule> mModules;
   ScopedCloseFile mOutFile;
   bool mAddTimestamp;
-  bool mIsSync;
 };
 
 StaticAutoPtr<LogModuleManager> sLogModuleManager;
 
 LogModule*
 LogModule::Get(const char* aName)
 {
   // This is just a pass through to the LogModuleManager so