From 2facca62f08fa7f38d943659b0e116d2b41b866c Mon Sep 17 00:00:00 2001 From: Jean-Yves Avenard Date: Fri, 8 May 2020 11:44:59 +0000 Subject: [PATCH] Bug 1634253 - P8. Remove use of MessageLoop in Canvas. r=mattwoodrow MessagePool brings no benefit over the traditional nsIThread. Additonally, replace some incorrect use of RefPtr for xpcom objects. Differential Revision: https://phabricator.services.mozilla.com/D73827 --- build/clang-plugin/ThreadAllows.txt | 1 + gfx/layers/ipc/CanvasThread.cpp | 31 +++++++++++++---------------- gfx/layers/ipc/CanvasThread.h | 6 +++--- 3 files changed, 18 insertions(+), 20 deletions(-) diff --git a/build/clang-plugin/ThreadAllows.txt b/build/clang-plugin/ThreadAllows.txt index b09bb32b7920..cb07842ba3be 100644 --- a/build/clang-plugin/ThreadAllows.txt +++ b/build/clang-plugin/ThreadAllows.txt @@ -14,6 +14,7 @@ COM MTA Cache Deleter Cache I/O Cameras IPC +Canvas ChainedPipePump ChainedPipeRecv Checker Test diff --git a/gfx/layers/ipc/CanvasThread.cpp b/gfx/layers/ipc/CanvasThread.cpp index c6d66b76fae1..4920ad7eef56 100644 --- a/gfx/layers/ipc/CanvasThread.cpp +++ b/gfx/layers/ipc/CanvasThread.cpp @@ -21,9 +21,9 @@ StaticDataMutex> CanvasThreadHolder::sCanvasThreadHolder("sCanvasThreadHolder"); CanvasThreadHolder::CanvasThreadHolder( - UniquePtr aCanvasThread, + already_AddRefed aCanvasThread, already_AddRefed aCanvasWorkers) - : mCanvasThread(std::move(aCanvasThread)), + : mCanvasThread(aCanvasThread), mCanvasWorkers(aCanvasWorkers), mCompositorThreadKeepAlive(CompositorThreadHolder::GetSingleton()) { MOZ_ASSERT(NS_IsInCompositorThread()); @@ -44,10 +44,9 @@ already_AddRefed CanvasThreadHolder::EnsureCanvasThread() { auto lockedCanvasThreadHolder = sCanvasThreadHolder.Lock(); if (!lockedCanvasThreadHolder.ref()) { - UniquePtr canvasThread = MakeUnique("Canvas"); - if (!canvasThread->Start()) { - return nullptr; - }; + nsCOMPtr canvasThread; + nsresult rv = NS_NewNamedThread("Canvas", getter_AddRefs(canvasThread)); + NS_ENSURE_SUCCESS(rv, nullptr); // Given that the canvas workers are receiving instructions from // content processes, it probably doesn't make sense to have more than @@ -56,14 +55,14 @@ already_AddRefed CanvasThreadHolder::EnsureCanvasThread() { // there is more than one window with canvas drawing, the OS can // manage the load between them. uint32_t threadLimit = std::max(2, PR_GetNumberOfProcessors() / 2); - RefPtr canvasWorkers = + nsCOMPtr canvasWorkers = SharedThreadPool::Get(NS_LITERAL_CSTRING("CanvasWorkers"), threadLimit); if (!canvasWorkers) { return nullptr; } lockedCanvasThreadHolder.ref() = - new CanvasThreadHolder(std::move(canvasThread), canvasWorkers.forget()); + new CanvasThreadHolder(canvasThread.forget(), canvasWorkers.forget()); } return do_AddRef(lockedCanvasThreadHolder.ref()); @@ -75,7 +74,8 @@ void CanvasThreadHolder::StaticRelease( RefPtr threadHolder = aCanvasThreadHolder; // Note we can't just use NS_IsInCompositorThread() here because // sCompositorThreadHolder might have already gone. - MOZ_ASSERT(threadHolder->mCompositorThreadKeepAlive->IsInCompositorThread()); + MOZ_ASSERT(threadHolder->mCompositorThreadKeepAlive->GetCompositorThread() + ->IsOnCurrentThread()); threadHolder = nullptr; auto lockedCanvasThreadHolder = sCanvasThreadHolder.Lock(); @@ -99,8 +99,7 @@ void CanvasThreadHolder::ReleaseOnCompositorThread( bool CanvasThreadHolder::IsInCanvasThread() { auto lockedCanvasThreadHolder = sCanvasThreadHolder.Lock(); return lockedCanvasThreadHolder.ref() && - lockedCanvasThreadHolder.ref()->mCanvasThread->thread_id() == - PlatformThread::CurrentId(); + lockedCanvasThreadHolder.ref()->mCanvasThread->IsOnCurrentThread(); } /* static */ @@ -115,8 +114,7 @@ bool CanvasThreadHolder::IsInCanvasThreadOrWorker() { auto lockedCanvasThreadHolder = sCanvasThreadHolder.Lock(); return lockedCanvasThreadHolder.ref() && (lockedCanvasThreadHolder.ref()->mCanvasWorkers->IsOnCurrentThread() || - lockedCanvasThreadHolder.ref()->mCanvasThread->thread_id() == - PlatformThread::CurrentId()); + lockedCanvasThreadHolder.ref()->mCanvasThread->IsOnCurrentThread()); } /* static */ @@ -125,17 +123,16 @@ void CanvasThreadHolder::MaybeDispatchToCanvasThread( auto lockedCanvasThreadHolder = sCanvasThreadHolder.Lock(); if (!lockedCanvasThreadHolder.ref()) { // There is no canvas thread just release the runnable. - RefPtr runnable = aRunnable; + nsCOMPtr runnable = aRunnable; return; } - lockedCanvasThreadHolder.ref()->mCanvasThread->message_loop()->PostTask( - std::move(aRunnable)); + lockedCanvasThreadHolder.ref()->mCanvasThread->Dispatch(std::move(aRunnable)); } void CanvasThreadHolder::DispatchToCanvasThread( already_AddRefed aRunnable) { - mCanvasThread->message_loop()->PostTask(std::move(aRunnable)); + mCanvasThread->Dispatch(std::move(aRunnable)); } already_AddRefed CanvasThreadHolder::CreateWorkerTaskQueue() { diff --git a/gfx/layers/ipc/CanvasThread.h b/gfx/layers/ipc/CanvasThread.h index a2b68add90ba..57a17326166c 100644 --- a/gfx/layers/ipc/CanvasThread.h +++ b/gfx/layers/ipc/CanvasThread.h @@ -7,13 +7,13 @@ #ifndef mozilla_layers_CanvasThread_h #define mozilla_layers_CanvasThread_h -#include "base/thread.h" #include "mozilla/layers/CompositorThread.h" #include "mozilla/DataMutex.h" #include "mozilla/StaticPtr.h" #include "mozilla/TaskQueue.h" #include "mozilla/UniquePtr.h" #include "nsISupports.h" +#include "nsIThread.h" #include "nsIThreadPool.h" namespace mozilla { @@ -85,12 +85,12 @@ class CanvasThreadHolder final { static StaticDataMutex> sCanvasThreadHolder; - CanvasThreadHolder(UniquePtr aCanvasThread, + CanvasThreadHolder(already_AddRefed aCanvasThread, already_AddRefed aCanvasWorkers); ~CanvasThreadHolder(); - UniquePtr mCanvasThread; + nsCOMPtr mCanvasThread; RefPtr mCanvasWorkers; // Hold a reference to prevent the compositor thread ending.