From 6cf9261d16c6e482e43c433af8f66096cd8cbaa4 Mon Sep 17 00:00:00 2001 From: Timothy Nikkel Date: Sun, 25 Feb 2024 04:54:33 +0000 Subject: [PATCH] Bug 1880054. Simplify some imagelib event target code. r=gfx-reviewers,lsalzman This code was added before we decided to do fission, when we wanted to separate out different sites that were in the same process so we could prioritize them better. We are not going down that path so we can simplify this code. There should be no change in functionality with this patch, just simpler code. Differential Revision: https://phabricator.services.mozilla.com/D201705 --- image/IDecodingTask.cpp | 73 +++++++++++---------------------------- image/IDecodingTask.h | 7 ---- image/IProgressObserver.h | 2 -- image/Image.cpp | 2 +- image/ProgressTracker.cpp | 4 --- image/ProgressTracker.h | 3 -- image/RasterImage.cpp | 7 +--- image/VectorImage.cpp | 7 +--- image/imgRequest.cpp | 22 +++++------- 9 files changed, 33 insertions(+), 94 deletions(-) diff --git a/image/IDecodingTask.cpp b/image/IDecodingTask.cpp index 1816c9056be6..670a6512754d 100644 --- a/image/IDecodingTask.cpp +++ b/image/IDecodingTask.cpp @@ -23,41 +23,9 @@ namespace image { // Helpers for sending notifications to the image associated with a decoder. /////////////////////////////////////////////////////////////////////////////// -void IDecodingTask::EnsureHasEventTarget(NotNull aImage) { - if (!mEventTarget) { - // We determine the event target as late as possible, at the first dispatch - // time, because the observers bound to an imgRequest will affect it. - // We cache it rather than query for the event target each time because the - // event target can change. We don't want to risk events being executed in - // a different order than they are dispatched, which can happen if we - // selected scheduler groups which have no ordering guarantees relative to - // each other (e.g. it moves from scheduler group A for doc group DA to - // scheduler group B for doc group DB due to changing observers -- if we - // dispatched the first event on A, and the second on B, we don't know which - // will execute first.) - RefPtr tracker = aImage->GetProgressTracker(); - if (tracker) { - mEventTarget = tracker->GetEventTarget(); - } else { - mEventTarget = GetMainThreadSerialEventTarget(); - } - } -} - -bool IDecodingTask::IsOnEventTarget() const { - // This is essentially equivalent to NS_IsOnMainThread() because all of the - // event targets are for the main thread (although perhaps with a different - // label / scheduler group). The observers in ProgressTracker may have - // different event targets from this, so this is just a best effort guess. - bool current = false; - mEventTarget->IsOnCurrentThread(¤t); - return current; -} - void IDecodingTask::NotifyProgress(NotNull aImage, NotNull aDecoder) { MOZ_ASSERT(aDecoder->HasProgress() && !aDecoder->IsMetadataDecode()); - EnsureHasEventTarget(aImage); // Capture the decoder's state. If we need to notify asynchronously, it's // important that we don't wait until the lambda actually runs to capture the @@ -72,7 +40,7 @@ void IDecodingTask::NotifyProgress(NotNull aImage, SurfaceFlags surfaceFlags = aDecoder->GetSurfaceFlags(); // Synchronously notify if we can. - if (IsOnEventTarget() && !(decoderFlags & DecoderFlags::ASYNC_NOTIFY)) { + if (NS_IsMainThread() && !(decoderFlags & DecoderFlags::ASYNC_NOTIFY)) { aImage->NotifyProgress(progress, invalidRect, frameCount, decoderFlags, surfaceFlags); return; @@ -86,21 +54,21 @@ void IDecodingTask::NotifyProgress(NotNull aImage, // We're forced to notify asynchronously. NotNull> image = aImage; - mEventTarget->Dispatch(CreateRenderBlockingRunnable(NS_NewRunnableFunction( - "IDecodingTask::NotifyProgress", - [=]() -> void { - image->NotifyProgress(progress, invalidRect, - frameCount, decoderFlags, - surfaceFlags); - })), - NS_DISPATCH_NORMAL); + nsCOMPtr eventTarget = GetMainThreadSerialEventTarget(); + eventTarget->Dispatch(CreateRenderBlockingRunnable(NS_NewRunnableFunction( + "IDecodingTask::NotifyProgress", + [=]() -> void { + image->NotifyProgress(progress, invalidRect, + frameCount, decoderFlags, + surfaceFlags); + })), + NS_DISPATCH_NORMAL); } void IDecodingTask::NotifyDecodeComplete(NotNull aImage, NotNull aDecoder) { MOZ_ASSERT(aDecoder->HasError() || !aDecoder->InFrame(), "Decode complete in the middle of a frame?"); - EnsureHasEventTarget(aImage); // Capture the decoder's state. DecoderFinalStatus finalStatus = aDecoder->FinalStatus(); @@ -113,7 +81,7 @@ void IDecodingTask::NotifyDecodeComplete(NotNull aImage, SurfaceFlags surfaceFlags = aDecoder->GetSurfaceFlags(); // Synchronously notify if we can. - if (IsOnEventTarget() && !(decoderFlags & DecoderFlags::ASYNC_NOTIFY)) { + if (NS_IsMainThread() && !(decoderFlags & DecoderFlags::ASYNC_NOTIFY)) { aImage->NotifyDecodeComplete(finalStatus, metadata, telemetry, progress, invalidRect, frameCount, decoderFlags, surfaceFlags); @@ -128,15 +96,16 @@ void IDecodingTask::NotifyDecodeComplete(NotNull aImage, // We're forced to notify asynchronously. NotNull> image = aImage; - mEventTarget->Dispatch(CreateRenderBlockingRunnable(NS_NewRunnableFunction( - "IDecodingTask::NotifyDecodeComplete", - [=]() -> void { - image->NotifyDecodeComplete( - finalStatus, metadata, telemetry, progress, - invalidRect, frameCount, decoderFlags, - surfaceFlags); - })), - NS_DISPATCH_NORMAL); + nsCOMPtr eventTarget = GetMainThreadSerialEventTarget(); + eventTarget->Dispatch(CreateRenderBlockingRunnable(NS_NewRunnableFunction( + "IDecodingTask::NotifyDecodeComplete", + [=]() -> void { + image->NotifyDecodeComplete( + finalStatus, metadata, telemetry, progress, + invalidRect, frameCount, decoderFlags, + surfaceFlags); + })), + NS_DISPATCH_NORMAL); } /////////////////////////////////////////////////////////////////////////////// diff --git a/image/IDecodingTask.h b/image/IDecodingTask.h index b3bce74757e4..380e680950cf 100644 --- a/image/IDecodingTask.h +++ b/image/IDecodingTask.h @@ -53,13 +53,6 @@ class IDecodingTask : public IResumable { /// Notify @aImage that @aDecoder has finished. void NotifyDecodeComplete(NotNull aImage, NotNull aDecoder); - - private: - void EnsureHasEventTarget(NotNull aImage); - - bool IsOnEventTarget() const; - - nsCOMPtr mEventTarget; }; /** diff --git a/image/IProgressObserver.h b/image/IProgressObserver.h index fea5c3f148c3..3a497bcfdc25 100644 --- a/image/IProgressObserver.h +++ b/image/IProgressObserver.h @@ -10,8 +10,6 @@ #include "nsISupports.h" #include "nsRect.h" -class nsIEventTarget; - namespace mozilla { namespace image { diff --git a/image/Image.cpp b/image/Image.cpp index 16f754f4907d..596ac54cedc0 100644 --- a/image/Image.cpp +++ b/image/Image.cpp @@ -218,7 +218,7 @@ void ImageResource::SendOnUnlockedDraw(uint32_t aFlags) { mProgressTracker->OnUnlockedDraw(); } else { NotNull> image = WrapNotNull(this); - nsCOMPtr eventTarget = mProgressTracker->GetEventTarget(); + nsCOMPtr eventTarget = GetMainThreadSerialEventTarget(); nsCOMPtr ev = NS_NewRunnableFunction( "image::ImageResource::SendOnUnlockedDraw", [=]() -> void { RefPtr tracker = image->GetProgressTracker(); diff --git a/image/ProgressTracker.cpp b/image/ProgressTracker.cpp index 7cc378323105..63c16f6ef722 100644 --- a/image/ProgressTracker.cpp +++ b/image/ProgressTracker.cpp @@ -408,10 +408,6 @@ void ProgressTracker::EmulateRequestFinished(IProgressObserver* aObserver) { } } -already_AddRefed ProgressTracker::GetEventTarget() const { - return do_AddRef(GetMainThreadSerialEventTarget()); -} - void ProgressTracker::AddObserver(IProgressObserver* aObserver) { MOZ_ASSERT(NS_IsMainThread()); RefPtr observer = aObserver; diff --git a/image/ProgressTracker.h b/image/ProgressTracker.h index ffa66cd3bf71..09b1f18a6a84 100644 --- a/image/ProgressTracker.h +++ b/image/ProgressTracker.h @@ -179,9 +179,6 @@ class ProgressTracker : public mozilla::SupportsWeakPtr { bool RemoveObserver(IProgressObserver* aObserver); uint32_t ObserverCount() const; - // Get the event target we should currently dispatch events to. - already_AddRefed GetEventTarget() const; - // Resets our weak reference to our image. Image subclasses should call this // in their destructor. void ResetImage(); diff --git a/image/RasterImage.cpp b/image/RasterImage.cpp index 775238b4609e..06e44bda018b 100644 --- a/image/RasterImage.cpp +++ b/image/RasterImage.cpp @@ -480,12 +480,7 @@ void RasterImage::OnSurfaceDiscarded(const SurfaceKey& aSurfaceKey) { bool animatedFramesDiscarded = mAnimationState && aSurfaceKey.Playback() == PlaybackType::eAnimated; - nsCOMPtr eventTarget; - if (mProgressTracker) { - eventTarget = mProgressTracker->GetEventTarget(); - } else { - eventTarget = do_GetMainThread(); - } + nsCOMPtr eventTarget = do_GetMainThread(); RefPtr image = this; nsCOMPtr ev = diff --git a/image/VectorImage.cpp b/image/VectorImage.cpp index b86f396e64b7..eccd04e3cfc0 100644 --- a/image/VectorImage.cpp +++ b/image/VectorImage.cpp @@ -1557,12 +1557,7 @@ void VectorImage::InvalidateObserversOnNextRefreshDriverTick() { // set by InvalidateFrameInternal in layout/generic/nsFrame.cpp. These bits // get cleared when we repaint the SVG into a surface by // nsIFrame::ClearInvalidationStateBits in nsDisplayList::PaintRoot. - nsCOMPtr eventTarget; - if (mProgressTracker) { - eventTarget = mProgressTracker->GetEventTarget(); - } else { - eventTarget = do_GetMainThread(); - } + nsCOMPtr eventTarget = do_GetMainThread(); RefPtr self(this); nsCOMPtr ev(NS_NewRunnableFunction( diff --git a/image/imgRequest.cpp b/image/imgRequest.cpp index 47bdd3480cf0..9713d2f33ea1 100644 --- a/image/imgRequest.cpp +++ b/image/imgRequest.cpp @@ -334,8 +334,7 @@ void imgRequest::Cancel(nsresult aStatus) { if (NS_IsMainThread()) { ContinueCancel(aStatus); } else { - RefPtr progressTracker = GetProgressTracker(); - nsCOMPtr eventTarget = progressTracker->GetEventTarget(); + nsCOMPtr eventTarget = GetMainThreadSerialEventTarget(); nsCOMPtr ev = new imgRequestMainThreadCancel(this, aStatus); eventTarget->Dispatch(ev.forget(), NS_DISPATCH_NORMAL); } @@ -1027,26 +1026,23 @@ imgRequest::OnDataAvailable(nsIRequest* aRequest, nsIInputStream* aInStr, if (result.mImage) { image = result.mImage; - nsCOMPtr eventTarget; // Update our state to reflect this new part. { MutexAutoLock lock(mMutex); mImage = image; - // We only get an event target if we are not on the main thread, because - // we have to dispatch in that case. If we are on the main thread, but - // on a different scheduler group than ProgressTracker would give us, - // that is okay because nothing in imagelib requires that, just our - // listeners (which have their own checks). - if (!NS_IsMainThread()) { - eventTarget = mProgressTracker->GetEventTarget(); - MOZ_ASSERT(eventTarget); - } - mProgressTracker = nullptr; } + // We only get an event target if we are not on the main thread, because + // we have to dispatch in that case. + nsCOMPtr eventTarget; + if (!NS_IsMainThread()) { + eventTarget = GetMainThreadSerialEventTarget(); + MOZ_ASSERT(eventTarget); + } + // Some property objects are not threadsafe, and we need to send // OnImageAvailable on the main thread, so finish on the main thread. if (!eventTarget) {