From 560edcbeffcf39fbd6cf73bd3e3affeb2733b8ce Mon Sep 17 00:00:00 2001 From: David Anderson Date: Tue, 2 Jul 2013 11:22:18 -0700 Subject: [PATCH] Remove unused run-to-completion feature in IPDL (bug 876989, r=cjones). --- dom/ipc/ContentParent.cpp | 44 -------- dom/ipc/ContentParent.h | 2 - ipc/glue/ProtocolUtils.h | 6 +- ipc/glue/RPCChannel.cpp | 136 +--------------------- ipc/glue/RPCChannel.h | 32 ------ ipc/glue/WindowsMessageLoop.cpp | 2 +- ipc/ipdl/ipdl/lower.py | 16 --- ipc/ipdl/test/cxx/Makefile.in | 1 - ipc/ipdl/test/cxx/PTestBlockChild.ipdl | 51 --------- ipc/ipdl/test/cxx/TestBlockChild.cpp | 149 ------------------------- ipc/ipdl/test/cxx/TestBlockChild.h | 89 --------------- ipc/ipdl/test/cxx/ipdl.mk | 1 - ipc/testshell/TestShellParent.cpp | 3 +- js/ipc/ContextWrapperParent.h | 8 +- 14 files changed, 8 insertions(+), 532 deletions(-) delete mode 100644 ipc/ipdl/test/cxx/PTestBlockChild.ipdl delete mode 100644 ipc/ipdl/test/cxx/TestBlockChild.cpp delete mode 100644 ipc/ipdl/test/cxx/TestBlockChild.h diff --git a/dom/ipc/ContentParent.cpp b/dom/ipc/ContentParent.cpp index 27c2dbb1567..b363472d8e2 100644 --- a/dom/ipc/ContentParent.cpp +++ b/dom/ipc/ContentParent.cpp @@ -905,8 +905,6 @@ ContentParent::ActorDestroy(ActorDestroyReason why) threadInt(do_QueryInterface(NS_GetCurrentThread())); if (threadInt) threadInt->RemoveObserver(this); - if (mRunToCompletionDepth) - mRunToCompletionDepth = 0; MarkAsDead(); @@ -1039,8 +1037,6 @@ ContentParent::ContentParent(mozIApplication* aApp, , mOSPrivileges(aOSPrivileges) , mChildID(gContentChildID++) , mGeolocationWatchID(-1) - , mRunToCompletionDepth(0) - , mShouldCallUnblockChild(false) , mForceKillTask(nullptr) , mNumDestroyingTabs(0) , mIsAlive(true) @@ -2051,32 +2047,6 @@ ContentParent::RecvPSpeechSynthesisConstructor(PSpeechSynthesisParent* aActor) #endif } -void -ContentParent::ReportChildAlreadyBlocked() -{ - if (!mRunToCompletionDepth) { -#ifdef DEBUG - printf("Running to completion...\n"); -#endif - mRunToCompletionDepth = 1; - mShouldCallUnblockChild = false; - } -} - -bool -ContentParent::RequestRunToCompletion() -{ - if (!mRunToCompletionDepth && - BlockChild()) { -#ifdef DEBUG - printf("Running to completion...\n"); -#endif - mRunToCompletionDepth = 1; - mShouldCallUnblockChild = true; - } - return !!mRunToCompletionDepth; -} - bool ContentParent::RecvStartVisitedQuery(const URIParams& aURI) { @@ -2245,9 +2215,6 @@ ContentParent::OnProcessNextEvent(nsIThreadInternal *thread, bool mayWait, uint32_t recursionDepth) { - if (mRunToCompletionDepth) - ++mRunToCompletionDepth; - return NS_OK; } @@ -2256,17 +2223,6 @@ NS_IMETHODIMP ContentParent::AfterProcessNextEvent(nsIThreadInternal *thread, uint32_t recursionDepth) { - if (mRunToCompletionDepth && - !--mRunToCompletionDepth) { -#ifdef DEBUG - printf("... ran to completion.\n"); -#endif - if (mShouldCallUnblockChild) { - mShouldCallUnblockChild = false; - UnblockChild(); - } - } - return NS_OK; } diff --git a/dom/ipc/ContentParent.h b/dom/ipc/ContentParent.h index 177d07a5cf5..538a53f45e1 100644 --- a/dom/ipc/ContentParent.h +++ b/dom/ipc/ContentParent.h @@ -414,8 +414,6 @@ private: uint64_t mChildID; int32_t mGeolocationWatchID; - int mRunToCompletionDepth; - bool mShouldCallUnblockChild; // This is a cache of all of the memory reporters // registered in the child process. To update this, one diff --git a/ipc/glue/ProtocolUtils.h b/ipc/glue/ProtocolUtils.h index f3d593de31f..6e01ad602d3 100644 --- a/ipc/glue/ProtocolUtils.h +++ b/ipc/glue/ProtocolUtils.h @@ -29,10 +29,8 @@ 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 - 7, - SHMEM_DESTROYED_MESSAGE_TYPE = kuint16max - 6, - UNBLOCK_CHILD_MESSAGE_TYPE = kuint16max - 5, - BLOCK_CHILD_MESSAGE_TYPE = kuint16max - 4, + CHANNEL_OPENED_MESSAGE_TYPE = kuint16max - 5, + SHMEM_DESTROYED_MESSAGE_TYPE = kuint16max - 4, SHMEM_CREATED_MESSAGE_TYPE = kuint16max - 3, GOODBYE_MESSAGE_TYPE = kuint16max - 2 diff --git a/ipc/glue/RPCChannel.cpp b/ipc/glue/RPCChannel.cpp index 63eb3f7f027..fe545df8d99 100644 --- a/ipc/glue/RPCChannel.cpp +++ b/ipc/glue/RPCChannel.cpp @@ -28,32 +28,6 @@ struct RunnableMethodTraits }; -namespace -{ - -// Async (from the sending side's perspective) -class BlockChildMessage : public IPC::Message -{ -public: - enum { ID = BLOCK_CHILD_MESSAGE_TYPE }; - BlockChildMessage() : - Message(MSG_ROUTING_NONE, ID, IPC::Message::PRIORITY_NORMAL) - { } -}; - -// Async -class UnblockChildMessage : public IPC::Message -{ -public: - enum { ID = UNBLOCK_CHILD_MESSAGE_TYPE }; - UnblockChildMessage() : - Message(MSG_ROUTING_NONE, ID, IPC::Message::PRIORITY_NORMAL) - { } -}; - -} // namespace - - namespace mozilla { namespace ipc { @@ -64,7 +38,6 @@ RPCChannel::RPCChannel(RPCListener* aListener) mOutOfTurnReplies(), mDeferred(), mRemoteStackDepthGuess(0), - mBlockedOnParent(false), mSawRPCOutMsg(false) { MOZ_COUNT_CTOR(RPCChannel); @@ -503,113 +476,6 @@ RPCChannel::DispatchIncall(const Message& call) } } -bool -RPCChannel::BlockChild() -{ - AssertWorkerThread(); - - if (mChild) - NS_RUNTIMEABORT("child tried to block parent"); - - MonitorAutoLock lock(*mMonitor); - SendSpecialMessage(new BlockChildMessage()); - return true; -} - -bool -RPCChannel::UnblockChild() -{ - AssertWorkerThread(); - - if (mChild) - NS_RUNTIMEABORT("child tried to unblock parent"); - - MonitorAutoLock lock(*mMonitor); - SendSpecialMessage(new UnblockChildMessage()); - return true; -} - -bool -RPCChannel::OnSpecialMessage(uint16_t id, const Message& msg) -{ - AssertWorkerThread(); - - switch (id) { - case BLOCK_CHILD_MESSAGE_TYPE: - BlockOnParent(); - return true; - - case UNBLOCK_CHILD_MESSAGE_TYPE: - UnblockFromParent(); - return true; - - default: - return SyncChannel::OnSpecialMessage(id, msg); - } -} - -void -RPCChannel::BlockOnParent() -{ - AssertWorkerThread(); - - if (!mChild) - NS_RUNTIMEABORT("child tried to block parent"); - - MonitorAutoLock lock(*mMonitor); - - if (mBlockedOnParent || AwaitingSyncReply() || 0 < StackDepth()) - NS_RUNTIMEABORT("attempt to block child when it's already blocked"); - - mBlockedOnParent = true; - do { - // XXX this dispatch loop shares some similarities with the - // one in Call(), but the logic is simpler and different - // enough IMHO to warrant its own dispatch loop - while (Connected() && mPending.empty() && mBlockedOnParent) { - WaitForNotify(); - } - - if (!Connected()) { - mBlockedOnParent = false; - ReportConnectionError("RPCChannel"); - break; - } - - if (!mPending.empty()) { - Message recvd = mPending.front(); - mPending.pop_front(); - - MonitorAutoUnlock unlock(*mMonitor); - - CxxStackFrame f(*this, IN_MESSAGE, &recvd); - if (recvd.is_rpc()) { - // stack depth must be 0 here - Incall(recvd, 0); - } - else if (recvd.is_sync()) { - SyncChannel::OnDispatchMessage(recvd); - } - else { - AsyncChannel::OnDispatchMessage(recvd); - } - } - } while (mBlockedOnParent); - - EnqueuePendingMessages(); -} - -void -RPCChannel::UnblockFromParent() -{ - AssertWorkerThread(); - - if (!mChild) - NS_RUNTIMEABORT("child tried to block parent"); - MonitorAutoLock lock(*mMonitor); - mBlockedOnParent = false; -} - void RPCChannel::ExitedCxxStack() { @@ -711,7 +577,7 @@ RPCChannel::OnMessageReceivedFromLink(const Message& msg) mPending.push_back(msg); - if (0 == StackDepth() && !mBlockedOnParent) { + if (0 == StackDepth()) { // the worker thread might be idle, make sure it wakes up if (!compressMessage) { // If we compressed away the previous message, we'll reuse diff --git a/ipc/glue/RPCChannel.h b/ipc/glue/RPCChannel.h index 893e4b68575..0acbfd10396 100644 --- a/ipc/glue/RPCChannel.h +++ b/ipc/glue/RPCChannel.h @@ -96,37 +96,11 @@ public: virtual bool Send(Message* msg) MOZ_OVERRIDE; virtual bool Send(Message* msg, Message* reply) MOZ_OVERRIDE; - // Asynchronously, send the child a message that puts it in such a - // state that it can't send messages to the parent unless the - // parent sends a message to it first. The child stays in this - // state until the parent calls |UnblockChild()|. - // - // It is an error to - // - call this on the child side of the channel. - // - nest |BlockChild()| calls - // - call this when the child is already blocked on a sync or RPC - // in-/out- message/call - // - // Return true iff successful. - bool BlockChild(); - - // Asynchronously undo |BlockChild()|. - // - // It is an error to - // - call this on the child side of the channel - // - call this without a matching |BlockChild()| - // - // Return true iff successful. - bool UnblockChild(); - // Return true iff this has code on the C++ stack. bool IsOnCxxStack() const { return !mCxxStackFrames.empty(); } - virtual bool OnSpecialMessage(uint16_t id, const Message& msg) MOZ_OVERRIDE; - - /** * If there is a pending RPC message, process all pending messages. * @@ -185,9 +159,6 @@ private: void Incall(const Message& call, size_t stackDepth); void DispatchIncall(const Message& call); - void BlockOnParent(); - void UnblockFromParent(); - // This helper class managed RPCChannel.mCxxStackDepth on behalf // of RPCChannel. When the stack depth is incremented from zero // to non-zero, it invokes an RPCChannel callback, and similarly @@ -391,9 +362,6 @@ private: // size_t mRemoteStackDepthGuess; - // True iff the parent has put us in a |BlockChild()| state. - bool mBlockedOnParent; - // Approximation of Sync/RPCChannel-code frames on the C++ stack. // It can only be interpreted as the implication // diff --git a/ipc/glue/WindowsMessageLoop.cpp b/ipc/glue/WindowsMessageLoop.cpp index 488e166e0bb..f7c15eded03 100644 --- a/ipc/glue/WindowsMessageLoop.cpp +++ b/ipc/glue/WindowsMessageLoop.cpp @@ -823,7 +823,7 @@ RPCChannel::WaitForNotify() { mMonitor->AssertCurrentThreadOwns(); - if (!StackDepth() && !mBlockedOnParent) { + if (!StackDepth()) { // There is currently no way to recover from this condition. NS_RUNTIMEABORT("StackDepth() is 0 in call to RPCChannel::WaitForNotify!"); } diff --git a/ipc/ipdl/ipdl/lower.py b/ipc/ipdl/ipdl/lower.py index 94136c1fcab..db90ad21089 100644 --- a/ipc/ipdl/ipdl/lower.py +++ b/ipc/ipdl/ipdl/lower.py @@ -3191,22 +3191,6 @@ class _GenerateProtocolActorCode(ipdl.ast.Visitor): self.cls.addstmts([ otherpid, Whitespace.NL, getdump, Whitespace.NL ]) - if (ptype.isToplevel() and self.side is 'parent' - and ptype.talksRpc()): - # offer BlockChild() and UnblockChild(). - # See ipc/glue/RPCChannel.h - blockchild = MethodDefn(MethodDecl( - 'BlockChild', ret=Type.BOOL)) - blockchild.addstmt(StmtReturn(ExprCall( - ExprSelect(p.channelVar(), '.', 'BlockChild')))) - - unblockchild = MethodDefn(MethodDecl( - 'UnblockChild', ret=Type.BOOL)) - unblockchild.addstmt(StmtReturn(ExprCall( - ExprSelect(p.channelVar(), '.', 'UnblockChild')))) - - self.cls.addstmts([ blockchild, unblockchild, Whitespace.NL ]) - ## private methods self.cls.addstmt(Label.PRIVATE) diff --git a/ipc/ipdl/test/cxx/Makefile.in b/ipc/ipdl/test/cxx/Makefile.in index ddf13a2568d..4b9d6bd52aa 100644 --- a/ipc/ipdl/test/cxx/Makefile.in +++ b/ipc/ipdl/test/cxx/Makefile.in @@ -16,7 +16,6 @@ EXPORT_LIBRARY = 1 IPDLTESTS = \ TestActorPunning \ - TestBlockChild \ TestBridgeMain \ TestCrashCleanup \ TestDataStructures \ diff --git a/ipc/ipdl/test/cxx/PTestBlockChild.ipdl b/ipc/ipdl/test/cxx/PTestBlockChild.ipdl deleted file mode 100644 index 8e638d7c906..00000000000 --- a/ipc/ipdl/test/cxx/PTestBlockChild.ipdl +++ /dev/null @@ -1,51 +0,0 @@ -namespace mozilla { -namespace _ipdltest { - -rpc protocol PTestBlockChild { -both: - rpc StackFrame(); - -child: - async Poke1(); - async Poke2(); - async LastPoke(); - async __delete__(); - -parent: - async P1(); - async P2(); - async Done(); - -state START: - // here we |BlockChild()| in the C++ - send Poke1 goto CHILD_BLOCKED; - -state CHILD_BLOCKED: - call StackFrame goto CHILD_BLOCKED_RPC; - -state CHILD_BLOCKED_RPC: - answer StackFrame goto CHILD_BLOCKED_RPC_POKE; - -state CHILD_BLOCKED_RPC_POKE: - send Poke2 goto CHILD_STILL_BLOCKED; - - // RPC stack frame gone. child should still be blocked - -state CHILD_STILL_BLOCKED: - send LastPoke goto CHILD_FLUSH_QUEUE; - - // here we |UnblockChild()| in the C++ - -state CHILD_FLUSH_QUEUE: - recv P1 goto CHILD_FLUSH_QUEUE_P2; -state CHILD_FLUSH_QUEUE_P2: - recv P2 goto CHILD_FLUSH_QUEUE_DONE; -state CHILD_FLUSH_QUEUE_DONE: - recv Done goto DONE; - -state DONE: - send __delete__; -}; - -} // namespace _ipdltest -} // namespace mozilla diff --git a/ipc/ipdl/test/cxx/TestBlockChild.cpp b/ipc/ipdl/test/cxx/TestBlockChild.cpp deleted file mode 100644 index 77a681b0167..00000000000 --- a/ipc/ipdl/test/cxx/TestBlockChild.cpp +++ /dev/null @@ -1,149 +0,0 @@ -#include "TestBlockChild.h" - -#include "IPDLUnitTests.h" // fail etc. - -template<> -struct RunnableMethodTraits -{ - typedef mozilla::_ipdltest::TestBlockChildChild T; - static void RetainCallee(T* obj) { } - static void ReleaseCallee(T* obj) { } -}; - - -namespace mozilla { -namespace _ipdltest { - -//----------------------------------------------------------------------------- -// parent - -void -TestBlockChildParent::Main() -{ - BlockChildInLowerFrame(); - - if (!SendPoke1()) - fail("couldn't poke the child"); - - if (!CallStackFrame()) - fail("couldn't poke the child"); - - if (!SendLastPoke()) - fail("couldn't poke the child"); - - if (!UnblockChild()) - fail("can't UnblockChild()!"); - mChildBlocked = false; -} - -void -TestBlockChildParent::BlockChildInLowerFrame() -{ - if (!BlockChild()) - fail("can't BlockChild()!"); - // child should be blocked and stay blocked after this returns - mChildBlocked = true; -} - -bool -TestBlockChildParent::AnswerStackFrame() -{ - if (!SendPoke2()) - fail("couldn't poke the child"); - return true; -} - -bool -TestBlockChildParent::RecvP1() -{ - if (mChildBlocked) - fail("child shouldn't been able to slip this past the blockade!"); - mGotP1 = true; - return true; -} - -bool -TestBlockChildParent::RecvP2() -{ - if (mChildBlocked) - fail("child shouldn't been able to slip this past the blockade!"); - mGotP2 = true; - return true; -} - -bool -TestBlockChildParent::RecvDone() -{ - if (mChildBlocked) - fail("child shouldn't been able to slip this past the blockade!"); - - // XXX IPDL transition checking makes this redundant, but never - // hurts to double-check, especially if we eventually turn off - // state checking in OPT builds - if (!mGotP1) - fail("should have received P1!"); - if (!mGotP2) - fail("should have received P2!"); - - Close(); - - return true; -} - - -//----------------------------------------------------------------------------- -// child - -bool -TestBlockChildChild::RecvPoke1() -{ - MessageLoop::current()->PostTask( - FROM_HERE, NewRunnableMethod(this, &TestBlockChildChild::OnPoke1)); - return true; -} - -bool -TestBlockChildChild::AnswerStackFrame() -{ - return CallStackFrame(); -} - -bool -TestBlockChildChild::RecvPoke2() -{ - MessageLoop::current()->PostTask( - FROM_HERE, NewRunnableMethod(this, &TestBlockChildChild::OnPoke2)); - return true; -} - -bool -TestBlockChildChild::RecvLastPoke() -{ - MessageLoop::current()->PostTask( - FROM_HERE, NewRunnableMethod(this, &TestBlockChildChild::OnLastPoke)); - return true; -} - -void -TestBlockChildChild::OnPoke1() -{ - if (!SendP1()) - fail("couldn't send P1"); -} - -void -TestBlockChildChild::OnPoke2() -{ - if (!SendP2()) - fail("couldn't send P2"); -} - -void -TestBlockChildChild::OnLastPoke() -{ - if (!SendDone()) - fail("couldn't send Done"); -} - -} // namespace _ipdltest -} // namespace mozilla diff --git a/ipc/ipdl/test/cxx/TestBlockChild.h b/ipc/ipdl/test/cxx/TestBlockChild.h deleted file mode 100644 index e9534c01238..00000000000 --- a/ipc/ipdl/test/cxx/TestBlockChild.h +++ /dev/null @@ -1,89 +0,0 @@ -#ifndef mozilla__ipdltest_TestBlockChild_h -#define mozilla__ipdltest_TestBlockChild_h 1 - -#include "mozilla/_ipdltest/IPDLUnitTests.h" - -#include "mozilla/_ipdltest/PTestBlockChildParent.h" -#include "mozilla/_ipdltest/PTestBlockChildChild.h" - -namespace mozilla { -namespace _ipdltest { - - -class TestBlockChildParent : - public PTestBlockChildParent -{ -public: - TestBlockChildParent() : mChildBlocked(false), - mGotP1(false), - mGotP2(false) - { } - virtual ~TestBlockChildParent() { } - - static bool RunTestInProcesses() { return true; } - static bool RunTestInThreads() { return true; } - - void Main(); - -protected: - virtual bool AnswerStackFrame() MOZ_OVERRIDE; - - virtual bool RecvP1() MOZ_OVERRIDE; - - virtual bool RecvP2() MOZ_OVERRIDE; - - virtual bool RecvDone() MOZ_OVERRIDE; - - virtual void ActorDestroy(ActorDestroyReason why) MOZ_OVERRIDE - { - if (NormalShutdown != why) - fail("unexpected destruction!"); - passed("ok"); - QuitParent(); - } - -private: - void BlockChildInLowerFrame(); - - bool mChildBlocked; - bool mGotP1; - bool mGotP2; - bool mGotP3; -}; - - -class TestBlockChildChild : - public PTestBlockChildChild -{ -public: - TestBlockChildChild() { } - virtual ~TestBlockChildChild() { } - -protected: - virtual bool RecvPoke1() MOZ_OVERRIDE; - - virtual bool AnswerStackFrame() MOZ_OVERRIDE; - - virtual bool RecvPoke2() MOZ_OVERRIDE; - - virtual bool RecvLastPoke() MOZ_OVERRIDE; - - virtual void ActorDestroy(ActorDestroyReason why) MOZ_OVERRIDE - { - if (NormalShutdown != why) - fail("unexpected destruction!"); - QuitChild(); - } - -private: - void OnPoke1(); - void OnPoke2(); - void OnLastPoke(); -}; - - -} // namespace _ipdltest -} // namespace mozilla - - -#endif // ifndef mozilla__ipdltest_TestBlockChild_h diff --git a/ipc/ipdl/test/cxx/ipdl.mk b/ipc/ipdl/test/cxx/ipdl.mk index d1b6d840b92..f4e43cfc9eb 100644 --- a/ipc/ipdl/test/cxx/ipdl.mk +++ b/ipc/ipdl/test/cxx/ipdl.mk @@ -2,7 +2,6 @@ IPDLSRCS = \ PTestActorPunning.ipdl \ PTestActorPunningPunned.ipdl \ PTestActorPunningSub.ipdl \ - PTestBlockChild.ipdl \ PTestBridgeMain.ipdl \ PTestBridgeSub.ipdl \ PTestBridgeMainSub.ipdl \ diff --git a/ipc/testshell/TestShellParent.cpp b/ipc/testshell/TestShellParent.cpp index bb9230be476..1b0ef4f6ad8 100644 --- a/ipc/testshell/TestShellParent.cpp +++ b/ipc/testshell/TestShellParent.cpp @@ -47,8 +47,7 @@ TestShellParent::CommandDone(TestShellCommandParent* command, PContextWrapperParent* TestShellParent::AllocPContextWrapper() { - ContentParent* cpp = static_cast(Manager()); - return new ContextWrapperParent(cpp); + return new ContextWrapperParent(); } bool diff --git a/js/ipc/ContextWrapperParent.h b/js/ipc/ContextWrapperParent.h index 19934c40697..6c950173da9 100644 --- a/js/ipc/ContextWrapperParent.h +++ b/js/ipc/ContextWrapperParent.h @@ -26,9 +26,8 @@ class ContextWrapperParent { public: - ContextWrapperParent(ContentParent* cpp) - : mContent(cpp) - , mGlobal(NULL) + ContextWrapperParent() + : mGlobal(NULL) {} JSBool GetGlobalJSObject(JSContext* cx, JSObject** globalp) { @@ -44,12 +43,11 @@ public: } bool RequestRunToCompletion() { - return mContent->RequestRunToCompletion(); + return false; } private: - ContentParent* mContent; ObjectWrapperParent* mGlobal; nsAutoJSValHolder mGlobalHolder;