From 79586c053487708e672a635f23cf1e4611bd6eda Mon Sep 17 00:00:00 2001 From: Jens Stutte Date: Tue, 21 Jun 2022 06:18:31 +0000 Subject: [PATCH] Bug 1775112 - Remove obsolete MessageChannel::Begin/StopPostponingSends feature. r=ipc-reviewers,nika Differential Revision: https://phabricator.services.mozilla.com/D149775 --- ipc/glue/MessageChannel.cpp | 32 --- ipc/glue/MessageChannel.h | 21 -- ipc/ipdl/sync-messages.ini | 2 - ipc/ipdl/test/cxx/PTestLayoutThread.ipdl | 22 -- .../test/cxx/TestOffMainThreadPainting.cpp | 237 ------------------ ipc/ipdl/test/cxx/TestOffMainThreadPainting.h | 109 -------- ipc/ipdl/test/cxx/moz.build | 1 - 7 files changed, 424 deletions(-) delete mode 100644 ipc/ipdl/test/cxx/PTestLayoutThread.ipdl delete mode 100644 ipc/ipdl/test/cxx/TestOffMainThreadPainting.cpp delete mode 100644 ipc/ipdl/test/cxx/TestOffMainThreadPainting.h diff --git a/ipc/glue/MessageChannel.cpp b/ipc/glue/MessageChannel.cpp index df3688dbe449..71092fcc7b5f 100644 --- a/ipc/glue/MessageChannel.cpp +++ b/ipc/glue/MessageChannel.cpp @@ -776,40 +776,9 @@ bool MessageChannel::Send(UniquePtr aMsg) { void MessageChannel::SendMessageToLink(UniquePtr aMsg) { AssertWorkerThread(); mMonitor->AssertCurrentThreadOwns(); - if (mIsPostponingSends) { - mPostponedSends.push_back(std::move(aMsg)); - return; - } mLink->SendMessage(std::move(aMsg)); } -void MessageChannel::BeginPostponingSends() { - AssertWorkerThread(); - mMonitor->AssertNotCurrentThreadOwns(); - - MonitorAutoLock lock(*mMonitor); - { - MOZ_ASSERT(!mIsPostponingSends); - mIsPostponingSends = true; - } -} - -void MessageChannel::StopPostponingSends() { - // Note: this can be called from any thread. - MonitorAutoLock lock(*mMonitor); - - MOZ_ASSERT(mIsPostponingSends); - - for (UniquePtr& iter : mPostponedSends) { - mLink->SendMessage(std::move(iter)); - } - - // We unset this after SendMessage so we can make correct thread - // assertions in MessageLink. - mIsPostponingSends = false; - mPostponedSends.clear(); -} - UniquePtr MessageChannel::PopCallback( const Message& aMsg) { auto iter = mPendingResponses.find(aMsg.seqno()); @@ -1247,7 +1216,6 @@ bool MessageChannel::Send(UniquePtr aMsg, UniquePtr* aReply) { if (aMsg->nested_level() < DispatchingSyncMessageNestedLevel() || aMsg->nested_level() < AwaitingSyncReplyNestedLevel()) { MOZ_RELEASE_ASSERT(DispatchingSyncMessage() || DispatchingAsyncMessage()); - MOZ_RELEASE_ASSERT(!mIsPostponingSends); IPC_LOG("Cancel from Send"); auto cancel = MakeUnique(CurrentNestedInsideSyncTransaction()); diff --git a/ipc/glue/MessageChannel.h b/ipc/glue/MessageChannel.h index c94f36d5a046..c6cf6b5e5793 100644 --- a/ipc/glue/MessageChannel.h +++ b/ipc/glue/MessageChannel.h @@ -291,22 +291,6 @@ class MessageChannel : HasResultCodes { void CancelCurrentTransaction() EXCLUDES(*mMonitor); - // Force all calls to Send to defer actually sending messages. This will - // cause sync messages to block until another thread calls - // StopPostponingSends. - // - // This must be called from the worker thread. - void BeginPostponingSends() EXCLUDES(*mMonitor); - - // Stop postponing sent messages, and immediately flush all postponed - // messages to the link. This may be called from any thread. - // - // Note that there are no ordering guarantees between two different - // MessageChannels. If channel B sends a message, then stops postponing - // channel A, messages from A may arrive before B. The easiest way to order - // this, if needed, is to make B send a sync message. - void StopPostponingSends() EXCLUDES(*mMonitor); - // IsClosed and NumQueuedMessages are safe to call from any thread, but // may provide an out-of-date value. bool IsClosed() EXCLUDES(*mMonitor) { @@ -752,11 +736,6 @@ class MessageChannel : HasResultCodes { // See SetChannelFlags ChannelFlags mFlags = REQUIRE_DEFAULT; - // Channels can enter messages are not sent immediately; instead, they are - // held in a queue until another thread deems it is safe to send them. - bool mIsPostponingSends GUARDED_BY(*mMonitor) = false; - std::vector> mPostponedSends GUARDED_BY(*mMonitor); - bool mBuildIDsConfirmedMatch GUARDED_BY(*mMonitor) = false; // If this is true, both ends of this message channel have event targets diff --git a/ipc/ipdl/sync-messages.ini b/ipc/ipdl/sync-messages.ini index 50e758b69f0a..163ee27c162e 100644 --- a/ipc/ipdl/sync-messages.ini +++ b/ipc/ipdl/sync-messages.ini @@ -240,8 +240,6 @@ description = Only used by C++ unit tests description = Only used by C++ unit tests [PTestUrgentHangs::Test5_1] description = Only used by C++ unit tests -[PTestLayoutThread::SyncMessage] -description = Only used by C++ unit tests [PTestPaintThread::FinishedPaint] description = Only used by C++ unit tests diff --git a/ipc/ipdl/test/cxx/PTestLayoutThread.ipdl b/ipc/ipdl/test/cxx/PTestLayoutThread.ipdl deleted file mode 100644 index b295d06f1547..000000000000 --- a/ipc/ipdl/test/cxx/PTestLayoutThread.ipdl +++ /dev/null @@ -1,22 +0,0 @@ -include protocol PTestPaintThread; - -include "mozilla/_ipdltest/TestOffMainThreadPainting.h"; - -namespace mozilla { -namespace _ipdltest { - -// This is supposed to be analagous to PLayerTransaction. -[ManualDealloc, ChildImpl="TestOffMainThreadPaintingChild", ParentImpl="TestOffMainThreadPaintingParent"] -sync protocol PTestLayoutThread -{ -parent: - async FinishedLayout(uint64_t aTxnId); - async AsyncMessage(uint64_t aTxnId); - sync SyncMessage(uint64_t aTxnId); - async EndTest(); -child: - async StartTest(Endpoint endpoint); -}; - -} // namespace mozilla -} // namespace _ipdltest diff --git a/ipc/ipdl/test/cxx/TestOffMainThreadPainting.cpp b/ipc/ipdl/test/cxx/TestOffMainThreadPainting.cpp deleted file mode 100644 index 7a1bf2cf8741..000000000000 --- a/ipc/ipdl/test/cxx/TestOffMainThreadPainting.cpp +++ /dev/null @@ -1,237 +0,0 @@ -#include "TestOffMainThreadPainting.h" - -#include "IPDLUnitTests.h" // fail etc. -#include "mozilla/Unused.h" -#include -#include - -namespace mozilla { -namespace _ipdltest { - -TestOffMainThreadPaintingParent::TestOffMainThreadPaintingParent() - : mAsyncMessages(0), mSyncMessages(0) {} - -TestOffMainThreadPaintingParent::~TestOffMainThreadPaintingParent() {} - -void TestOffMainThreadPaintingParent::Main() { - ipc::Endpoint parentPipe; - ipc::Endpoint childPipe; - nsresult rv = PTestPaintThread::CreateEndpoints( - base::GetCurrentProcId(), OtherPid(), &parentPipe, &childPipe); - if (NS_FAILED(rv)) { - fail("create pipes"); - } - - mPaintActor = new TestPaintThreadParent(this); - if (!mPaintActor->Bind(std::move(parentPipe))) { - fail("bind parent pipe"); - } - - if (!SendStartTest(std::move(childPipe))) { - fail("sending Start"); - } -} - -ipc::IPCResult TestOffMainThreadPaintingParent::RecvFinishedLayout( - const uint64_t& aTxnId) { - if (!mPaintedTxn || mPaintedTxn.value() != aTxnId) { - fail("received transaction before receiving paint"); - } - mPaintedTxn = Nothing(); - mCompletedTxn = Some(aTxnId); - return IPC_OK(); -} - -void TestOffMainThreadPaintingParent::NotifyFinishedPaint( - const uint64_t& aTxnId) { - if (mCompletedTxn && mCompletedTxn.value() >= aTxnId) { - fail("received paint after receiving transaction"); - } - if (mPaintedTxn) { - fail("painted again before completing previous transaction"); - } - mPaintedTxn = Some(aTxnId); -} - -ipc::IPCResult TestOffMainThreadPaintingParent::RecvAsyncMessage( - const uint64_t& aTxnId) { - if (!mCompletedTxn || mCompletedTxn.value() != aTxnId) { - fail("sync message received out of order"); - return IPC_FAIL_NO_REASON(this); - } - mAsyncMessages++; - return IPC_OK(); -} - -ipc::IPCResult TestOffMainThreadPaintingParent::RecvSyncMessage( - const uint64_t& aTxnId) { - if (!mCompletedTxn || mCompletedTxn.value() != aTxnId) { - fail("sync message received out of order"); - return IPC_FAIL_NO_REASON(this); - } - if (mSyncMessages >= mAsyncMessages) { - fail("sync message received before async message"); - return IPC_FAIL_NO_REASON(this); - } - mSyncMessages++; - return IPC_OK(); -} - -ipc::IPCResult TestOffMainThreadPaintingParent::RecvEndTest() { - if (!mCompletedTxn || mCompletedTxn.value() != 1) { - fail("expected to complete a transaction"); - } - if (mAsyncMessages != 1) { - fail("expected to get 1 async message"); - } - if (mSyncMessages != 1) { - fail("expected to get 1 sync message"); - } - - passed("ok"); - - mPaintActor->Close(); - Close(); - return IPC_OK(); -} - -void TestOffMainThreadPaintingParent::ActorDestroy(ActorDestroyReason aWhy) { - if (aWhy != NormalShutdown) { - fail("child process aborted"); - } - QuitParent(); -} - -/************************** - * PTestLayoutThreadChild * - **************************/ - -TestOffMainThreadPaintingChild::TestOffMainThreadPaintingChild() - : mNextTxnId(1) {} - -TestOffMainThreadPaintingChild::~TestOffMainThreadPaintingChild() {} - -ipc::IPCResult TestOffMainThreadPaintingChild::RecvStartTest( - ipc::Endpoint&& aEndpoint) { - mPaintThread = MakeUnique("PaintThread"); - if (!mPaintThread->Start()) { - return IPC_FAIL_NO_REASON(this); - } - - mPaintActor = new TestPaintThreadChild(GetIPCChannel()); - RefPtr task = - NewRunnableMethod&&>( - "TestPaintthreadChild::Bind", mPaintActor, - &TestPaintThreadChild::Bind, std::move(aEndpoint)); - mPaintThread->message_loop()->PostTask(task.forget()); - - IssueTransaction(); - return IPC_OK(); -} - -void TestOffMainThreadPaintingChild::ActorDestroy(ActorDestroyReason aWhy) { - RefPtr task = NewRunnableMethod( - "TestPaintThreadChild::Close", mPaintActor, &TestPaintThreadChild::Close); - mPaintThread->message_loop()->PostTask(task.forget()); - mPaintThread = nullptr; - - QuitChild(); -} - -void TestOffMainThreadPaintingChild::ProcessingError(Result aCode, - const char* aReason) { - MOZ_CRASH("Aborting child due to IPC error"); -} - -void TestOffMainThreadPaintingChild::IssueTransaction() { - GetIPCChannel()->BeginPostponingSends(); - - uint64_t txnId = mNextTxnId++; - - // Start painting before we send the message. - RefPtr task = NewRunnableMethod( - "TestPaintThreadChild::BeginPaintingForTxn", mPaintActor, - &TestPaintThreadChild::BeginPaintingForTxn, txnId); - mPaintThread->message_loop()->PostTask(task.forget()); - - // Simulate some gecko main thread stuff. - SendFinishedLayout(txnId); - SendAsyncMessage(txnId); - SendSyncMessage(txnId); - SendEndTest(); -} - -/************************** - * PTestPaintThreadParent * - **************************/ - -TestPaintThreadParent::TestPaintThreadParent( - TestOffMainThreadPaintingParent* aMainBridge) - : mMainBridge(aMainBridge) {} - -TestPaintThreadParent::~TestPaintThreadParent() {} - -bool TestPaintThreadParent::Bind( - ipc::Endpoint&& aEndpoint) { - if (!aEndpoint.Bind(this)) { - return false; - } - - AddRef(); - return true; -} - -ipc::IPCResult TestPaintThreadParent::RecvFinishedPaint( - const uint64_t& aTxnId) { - mMainBridge->NotifyFinishedPaint(aTxnId); - return IPC_OK(); -} - -void TestPaintThreadParent::ActorDestroy(ActorDestroyReason aWhy) {} - -void TestPaintThreadParent::DeallocPTestPaintThreadParent() { Release(); } - -/************************* - * PTestPaintThreadChild * - *************************/ - -TestPaintThreadChild::TestPaintThreadChild(MessageChannel* aMainChannel) - : mCanSend(false), mMainChannel(aMainChannel) {} - -TestPaintThreadChild::~TestPaintThreadChild() {} - -void TestPaintThreadChild::Bind( - ipc::Endpoint&& aEndpoint) { - if (!aEndpoint.Bind(this)) { - MOZ_CRASH("could not bind paint child endpoint"); - } - AddRef(); - mCanSend = true; -} - -void TestPaintThreadChild::BeginPaintingForTxn(uint64_t aTxnId) { - MOZ_RELEASE_ASSERT(!NS_IsMainThread()); - - // Sleep for some time to simulate painting being slow. - PR_Sleep(PR_MillisecondsToInterval(500)); - SendFinishedPaint(aTxnId); - - mMainChannel->StopPostponingSends(); -} - -void TestPaintThreadChild::ActorDestroy(ActorDestroyReason aWhy) { - mCanSend = false; -} - -void TestPaintThreadChild::Close() { - MOZ_RELEASE_ASSERT(!NS_IsMainThread()); - - if (mCanSend) { - PTestPaintThreadChild::Close(); - } -} - -void TestPaintThreadChild::DeallocPTestPaintThreadChild() { Release(); } - -} // namespace _ipdltest -} // namespace mozilla diff --git a/ipc/ipdl/test/cxx/TestOffMainThreadPainting.h b/ipc/ipdl/test/cxx/TestOffMainThreadPainting.h deleted file mode 100644 index 7cb935a2ee57..000000000000 --- a/ipc/ipdl/test/cxx/TestOffMainThreadPainting.h +++ /dev/null @@ -1,109 +0,0 @@ -#ifndef mozilla__ipdltest_TestOffMainThreadPainting_h -#define mozilla__ipdltest_TestOffMainThreadPainting_h - -#include "mozilla/Maybe.h" -#include "mozilla/_ipdltest/IPDLUnitTests.h" -#include "mozilla/_ipdltest/PTestLayoutThreadChild.h" -#include "mozilla/_ipdltest/PTestLayoutThreadParent.h" -#include "mozilla/_ipdltest/PTestPaintThreadChild.h" -#include "mozilla/_ipdltest/PTestPaintThreadParent.h" -#include "base/thread.h" - -namespace mozilla { -namespace _ipdltest { - -class TestPaintThreadChild; -class TestPaintThreadParent; - -// Analagous to LayerTransactionParent. -class TestOffMainThreadPaintingParent final : public PTestLayoutThreadParent { - public: - static bool RunTestInThreads() { return false; } - static bool RunTestInProcesses() { return true; } - - void Main(); - - MOZ_IMPLICIT TestOffMainThreadPaintingParent(); - ~TestOffMainThreadPaintingParent() override; - - ipc::IPCResult RecvFinishedLayout(const uint64_t& aTxnId); - ipc::IPCResult RecvAsyncMessage(const uint64_t& aTxnId); - ipc::IPCResult RecvSyncMessage(const uint64_t& aTxnId); - ipc::IPCResult RecvEndTest(); - void ActorDestroy(ActorDestroyReason aWhy) override; - - void NotifyFinishedPaint(const uint64_t& aTxnId); - - private: - RefPtr mPaintActor; - Maybe mCompletedTxn; - Maybe mPaintedTxn; - uint32_t mAsyncMessages; - uint32_t mSyncMessages; -}; - -// Analagous to LayerTransactionChild. -class TestOffMainThreadPaintingChild final : public PTestLayoutThreadChild { - public: - TestOffMainThreadPaintingChild(); - ~TestOffMainThreadPaintingChild() override; - - ipc::IPCResult RecvStartTest( - ipc::Endpoint&& aEndpoint); - void ActorDestroy(ActorDestroyReason aWhy) override; - void ProcessingError(Result aCode, const char* aReason) override; - - private: - void IssueTransaction(); - - private: - UniquePtr mPaintThread; - RefPtr mPaintActor; - uint64_t mNextTxnId; -}; - -/**************** - * Paint Actors * - ****************/ - -class TestPaintThreadParent final : public PTestPaintThreadParent { - public: - explicit TestPaintThreadParent(TestOffMainThreadPaintingParent* aMainBridge); - - NS_INLINE_DECL_THREADSAFE_REFCOUNTING(TestPaintThreadParent); - - bool Bind(ipc::Endpoint&& aEndpoint); - ipc::IPCResult RecvFinishedPaint(const uint64_t& aTxnId); - void ActorDestroy(ActorDestroyReason aWhy) override; - void DeallocPTestPaintThreadParent() override; - - private: - ~TestPaintThreadParent() override; - - private: - TestOffMainThreadPaintingParent* mMainBridge; -}; - -class TestPaintThreadChild final : public PTestPaintThreadChild { - public: - explicit TestPaintThreadChild(MessageChannel* aOtherChannel); - - NS_INLINE_DECL_THREADSAFE_REFCOUNTING(TestPaintThreadChild); - - void Bind(ipc::Endpoint&& aEndpoint); - void BeginPaintingForTxn(uint64_t aTxnId); - void ActorDestroy(ActorDestroyReason aWhy) override; - void DeallocPTestPaintThreadChild() override; - void Close(); - - private: - ~TestPaintThreadChild() override; - - bool mCanSend; - MessageChannel* mMainChannel; -}; - -} // namespace _ipdltest -} // namespace mozilla - -#endif // ifndef mozilla__ipdltest_TestOffMainThreadPainting_h diff --git a/ipc/ipdl/test/cxx/moz.build b/ipc/ipdl/test/cxx/moz.build index 591fa269a953..e2650df0aae4 100644 --- a/ipc/ipdl/test/cxx/moz.build +++ b/ipc/ipdl/test/cxx/moz.build @@ -136,7 +136,6 @@ IPDL_SOURCES += [ "PTestInterruptShutdownRace.ipdl", "PTestJSON.ipdl", "PTestLatency.ipdl", - "PTestLayoutThread.ipdl", "PTestManyChildAllocs.ipdl", "PTestManyChildAllocsSub.ipdl", "PTestMultiMgrs.ipdl",