Backed out 9 changesets (bug 1240985) for various Windows build failures CLOSED TREE
authorWes Kocher <wkocher@mozilla.com>
Wed, 20 Jan 2016 17:22:12 -0800
changeset 280799 10d4e8736dbbb4f79320273906d148c448cb8e0b
parent 280798 1a5165c7be1825287da5541e99b505490444974a
child 280800 a6c0026d360b24f9e256df25ca301628d060f67a
push id70593
push userkwierso@gmail.com
push dateThu, 21 Jan 2016 01:22:17 +0000
treeherdermozilla-inbound@10d4e8736dbb [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1240985
milestone46.0a1
backs out1a5165c7be1825287da5541e99b505490444974a
26e6fe3875a56c088588fb04f72036c24e2c4c11
b87f893bd6aa3ba610607cf88ec38b13f73b4949
3fcd50d8382165d89359b3a238b686a3624e35f4
488690ba4c8fa9a09f55ca7a77614219f2a0c57a
a7eecc68ca20da93faf415819679236d3196491a
12de76467424461300853e4953dfd3c6229ff5cb
34f87cc8ac248ec137f1450253f561cad84a2172
fddcf8b3b08835eb75c1bd986df3af677bae3f34
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 9 changesets (bug 1240985) for various Windows build failures CLOSED TREE Backed out changeset 1a5165c7be18 (bug 1240985) Backed out changeset 26e6fe3875a5 (bug 1240985) Backed out changeset b87f893bd6aa (bug 1240985) Backed out changeset 3fcd50d83821 (bug 1240985) Backed out changeset 488690ba4c8f (bug 1240985) Backed out changeset a7eecc68ca20 (bug 1240985) Backed out changeset 12de76467424 (bug 1240985) Backed out changeset 34f87cc8ac24 (bug 1240985) Backed out changeset fddcf8b3b088 (bug 1240985)
ipc/chromium/src/base/message_pump_glib.cc
ipc/chromium/src/base/message_pump_glib.h
ipc/glue/MessageChannel.cpp
ipc/glue/MessageChannel.h
--- a/ipc/chromium/src/base/message_pump_glib.cc
+++ b/ipc/chromium/src/base/message_pump_glib.cc
@@ -119,18 +119,17 @@ GSourceFuncs WorkSourceFuncs = {
 }  // namespace
 
 
 namespace base {
 
 MessagePumpForUI::MessagePumpForUI()
     : state_(NULL),
       context_(g_main_context_default()),
-      wakeup_gpollfd_(new GPollFD),
-      pipe_full_(false) {
+      wakeup_gpollfd_(new GPollFD) {
   // Create our wakeup pipe, which is used to flag when work was scheduled.
   int fds[2];
   CHECK(pipe(fds) == 0);
   wakeup_pipe_read_  = fds[0];
   wakeup_pipe_write_ = fds[1];
   wakeup_gpollfd_->fd = wakeup_pipe_read_;
   wakeup_gpollfd_->events = G_IO_IN;
 
@@ -226,22 +225,20 @@ int MessagePumpForUI::HandlePrepare() {
   // longer than the next time we need to run delayed work.
   return GetTimeIntervalMilliseconds(delayed_work_time_);
 }
 
 bool MessagePumpForUI::HandleCheck() {
   if (!state_)  // state_ may be null during tests.
     return false;
 
-  // We should only ever have a single message on the wakeup pipe since we only
-  // write to the pipe when pipe_full_ is false. The glib poll will tell us
-  // whether there was data, so this read shouldn't block.
+  // We should only ever have a single message on the wakeup pipe, since we
+  // are only signaled when the queue went from empty to non-empty.  The glib
+  // poll will tell us whether there was data, so this read shouldn't block.
   if (wakeup_gpollfd_->revents & G_IO_IN) {
-    pipe_full_ = false;
-
     char msg;
     if (HANDLE_EINTR(read(wakeup_pipe_read_, &msg, 1)) != 1 || msg != '!') {
       NOTREACHED() << "Error reading from the wakeup pipe.";
     }
     // Since we ate the message, we need to record that we have more work,
     // because HandleCheck() may be called without HandleDispatch being called
     // afterwards.
     state_->has_work = true;
@@ -295,21 +292,16 @@ void MessagePumpForUI::Quit() {
   if (state_) {
     state_->should_quit = true;
   } else {
     NOTREACHED() << "Quit called outside Run!";
   }
 }
 
 void MessagePumpForUI::ScheduleWork() {
-  bool was_full = pipe_full_.exchange(true);
-  if (was_full) {
-    return;
-  }
-
   // This can be called on any thread, so we don't want to touch any state
   // variables as we would then need locks all over.  This ensures that if
   // we are sleeping in a poll that we will wake up.
   char msg = '!';
   if (HANDLE_EINTR(write(wakeup_pipe_write_, &msg, 1)) != 1) {
     NOTREACHED() << "Could not write to the UI message loop wakeup pipe!";
   }
 }
--- a/ipc/chromium/src/base/message_pump_glib.h
+++ b/ipc/chromium/src/base/message_pump_glib.h
@@ -4,17 +4,16 @@
 
 #ifndef BASE_MESSAGE_PUMP_GLIB_H_
 #define BASE_MESSAGE_PUMP_GLIB_H_
 
 #include "base/message_pump.h"
 #include "base/observer_list.h"
 #include "base/time.h"
 #include "mozilla/UniquePtr.h"
-#include "mozilla/Atomics.h"
 
 typedef union _GdkEvent GdkEvent;
 typedef struct _GMainContext GMainContext;
 typedef struct _GPollFD GPollFD;
 typedef struct _GSource GSource;
 
 namespace base {
 
@@ -127,18 +126,16 @@ class MessagePumpForUI : public MessageP
   // when another thread has scheduled us to do some work.  There is a glib
   // mechanism g_main_context_wakeup, but this won't guarantee that our event's
   // Dispatch() will be called.
   int wakeup_pipe_read_;
   int wakeup_pipe_write_;
   // Use an autoptr to avoid needing the definition of GPollFD in the header.
   mozilla::UniquePtr<GPollFD> wakeup_gpollfd_;
 
-  mozilla::Atomic<bool> pipe_full_;
-
   // List of observers.
   ObserverList<Observer> observers_;
 
   DISALLOW_COPY_AND_ASSIGN(MessagePumpForUI);
 };
 
 }  // namespace base
 
--- a/ipc/glue/MessageChannel.cpp
+++ b/ipc/glue/MessageChannel.cpp
@@ -18,20 +18,16 @@
 #include "nsDebug.h"
 #include "nsISupportsImpl.h"
 #include "nsContentUtils.h"
 #include "mozilla/Snprintf.h"
 
 // Undo the damage done by mozzconf.h
 #undef compress
 
-static LazyLogModule sLogModule("ipc");
-
-#define IPC_LOG(...) MOZ_LOG(sLogModule, LogLevel::Debug, __VA_ARGS__)
-
 /*
  * IPC design:
  *
  * There are three kinds of messages: async, sync, and intr. Sync and intr
  * messages are blocking. Only intr and high-priority sync messages can nest.
  *
  * Terminology: To dispatch a message Foo is to run the RecvFoo code for
  * it. This is also called "handling" the message.
@@ -318,16 +314,17 @@ MessageChannel::MessageChannel(MessageLi
     mAwaitingSyncReply(false),
     mAwaitingSyncReplyPriority(0),
     mDispatchingSyncMessage(false),
     mDispatchingSyncMessagePriority(0),
     mDispatchingAsyncMessage(false),
     mDispatchingAsyncMessagePriority(0),
     mCurrentTransaction(0),
     mTimedOutMessageSeqno(0),
+    mTimedOutMessagePriority(0),
     mRecvdErrors(0),
     mRemoteStackDepthGuess(false),
     mSawInterruptOutMsg(false),
     mIsWaitingForIncoming(false),
     mAbortOnError(false),
     mBlockScripts(false),
     mFlags(REQUIRE_DEFAULT),
     mPeerPidSet(false),
@@ -592,31 +589,19 @@ MessageChannel::MaybeInterceptSpecialIOM
             // other side
             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");
-
-            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;
-            }
+            CancelCurrentTransactionInternal();
+            NotifyWorkerThread();
+            return true;
         }
     }
     return false;
 }
 
 bool
 MessageChannel::ShouldDeferMessage(const Message& aMsg)
 {
@@ -676,29 +661,25 @@ MessageChannel::OnMessageReceivedFromLin
     mMonitor->AssertCurrentThreadOwns();
 
     if (MaybeInterceptSpecialIOMessage(aMsg))
         return;
 
     // 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);
             mTimedOutMessageSeqno = 0;
             return;
         }
 
         MOZ_ASSERT(aMsg.transaction_id() == mCurrentTransaction);
         MOZ_ASSERT(AwaitingSyncReply());
         MOZ_ASSERT(!mRecvd);
-        MOZ_ASSERT(!mTimedOutMessageSeqno);
 
         // Rather than storing errors in mRecvd, we mark them in
         // mRecvdErrors. We need a counter because multiple replies can arrive
         // when a timeout happens, as in the following example. Imagine the
         // child is running slowly. The parent sends a sync message P1. It times
         // out. The child eventually sends a sync message C1. While waiting for
         // the C1 response, the child dispatches P1. In doing so, it sends sync
         // message C2. At that point, it's valid for the parent to send error
@@ -745,19 +726,16 @@ MessageChannel::OnMessageReceivedFromLin
             mPending.erase((++it).base());
         }
     }
 
     bool shouldWakeUp = AwaitingInterruptReply() ||
                         (AwaitingSyncReply() && !ShouldDeferMessage(aMsg)) ||
                         AwaitingIncomingMessage();
 
-    IPC_LOG("Receive on link thread; seqno=%d, xid=%d, shouldWakeUp=%d",
-            aMsg.seqno(), aMsg.transaction_id(), shouldWakeUp);
-
     // There are three cases we're concerned about, relating to the state of the
     // main thread:
     //
     // (1) We are waiting on a sync reply - main thread is blocked on the
     //     IPC monitor.
     //   - If the message is high priority, we wake up the main thread to
     //     deliver the message depending on ShouldDeferMessage. Otherwise, we
     //     leave it in the mPending queue, posting a task to the main event
@@ -787,60 +765,40 @@ MessageChannel::OnMessageReceivedFromLin
             // 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 transaction, int prio)
+MessageChannel::ProcessPendingRequests()
 {
-    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;
-
-            bool defer = ShouldDeferMessage(msg);
-
-            // Only log the interesting messages.
-            if (msg.is_sync() || msg.priority() == IPC::Message::PRIORITY_URGENT) {
-                IPC_LOG("ShouldDeferMessage(seqno=%d) = %d", msg.seqno(), defer);
-            }
-
-            if (!defer) {
+            if (!ShouldDeferMessage(msg)) {
                 if (!toProcess.append(Move(msg)))
                     MOZ_CRASH();
                 it = mPending.erase(it);
                 continue;
             }
             it++;
         }
 
         if (toProcess.empty())
             break;
 
         // Processing these messages could result in more messages, so we
         // loop around to check for more afterwards.
-
         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, prio)) {
-            return;
-        }
     }
 }
 
 bool
 MessageChannel::WasTransactionCanceled(int transaction, int prio)
 {
     if (transaction == mCurrentTransaction) {
         return false;
@@ -900,51 +858,34 @@ 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");
         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");
-        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);
         return false;
     }
 
     if (mCurrentTransaction &&
         (msg->priority() < DispatchingSyncMessagePriority() ||
-         msg->priority() < AwaitingSyncReplyPriority()))
+         mAwaitingSyncReplyPriority > msg->priority()))
     {
-        MOZ_ASSERT(DispatchingSyncMessage() || DispatchingAsyncMessage());
-        IPC_LOG("Cancel from Send");
-        CancelMessage *cancel = new CancelMessage();
-        cancel->set_transaction_id(mCurrentTransaction);
-        mLink->SendMessage(cancel);
         CancelCurrentTransactionInternal();
+        mLink->SendMessage(new CancelMessage());
     }
 
     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(),
@@ -969,80 +910,71 @@ MessageChannel::Send(Message* aMsg, Mess
 
     AutoSetValue<bool> replies(mAwaitingSyncReply, true);
     AutoSetValue<int> prioSet(mAwaitingSyncReplyPriority, prio);
     AutoEnterTransaction transact(this, seqno);
 
     int32_t transaction = mCurrentTransaction;
     msg->set_transaction_id(transaction);
 
-    IPC_LOG("Send seqno=%d, xid=%d", seqno, transaction);
-
-    ProcessPendingRequests(transaction, prio);
+    ProcessPendingRequests();
     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(transaction, prio);
+        ProcessPendingRequests();
         if (WasTransactionCanceled(transaction, prio)) {
-            IPC_LOG("Other side canceled seqno=%d, xid=%d", seqno, transaction);
             return false;
         }
 
         // See if we've received a reply.
         if (mRecvdErrors) {
-            IPC_LOG("Error: seqno=%d, xid=%d", seqno, transaction);
             mRecvdErrors--;
             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");
             return false;
         }
 
         if (WasTransactionCanceled(transaction, prio)) {
-            IPC_LOG("Other side canceled seqno=%d, xid=%d", seqno, transaction);
             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()) {
-            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--;
                 return false;
             }
             if (mRecvd) {
                 break;
             }
 
             mTimedOutMessageSeqno = seqno;
+            mTimedOutMessagePriority = prio;
             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");
@@ -1278,18 +1210,16 @@ MessageChannel::ProcessPendingRequest(co
     // therefore mPendingUrgentRequest is set *and* mRecvd is set as
     // well, because the link thread received both before the worker
     // thread woke up.
     //
     // In this case, we process the urgent message first, but we need
     // to save the reply.
     nsAutoPtr<Message> savedReply(mRecvd.forget());
 
-    IPC_LOG("Process pending: seqno=%d, xid=%d", aUrgent.seqno(), aUrgent.transaction_id());
-
     DispatchMessage(aUrgent);
     if (!Connected()) {
         ReportConnectionError("MessageChannel::ProcessPendingRequest");
         return false;
     }
 
     // In between having dispatched our reply to the parent process, and
     // re-acquiring the monitor, the parent process could have already
@@ -1353,18 +1283,16 @@ void
 MessageChannel::DispatchMessage(const Message &aMsg)
 {
     Maybe<AutoNoJSAPI> nojsapi;
     if (ScriptSettingsInitialized() && NS_IsMainThread())
         nojsapi.emplace();
 
     nsAutoPtr<Message> reply;
 
-    IPC_LOG("DispatchMessage: seqno=%d, xid=%d", aMsg.seqno(), aMsg.transaction_id());
-
     {
         AutoEnterTransaction transaction(this, aMsg);
 
         int id = aMsg.transaction_id();
         MOZ_ASSERT_IF(aMsg.is_sync(), id == mCurrentTransaction);
 
         {
             MonitorAutoUnlock unlock(*mMonitor);
@@ -1402,17 +1330,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();
@@ -2072,44 +2015,32 @@ MessageChannel::GetTopmostMessageRouting
     }
     const InterruptFrame& frame = mCxxStackFrames.back();
     return frame.GetRoutingId();
 }
 
 void
 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("CancelInternal: current xid=%d", mCurrentTransaction);
-
     MOZ_ASSERT(mCurrentTransaction);
     mCurrentTransaction = 0;
 
     mAwaitingSyncReply = false;
     mAwaitingSyncReplyPriority = 0;
 
-    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));
-    }
-
     // 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()
--- a/ipc/glue/MessageChannel.h
+++ b/ipc/glue/MessageChannel.h
@@ -245,17 +245,17 @@ class MessageChannel : HasResultCodes
     void Clear();
 
     // Send OnChannelConnected notification to listeners.
     void DispatchOnChannelConnected();
 
     bool InterruptEventOccurred();
     bool HasPendingEvents();
 
-    void ProcessPendingRequests(int transaction, int prio);
+    void ProcessPendingRequests();
     bool ProcessPendingRequest(const Message &aUrgent);
 
     void MaybeUndeferIncall();
     void EnqueuePendingMessages();
 
     // Executed on the worker thread. Dequeues one pending message.
     bool OnMaybeDequeueOne();
     bool DequeueOne(Message *recvd);
@@ -520,32 +520,26 @@ class MessageChannel : HasResultCodes
     int32_t mNextSeqno;
 
     static bool sIsPumpingMessages;
 
     template<class T>
     class AutoSetValue {
       public:
         explicit AutoSetValue(T &var, const T &newValue)
-          : mVar(var), mPrev(var), mNew(newValue)
+          : mVar(var), mPrev(var)
         {
             mVar = newValue;
         }
         ~AutoSetValue() {
-            // The value may have been zeroed if the transaction was
-            // canceled. In that case we shouldn't return it to its previous
-            // value.
-            if (mVar == mNew) {
-                mVar = mPrev;
-            }
+            mVar = mPrev;
         }
       private:
         T& mVar;
         T mPrev;
-        T mNew;
     };
 
     // Worker thread only.
     bool mAwaitingSyncReply;
     int mAwaitingSyncReplyPriority;
 
     // Set while we are dispatching a synchronous message. Only for use on the
     // worker thread.
@@ -625,16 +619,17 @@ class MessageChannel : HasResultCodes
     // side is responsible for replying to all sync messages sent by the other
     // side when it dispatches the timed out message. The response is always an
     // error.
     //
     // A message is only timed out if it initiated a transaction. This avoids
     // hitting a lot of corner cases with message nesting that we don't really
     // care about.
     int32_t mTimedOutMessageSeqno;
+    int mTimedOutMessagePriority;
 
     // If waiting for the reply to a sync out-message, it will be saved here
     // on the I/O thread and then read and cleared by the worker thread.
     nsAutoPtr<Message> mRecvd;
 
     // If a sync message reply that is an error arrives, we increment this
     // counter rather than storing it in mRecvd.
     size_t mRecvdErrors;