From 0783fb05a10fcaadb966e93d544270b2b43fe835 Mon Sep 17 00:00:00 2001 From: Iulian Moraru Date: Tue, 27 Feb 2024 20:41:39 +0200 Subject: [PATCH] Backed out 6 changesets (bug 1880192) for causing build bustages on MozPromise.h. CLOSED TREE Backed out changeset b73f06a9fdb1 (bug 1880192) Backed out changeset ee04f994bad8 (bug 1880192) Backed out changeset 8c7a686c8691 (bug 1880192) Backed out changeset 15e93c260557 (bug 1880192) Backed out changeset b5f62ac62087 (bug 1880192) Backed out changeset ff2d8eae2a68 (bug 1880192) --- dom/media/AudioSegment.h | 2 +- dom/media/VideoFrameConverter.h | 17 +- .../recognition/SpeechRecognition.cpp | 13 +- gfx/layers/apz/src/APZCTreeManager.cpp | 3 +- netwerk/wifi/nsWifiMonitor.cpp | 4 +- netwerk/wifi/nsWifiMonitor.h | 2 +- xpcom/tests/gtest/TestThreadUtils.cpp | 333 ++++++++++++++---- xpcom/threads/MozPromise.h | 198 ++++++----- xpcom/threads/nsThreadUtils.h | 319 ++++++++++------- 9 files changed, 602 insertions(+), 289 deletions(-) diff --git a/dom/media/AudioSegment.h b/dom/media/AudioSegment.h index 4b4434ff0b7b..006f996c398f 100644 --- a/dom/media/AudioSegment.h +++ b/dom/media/AudioSegment.h @@ -318,7 +318,7 @@ struct AudioChunk { * A list of audio samples consisting of a sequence of slices of SharedBuffers. * The audio rate is determined by the track, not stored in this class. */ -class AudioSegment final : public MediaSegmentBase { +class AudioSegment : public MediaSegmentBase { // The channel count that MaxChannelCount() returned last time it was called. uint32_t mMemoizedMaxChannelCount = 0; diff --git a/dom/media/VideoFrameConverter.h b/dom/media/VideoFrameConverter.h index 7b4b259e3e05..36132c1b45be 100644 --- a/dom/media/VideoFrameConverter.h +++ b/dom/media/VideoFrameConverter.h @@ -109,8 +109,8 @@ class VideoFrameConverter { // for processing so it can be immediately sent. mLastFrameQueuedForProcessing.mTime = time; - MOZ_ALWAYS_SUCCEEDS( - mTaskQueue->Dispatch(NewRunnableMethod( + MOZ_ALWAYS_SUCCEEDS(mTaskQueue->Dispatch( + NewRunnableMethod>( "VideoFrameConverter::ProcessVideoFrame", this, &VideoFrameConverter::ProcessVideoFrame, mLastFrameQueuedForProcessing))); @@ -138,8 +138,8 @@ class VideoFrameConverter { mLastFrameQueuedForProcessing.mForceBlack = true; mLastFrameQueuedForProcessing.mImage = nullptr; - MOZ_ALWAYS_SUCCEEDS( - mTaskQueue->Dispatch(NewRunnableMethod( + MOZ_ALWAYS_SUCCEEDS(mTaskQueue->Dispatch( + NewRunnableMethod>( "VideoFrameConverter::ProcessVideoFrame", this, &VideoFrameConverter::ProcessVideoFrame, mLastFrameQueuedForProcessing))); @@ -293,10 +293,11 @@ class VideoFrameConverter { return; } - MOZ_ALWAYS_SUCCEEDS(mTaskQueue->Dispatch(NewRunnableMethod( - "VideoFrameConverter::ProcessVideoFrame", this, - &VideoFrameConverter::ProcessVideoFrame, - mLastFrameQueuedForProcessing))); + MOZ_ALWAYS_SUCCEEDS(mTaskQueue->Dispatch( + NewRunnableMethod>( + "VideoFrameConverter::ProcessVideoFrame", this, + &VideoFrameConverter::ProcessVideoFrame, + mLastFrameQueuedForProcessing))); } void ProcessVideoFrame(const FrameToProcess& aFrame) { diff --git a/dom/media/webspeech/recognition/SpeechRecognition.cpp b/dom/media/webspeech/recognition/SpeechRecognition.cpp index 7239a882374e..75d1ba77094d 100644 --- a/dom/media/webspeech/recognition/SpeechRecognition.cpp +++ b/dom/media/webspeech/recognition/SpeechRecognition.cpp @@ -437,13 +437,12 @@ uint32_t SpeechRecognition::ProcessAudioSegment(AudioSegment* aSegment, // we need to call the nsISpeechRecognitionService::ProcessAudioSegment // in a separate thread so that any eventual encoding or pre-processing // of the audio does not block the main thread - nsresult rv = mEncodeTaskQueue->Dispatch(NS_NewRunnableFunction( - "nsISpeechRecognitionService::ProcessAudioSegment", - [=, service = mRecognitionService, - segment = std::move(*aSegment)]() mutable { - service->ProcessAudioSegment(&segment, aTrackRate); - })); - + nsresult rv = mEncodeTaskQueue->Dispatch( + NewRunnableMethod, TrackRate>( + "nsISpeechRecognitionService::ProcessAudioSegment", + mRecognitionService, + &nsISpeechRecognitionService::ProcessAudioSegment, + std::move(*aSegment), aTrackRate)); MOZ_DIAGNOSTIC_ASSERT(NS_SUCCEEDED(rv)); Unused << rv; return samples; diff --git a/gfx/layers/apz/src/APZCTreeManager.cpp b/gfx/layers/apz/src/APZCTreeManager.cpp index c37679712d93..61e0220e6496 100644 --- a/gfx/layers/apz/src/APZCTreeManager.cpp +++ b/gfx/layers/apz/src/APZCTreeManager.cpp @@ -394,7 +394,8 @@ void APZCTreeManager::SetAllowedTouchBehavior( uint64_t aInputBlockId, const nsTArray& aValues) { if (!APZThreadUtils::IsControllerThread()) { APZThreadUtils::RunOnControllerThread( - NewRunnableMethod>( + NewRunnableMethod>>( "layers::APZCTreeManager::SetAllowedTouchBehavior", this, &APZCTreeManager::SetAllowedTouchBehavior, aInputBlockId, aValues.Clone())); diff --git a/netwerk/wifi/nsWifiMonitor.cpp b/netwerk/wifi/nsWifiMonitor.cpp index f437dec4bd08..5a7be15d162c 100644 --- a/netwerk/wifi/nsWifiMonitor.cpp +++ b/netwerk/wifi/nsWifiMonitor.cpp @@ -364,13 +364,13 @@ nsresult nsWifiMonitor::DoScan() { } return NS_DispatchToMainThread( - NewRunnableMethod>, bool>( + NewRunnableMethod>&&, bool>( "CallWifiListeners", this, &nsWifiMonitor::CallWifiListeners, mLastAccessPoints.Clone(), accessPointsChanged)); } nsresult nsWifiMonitor::CallWifiListeners( - const nsTArray>& aAccessPoints, + nsTArray>&& aAccessPoints, bool aAccessPointsChanged) { MOZ_ASSERT(NS_IsMainThread()); LOG(("Sending wifi access points to the listeners")); diff --git a/netwerk/wifi/nsWifiMonitor.h b/netwerk/wifi/nsWifiMonitor.h index 55a3feedeaff..5a37cc75be05 100644 --- a/netwerk/wifi/nsWifiMonitor.h +++ b/netwerk/wifi/nsWifiMonitor.h @@ -69,7 +69,7 @@ class nsWifiMonitor final : public nsIWifiMonitor, public nsIObserver { nsresult DoScan(); nsresult CallWifiListeners( - const nsTArray>& aAccessPoints, + nsTArray>&& aAccessPoints, bool aAccessPointsChanged); nsresult PassErrorToWifiListeners(nsresult rv); diff --git a/xpcom/tests/gtest/TestThreadUtils.cpp b/xpcom/tests/gtest/TestThreadUtils.cpp index 96b134288597..2b9ff97192cc 100644 --- a/xpcom/tests/gtest/TestThreadUtils.cpp +++ b/xpcom/tests/gtest/TestThreadUtils.cpp @@ -885,88 +885,122 @@ TEST(ThreadUtils, IdleTaskRunner) TEST(ThreadUtils, TypeTraits) { - static_assert(std::is_same_v>, - "RemoveSmartPointer should be int"); - static_assert(std::is_same_v>, - "RemoveSmartPointer should be int*"); - static_assert(std::is_same_v, - mozilla::RemoveSmartPointer>>, - "RemoveSmartPointer> should be UniquePtr"); - static_assert(std::is_same_v>>, - "RemoveSmartPointer> should be int"); + static_assert(!mozilla::IsRefcountedSmartPointer::value, + "IsRefcountedSmartPointer should be false"); + static_assert(mozilla::IsRefcountedSmartPointer>::value, + "IsRefcountedSmartPointer> should be true"); + static_assert(mozilla::IsRefcountedSmartPointer>::value, + "IsRefcountedSmartPointer> should be true"); static_assert( - std::is_same_v>>, - "RemoveSmartPointer> should be int"); + mozilla::IsRefcountedSmartPointer>::value, + "IsRefcountedSmartPointer> should be true"); static_assert( - std::is_same_v>>, - "RemoveSmartPointer> should be int"); + mozilla::IsRefcountedSmartPointer>::value, + "IsRefcountedSmartPointer> should be true"); + static_assert(mozilla::IsRefcountedSmartPointer>::value, + "IsRefcountedSmartPointer> should be true"); + static_assert(mozilla::IsRefcountedSmartPointer>::value, + "IsRefcountedSmartPointer> should be true"); static_assert( - std::is_same_v>>, - "RemoveSmartPointer> should be int"); - static_assert(std::is_same_v>>, - "RemoveSmartPointer> should be int"); + mozilla::IsRefcountedSmartPointer>::value, + "IsRefcountedSmartPointer> should be true"); static_assert( - std::is_same_v>>, - "RemoveSmartPointer> should be int"); - static_assert( - std::is_same_v>>, - "RemoveSmartPointer> should be int"); - static_assert( - std::is_same_v>>, - "RemoveSmartPointer> should be int"); + mozilla::IsRefcountedSmartPointer>::value, + "IsRefcountedSmartPointer> should be true"); - static_assert(std::is_same_v>, - "RemoveRawOrSmartPointer should be int"); + static_assert(std::is_same_v::Type>, + "RemoveSmartPointer::Type should be int"); + static_assert(std::is_same_v::Type>, + "RemoveSmartPointer::Type should be int*"); static_assert( std::is_same_v, - mozilla::RemoveRawOrSmartPointer>>, - "RemoveRawOrSmartPointer> should be UniquePtr"); - static_assert(std::is_same_v>, - "RemoveRawOrSmartPointer should be int"); + mozilla::RemoveSmartPointer>::Type>, + "RemoveSmartPointer>::Type should be UniquePtr"); static_assert( - std::is_same_v>, - "RemoveRawOrSmartPointer should be const int"); + std::is_same_v>::Type>, + "RemoveSmartPointer>::Type should be int"); + static_assert( + std::is_same_v>::Type>, + "RemoveSmartPointer>::Type should be int"); + static_assert( + std::is_same_v>::Type>, + "RemoveSmartPointer>::Type should be int"); + static_assert( + std::is_same_v< + int, mozilla::RemoveSmartPointer>::Type>, + "RemoveSmartPointer>::Type should be int"); + static_assert( + std::is_same_v>::Type>, + "RemoveSmartPointer>::Type should be int"); + static_assert( + std::is_same_v>::Type>, + "RemoveSmartPointer>::Type should be int"); + static_assert( + std::is_same_v>::Type>, + "RemoveSmartPointer>::Type should be int"); + static_assert( + std::is_same_v< + int, mozilla::RemoveSmartPointer>::Type>, + "RemoveSmartPointer>::Type should be int"); + + static_assert( + std::is_same_v::Type>, + "RemoveRawOrSmartPointer::Type should be int"); + static_assert( + std::is_same_v, + mozilla::RemoveRawOrSmartPointer>::Type>, + "RemoveRawOrSmartPointer>::Type should be UniquePtr"); + static_assert( + std::is_same_v::Type>, + "RemoveRawOrSmartPointer::Type should be int"); + static_assert( + std::is_same_v::Type>, + "RemoveRawOrSmartPointer::Type should be const int"); static_assert( std::is_same_v>, - "RemoveRawOrSmartPointer should be volatile int"); + mozilla::RemoveRawOrSmartPointer::Type>, + "RemoveRawOrSmartPointer::Type should be volatile int"); static_assert( - std::is_same_v>, - "RemoveRawOrSmartPointer should be const " + std::is_same_v::Type>, + "RemoveRawOrSmartPointer::Type should be const " "volatile int"); static_assert( - std::is_same_v>>, - "RemoveRawOrSmartPointer> should be int"); - static_assert( - std::is_same_v>>, - "RemoveRawOrSmartPointer> should be int"); + std::is_same_v>::Type>, + "RemoveRawOrSmartPointer>::Type should be int"); static_assert( std::is_same_v>>, - "RemoveRawOrSmartPointer> should be int"); + mozilla::RemoveRawOrSmartPointer>::Type>, + "RemoveRawOrSmartPointer>::Type should be int"); static_assert( std::is_same_v< - int, mozilla::RemoveRawOrSmartPointer>>, - "RemoveRawOrSmartPointer> should be " + int, mozilla::RemoveRawOrSmartPointer>::Type>, + "RemoveRawOrSmartPointer>::Type should be int"); + static_assert( + std::is_same_v>::Type>, + "RemoveRawOrSmartPointer>::Type should be " "int"); - static_assert( - std::is_same_v>>, - "RemoveRawOrSmartPointer> should be int"); static_assert( std::is_same_v>>, - "RemoveRawOrSmartPointer> should be int"); - static_assert( - std::is_same_v>>, - "RemoveRawOrSmartPointer> should be int"); + mozilla::RemoveRawOrSmartPointer>::Type>, + "RemoveRawOrSmartPointer>::Type should be int"); static_assert( std::is_same_v< - int, mozilla::RemoveRawOrSmartPointer>>, - "RemoveRawOrSmartPointer> should be " + int, mozilla::RemoveRawOrSmartPointer>::Type>, + "RemoveRawOrSmartPointer>::Type should be int"); + static_assert( + std::is_same_v< + int, mozilla::RemoveRawOrSmartPointer>::Type>, + "RemoveRawOrSmartPointer>::Type should be int"); + static_assert( + std::is_same_v>::Type>, + "RemoveRawOrSmartPointer>::Type should be " "int"); } @@ -1254,9 +1288,15 @@ TEST(ThreadUtils, main) static_assert(!IsParameterStorageClass::value, "'int' should not be recognized as Storage Class"); + static_assert( + IsParameterStorageClass>::value, + "StoreCopyPassByValue should be recognized as Storage Class"); static_assert( IsParameterStorageClass>::value, "StoreCopyPassByConstLRef should be recognized as Storage Class"); + static_assert( + IsParameterStorageClass>::value, + "StoreCopyPassByLRef should be recognized as Storage Class"); static_assert( IsParameterStorageClass>::value, "StoreCopyPassByRRef should be recognized as Storage Class"); @@ -1275,6 +1315,12 @@ TEST(ThreadUtils, main) static_assert( IsParameterStorageClass>::value, "StoreConstPtrPassByConstPtr should be recognized as Storage Class"); + static_assert( + IsParameterStorageClass>::value, + "StoreCopyPassByConstPtr should be recognized as Storage Class"); + static_assert( + IsParameterStorageClass>::value, + "StoreCopyPassByPtr should be recognized as Storage Class"); RefPtr rpt(new ThreadUtilsObject); int count = 0; @@ -1313,6 +1359,11 @@ TEST(ThreadUtils, main) StoreCopyPassByConstLRef>, "detail::ParameterStorage::Type should be " "StoreCopyPassByConstLRef"); + static_assert(std::is_same_v< + ::detail::ParameterStorage>::Type, + StoreCopyPassByValue>, + "detail::ParameterStorage>::Type " + "should be StoreCopyPassByValue"); r1 = NewRunnableMethod("TestThreadUtils::ThreadUtilsObject::Test1i", rpt, &ThreadUtilsObject::Test1i, 12); @@ -1429,6 +1480,37 @@ TEST(ThreadUtils, main) EXPECT_EQ(i, rpt->mA0); } + // Raw pointer to copy. + static_assert(std::is_same_v::stored_type, int>, + "StoreCopyPassByPtr::stored_type should be int"); + static_assert(std::is_same_v::passed_type, int*>, + "StoreCopyPassByPtr::passed_type should be int*"); + { + int i = 1202; + r1 = NewRunnableMethod>( + "TestThreadUtils::ThreadUtilsObject::Test1pi", rpt, + &ThreadUtilsObject::Test1pi, i); + r1->Run(); + EXPECT_EQ(count += 2, rpt->mCount); + EXPECT_EQ(i, rpt->mA0); + } + + // Raw pointer to const copy. + static_assert(std::is_same_v::stored_type, int>, + "StoreCopyPassByConstPtr::stored_type should be int"); + static_assert( + std::is_same_v::passed_type, const int*>, + "StoreCopyPassByConstPtr::passed_type should be const int*"); + { + int i = 1203; + r1 = NewRunnableMethod>( + "TestThreadUtils::ThreadUtilsObject::Test1pci", rpt, + &ThreadUtilsObject::Test1pci, i); + r1->Run(); + EXPECT_EQ(count += 2, rpt->mCount); + EXPECT_EQ(i, rpt->mA0); + } + // nsRefPtr to pointer. static_assert( std::is_same_v<::detail::ParameterStorage< @@ -1454,7 +1536,7 @@ TEST(ThreadUtils, main) // (more nsRefPtr tests below) // nsRefPtr for ref-countable classes that do not derive from ISupports. - static_assert(::detail::HasRefCountMethods, + static_assert(::detail::HasRefCountMethods::value, "ThreadUtilsRefCountedFinal has AddRef() and Release()"); static_assert( std::is_same_v< @@ -1462,7 +1544,7 @@ TEST(ThreadUtils, main) StoreRefPtrPassByPtr>, "ParameterStorage::Type should be " "StoreRefPtrPassByPtr"); - static_assert(::detail::HasRefCountMethods, + static_assert(::detail::HasRefCountMethods::value, "ThreadUtilsRefCountedBase has AddRef() and Release()"); static_assert( std::is_same_v< @@ -1470,8 +1552,9 @@ TEST(ThreadUtils, main) StoreRefPtrPassByPtr>, "ParameterStorage::Type should be " "StoreRefPtrPassByPtr"); - static_assert(::detail::HasRefCountMethods, - "ThreadUtilsRefCountedDerived has AddRef() and Release()"); + static_assert( + ::detail::HasRefCountMethods::value, + "ThreadUtilsRefCountedDerived has AddRef() and Release()"); static_assert( std::is_same_v< ::detail::ParameterStorage::Type, @@ -1479,7 +1562,7 @@ TEST(ThreadUtils, main) "ParameterStorage::Type should be " "StoreRefPtrPassByPtr"); - static_assert(!::detail::HasRefCountMethods, + static_assert(!::detail::HasRefCountMethods::value, "ThreadUtilsNonRefCounted doesn't have AddRef() and Release()"); static_assert(!std::is_same_v< ::detail::ParameterStorage::Type, @@ -1653,6 +1736,126 @@ TEST(ThreadUtils, main) // Verify copy/move assumptions. + Spy::ClearAll(); + if (gDebug) { + printf("%d - Test: Store copy from lvalue, pass by value\n", __LINE__); + } + { // Block around nsCOMPtr lifetime. + nsCOMPtr r2; + { // Block around Spy lifetime. + if (gDebug) { + printf("%d - Spy s(10)\n", __LINE__); + } + Spy s(10); + EXPECT_EQ(1, gConstructions); + EXPECT_EQ(1, gAlive); + if (gDebug) { + printf( + "%d - r2 = " + "NewRunnableMethod>(&TestByValue, s)\n", + __LINE__); + } + r2 = NewRunnableMethod>( + "TestThreadUtils::ThreadUtilsObject::TestByValue", rpt, + &ThreadUtilsObject::TestByValue, s); + EXPECT_EQ(2, gAlive); + EXPECT_LE(1, gCopyConstructions); // At least 1 copy-construction. + Spy::ClearActions(); + if (gDebug) { + printf("%d - End block with Spy s(10)\n", __LINE__); + } + } + EXPECT_EQ(1, gDestructions); + EXPECT_EQ(1, gAlive); + Spy::ClearActions(); + if (gDebug) { + printf("%d - Run()\n", __LINE__); + } + r2->Run(); + EXPECT_LE(1, gCopyConstructions); // Another copy-construction in call. + EXPECT_EQ(10, rpt->mSpy.mID); + EXPECT_LE(1, gDestructions); + EXPECT_EQ(1, gAlive); + Spy::ClearActions(); + if (gDebug) { + printf("%d - End block with r\n", __LINE__); + } + } + if (gDebug) { + printf("%d - After end block with r\n", __LINE__); + } + EXPECT_EQ(1, gDestructions); + EXPECT_EQ(0, gAlive); + + Spy::ClearAll(); + if (gDebug) { + printf("%d - Test: Store copy from prvalue, pass by value\n", __LINE__); + } + { + if (gDebug) { + printf( + "%d - r3 = " + "NewRunnableMethod>(&TestByValue, " + "Spy(11))\n", + __LINE__); + } + nsCOMPtr r3 = NewRunnableMethod>( + "TestThreadUtils::ThreadUtilsObject::TestByValue", rpt, + &ThreadUtilsObject::TestByValue, Spy(11)); + EXPECT_EQ(1, gAlive); + EXPECT_EQ(1, gConstructions); + EXPECT_LE(1, gMoveConstructions); + Spy::ClearActions(); + if (gDebug) { + printf("%d - Run()\n", __LINE__); + } + r3->Run(); + EXPECT_LE(1, gCopyConstructions); // Another copy-construction in call. + EXPECT_EQ(11, rpt->mSpy.mID); + EXPECT_LE(1, gDestructions); + EXPECT_EQ(1, gAlive); + Spy::ClearActions(); + if (gDebug) { + printf("%d - End block with r\n", __LINE__); + } + } + if (gDebug) { + printf("%d - After end block with r\n", __LINE__); + } + EXPECT_EQ(1, gDestructions); + EXPECT_EQ(0, gAlive); + + Spy::ClearAll(); + { // Store copy from xvalue, pass by value. + nsCOMPtr r4; + { + Spy s(12); + EXPECT_EQ(1, gConstructions); + EXPECT_EQ(1, gAlive); + Spy::ClearActions(); + r4 = NewRunnableMethod>( + "TestThreadUtils::ThreadUtilsObject::TestByValue", rpt, + &ThreadUtilsObject::TestByValue, std::move(s)); + EXPECT_LE(1, gMoveConstructions); + EXPECT_EQ(1, gAlive); + EXPECT_EQ(1, gZombies); + Spy::ClearActions(); + } + EXPECT_EQ(1, gDestructions); + EXPECT_EQ(1, gAlive); + EXPECT_EQ(0, gZombies); + Spy::ClearActions(); + r4->Run(); + EXPECT_LE(1, gCopyConstructions); // Another copy-construction in call. + EXPECT_EQ(12, rpt->mSpy.mID); + EXPECT_LE(1, gDestructions); + EXPECT_EQ(1, gAlive); + Spy::ClearActions(); + } + EXPECT_EQ(1, gDestructions); + EXPECT_EQ(0, gAlive); + // Won't test xvalues anymore, prvalues are enough to verify all rvalues. + Spy::ClearAll(); if (gDebug) { printf("%d - Test: Store copy from lvalue, pass by const lvalue ref\n", diff --git a/xpcom/threads/MozPromise.h b/xpcom/threads/MozPromise.h index af4ac657fd12..0700b8dda4ea 100644 --- a/xpcom/threads/MozPromise.h +++ b/xpcom/threads/MozPromise.h @@ -82,22 +82,24 @@ struct MethodTrait : MethodTraitsHelper> {}; } // namespace detail -template -using MethodReturnType = typename detail::MethodTrait::ReturnType; - template -constexpr bool TakesAnyArguments = - detail::MethodTrait::ArgSize != 0; +using TakesArgument = + std::integral_constant::ArgSize != 0>; + +template +using ReturnTypeIs = + std::is_convertible::ReturnType, + TargetType>; template class MozPromise; -template -constexpr bool IsMozPromise = false; +template +struct IsMozPromise : std::false_type {}; template -constexpr bool - IsMozPromise> = true; +struct IsMozPromise> + : std::true_type {}; /* * A promise manages an asynchronous request that may or may not be able to be @@ -174,8 +176,8 @@ class MozPromise : public MozPromiseBase { } public: - using ResolveValueType = ResolveValueT; - using RejectValueType = RejectValueT; + typedef ResolveValueT ResolveValueType; + typedef RejectValueT RejectValueType; class ResolveOrRejectValue { public: template @@ -286,10 +288,12 @@ class MozPromise : public MozPromiseBase { return p; } - using AllPromiseType = MozPromise, - RejectValueType, IsExclusive>; - using AllSettledPromiseType = - MozPromise, bool, IsExclusive>; + typedef MozPromise, RejectValueType, + IsExclusive> + AllPromiseType; + + typedef MozPromise, bool, IsExclusive> + AllSettledPromiseType; private: class AllPromiseHolder : public MozPromiseRefcountable { @@ -346,17 +350,17 @@ class MozPromise : public MozPromiseBase { // Trying to pass ResolveOrRejectValue by value fails static analysis checks, // so we need to use either a const& or an rvalue reference, depending on // whether IsExclusive is true or not. - using ResolveOrRejectValueParam = - std::conditional_t; + typedef std::conditional_t + ResolveOrRejectValueParam; - using ResolveValueTypeParam = - std::conditional_t; + typedef std::conditional_t + ResolveValueTypeParam; - using RejectValueTypeParam = - std::conditional_t; + typedef std::conditional_t + RejectValueTypeParam; class AllSettledPromiseHolder : public MozPromiseRefcountable { public: @@ -644,15 +648,15 @@ class MozPromise : public MozPromiseBase { * make the resolve/reject value argument "optional". */ template - static std::enable_if_t, - MethodReturnType> + static std::enable_if_t::value, + typename detail::MethodTrait::ReturnType> InvokeMethod(ThisType* aThisVal, MethodType aMethod, ValueType&& aValue) { return (aThisVal->*aMethod)(std::forward(aValue)); } template - static std::enable_if_t, - MethodReturnType> + static std::enable_if_t::value, + typename detail::MethodTrait::ReturnType> InvokeMethod(ThisType* aThisVal, MethodType aMethod, ValueType&& aValue) { return (aThisVal->*aMethod)(); } @@ -693,14 +697,18 @@ class MozPromise : public MozPromiseBase { : public ThenValueBase { friend class ThenCommand; - using R1 = RemoveSmartPointer>; - using R2 = RemoveSmartPointer>; - constexpr static bool SupportChaining = - IsMozPromise && std::is_same_v; + using R1 = typename RemoveSmartPointer< + typename detail::MethodTrait::ReturnType>::Type; + using R2 = typename RemoveSmartPointer< + typename detail::MethodTrait::ReturnType>::Type; + using SupportChaining = + std::integral_constant::value && + std::is_same_v>; // Fall back to MozPromise when promise chaining is not supported to make // code compile. - using PromiseType = std::conditional_t; + using PromiseType = + std::conditional_t; public: ThenValue(nsISerialEventTarget* aResponseTarget, ThisType* aThisVal, @@ -728,13 +736,13 @@ class MozPromise : public MozPromiseBase { void DoResolveOrRejectInternal(ResolveOrRejectValue& aValue) override { if (aValue.IsResolve()) { - InvokeCallbackMethod(mThisVal.get(), mResolveMethod, - MaybeMove(aValue.ResolveValue()), - std::move(mCompletionPromise)); + InvokeCallbackMethod( + mThisVal.get(), mResolveMethod, MaybeMove(aValue.ResolveValue()), + std::move(mCompletionPromise)); } else { - InvokeCallbackMethod(mThisVal.get(), mRejectMethod, - MaybeMove(aValue.RejectValue()), - std::move(mCompletionPromise)); + InvokeCallbackMethod( + mThisVal.get(), mRejectMethod, MaybeMove(aValue.RejectValue()), + std::move(mCompletionPromise)); } // Null out mThisVal after invoking the callback so that any references @@ -756,12 +764,15 @@ class MozPromise : public MozPromiseBase { class ThenValue : public ThenValueBase { friend class ThenCommand; - using R1 = RemoveSmartPointer>; - constexpr static bool SupportChaining = IsMozPromise; + using R1 = typename RemoveSmartPointer::ReturnType>::Type; + using SupportChaining = + std::integral_constant::value>; // Fall back to MozPromise when promise chaining is not supported to make // code compile. - using PromiseType = std::conditional_t; + using PromiseType = + std::conditional_t; public: ThenValue(nsISerialEventTarget* aResponseTarget, ThisType* aThisVal, @@ -787,7 +798,7 @@ class MozPromise : public MozPromiseBase { } void DoResolveOrRejectInternal(ResolveOrRejectValue& aValue) override { - InvokeCallbackMethod( + InvokeCallbackMethod( mThisVal.get(), mResolveRejectMethod, MaybeMove(aValue), std::move(mCompletionPromise)); @@ -811,14 +822,18 @@ class MozPromise : public MozPromiseBase { class ThenValue : public ThenValueBase { friend class ThenCommand; - using R1 = RemoveSmartPointer>; - using R2 = RemoveSmartPointer>; - constexpr static bool SupportChaining = - IsMozPromise && std::is_same_v; + using R1 = typename RemoveSmartPointer< + typename detail::MethodTrait::ReturnType>::Type; + using R2 = typename RemoveSmartPointer< + typename detail::MethodTrait::ReturnType>::Type; + using SupportChaining = + std::integral_constant::value && + std::is_same_v>; // Fall back to MozPromise when promise chaining is not supported to make // code compile. - using PromiseType = std::conditional_t; + using PromiseType = + std::conditional_t; public: ThenValue(nsISerialEventTarget* aResponseTarget, @@ -852,11 +867,11 @@ class MozPromise : public MozPromiseBase { // easily. We could fix this if need be, though it's quite easy to work // around by just capturing something. if (aValue.IsResolve()) { - InvokeCallbackMethod( + InvokeCallbackMethod( mResolveFunction.ptr(), &ResolveFunction::operator(), MaybeMove(aValue.ResolveValue()), std::move(mCompletionPromise)); } else { - InvokeCallbackMethod( + InvokeCallbackMethod( mRejectFunction.ptr(), &RejectFunction::operator(), MaybeMove(aValue.RejectValue()), std::move(mCompletionPromise)); } @@ -881,12 +896,15 @@ class MozPromise : public MozPromiseBase { class ThenValue : public ThenValueBase { friend class ThenCommand; - using R1 = RemoveSmartPointer>; - constexpr static bool SupportChaining = IsMozPromise; + using R1 = typename RemoveSmartPointer< + typename detail::MethodTrait::ReturnType>::Type; + using SupportChaining = + std::integral_constant::value>; // Fall back to MozPromise when promise chaining is not supported to make // code compile. - using PromiseType = std::conditional_t; + using PromiseType = + std::conditional_t; public: ThenValue(nsISerialEventTarget* aResponseTarget, @@ -917,7 +935,7 @@ class MozPromise : public MozPromiseBase { // classes with ::operator()), since it allows us to share code more // easily. We could fix this if need be, though it's quite easy to work // around by just capturing something. - InvokeCallbackMethod( + InvokeCallbackMethod( mResolveRejectFunction.ptr(), &ResolveRejectFunction::operator(), MaybeMove(aValue), std::move(mCompletionPromise)); @@ -980,7 +998,7 @@ class MozPromise : public MozPromiseBase { MozPromise* aReceiver) : mCallSite(aCallSite), mThenValue(aThenValue), mReceiver(aReceiver) {} - ThenCommand(ThenCommand&& aOther) noexcept = default; + ThenCommand(ThenCommand&& aOther) = default; public: ~ThenCommand() { @@ -995,7 +1013,7 @@ class MozPromise : public MozPromiseBase { // p->Then(thread2, ...); operator RefPtr() { static_assert( - ThenValueType::SupportChaining, + ThenValueType::SupportChaining::value, "The resolve/reject callback needs to return a RefPtr " "in order to do promise chaining."); @@ -1318,18 +1336,12 @@ class MozPromise::Private }; // A generic promise type that does the trick for simple use cases. -// -// Vaguely deprecated: prefer explicitly naming the resolve- and reject-type. -// Additionally, prefer `mozilla::Ok` as the resolve-type if the boolean's value -// is irrelevant. -using GenericPromise = MozPromise; +typedef MozPromise GenericPromise; // A generic, non-exclusive promise type that does the trick for simple use // cases. -// -// Vaguely deprecated, as above. -using GenericNonExclusivePromise = - MozPromise; +typedef MozPromise + GenericNonExclusivePromise; /* * Class to encapsulate a promise for a particular role. Use this as the member @@ -1340,9 +1352,8 @@ class MozPromiseHolderBase { public: MozPromiseHolderBase() = default; - MozPromiseHolderBase(MozPromiseHolderBase&& aOther) noexcept = default; - MozPromiseHolderBase& operator=(MozPromiseHolderBase&& aOther) noexcept = - default; + MozPromiseHolderBase(MozPromiseHolderBase&& aOther) = default; + MozPromiseHolderBase& operator=(MozPromiseHolderBase&& aOther) = default; ~MozPromiseHolderBase() { MOZ_ASSERT(!mPromise); } @@ -1689,10 +1700,38 @@ class ProxyFunctionRunnable : public CancelableRunnable { UniquePtr mFunction; }; -template -constexpr static bool IsRefPtrMozPromise = false; -template -constexpr static bool IsRefPtrMozPromise>> = true; +// Note: The following struct and function are not for public consumption (yet?) +// as we would prefer all calls to pass on-the-spot lambdas (or at least moved +// function objects). They could be moved outside of detail if really needed. + +// We prefer getting function objects by non-lvalue-ref (to avoid copying them +// and their captures). This struct is a tag that allows the use of objects +// through lvalue-refs where necessary. +struct AllowInvokeAsyncFunctionLVRef {}; + +// Invoke a function object (e.g., lambda or std/mozilla::function) +// asynchronously; note that the object will be copied if provided by +// lvalue-ref. Return a promise that the function should eventually resolve or +// reject. +template +static auto InvokeAsync(nsISerialEventTarget* aTarget, const char* aCallerName, + AllowInvokeAsyncFunctionLVRef, Function&& aFunction) + -> decltype(aFunction()) { + static_assert( + IsRefcountedSmartPointer::value && + IsMozPromise< + typename RemoveSmartPointer::Type>::value, + "Function object must return RefPtr"); + MOZ_ASSERT(aTarget); + typedef typename RemoveSmartPointer::Type PromiseType; + typedef detail::ProxyFunctionRunnable + ProxyRunnableType; + + auto p = MakeRefPtr(aCallerName); + auto r = MakeRefPtr(p, std::forward(aFunction)); + aTarget->Dispatch(r.forget()); + return p; +} } // namespace detail @@ -1704,18 +1743,9 @@ static auto InvokeAsync(nsISerialEventTarget* aTarget, const char* aCallerName, static_assert(!std::is_lvalue_reference_v, "Function object must not be passed by lvalue-ref (to avoid " "unplanned copies); Consider move()ing the object."); - - static_assert(detail::IsRefPtrMozPromise, - "Function object must return RefPtr"); - MOZ_ASSERT(aTarget); - typedef RemoveSmartPointer PromiseType; - typedef detail::ProxyFunctionRunnable - ProxyRunnableType; - - auto p = MakeRefPtr(aCallerName); - auto r = MakeRefPtr(p, std::forward(aFunction)); - aTarget->Dispatch(r.forget()); - return p; + return detail::InvokeAsync(aTarget, aCallerName, + detail::AllowInvokeAsyncFunctionLVRef(), + std::forward(aFunction)); } # undef PROMISE_LOG diff --git a/xpcom/threads/nsThreadUtils.h b/xpcom/threads/nsThreadUtils.h index 755ec21c9563..72041da295ab 100644 --- a/xpcom/threads/nsThreadUtils.h +++ b/xpcom/threads/nsThreadUtils.h @@ -562,40 +562,71 @@ using RunnableFunctionImpl = namespace detail { +template +struct IsRefcountedSmartPointerHelper : std::false_type {}; + +template +struct IsRefcountedSmartPointerHelper> : std::true_type {}; + +template +struct IsRefcountedSmartPointerHelper> : std::true_type {}; + +} // namespace detail + template +struct IsRefcountedSmartPointer + : detail::IsRefcountedSmartPointerHelper> {}; + +namespace detail { + +template struct RemoveSmartPointerHelper { - using Type = T; + typedef T Type; }; -template -struct RemoveSmartPointerHelper> { - using Type = T; +template +struct RemoveSmartPointerHelper> { + typedef Pointee Type; }; -template -struct RemoveSmartPointerHelper> { - using Type = T; -}; - -template -struct RemoveRawOrSmartPointerHelper { - using Type = typename RemoveSmartPointerHelper::Type; -}; - -template -struct RemoveRawOrSmartPointerHelper { - using Type = T; +template +struct RemoveSmartPointerHelper> { + typedef Pointee Type; }; } // namespace detail template -using RemoveSmartPointer = - typename detail::RemoveSmartPointerHelper>::Type; +struct RemoveSmartPointer + : detail::RemoveSmartPointerHelper> {}; + +namespace detail { + +template +struct RemoveRawOrSmartPointerHelper { + typedef T Type; +}; + +template +struct RemoveRawOrSmartPointerHelper { + typedef Pointee Type; +}; + +template +struct RemoveRawOrSmartPointerHelper> { + typedef Pointee Type; +}; + +template +struct RemoveRawOrSmartPointerHelper> { + typedef Pointee Type; +}; + +} // namespace detail template -using RemoveRawOrSmartPointer = - typename detail::RemoveRawOrSmartPointerHelper>::Type; +struct RemoveRawOrSmartPointer + : detail::RemoveRawOrSmartPointerHelper> {}; } // namespace mozilla @@ -766,22 +797,23 @@ struct nsRunnableMethodTraits; template struct nsRunnableMethodTraits { - using class_type = mozilla::RemoveRawOrSmartPointer; + typedef typename mozilla::RemoveRawOrSmartPointer::Type class_type; static_assert(std::is_base_of::value, "Stored class must inherit from method's class"); - using return_type = R; - using base_type = nsRunnableMethod; + typedef R return_type; + typedef nsRunnableMethod base_type; static const bool can_cancel = Kind == mozilla::RunnableKind::Cancelable; }; template struct nsRunnableMethodTraits { - using class_type = const mozilla::RemoveRawOrSmartPointer; + typedef const typename mozilla::RemoveRawOrSmartPointer::Type + class_type; static_assert(std::is_base_of::value, "Stored class must inherit from method's class"); - using return_type = R; - using base_type = nsRunnableMethod; + typedef R return_type; + typedef nsRunnableMethod base_type; static const bool can_cancel = Kind == mozilla::RunnableKind::Cancelable; }; @@ -790,22 +822,22 @@ template struct nsRunnableMethodTraits { - using class_type = mozilla::RemoveRawOrSmartPointer; + typedef typename mozilla::RemoveRawOrSmartPointer::Type class_type; static_assert(std::is_base_of::value, "Stored class must inherit from method's class"); - using return_type = R; - using base_type = nsRunnableMethod; + typedef R return_type; + typedef nsRunnableMethod base_type; static const bool can_cancel = Kind == mozilla::RunnableKind::Cancelable; }; template struct nsRunnableMethodTraits { - using class_type = mozilla::RemoveRawOrSmartPointer; + typedef typename mozilla::RemoveRawOrSmartPointer::Type class_type; static_assert(std::is_base_of::value, "Stored class must inherit from method's class"); - using return_type = R; - using base_type = nsRunnableMethod; + typedef R return_type; + typedef nsRunnableMethod base_type; static const bool can_cancel = Kind == mozilla::RunnableKind::Cancelable; }; @@ -813,11 +845,12 @@ template struct nsRunnableMethodTraits { - using class_type = const mozilla::RemoveRawOrSmartPointer; + typedef const typename mozilla::RemoveRawOrSmartPointer::Type + class_type; static_assert(std::is_base_of::value, "Stored class must inherit from method's class"); - using return_type = R; - using base_type = nsRunnableMethod; + typedef R return_type; + typedef nsRunnableMethod base_type; static const bool can_cancel = Kind == mozilla::RunnableKind::Cancelable; }; @@ -825,11 +858,12 @@ template struct nsRunnableMethodTraits { - using class_type = const mozilla::RemoveRawOrSmartPointer; + typedef const typename mozilla::RemoveRawOrSmartPointer::Type + class_type; static_assert(std::is_base_of::value, "Stored class must inherit from method's class"); - using return_type = R; - using base_type = nsRunnableMethod; + typedef R return_type; + typedef nsRunnableMethod base_type; static const bool can_cancel = Kind == mozilla::RunnableKind::Cancelable; }; # endif @@ -846,6 +880,19 @@ struct IsParameterStorageClass : public std::false_type {}; // StoreXPassByY structs used to inform nsRunnableMethodArguments how to // store arguments, and how to pass them to the target method. +template +struct StoreCopyPassByValue { + using stored_type = std::decay_t; + typedef stored_type passed_type; + stored_type m; + template + MOZ_IMPLICIT StoreCopyPassByValue(A&& a) : m(std::forward(a)) {} + passed_type PassAsParameter() { return m; } +}; +template +struct IsParameterStorageClass> + : public std::true_type {}; + template struct StoreCopyPassByConstLRef { using stored_type = std::decay_t; @@ -859,6 +906,19 @@ template struct IsParameterStorageClass> : public std::true_type {}; +template +struct StoreCopyPassByLRef { + using stored_type = std::decay_t; + typedef stored_type& passed_type; + stored_type m; + template + MOZ_IMPLICIT StoreCopyPassByLRef(A&& a) : m(std::forward(a)) {} + passed_type PassAsParameter() { return m; } +}; +template +struct IsParameterStorageClass> : public std::true_type { +}; + template struct StoreCopyPassByRRef { using stored_type = std::decay_t; @@ -936,6 +996,32 @@ template struct IsParameterStorageClass> : public std::true_type {}; +template +struct StoreCopyPassByConstPtr { + typedef T stored_type; + typedef const T* passed_type; + stored_type m; + template + MOZ_IMPLICIT StoreCopyPassByConstPtr(A&& a) : m(std::forward(a)) {} + passed_type PassAsParameter() { return &m; } +}; +template +struct IsParameterStorageClass> + : public std::true_type {}; + +template +struct StoreCopyPassByPtr { + typedef T stored_type; + typedef T* passed_type; + stored_type m; + template + MOZ_IMPLICIT StoreCopyPassByPtr(A&& a) : m(std::forward(a)) {} + passed_type PassAsParameter() { return &m; } +}; +template +struct IsParameterStorageClass> : public std::true_type { +}; + namespace detail { template @@ -949,8 +1035,69 @@ template static auto HasRefCountMethodsTest(long) -> std::false_type; template -constexpr static bool HasRefCountMethods = - decltype(HasRefCountMethodsTest(0))::value; +struct HasRefCountMethods : decltype(HasRefCountMethodsTest(0)) {}; + +template +struct NonnsISupportsPointerStorageClass + : std::conditional< + std::is_const_v, + StoreConstPtrPassByConstPtr>, + StorePtrPassByPtr> { + using Type = typename NonnsISupportsPointerStorageClass::conditional::type; +}; + +template +struct PointerStorageClass + : std::conditional< + HasRefCountMethods::value, + StoreRefPtrPassByPtr, + typename NonnsISupportsPointerStorageClass::Type> { + using Type = typename PointerStorageClass::conditional::type; +}; + +template +struct LValueReferenceStorageClass + : std::conditional< + std::is_const_v, + StoreConstRefPassByConstLRef>, + StoreRefPassByLRef> { + using Type = typename LValueReferenceStorageClass::conditional::type; +}; + +template +struct SmartPointerStorageClass + : std::conditional< + mozilla::IsRefcountedSmartPointer::value, + StoreRefPtrPassByPtr::Type>, + StoreCopyPassByConstLRef> { + using Type = typename SmartPointerStorageClass::conditional::type; +}; + +template +struct NonLValueReferenceStorageClass + : std::conditional, + StoreCopyPassByRRef>, + typename SmartPointerStorageClass::Type> { + using Type = typename NonLValueReferenceStorageClass::conditional::type; +}; + +template +struct NonPointerStorageClass + : std::conditional, + typename LValueReferenceStorageClass< + std::remove_reference_t>::Type, + typename NonLValueReferenceStorageClass::Type> { + using Type = typename NonPointerStorageClass::conditional::type; +}; + +template +struct NonParameterStorageClass + : std::conditional< + std::is_pointer_v, + typename PointerStorageClass>::Type, + typename NonPointerStorageClass::Type> { + using Type = typename NonParameterStorageClass::conditional::type; +}; // Choose storage&passing strategy based on preferred storage type: // - If IsParameterStorageClass::value is true, use as-is. @@ -964,86 +1111,18 @@ constexpr static bool HasRefCountMethods = // - RefPtr, nsCOMPtr // -> StoreRefPtrPassByPtr :Store RefPtr, pass T* // - Other T -> StoreCopyPassByConstLRef :Store T, pass const T&. -// -// For anything less common, please use a lambda function rather than devising -// new parameter-storage classes. (In fact, consider doing that anyway.) - +// Other available explicit options: +// - StoreCopyPassByValue :Store T, pass T. +// - StoreCopyPassByLRef :Store T, pass T& (of copy!) +// - StoreCopyPassByConstPtr :Store T, pass const T* +// - StoreCopyPassByPtr :Store T, pass T* (of copy!) +// Or create your own class with PassAsParameter() method, optional +// clean-up in destructor, and with associated IsParameterStorageClass<>. template -struct OtherParameterStorage; - -// The `IsParameterStorageClass` and `RC*` cases must be handled separately (see -// `ParameterStorageHelper`, below) until we can use C++20 concepts. - -template -struct OtherParameterStorage { - using Type = StoreConstPtrPassByConstPtr; -}; - -template -struct OtherParameterStorage { - using Type = StorePtrPassByPtr; -}; - -template -struct OtherParameterStorage { - using Type = StoreConstRefPassByConstLRef; -}; - -template -struct OtherParameterStorage { - using Type = StoreRefPassByLRef; -}; - -template -struct OtherParameterStorage> { - using Type = StoreRefPtrPassByPtr; -}; - -template -struct OtherParameterStorage> { - using Type = StoreRefPtrPassByPtr; -}; - -template -struct OtherParameterStorage { - using Type = StoreCopyPassByRRef; -}; - -template -struct OtherParameterStorage { - // This is good advice regardless of the types you're handling. - static_assert(!SFINAE1True::value, "please use a lambda function"); -}; - -// default impl. -template -struct OtherParameterStorage { - using Type = StoreCopyPassByConstLRef; -}; - -template ::value, - bool B = std::is_pointer_v && - HasRefCountMethods>> -struct ParameterStorageHelper; - -template -struct ParameterStorageHelper { - using Type = T; -}; - -template -struct ParameterStorageHelper { - using Type = StoreRefPtrPassByPtr>; -}; - -template -struct ParameterStorageHelper { - using Type = typename OtherParameterStorage>::Type; -}; - -template -struct ParameterStorage { - using Type = typename ParameterStorageHelper::Type; +struct ParameterStorage + : std::conditional::value, T, + typename NonParameterStorageClass::Type> { + using Type = typename ParameterStorage::conditional::type; }; template