diff --git a/browser/base/content/content.js b/browser/base/content/content.js index ea7ff76c86b3..c1fef6a25c2e 100644 --- a/browser/base/content/content.js +++ b/browser/base/content/content.js @@ -150,12 +150,12 @@ let handleContentContextMenu = function (event) { let customMenuItems = PageMenuChild.build(event.target); let principal = doc.nodePrincipal; - sendRpcMessage("contextmenu", - { editFlags, spellInfo, customMenuItems, addonInfo, - principal, docLocation, charSet, baseURI, referrer, - referrerPolicy, contentType, contentDisposition, - frameOuterWindowID, selectionInfo, disableSetDesktopBg }, - { event, popupNode: event.target }); + sendSyncMessage("contextmenu", + { editFlags, spellInfo, customMenuItems, addonInfo, + principal, docLocation, charSet, baseURI, referrer, + referrerPolicy, contentType, contentDisposition, + frameOuterWindowID, selectionInfo, disableSetDesktopBg }, + { event, popupNode: event.target }); } else { // Break out to the parent window and pass the add-on info along diff --git a/dom/base/test/chrome/cpows_child.js b/dom/base/test/chrome/cpows_child.js index dd58273aa942..440225727ed6 100644 --- a/dom/base/test/chrome/cpows_child.js +++ b/dom/base/test/chrome/cpows_child.js @@ -1,40 +1,38 @@ dump('loaded child cpow test\n'); +content.document.title = "Hello, Kitty"; const Cu = Components.utils; +var done_count = 0; +var is_remote; + (function start() { - [is_remote] = sendRpcMessage("cpows:is_remote"); - - var tests = [ - parent_test, - error_reporting_test, - dom_test, - xray_test, - symbol_test, - compartment_test, - regexp_test, - postmessage_test, - sync_test, - async_test, - rpc_test, - lifetime_test - ]; - - function go() { - if (tests.length == 0) { - sendRpcMessage("cpows:done", {}); - return; + [is_remote] = sendSyncMessage("cpows:is_remote"); + parent_test(); + error_reporting_test(); + dom_test(); + xray_test(); + if (typeof Symbol === "function") { + symbol_test(); } - - var test = tests[0]; - tests.shift(); - test(function() { - go(); + compartment_test(); + regexp_test(); + postmessage_test(); + sync_test(); + async_test(); + rpc_test(); + nested_sync_test(); + // The sync-ness of this call is important, because otherwise + // we tear down the child's document while we are + // still in the async test in the parent. + // This test races with itself to be the final test. + lifetime_test(function() { + done_count++; + if (done_count == 2) + sendSyncMessage("cpows:done", {}); }); } - - go(); -})(); +)(); function ok(condition, message) { dump('condition: ' + condition + ', ' + message + '\n'); @@ -71,7 +69,6 @@ function make_object() let with_null_proto = Object.create(null); - content.document.title = "Hello, Kitty"; return { "data": o, "throwing": throwing, "document": content.document, @@ -87,7 +84,7 @@ function make_json() return { check: "ok" }; } -function parent_test(finish) +function parent_test() { function f(check_func) { let result = check_func(10); @@ -107,39 +104,38 @@ function parent_test(finish) sb.func = func; ok(sb.eval('func()') == 101, "can call parent's function in child"); - finish(); + done_count++; + if (done_count == 2) + sendSyncMessage("cpows:done", {}); }); - sendRpcMessage("cpows:parent_test", {}, {func: f}); + sendSyncMessage("cpows:parent_test", {}, {func: f}); } -function error_reporting_test(finish) { - sendRpcMessage("cpows:error_reporting_test", {}, {}); - finish(); +function error_reporting_test() { + sendSyncMessage("cpows:error_reporting_test", {}, {}); } -function dom_test(finish) +function dom_test() { let element = content.document.createElement("div"); element.id = "it_works"; content.document.body.appendChild(element); - sendRpcMessage("cpows:dom_test", {}, {element: element}); + sendAsyncMessage("cpows:dom_test", {}, {element: element}); Components.utils.schedulePreciseGC(function() { - sendRpcMessage("cpows:dom_test_after_gc"); - finish(); + sendSyncMessage("cpows:dom_test_after_gc"); }); } -function xray_test(finish) +function xray_test() { let element = content.document.createElement("div"); element.wrappedJSObject.foo = "hello"; - sendRpcMessage("cpows:xray_test", {}, {element: element}); - finish(); + sendSyncMessage("cpows:xray_test", {}, {element: element}); } -function symbol_test(finish) +function symbol_test() { let iterator = Symbol.iterator; let named = Symbol.for("cpow-test"); @@ -149,18 +145,16 @@ function symbol_test(finish) [named]: named, }; let test = ['a']; - sendRpcMessage("cpows:symbol_test", {}, {object: object, test: test}); - finish(); + sendSyncMessage("cpows:symbol_test", {}, {object: object, test: test}); } // Parent->Child references should go X->parent.privilegedJunkScope->child.privilegedJunkScope->Y // Child->Parent references should go X->child.privilegedJunkScope->parent.unprivilegedJunkScope->Y -function compartment_test(finish) +function compartment_test() { // This test primarily checks various compartment invariants for CPOWs, and // doesn't make sense to run in-process. if (!is_remote) { - finish(); return; } @@ -184,47 +178,41 @@ function compartment_test(finish) return results; } - sendRpcMessage("cpows:compartment_test", {}, { getUnprivilegedObject: sb.getUnprivilegedObject, - testParentObject: testParentObject }); - finish(); + sendSyncMessage("cpows:compartment_test", {}, { getUnprivilegedObject: sb.getUnprivilegedObject, + testParentObject: testParentObject }); } -function regexp_test(finish) +function regexp_test() { - sendRpcMessage("cpows:regexp_test", {}, { regexp: /myRegExp/g }); - finish(); + sendSyncMessage("cpows:regexp_test", {}, { regexp: /myRegExp/g }); } -function postmessage_test(finish) +function postmessage_test() { - sendRpcMessage("cpows:postmessage_test", {}, { win: content.window }); - finish(); + sendSyncMessage("cpows:postmessage_test", {}, { win: content.window }); } -function sync_test(finish) +function sync_test() { dump('beginning cpow sync test\n'); sync_obj = make_object(); - sendRpcMessage("cpows:sync", + sendSyncMessage("cpows:sync", make_json(), make_object()); - finish(); } -function async_test(finish) +function async_test() { dump('beginning cpow async test\n'); async_obj = make_object(); sendAsyncMessage("cpows:async", make_json(), async_obj); - - addMessageListener("cpows:async_done", finish); } var rpc_obj; -function rpc_test(finish) +function rpc_test() { dump('beginning cpow rpc test\n'); rpc_obj = make_object(); @@ -235,7 +223,26 @@ function rpc_test(finish) sendRpcMessage("cpows:rpc", make_json(), rpc_obj); - finish(); +} + +function nested_sync_test() +{ + dump('beginning cpow nested sync test\n'); + sync_obj = make_object(); + sync_obj.data.reenter = function () { + let caught = false; + try { + sendSyncMessage("cpows:reenter_sync", { }, { }); + } catch (e) { + caught = true; + } + if (!ok(caught, "should not allow nested sync")) + return "fail"; + return "ok"; + } + sendSyncMessage("cpows:nested_sync", + make_json(), + rpc_obj); } function lifetime_test(finish) @@ -250,7 +257,7 @@ function lifetime_test(finish) dump("beginning lifetime test\n"); var obj = {"will_die": {"f": 1}}; - let [result] = sendRpcMessage("cpows:lifetime_test_1", {}, {obj: obj}); + let [result] = sendSyncMessage("cpows:lifetime_test_1", {}, {obj: obj}); ok(result == 10, "got sync result"); ok(obj.wont_die.f == 2, "got reverse CPOW"); obj.will_die = null; @@ -259,6 +266,6 @@ function lifetime_test(finish) ok(obj.wont_die.f == 2, "reverse CPOW still works"); finish(); }); - sendRpcMessage("cpows:lifetime_test_2"); + sendSyncMessage("cpows:lifetime_test_2"); }); } diff --git a/dom/base/test/chrome/cpows_parent.xul b/dom/base/test/chrome/cpows_parent.xul index e9e0b6482ef8..370ba101631a 100644 --- a/dom/base/test/chrome/cpows_parent.xul +++ b/dom/base/test/chrome/cpows_parent.xul @@ -64,7 +64,7 @@ "getOwnPropertyDescriptor.value works"); let obj = new data.ctor(); ok(obj.a === 3, "constructor call"); - is(document.title, "Hello, Kitty", "document node"); + ok(document.title === "Hello, Kitty", "document node"); is(typeof document.cookie, "string", "can get document.cookie"); is(typeof document.defaultView.navigator.userAgent, "string", "can get navigator.userAgent"); @@ -141,7 +141,6 @@ function recvAsyncMessage(message) { testCpowMessage(message); - savedMM.sendAsyncMessage("cpows:async_done"); } function recvSyncMessage(message) { diff --git a/ipc/glue/MessageChannel.cpp b/ipc/glue/MessageChannel.cpp index bbc94769d189..74cae198da4f 100644 --- a/ipc/glue/MessageChannel.cpp +++ b/ipc/glue/MessageChannel.cpp @@ -14,7 +14,6 @@ #include "mozilla/DebugOnly.h" #include "mozilla/Move.h" #include "mozilla/SizePrintfMacros.h" -#include "mozilla/Telemetry.h" #include "nsDebug.h" #include "nsISupportsImpl.h" #include "nsContentUtils.h" @@ -33,31 +32,31 @@ * Terminology: To dispatch a message Foo is to run the RecvFoo code for * it. This is also called "handling" the message. * - * Sync and async messages have priorities while intr messages always have + * Sync messages have priorities while async and intr messages always have * normal priority. The three possible priorities are normal, high, and urgent. * The intended uses of these priorities are: * NORMAL - most messages. * HIGH - CPOW-related messages, which can go in either direction. * URGENT - messages where we don't want to dispatch * incoming CPOWs while waiting for the response. - * Async messages cannot have HIGH priority. * * To avoid jank, the parent process is not allowed to send sync messages of - * normal priority. When a process is waiting for a response to a sync message - * M0, it will dispatch an incoming message M if: + * normal priority. The parent also is not allowed to send urgent messages at + * all. When a process is waiting for a response to a sync message M0, it will + * dispatch an incoming message M if: * 1. M has a higher priority than M0, or * 2. if M has the same priority as M0 and we're in the child, or * 3. if M has the same priority as M0 and it was sent by the other side - * while dispatching M0 (nesting). + while dispatching M0 (nesting). * The idea is that higher priority messages should take precendence, and we * also want to allow nesting. The purpose of rule 2 is to handle a race where * both processes send to each other simultaneously. In this case, we resolve * the race in favor of the parent (so the child dispatches first). * - * Messages satisfy the following properties: + * Sync messages satisfy the following properties: * A. When waiting for a response to a sync message, we won't dispatch any * messages of lower priority. - * B. Messages of the same priority will be dispatched roughly in the + * B. Sync messages of the same priority will be dispatched roughly in the * order they were sent. The exception is when the parent and child send * sync messages to each other simulataneously. In this case, the parent's * message is dispatched first. While it is dispatched, the child may send @@ -66,11 +65,6 @@ * because we pretend that the child's original message wasn't sent until * after the parent's message is finished being dispatched. * - * When waiting for a sync message reply, we dispatch an async message only if - * it has URGENT priority. Normally URGENT async messages are sent only from the - * child. However, the parent can send URGENT async messages when it is creating - * a bridged protocol. - * * Intr messages are blocking but not prioritized. While waiting for an intr * response, all incoming messages are dispatched until a response is * received. Intr messages also can be nested. When two intr messages race with @@ -106,7 +100,7 @@ struct RunnableMethodTraits DebugAbort(__FILE__, __LINE__, #_cond,## __VA_ARGS__); \ } while (0) -static MessageChannel* gParentProcessBlocker; +static bool gParentIsBlocked; namespace mozilla { namespace ipc { @@ -404,10 +398,6 @@ MessageChannel::Clear() // In practice, mListener owns the channel, so the channel gets deleted // before mListener. But just to be safe, mListener is a weak pointer. - if (gParentProcessBlocker == this) { - gParentProcessBlocker = nullptr; - } - mDequeueOneTask->Cancel(); mWorkerLoop = nullptr; @@ -562,42 +552,23 @@ MessageChannel::Send(Message* aMsg) return true; } -class CancelMessage : public IPC::Message -{ -public: - CancelMessage() : - IPC::Message(MSG_ROUTING_NONE, CANCEL_MESSAGE_TYPE, PRIORITY_NORMAL) - { - } - static bool Read(const Message* msg) { - return true; - } - void Log(const std::string& aPrefix, FILE* aOutf) const { - fputs("(special `Cancel' message)", aOutf); - } -}; - bool MessageChannel::MaybeInterceptSpecialIOMessage(const Message& aMsg) { AssertLinkThread(); mMonitor->AssertCurrentThreadOwns(); - if (MSG_ROUTING_NONE == aMsg.routing_id()) { - if (GOODBYE_MESSAGE_TYPE == aMsg.type()) { - // :TODO: Sort out Close() on this side racing with Close() on the - // 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()) { - CancelCurrentTransactionInternal(); - NotifyWorkerThread(); - return true; + if (MSG_ROUTING_NONE == aMsg.routing_id() && + GOODBYE_MESSAGE_TYPE == aMsg.type()) + { + // :TODO: Sort out Close() on this side racing with Close() on the + // other side + mChannelState = ChannelClosing; + if (LoggingEnabled()) { + printf("NOTE: %s process received `Goodbye', closing down\n", + (mSide == ChildSide) ? "child" : "parent"); } + return true; } return false; } @@ -672,7 +643,6 @@ MessageChannel::OnMessageReceivedFromLink(const Message& aMsg) return; } - MOZ_ASSERT(aMsg.transaction_id() == mCurrentTransaction); MOZ_ASSERT(AwaitingSyncReply()); MOZ_ASSERT(!mRecvd); @@ -796,51 +766,9 @@ MessageChannel::ProcessPendingRequests() } } -bool -MessageChannel::WasTransactionCanceled(int transaction, int prio) -{ - if (transaction == mCurrentTransaction) { - 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 msg(aMsg); - // See comment in DispatchSyncMessage. MaybeScriptBlocker scriptBlocker(this, true); @@ -855,7 +783,7 @@ MessageChannel::Send(Message* aMsg, Message* aReply) SyncStackFrame frame(this, false); #endif - CxxStackFrame f(*this, OUT_MESSAGE, msg); + CxxStackFrame f(*this, OUT_MESSAGE, aMsg); MonitorAutoLock lock(*mMonitor); @@ -867,28 +795,10 @@ MessageChannel::Send(Message* aMsg, Message* aReply) return false; } - if (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. - return false; - } - - if (mCurrentTransaction && - (msg->priority() < DispatchingSyncMessagePriority() || - mAwaitingSyncReplyPriority > msg->priority() || - DispatchingSyncMessagePriority() == IPC::Message::PRIORITY_URGENT || - DispatchingAsyncMessagePriority() == IPC::Message::PRIORITY_URGENT)) - { - CancelCurrentTransactionInternal(); - mLink->SendMessage(new CancelMessage()); - } - - IPC_ASSERT(msg->is_sync(), "can only Send() sync messages here"); - IPC_ASSERT(msg->priority() >= DispatchingSyncMessagePriority(), + IPC_ASSERT(aMsg->is_sync(), "can only Send() sync messages here"); + IPC_ASSERT(aMsg->priority() >= DispatchingSyncMessagePriority(), "can't send sync message of a lesser priority than what's being dispatched"); - IPC_ASSERT(AwaitingSyncReplyPriority() <= msg->priority(), + IPC_ASSERT(mAwaitingSyncReplyPriority <= aMsg->priority(), "nested sync message sends must be of increasing priority"); IPC_ASSERT(DispatchingSyncMessagePriority() != IPC::Message::PRIORITY_URGENT, @@ -896,6 +806,8 @@ MessageChannel::Send(Message* aMsg, Message* aReply) IPC_ASSERT(DispatchingAsyncMessagePriority() != IPC::Message::PRIORITY_URGENT, "not allowed to send messages while dispatching urgent messages"); + nsAutoPtr msg(aMsg); + if (!Connected()) { ReportConnectionError("MessageChannel::SendAndWait", msg); return false; @@ -915,17 +827,11 @@ MessageChannel::Send(Message* aMsg, Message* aReply) msg->set_transaction_id(transaction); ProcessPendingRequests(); - if (WasTransactionCanceled(transaction, prio)) { - return false; - } mLink->SendMessage(msg.forget()); while (true) { ProcessPendingRequests(); - if (WasTransactionCanceled(transaction, prio)) { - return false; - } // See if we've received a reply. if (mRecvdErrors) { @@ -946,10 +852,6 @@ MessageChannel::Send(Message* aMsg, Message* aReply) return false; } - if (WasTransactionCanceled(transaction, prio)) { - 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; @@ -1072,7 +974,12 @@ MessageChannel::Call(Message* aMsg, Message* aReply) // If the message is not Interrupt, we can dispatch it as normal. if (!recvd.is_interrupt()) { - DispatchMessage(recvd); + { + AutoEnterTransaction transaction(this, recvd); + MonitorAutoUnlock unlock(*mMonitor); + CxxStackFrame frame(*this, IN_MESSAGE, &recvd); + DispatchMessage(recvd); + } if (!Connected()) { ReportConnectionError("MessageChannel::DispatchMessage"); return false; @@ -1200,7 +1107,14 @@ MessageChannel::ProcessPendingRequest(const Message &aUrgent) // to save the reply. nsAutoPtr savedReply(mRecvd.forget()); - DispatchMessage(aUrgent); + { + // In order to send the parent RPC messages and guarantee it will + // wake up, we must re-use its transaction. + AutoEnterTransaction transaction(this, aUrgent); + + MonitorAutoUnlock unlock(*mMonitor); + DispatchMessage(aUrgent); + } if (!Connected()) { ReportConnectionError("MessageChannel::ProcessPendingRequest"); return false; @@ -1257,10 +1171,16 @@ MessageChannel::OnMaybeDequeueOne() return false; } - // We should not be in a transaction yet if we're not blocked. - MOZ_ASSERT(mCurrentTransaction == 0); - DispatchMessage(recvd); + { + // We should not be in a transaction yet if we're not blocked. + MOZ_ASSERT(mCurrentTransaction == 0); + AutoEnterTransaction transaction(this, recvd); + MonitorAutoUnlock unlock(*mMonitor); + + CxxStackFrame frame(*this, IN_MESSAGE, &recvd); + DispatchMessage(recvd); + } return true; } @@ -1270,43 +1190,21 @@ MessageChannel::DispatchMessage(const Message &aMsg) Maybe nojsapi; if (ScriptSettingsInitialized() && NS_IsMainThread()) nojsapi.emplace(); - - nsAutoPtr reply; - - { - AutoEnterTransaction transaction(this, aMsg); - - int id = aMsg.transaction_id(); - MOZ_ASSERT_IF(aMsg.is_sync(), id == mCurrentTransaction); - - { - MonitorAutoUnlock unlock(*mMonitor); - CxxStackFrame frame(*this, IN_MESSAGE, &aMsg); - - if (aMsg.is_sync()) - DispatchSyncMessage(aMsg, *getter_Transfers(reply)); - else if (aMsg.is_interrupt()) - DispatchInterruptMessage(aMsg, 0); - else - DispatchAsyncMessage(aMsg); - } - - if (mCurrentTransaction != id) { - // The transaction has been canceled. Don't send a reply. - reply = nullptr; - } - } - - if (reply && ChannelConnected == mChannelState) { - mLink->SendMessage(reply.forget()); - } + if (aMsg.is_sync()) + DispatchSyncMessage(aMsg); + else if (aMsg.is_interrupt()) + DispatchInterruptMessage(aMsg, 0); + else + DispatchAsyncMessage(aMsg); } void -MessageChannel::DispatchSyncMessage(const Message& aMsg, Message*& aReply) +MessageChannel::DispatchSyncMessage(const Message& aMsg) { AssertWorkerThread(); + nsAutoPtr reply; + int prio = aMsg.priority(); // We don't want to run any code that might run a nested event loop here, so @@ -1316,13 +1214,13 @@ MessageChannel::DispatchSyncMessage(const Message& aMsg, Message*& aReply) MOZ_ASSERT_IF(prio > IPC::Message::PRIORITY_NORMAL, NS_IsMainThread()); MaybeScriptBlocker scriptBlocker(this, prio > IPC::Message::PRIORITY_NORMAL); - IPC_ASSERT(prio >= DispatchingSyncMessagePriority(), + IPC_ASSERT(prio >= mDispatchingSyncMessagePriority, "priority inversion while dispatching sync message"); IPC_ASSERT(prio >= mAwaitingSyncReplyPriority, "dispatching a message of lower priority while waiting for a response"); - MessageChannel* dummy; - MessageChannel*& blockingVar = ShouldBlockScripts() ? gParentProcessBlocker : dummy; + bool dummy; + bool& blockingVar = ShouldBlockScripts() ? gParentIsBlocked : dummy; Result rv; if (mTimedOutMessageSeqno && mTimedOutMessagePriority >= prio) { @@ -1341,21 +1239,26 @@ MessageChannel::DispatchSyncMessage(const Message& aMsg, Message*& aReply) // for a response to its urgent message). rv = MsgNotAllowed; } else { - AutoSetValue blocked(blockingVar, this); + AutoSetValue blocked(blockingVar, true); AutoSetValue sync(mDispatchingSyncMessage, true); AutoSetValue prioSet(mDispatchingSyncMessagePriority, prio); - rv = mListener->OnMessageReceived(aMsg, aReply); + rv = mListener->OnMessageReceived(aMsg, *getter_Transfers(reply)); } if (!MaybeHandleError(rv, aMsg, "DispatchSyncMessage")) { - aReply = new Message(); - aReply->set_sync(); - aReply->set_priority(aMsg.priority()); - aReply->set_reply(); - aReply->set_reply_error(); + reply = new Message(); + reply->set_sync(); + reply->set_priority(aMsg.priority()); + reply->set_reply(); + reply->set_reply_error(); + } + reply->set_seqno(aMsg.seqno()); + reply->set_transaction_id(aMsg.transaction_id()); + + MonitorAutoLock lock(*mMonitor); + if (ChannelConnected == mChannelState) { + mLink->SendMessage(reply.forget()); } - aReply->set_seqno(aMsg.seqno()); - aReply->set_transaction_id(aMsg.transaction_id()); } void @@ -2012,38 +1915,10 @@ MessageChannel::GetTopmostMessageRoutingId() const return frame.GetRoutingId(); } -void -MessageChannel::CancelCurrentTransactionInternal() +bool +ParentProcessIsBlocked() { - // 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. - - MOZ_ASSERT(!mCurrentTransaction); - mCurrentTransaction = 0; -} - -void -MessageChannel::CancelCurrentTransaction() -{ - MonitorAutoLock lock(*mMonitor); - CancelCurrentTransactionInternal(); - mLink->SendMessage(new CancelMessage()); -} - -void -CancelCPOWs() -{ - if (gParentProcessBlocker) { - mozilla::Telemetry::Accumulate(mozilla::Telemetry::IPC_TRANSACTION_CANCEL, true); - gParentProcessBlocker->CancelCurrentTransaction(); - } + return gParentIsBlocked; } } // ipc diff --git a/ipc/glue/MessageChannel.h b/ipc/glue/MessageChannel.h index 66da5da25c12..dff329add02c 100644 --- a/ipc/glue/MessageChannel.h +++ b/ipc/glue/MessageChannel.h @@ -135,8 +135,6 @@ class MessageChannel : HasResultCodes return !mCxxStackFrames.empty(); } - void CancelCurrentTransaction(); - /** * This function is used by hang annotation code to determine which IPDL * actor is highest in the call stack at the time of the hang. It should @@ -244,7 +242,7 @@ class MessageChannel : HasResultCodes // DispatchMessage will route to one of these functions depending on the // protocol type of the message. - void DispatchSyncMessage(const Message &aMsg, Message*& aReply); + void DispatchSyncMessage(const Message &aMsg); void DispatchUrgentMessage(const Message &aMsg); void DispatchAsyncMessage(const Message &aMsg); void DispatchRPCMessage(const Message &aMsg); @@ -267,8 +265,6 @@ class MessageChannel : HasResultCodes bool ShouldContinueFromTimeout(); - 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 @@ -405,7 +401,6 @@ class MessageChannel : HasResultCodes // Tell the IO thread to close the channel and wait for it to ACK. void SynchronouslyClose(); - bool WasTransactionCanceled(int transaction, int prio); bool ShouldDeferMessage(const Message& aMsg); void OnMessageReceivedFromLink(const Message& aMsg); void OnChannelErrorFromLink(); @@ -551,21 +546,17 @@ class MessageChannel : HasResultCodes class AutoEnterTransaction { - public: + public: explicit AutoEnterTransaction(MessageChannel *aChan, int32_t aMsgSeqno) : mChan(aChan), - mNewTransaction(0), mOldTransaction(mChan->mCurrentTransaction) { mChan->mMonitor->AssertCurrentThreadOwns(); - if (mChan->mCurrentTransaction == 0) { - mNewTransaction = aMsgSeqno; + if (mChan->mCurrentTransaction == 0) mChan->mCurrentTransaction = aMsgSeqno; - } } explicit AutoEnterTransaction(MessageChannel *aChan, const Message &aMessage) : mChan(aChan), - mNewTransaction(aMessage.transaction_id()), mOldTransaction(mChan->mCurrentTransaction) { mChan->mMonitor->AssertCurrentThreadOwns(); @@ -579,14 +570,12 @@ class MessageChannel : HasResultCodes } ~AutoEnterTransaction() { mChan->mMonitor->AssertCurrentThreadOwns(); - if (mChan->mCurrentTransaction == mNewTransaction) { - mChan->mCurrentTransaction = mOldTransaction; - } + mChan->mCurrentTransaction = mOldTransaction; } private: MessageChannel *mChan; - int32_t mNewTransaction, mOldTransaction; + int32_t mOldTransaction; }; // If a sync message times out, we store its sequence number here. Any @@ -732,8 +721,8 @@ class MessageChannel : HasResultCodes int32_t mPeerPid; }; -void -CancelCPOWs(); +bool +ParentProcessIsBlocked(); } // namespace ipc } // namespace mozilla diff --git a/ipc/glue/ProtocolUtils.cpp b/ipc/glue/ProtocolUtils.cpp index e26e00adb066..ee5995602adb 100644 --- a/ipc/glue/ProtocolUtils.cpp +++ b/ipc/glue/ProtocolUtils.cpp @@ -45,9 +45,6 @@ IToplevelProtocol::IToplevelProtocol(ProtocolId aProtoId) , mProtocolId(aProtoId) , mTrans(nullptr) { -#ifdef DEBUG - StaticMutexAutoLock al(gProtocolMutex); -#endif } IToplevelProtocol::~IToplevelProtocol() diff --git a/ipc/glue/ProtocolUtils.h b/ipc/glue/ProtocolUtils.h index fa7355f54f37..4a7b24467a69 100644 --- a/ipc/glue/ProtocolUtils.h +++ b/ipc/glue/ProtocolUtils.h @@ -38,11 +38,10 @@ namespace { // protocol 0. Oops! We can get away with this until protocol 0 // starts approaching its 65,536th message. enum { - CHANNEL_OPENED_MESSAGE_TYPE = kuint16max - 6, - SHMEM_DESTROYED_MESSAGE_TYPE = kuint16max - 5, - SHMEM_CREATED_MESSAGE_TYPE = kuint16max - 4, - GOODBYE_MESSAGE_TYPE = kuint16max - 3, - CANCEL_MESSAGE_TYPE = kuint16max - 2, + CHANNEL_OPENED_MESSAGE_TYPE = kuint16max - 5, + SHMEM_DESTROYED_MESSAGE_TYPE = kuint16max - 4, + SHMEM_CREATED_MESSAGE_TYPE = kuint16max - 3, + GOODBYE_MESSAGE_TYPE = kuint16max - 2 // kuint16max - 1 is used by ipc_channel.h. }; diff --git a/ipc/ipdl/test/cxx/PTestCancel.ipdl b/ipc/ipdl/test/cxx/PTestCancel.ipdl deleted file mode 100644 index f7622e580343..000000000000 --- a/ipc/ipdl/test/cxx/PTestCancel.ipdl +++ /dev/null @@ -1,36 +0,0 @@ -namespace mozilla { -namespace _ipdltest { - -prio(normal upto high) sync protocol PTestCancel -{ -// Test1 -child: - prio(high) sync Test1_1(); -parent: - async Done1(); - -// Test2 -child: - async Start2(); - prio(high) sync Test2_2(); -parent: - sync Test2_1(); - -// Test3 -child: - prio(high) sync Test3_1(); -parent: - async Start3(); - prio(high) sync Test3_2(); - -parent: - async Done(); - -child: - prio(high) sync CheckChild() returns (uint32_t reply); -parent: - prio(high) sync CheckParent() returns (uint32_t reply); -}; - -} // namespace _ipdltest -} // namespace mozilla diff --git a/ipc/ipdl/test/cxx/PTestHighestPrio.ipdl b/ipc/ipdl/test/cxx/PTestHighestPrio.ipdl index 692b7e83b1d5..ce3a41b54656 100644 --- a/ipc/ipdl/test/cxx/PTestHighestPrio.ipdl +++ b/ipc/ipdl/test/cxx/PTestHighestPrio.ipdl @@ -5,7 +5,7 @@ prio(normal upto urgent) sync protocol PTestHighestPrio { parent: prio(urgent) async Msg1(); - prio(high) sync Msg2(); + sync Msg2(); prio(urgent) async Msg3(); prio(urgent) sync Msg4(); diff --git a/ipc/ipdl/test/cxx/PTestRPC.ipdl b/ipc/ipdl/test/cxx/PTestRPC.ipdl index dc214fcc2045..ba6c36b0568e 100644 --- a/ipc/ipdl/test/cxx/PTestRPC.ipdl +++ b/ipc/ipdl/test/cxx/PTestRPC.ipdl @@ -8,6 +8,8 @@ parent: prio(high) sync Test1_InnerEvent() returns (uint32_t result); async Test2_Start(); prio(high) sync Test2_OutOfOrder(); + sync Test3_Start() returns (uint32_t result); + prio(high) sync Test3_InnerEvent() returns (uint32_t result); child: async Start(); @@ -15,6 +17,7 @@ child: prio(high) sync Test1_NoReenter() returns (uint32_t result); prio(high) sync Test2_FirstUrgent(); prio(high) sync Test2_SecondUrgent(); + prio(high) sync Test3_WakeUp() returns (uint32_t result); }; } // namespace _ipdltest diff --git a/ipc/ipdl/test/cxx/PTestUrgency.ipdl b/ipc/ipdl/test/cxx/PTestUrgency.ipdl index d1ae87e6fd14..dcc725333646 100644 --- a/ipc/ipdl/test/cxx/PTestUrgency.ipdl +++ b/ipc/ipdl/test/cxx/PTestUrgency.ipdl @@ -4,7 +4,7 @@ namespace _ipdltest { prio(normal upto high) sync protocol PTestUrgency { parent: - prio(high) sync Test1() returns (uint32_t result); + sync Test1() returns (uint32_t result); async Test2(); sync Test3() returns (uint32_t result); sync FinalTest_Begin(); diff --git a/ipc/ipdl/test/cxx/TestCancel.cpp b/ipc/ipdl/test/cxx/TestCancel.cpp deleted file mode 100644 index 160bdc71c8ec..000000000000 --- a/ipc/ipdl/test/cxx/TestCancel.cpp +++ /dev/null @@ -1,175 +0,0 @@ -#include "TestCancel.h" - -#include "IPDLUnitTests.h" // fail etc. - -template<> -struct RunnableMethodTraits -{ - static void RetainCallee(mozilla::_ipdltest::TestCancelParent* obj) { } - static void ReleaseCallee(mozilla::_ipdltest::TestCancelParent* obj) { } -}; - -namespace mozilla { -namespace _ipdltest { - -//----------------------------------------------------------------------------- -// parent - -TestCancelParent::TestCancelParent() -{ - MOZ_COUNT_CTOR(TestCancelParent); -} - -TestCancelParent::~TestCancelParent() -{ - MOZ_COUNT_DTOR(TestCancelParent); -} - -void -TestCancelParent::Main() -{ - if (SendTest1_1()) - fail("sending Test1_1"); - - uint32_t value = 0; - if (!SendCheckChild(&value)) - fail("Test1 CheckChild"); - - if (value != 12) - fail("Test1 CheckChild reply"); -} - -bool -TestCancelParent::RecvDone1() -{ - if (!SendStart2()) - fail("sending Start2"); - - return true; -} - -bool -TestCancelParent::RecvTest2_1() -{ - if (SendTest2_2()) - fail("sending Test2_2"); - - return true; -} - -bool -TestCancelParent::RecvStart3() -{ - if (SendTest3_1()) - fail("sending Test3_1"); - - uint32_t value = 0; - if (!SendCheckChild(&value)) - fail("Test1 CheckChild"); - - if (value != 12) - fail("Test1 CheckChild reply"); - - return true; -} - -bool -TestCancelParent::RecvTest3_2() -{ - GetIPCChannel()->CancelCurrentTransaction(); - return true; -} - -bool -TestCancelParent::RecvDone() -{ - MessageLoop::current()->PostTask( - FROM_HERE, NewRunnableMethod(this, &TestCancelParent::Close)); - return true; -} - -bool -TestCancelParent::RecvCheckParent(uint32_t *reply) -{ - *reply = 12; - return true; -} - -//----------------------------------------------------------------------------- -// child - -bool -TestCancelChild::RecvTest1_1() -{ - GetIPCChannel()->CancelCurrentTransaction(); - - uint32_t value = 0; - if (!SendCheckParent(&value)) - fail("Test1 CheckParent"); - - if (value != 12) - fail("Test1 CheckParent reply"); - - if (!SendDone1()) - fail("Test1 CheckParent"); - - return true; -} - -bool -TestCancelChild::RecvStart2() -{ - if (!SendTest2_1()) - fail("sending Test2_1"); - - if (!SendStart3()) - fail("sending Start3"); - - return true; -} - -bool -TestCancelChild::RecvTest2_2() -{ - GetIPCChannel()->CancelCurrentTransaction(); - return true; -} - -bool -TestCancelChild::RecvTest3_1() -{ - if (SendTest3_2()) - fail("sending Test3_2"); - - uint32_t value = 0; - if (!SendCheckParent(&value)) - fail("Test1 CheckParent"); - - if (value != 12) - fail("Test1 CheckParent reply"); - - if (!SendDone()) - fail("sending Done"); - - return true; -} - -bool -TestCancelChild::RecvCheckChild(uint32_t *reply) -{ - *reply = 12; - return true; -} - -TestCancelChild::TestCancelChild() -{ - MOZ_COUNT_CTOR(TestCancelChild); -} - -TestCancelChild::~TestCancelChild() -{ - MOZ_COUNT_DTOR(TestCancelChild); -} - -} // namespace _ipdltest -} // namespace mozilla diff --git a/ipc/ipdl/test/cxx/TestCancel.h b/ipc/ipdl/test/cxx/TestCancel.h deleted file mode 100644 index 8cd51acd1670..000000000000 --- a/ipc/ipdl/test/cxx/TestCancel.h +++ /dev/null @@ -1,66 +0,0 @@ -#ifndef mozilla__ipdltest_TestCancel_h -#define mozilla__ipdltest_TestCancel_h 1 - -#include "mozilla/_ipdltest/IPDLUnitTests.h" - -#include "mozilla/_ipdltest/PTestCancelParent.h" -#include "mozilla/_ipdltest/PTestCancelChild.h" - -namespace mozilla { -namespace _ipdltest { - - -class TestCancelParent : - public PTestCancelParent -{ -public: - TestCancelParent(); - virtual ~TestCancelParent(); - - static bool RunTestInProcesses() { return true; } - static bool RunTestInThreads() { return false; } - - void Main(); - - virtual bool RecvDone1() override; - virtual bool RecvTest2_1() override; - virtual bool RecvStart3() override; - virtual bool RecvTest3_2() override; - virtual bool RecvDone() override; - - virtual bool RecvCheckParent(uint32_t *reply) override; - - virtual void ActorDestroy(ActorDestroyReason why) override - { - passed("ok"); - QuitParent(); - } -}; - - -class TestCancelChild : - public PTestCancelChild -{ -public: - TestCancelChild(); - virtual ~TestCancelChild(); - - virtual bool RecvTest1_1() override; - virtual bool RecvStart2() override; - virtual bool RecvTest2_2() override; - virtual bool RecvTest3_1() override; - - virtual bool RecvCheckChild(uint32_t *reply) override; - - virtual void ActorDestroy(ActorDestroyReason why) override - { - QuitChild(); - } -}; - - -} // namespace _ipdltest -} // namespace mozilla - - -#endif // ifndef mozilla__ipdltest_TestCancel_h diff --git a/ipc/ipdl/test/cxx/TestRPC.cpp b/ipc/ipdl/test/cxx/TestRPC.cpp index 2dfc51d33b5c..b92088475acb 100644 --- a/ipc/ipdl/test/cxx/TestRPC.cpp +++ b/ipc/ipdl/test/cxx/TestRPC.cpp @@ -83,6 +83,22 @@ TestRPCParent::RecvTest2_OutOfOrder() return true; } +bool +TestRPCParent::RecvTest3_Start(uint32_t* aResult) +{ + if (!SendTest3_WakeUp(aResult)) + fail("SendTest3_WakeUp"); + + return true; +} + +bool +TestRPCParent::RecvTest3_InnerEvent(uint32_t* aResult) +{ + *aResult = 200; + return true; +} + //----------------------------------------------------------------------------- // child @@ -112,6 +128,12 @@ TestRPCChild::RecvStart() if (!SendTest2_OutOfOrder()) fail("SendTest2_OutOfOrder"); + result = 0; + if (!SendTest3_Start(&result)) + fail("SendTest3_Start"); + if (result != 200) + fail("Wrong result (expected 200)"); + Close(); return true; } @@ -148,5 +170,14 @@ TestRPCChild::RecvTest2_SecondUrgent() return true; } +bool +TestRPCChild::RecvTest3_WakeUp(uint32_t* aResult) +{ + if (!SendTest3_InnerEvent(aResult)) + fail("SendTest3_InnerEvent"); + + return true; +} + } // namespace _ipdltest } // namespace mozilla diff --git a/ipc/ipdl/test/cxx/TestRPC.h b/ipc/ipdl/test/cxx/TestRPC.h index 0a11b3e76153..71a56266c6dc 100644 --- a/ipc/ipdl/test/cxx/TestRPC.h +++ b/ipc/ipdl/test/cxx/TestRPC.h @@ -26,6 +26,8 @@ public: bool RecvTest1_InnerEvent(uint32_t* aResult) override; bool RecvTest2_Start() override; bool RecvTest2_OutOfOrder() override; + bool RecvTest3_Start(uint32_t* aResult) override; + bool RecvTest3_InnerEvent(uint32_t* aResult) override; virtual void ActorDestroy(ActorDestroyReason why) override { @@ -57,6 +59,7 @@ public: bool RecvTest1_NoReenter(uint32_t* aResult) override; bool RecvTest2_FirstUrgent() override; bool RecvTest2_SecondUrgent() override; + bool RecvTest3_WakeUp(uint32_t* aResult) override; virtual void ActorDestroy(ActorDestroyReason why) override { diff --git a/ipc/ipdl/test/cxx/moz.build b/ipc/ipdl/test/cxx/moz.build index 33fd41a038ed..8e1e53723610 100644 --- a/ipc/ipdl/test/cxx/moz.build +++ b/ipc/ipdl/test/cxx/moz.build @@ -17,7 +17,6 @@ SOURCES += [ 'TestActorPunning.cpp', 'TestBadActor.cpp', 'TestBridgeMain.cpp', - 'TestCancel.cpp', 'TestCrashCleanup.cpp', 'TestDataStructures.cpp', 'TestDesc.cpp', @@ -70,7 +69,6 @@ IPDL_SOURCES += [ 'PTestBridgeMain.ipdl', 'PTestBridgeMainSub.ipdl', 'PTestBridgeSub.ipdl', - 'PTestCancel.ipdl', 'PTestCrashCleanup.ipdl', 'PTestDataStructures.ipdl', 'PTestDataStructuresCommon.ipdlh', diff --git a/js/ipc/WrapperOwner.cpp b/js/ipc/WrapperOwner.cpp index f88aa5634e5f..f515174f91c0 100644 --- a/js/ipc/WrapperOwner.cpp +++ b/js/ipc/WrapperOwner.cpp @@ -1000,7 +1000,7 @@ WrapperOwner::ActorDestroy(ActorDestroyReason why) bool WrapperOwner::ipcfail(JSContext* cx) { - JS_ReportError(cx, "cross-process JS call failed"); + JS_ReportError(cx, "child process crashed or timedout"); return false; } diff --git a/toolkit/components/telemetry/Histograms.json b/toolkit/components/telemetry/Histograms.json index bb23ec54ed9d..fc6dbb8a4da7 100644 --- a/toolkit/components/telemetry/Histograms.json +++ b/toolkit/components/telemetry/Histograms.json @@ -4843,12 +4843,6 @@ "keyed" : true, "description" : "Exceptions thrown by add-ons" }, - "IPC_TRANSACTION_CANCEL": { - "alert_emails": ["billm@mozilla.com"], - "expires_in_version": "never", - "kind": "boolean", - "description": "True when an IPC transaction is canceled" - }, "MISBEHAVING_ADDONS_CPOW_TIME_MS": { "expires_in_version": "never", "kind": "exponential", diff --git a/toolkit/mozapps/extensions/test/xpinstall/head.js b/toolkit/mozapps/extensions/test/xpinstall/head.js index 76cd63ab8bd8..1bbf2e1da839 100644 --- a/toolkit/mozapps/extensions/test/xpinstall/head.js +++ b/toolkit/mozapps/extensions/test/xpinstall/head.js @@ -287,17 +287,15 @@ var Harness = { if (this.finalContentEvent && !this.waitingForEvent) { this.waitingForEvent = true; info("Waiting for " + this.finalContentEvent); - let mm = gBrowser.selectedBrowser.messageManager; - mm.loadFrameScript(`data:,content.addEventListener("${this.finalContentEvent}", () => { sendAsyncMessage("Test:GotNewInstallEvent"); });`, false); let win = gBrowser.contentWindow; let listener = () => { info("Saw " + this.finalContentEvent); - mm.removeMessageListener("Test:GotNewInstallEvent", listener); + win.removeEventListener(this.finalContentEvent, listener, false); this.waitingForEvent = false; if (this.pendingCount == 0) this.endTest(); } - mm.addMessageListener("Test:GotNewInstallEvent", listener); + win.addEventListener(this.finalContentEvent, listener, false); } }, diff --git a/widget/windows/nsWindow.cpp b/widget/windows/nsWindow.cpp index a644a8e2d633..fc735f9b1cee 100644 --- a/widget/windows/nsWindow.cpp +++ b/widget/windows/nsWindow.cpp @@ -4344,7 +4344,7 @@ inline static mozilla::HangMonitor::ActivityType ActivityTypeForMessage(UINT msg // and http://msdn.microsoft.com/en-us/library/ms633573%28VS.85%29.aspx LRESULT CALLBACK nsWindow::WindowProc(HWND hWnd, UINT msg, WPARAM wParam, LPARAM lParam) { - ipc::CancelCPOWs(); + MOZ_RELEASE_ASSERT(!ipc::ParentProcessIsBlocked()); HangMonitor::NotifyActivity(ActivityTypeForMessage(msg)); diff --git a/xpcom/threads/nsThread.cpp b/xpcom/threads/nsThread.cpp index b0d2b70cd0c2..acfd6cd3eba7 100644 --- a/xpcom/threads/nsThread.cpp +++ b/xpcom/threads/nsThread.cpp @@ -738,9 +738,8 @@ nsThread::ProcessNextEvent(bool aMayWait, bool* aResult) #if !defined(MOZILLA_XPCOMRT_API) // If we're on the main thread, we shouldn't be dispatching CPOWs. - if (mIsMainThread == MAIN_THREAD) { - ipc::CancelCPOWs(); - } + MOZ_RELEASE_ASSERT(mIsMainThread != MAIN_THREAD || + !ipc::ParentProcessIsBlocked()); #endif // !defined(MOZILLA_XPCOMRT_API) if (NS_WARN_IF(PR_GetCurrentThread() != mThread)) {