Bug 1772908 - [1/6] Drive-by cleanup: simplify use of SyncRunnable r=xpcom-reviewers,necko-reviewers,nika,valentin

`SyncRunnable`'s helper functions take an `nsIRunnable *`; but the most
common way of building nsIRunnables, `NS_NewRunnableFunction`, returns
an `already_AddRefed<nsIRunnable>` instead. Add two new overloads of the
helper functions to eliminate the impedance mismatch.

(This does result in an uncomfortable amount of code duplication. While
we could eliminate that with appropriate use of SFINAE, it'll be simpler
if we wait for C++20 and its `requires` keyword.)

Additionally, add two explicitly-deleted overloads to catch and prevent
a previously-common antipattern that presumably resulted from this type
mismatch: accidentally wrapping the actual runnable in two layers of
`SyncRunnable`. Fix the former use-sites appropriately. (This was
probably harmless, but is also probably best avoided.)

No functional changes. This is in some sense a continuation of bug
1281626.

(This is no longer actually relevant to bug 1772908 due to a different
approach being taken. It remains in the patchset anyway, for
simplicity's sake.)

Differential Revision: https://phabricator.services.mozilla.com/D157131
This commit is contained in:
Ray Kraesig 2022-11-04 21:04:18 +00:00
Родитель ee21435fb3
Коммит 54ac39289a
9 изменённых файлов: 59 добавлений и 35 удалений

Просмотреть файл

@ -976,8 +976,8 @@ nsresult nsSocketTransport::ResolveHost() {
SyncRunnable::DispatchToThread(
mainThread,
new SyncRunnable(NS_NewRunnableFunction(
"nsSocketTransport::ResolveHost->GetDNSService", initTask)));
NS_NewRunnableFunction("nsSocketTransport::ResolveHost->GetDNSService",
initTask));
} else {
initTask();
}

Просмотреть файл

@ -58,8 +58,8 @@ nsresult DNSUtils::CreateChannelHelper(nsIURI* aUri, nsIChannel** aResult) {
if (main) {
// Forward to the main thread synchronously.
SyncRunnable::DispatchToThread(
main, new SyncRunnable(NS_NewRunnableFunction(
"InitHttpHandler", []() { InitHttpHandler(); })));
main, NS_NewRunnableFunction("InitHttpHandler",
[]() { InitHttpHandler(); }));
}
}

Просмотреть файл

@ -682,9 +682,8 @@ already_AddRefed<nsIDNSService> GetOrInitDNSService() {
return nullptr;
}
SyncRunnable::DispatchToThread(mainThread,
new SyncRunnable(NS_NewRunnableFunction(
"GetOrInitDNSService", initTask)));
SyncRunnable::DispatchToThread(
mainThread, NS_NewRunnableFunction("GetOrInitDNSService", initTask));
} else {
initTask();
}

Просмотреть файл

@ -898,8 +898,8 @@ void AltSvcCache::EnsureStorageInited() {
}
SyncRunnable::DispatchToThread(
main, new SyncRunnable(NS_NewRunnableFunction(
"AltSvcCache::EnsureStorageInited", initTask)));
main,
NS_NewRunnableFunction("AltSvcCache::EnsureStorageInited", initTask));
}
already_AddRefed<AltSvcMapping> AltSvcCache::LookupMapping(

Просмотреть файл

@ -1839,9 +1839,10 @@ nsresult nsHttpHandler::SetAcceptLanguages() {
// Forward to the main thread synchronously.
SyncRunnable::DispatchToThread(
mainThread, new SyncRunnable(NS_NewRunnableFunction(
"nsHttpHandler::SetAcceptLanguages",
[&rv]() { rv = gHttpHandler->SetAcceptLanguages(); })));
mainThread,
NS_NewRunnableFunction("nsHttpHandler::SetAcceptLanguages", [&rv]() {
rv = gHttpHandler->SetAcceptLanguages();
}));
return rv;
}

Просмотреть файл

@ -62,10 +62,10 @@ nsresult NSSKeyStore::Lock() {
// Forward to the main thread synchronously.
SyncRunnable::DispatchToThread(
mainThread, new SyncRunnable(NS_NewRunnableFunction(
"NSSKeyStoreMainThreadLock", [slot = mSlot.get()]() {
NSSKeyStoreMainThreadLock(slot);
})));
mainThread, NS_NewRunnableFunction("NSSKeyStoreMainThreadLock",
[slot = mSlot.get()]() {
NSSKeyStoreMainThreadLock(slot);
}));
return NS_OK;
}
@ -91,11 +91,11 @@ nsresult NSSKeyStore::Unlock() {
// Forward to the main thread synchronously.
nsresult result = NS_ERROR_FAILURE;
SyncRunnable::DispatchToThread(
mainThread, new SyncRunnable(NS_NewRunnableFunction(
"NSSKeyStoreMainThreadUnlock",
[slot = mSlot.get(), result = &result]() {
*result = NSSKeyStoreMainThreadUnlock(slot);
})));
mainThread,
NS_NewRunnableFunction("NSSKeyStoreMainThreadUnlock",
[slot = mSlot.get(), result = &result]() {
*result = NSSKeyStoreMainThreadUnlock(slot);
}));
return result;
}

Просмотреть файл

@ -185,9 +185,10 @@ bool EnsureNSSInitializedChromeOrContent() {
// Forward to the main thread synchronously.
mozilla::SyncRunnable::DispatchToThread(
mainThread, new SyncRunnable(NS_NewRunnableFunction(
"EnsureNSSInitializedChromeOrContent",
[]() { EnsureNSSInitializedChromeOrContent(); })));
mainThread,
NS_NewRunnableFunction("EnsureNSSInitializedChromeOrContent", []() {
EnsureNSSInitializedChromeOrContent();
}));
return initialized;
}

Просмотреть файл

@ -622,12 +622,11 @@ NS_IMETHODIMP IPCFuzzController::IPCFuzzLoop::Run() {
SyncRunnable::DispatchToThread(
GetMainThreadEventTarget(),
new SyncRunnable(NS_NewRunnableFunction(
"IPCFuzzController::StartFuzzing", [&]() -> void {
MOZ_FUZZING_NYX_PRINT("INFO: Main thread runnable start.\n");
NS_ProcessPendingEvents(NS_GetCurrentThread());
MOZ_FUZZING_NYX_PRINT("INFO: Main thread runnable done.\n");
})));
NS_NewRunnableFunction("IPCFuzzController::StartFuzzing", [&]() -> void {
MOZ_FUZZING_NYX_PRINT("INFO: Main thread runnable start.\n");
NS_ProcessPendingEvents(NS_GetCurrentThread());
MOZ_FUZZING_NYX_PRINT("INFO: Main thread runnable done.\n");
}));
MOZ_FUZZING_NYX_PRINT("INFO: Performing snapshot...\n");
Nyx::instance().start();

Просмотреть файл

@ -17,7 +17,7 @@
namespace mozilla {
/**
* This class will wrap a nsIRunnable and dispatch it to the main thread
* This class will wrap a nsIRunnable and dispatch it to the target thread
* synchronously. This is different from nsIEventTarget.DISPATCH_SYNC:
* this class does not spin the event loop waiting for the event to be
* dispatched. This means that you don't risk reentrance from pending
@ -25,11 +25,12 @@ namespace mozilla {
* on this thread, or else you will deadlock.
*
* Typical usage:
* RefPtr<SyncRunnable> sr = new SyncRunnable(new myrunnable...());
* sr->DispatchToThread(t);
* RefPtr<SyncRunnable> sr = new SyncRunnable(new myrunnable...());
* sr->DispatchToThread(t);
*
* We also provide a convenience wrapper:
* SyncRunnable::DispatchToThread(new myrunnable...());
* We also provide convenience wrappers:
* SyncRunnable::DispatchToThread(pThread, new myrunnable...());
* SyncRunnable::DispatchToThread(pThread, NS_NewRunnableFunction(...));
*
*/
class SyncRunnable : public Runnable {
@ -109,6 +110,29 @@ class SyncRunnable : public Runnable {
return s->DispatchToThread(aThread, aForceDispatch);
}
static nsresult DispatchToThread(nsIEventTarget* aThread,
already_AddRefed<nsIRunnable> aRunnable,
bool aForceDispatch = false) {
RefPtr<SyncRunnable> s(new SyncRunnable(std::move(aRunnable)));
return s->DispatchToThread(aThread, aForceDispatch);
}
static nsresult DispatchToThread(AbstractThread* aThread,
already_AddRefed<nsIRunnable> aRunnable,
bool aForceDispatch = false) {
RefPtr<SyncRunnable> s(new SyncRunnable(std::move(aRunnable)));
return s->DispatchToThread(aThread, aForceDispatch);
}
// These deleted overloads prevent accidentally (if harmlessly) double-
// wrapping SyncRunnable, which was previously a common anti-pattern.
static nsresult DispatchToThread(nsIEventTarget* aThread,
SyncRunnable* aRunnable,
bool aForceDispatch = false) = delete;
static nsresult DispatchToThread(AbstractThread* aThread,
SyncRunnable* aRunnable,
bool aForceDispatch = false) = delete;
protected:
NS_IMETHOD Run() override {
mRunnable->Run();