From 7d75c2717f8398c7cece4ae343db6dd83ad47374 Mon Sep 17 00:00:00 2001 From: Martok Date: Fri, 30 Dec 2022 21:37:59 +0100 Subject: [PATCH 01/11] Issue #2073 - m-c 1382683: Accelerate GIF decoding to SurfacePipe 1. Implement SurfacePipe::WritePixelBlocks for faster writing of pixels 2. Switch nsGIFDecoder2 to write pixels in blocks instead of individually --- image/SurfacePipe.h | 96 ++++++++++++++ image/decoders/nsGIFDecoder2.cpp | 209 +++++++++++++++++-------------- image/decoders/nsGIFDecoder2.h | 8 +- 3 files changed, 215 insertions(+), 98 deletions(-) diff --git a/image/SurfacePipe.h b/image/SurfacePipe.h index 2c9d07b2f4..616062dd62 100644 --- a/image/SurfacePipe.h +++ b/image/SurfacePipe.h @@ -28,6 +28,7 @@ #include "mozilla/Likely.h" #include "mozilla/Maybe.h" #include "mozilla/Move.h" +#include "mozilla/Tuple.h" #include "mozilla/UniquePtr.h" #include "mozilla/Unused.h" #include "mozilla/Variant.h" @@ -175,6 +176,43 @@ public: return *result; } + /** + * Write pixels to the surface by calling a lambda which may write as many + * pixels as there is remaining to complete the row. It is not completely + * memory safe as it trusts the underlying decoder not to overrun the given + * buffer, however it is an acceptable tradeoff for performance. + * + * Writing continues until every pixel in the surface has been written to + * (i.e., IsSurfaceFinished() returns true) or the lambda returns a WriteState + * which WritePixelBlocks() will return to the caller. + * + * The template parameter PixelType must be uint8_t (for paletted surfaces) or + * uint32_t (for BGRA/BGRX surfaces) and must be in agreement with the pixel + * size passed to ConfigureFilter(). + * + * XXX(seth): We'll remove all support for paletted surfaces in bug 1247520, + * which means we can remove the PixelType template parameter from this + * method. + * + * @param aFunc A lambda that functions as a generator, yielding at most the + * maximum number of pixels requested. The lambda must accept a + * pointer argument to the first pixel to write, a maximum + * number of pixels to write as part of the block, and return a + * NextPixel value. + * + * @return A WriteState value indicating the lambda generator's state. + * WritePixelBlocks() itself will return WriteState::FINISHED if + * writing has finished, regardless of the lambda's internal state. + */ + template + WriteState WritePixelBlocks(Func aFunc) + { + Maybe result; + while (!(result = DoWritePixelBlockToRow(Forward(aFunc)))) { } + + return *result; + } + /** * A variant of WritePixels() that writes a single row of pixels to the * surface one at a time by repeatedly calling a lambda that yields pixels. @@ -449,6 +487,50 @@ protected: private: + /** + * An internal method used to implement WritePixelBlocks. This method writes + * up to the number of pixels necessary to complete the row and returns Some() + * if we either finished the entire surface or the lambda returned a + * WriteState indicating that we should return to the caller. If the row was + * successfully written without either of those things happening, it returns + * Nothing(), allowing WritePixelBlocks() to iterate to fill as many rows as + * possible. + */ + template + Maybe DoWritePixelBlockToRow(Func aFunc) + { + MOZ_ASSERT(mPixelSize == 1 || mPixelSize == 4); + MOZ_ASSERT_IF(mPixelSize == 1, sizeof(PixelType) == sizeof(uint8_t)); + MOZ_ASSERT_IF(mPixelSize == 4, sizeof(PixelType) == sizeof(uint32_t)); + + if (IsSurfaceFinished()) { + return Some(WriteState::FINISHED); // We're already done. + } + + PixelType* rowPtr = reinterpret_cast(mRowPointer); + int32_t remainder = mInputSize.width - mCol; + int32_t written; + Maybe result; + Tie(written, result) = aFunc(&rowPtr[mCol], remainder); + if (written == remainder) { + MOZ_ASSERT(result.isNothing()); + mCol = mInputSize.width; + AdvanceRow(); // We've finished the row. + return IsSurfaceFinished() ? Some(WriteState::FINISHED) + : Nothing(); + } + + MOZ_ASSERT(written >= 0 && written < remainder); + MOZ_ASSERT(result.isSome()); + + mCol += written; + if (*result == WriteState::FINISHED) { + ZeroOutRestOfSurface(); + } + + return result; + } + /** * An internal method used to implement both WritePixels() and * WritePixelsToRow(). Those methods differ only in their behavior after a row @@ -607,6 +689,20 @@ public: return mHead->WritePixels(Forward(aFunc)); } + /** + * A variant of WritePixels() that writes up to a single row of pixels to the + * surface in blocks by repeatedly calling a lambda that yields up to the + * requested number of pixels. + * + * @see SurfaceFilter::WritePixelBlocks() for the canonical documentation. + */ + template + WriteState WritePixelBlocks(Func aFunc) + { + MOZ_ASSERT(mHead, "Use before configured!"); + return mHead->WritePixelBlocks(Forward(aFunc)); + } + /** * A variant of WritePixels() that writes a single row of pixels to the * surface one at a time by repeatedly calling a lambda that yields pixels. diff --git a/image/decoders/nsGIFDecoder2.cpp b/image/decoders/nsGIFDecoder2.cpp index 7a0628e8eb..5a229dbe61 100644 --- a/image/decoders/nsGIFDecoder2.cpp +++ b/image/decoders/nsGIFDecoder2.cpp @@ -299,10 +299,12 @@ nsGIFDecoder2::ColormapIndexToPixel(uint8_t aIndex) } template -NextPixel -nsGIFDecoder2::YieldPixel(const uint8_t* aData, - size_t aLength, - size_t* aBytesReadOut) +Tuple> +nsGIFDecoder2::YieldPixels(const uint8_t* aData, + size_t aLength, + size_t* aBytesReadOut, + PixelSize* aPixelBlock, + int32_t aBlockSize) { MOZ_ASSERT(aData); MOZ_ASSERT(aBytesReadOut); @@ -311,108 +313,119 @@ nsGIFDecoder2::YieldPixel(const uint8_t* aData, // Advance to the next byte we should read. const uint8_t* data = aData + *aBytesReadOut; - // If we don't have any decoded data to yield, try to read some input and - // produce some. - if (mGIFStruct.stackp == mGIFStruct.stack) { - while (mGIFStruct.bits < mGIFStruct.codesize && *aBytesReadOut < aLength) { - // Feed the next byte into the decoder's 32-bit input buffer. - mGIFStruct.datum += int32_t(*data) << mGIFStruct.bits; - mGIFStruct.bits += 8; - data += 1; - *aBytesReadOut += 1; - } - - if (mGIFStruct.bits < mGIFStruct.codesize) { - return AsVariant(WriteState::NEED_MORE_DATA); - } - - // Get the leading variable-length symbol from the data stream. - int code = mGIFStruct.datum & mGIFStruct.codemask; - mGIFStruct.datum >>= mGIFStruct.codesize; - mGIFStruct.bits -= mGIFStruct.codesize; - - const int clearCode = ClearCode(); - - // Reset the dictionary to its original state, if requested - if (code == clearCode) { - mGIFStruct.codesize = mGIFStruct.datasize + 1; - mGIFStruct.codemask = (1 << mGIFStruct.codesize) - 1; - mGIFStruct.avail = clearCode + 2; - mGIFStruct.oldcode = -1; - return AsVariant(WriteState::NEED_MORE_DATA); - } - - // Check for explicit end-of-stream code. It should only appear after all - // image data, but if that was the case we wouldn't be in this function, so - // this is always an error condition. - if (code == (clearCode + 1)) { - return AsVariant(WriteState::FAILURE); - } - - if (mGIFStruct.oldcode == -1) { - if (code >= MAX_BITS) { - return AsVariant(WriteState::FAILURE); // The code's too big; something's wrong. + int32_t written = 0; + while (aBlockSize > written) { + // If we don't have any decoded data to yield, try to read some input and + // produce some. + if (mGIFStruct.stackp == mGIFStruct.stack) { + while (mGIFStruct.bits < mGIFStruct.codesize && *aBytesReadOut < aLength) { + // Feed the next byte into the decoder's 32-bit input buffer. + mGIFStruct.datum += int32_t(*data) << mGIFStruct.bits; + mGIFStruct.bits += 8; + data += 1; + *aBytesReadOut += 1; } - mGIFStruct.firstchar = mGIFStruct.oldcode = code; - - // Yield a pixel at the appropriate index in the colormap. - mGIFStruct.pixels_remaining--; - return AsVariant(ColormapIndexToPixel(mGIFStruct.suffix[code])); - } - - int incode = code; - if (code >= mGIFStruct.avail) { - *mGIFStruct.stackp++ = mGIFStruct.firstchar; - code = mGIFStruct.oldcode; - - if (mGIFStruct.stackp >= mGIFStruct.stack + MAX_BITS) { - return AsVariant(WriteState::FAILURE); // Stack overflow; something's wrong. - } - } - - while (code >= clearCode) { - if ((code >= MAX_BITS) || (code == mGIFStruct.prefix[code])) { - return AsVariant(WriteState::FAILURE); + if (mGIFStruct.bits < mGIFStruct.codesize) { + return MakeTuple(written, Some(WriteState::NEED_MORE_DATA)); } - *mGIFStruct.stackp++ = mGIFStruct.suffix[code]; - code = mGIFStruct.prefix[code]; + // Get the leading variable-length symbol from the data stream. + int code = mGIFStruct.datum & mGIFStruct.codemask; + mGIFStruct.datum >>= mGIFStruct.codesize; + mGIFStruct.bits -= mGIFStruct.codesize; - if (mGIFStruct.stackp >= mGIFStruct.stack + MAX_BITS) { - return AsVariant(WriteState::FAILURE); // Stack overflow; something's wrong. + const int clearCode = ClearCode(); + + // Reset the dictionary to its original state, if requested + if (code == clearCode) { + mGIFStruct.codesize = mGIFStruct.datasize + 1; + mGIFStruct.codemask = (1 << mGIFStruct.codesize) - 1; + mGIFStruct.avail = clearCode + 2; + mGIFStruct.oldcode = -1; + return MakeTuple(written, Some(WriteState::NEED_MORE_DATA)); } + + // Check for explicit end-of-stream code. It should only appear after all + // image data, but if that was the case we wouldn't be in this function, so + // this is always an error condition. + if (code == (clearCode + 1)) { + return MakeTuple(written, Some(WriteState::FAILURE)); + } + + if (mGIFStruct.oldcode == -1) { + if (code >= MAX_BITS) { + // The code's too big; something's wrong. + return MakeTuple(written, Some(WriteState::FAILURE)); + } + + mGIFStruct.firstchar = mGIFStruct.oldcode = code; + + // Yield a pixel at the appropriate index in the colormap. + mGIFStruct.pixels_remaining--; + aPixelBlock[written++] = + ColormapIndexToPixel(mGIFStruct.suffix[code]); + continue; + } + + int incode = code; + if (code >= mGIFStruct.avail) { + *mGIFStruct.stackp++ = mGIFStruct.firstchar; + code = mGIFStruct.oldcode; + + if (mGIFStruct.stackp >= mGIFStruct.stack + MAX_BITS) { + // Stack overflow; something's wrong. + return MakeTuple(written, Some(WriteState::FAILURE)); + } + } + + while (code >= clearCode) { + if ((code >= MAX_BITS) || (code == mGIFStruct.prefix[code])) { + return MakeTuple(written, Some(WriteState::FAILURE)); + } + + *mGIFStruct.stackp++ = mGIFStruct.suffix[code]; + code = mGIFStruct.prefix[code]; + + if (mGIFStruct.stackp >= mGIFStruct.stack + MAX_BITS) { + // Stack overflow; something's wrong. + return MakeTuple(written, Some(WriteState::FAILURE)); + } + } + + *mGIFStruct.stackp++ = mGIFStruct.firstchar = mGIFStruct.suffix[code]; + + // Define a new codeword in the dictionary. + if (mGIFStruct.avail < 4096) { + mGIFStruct.prefix[mGIFStruct.avail] = mGIFStruct.oldcode; + mGIFStruct.suffix[mGIFStruct.avail] = mGIFStruct.firstchar; + mGIFStruct.avail++; + + // If we've used up all the codewords of a given length increase the + // length of codewords by one bit, but don't exceed the specified maximum + // codeword size of 12 bits. + if (((mGIFStruct.avail & mGIFStruct.codemask) == 0) && + (mGIFStruct.avail < 4096)) { + mGIFStruct.codesize++; + mGIFStruct.codemask += mGIFStruct.avail; + } + } + + mGIFStruct.oldcode = incode; } - *mGIFStruct.stackp++ = mGIFStruct.firstchar = mGIFStruct.suffix[code]; - - // Define a new codeword in the dictionary. - if (mGIFStruct.avail < 4096) { - mGIFStruct.prefix[mGIFStruct.avail] = mGIFStruct.oldcode; - mGIFStruct.suffix[mGIFStruct.avail] = mGIFStruct.firstchar; - mGIFStruct.avail++; - - // If we've used up all the codewords of a given length increase the - // length of codewords by one bit, but don't exceed the specified maximum - // codeword size of 12 bits. - if (((mGIFStruct.avail & mGIFStruct.codemask) == 0) && - (mGIFStruct.avail < 4096)) { - mGIFStruct.codesize++; - mGIFStruct.codemask += mGIFStruct.avail; - } + if (MOZ_UNLIKELY(mGIFStruct.stackp <= mGIFStruct.stack)) { + MOZ_ASSERT_UNREACHABLE("No decoded data but we didn't return early?"); + return MakeTuple(written, Some(WriteState::FAILURE)); } - mGIFStruct.oldcode = incode; + // Yield a pixel at the appropriate index in the colormap. + mGIFStruct.pixels_remaining--; + aPixelBlock[written++] + = ColormapIndexToPixel(*--mGIFStruct.stackp); } - if (MOZ_UNLIKELY(mGIFStruct.stackp <= mGIFStruct.stack)) { - MOZ_ASSERT_UNREACHABLE("No decoded data but we didn't return early?"); - return AsVariant(WriteState::FAILURE); - } - - // Yield a pixel at the appropriate index in the colormap. - mGIFStruct.pixels_remaining--; - return AsVariant(ColormapIndexToPixel(*--mGIFStruct.stackp)); + return MakeTuple(written, Maybe()); } /// Expand the colormap from RGB to Packed ARGB as needed by Cairo. @@ -1030,8 +1043,12 @@ nsGIFDecoder2::ReadLZWData(const char* aData, size_t aLength) size_t bytesRead = 0; auto result = mGIFStruct.images_decoded == 0 - ? mPipe.WritePixels([&]{ return YieldPixel(data, length, &bytesRead); }) - : mPipe.WritePixels([&]{ return YieldPixel(data, length, &bytesRead); }); + ? mPipe.WritePixelBlocks([&](uint32_t* aPixelBlock, int32_t aBlockSize) { + return YieldPixels(data, length, &bytesRead, aPixelBlock, aBlockSize); + }) + : mPipe.WritePixelBlocks([&](uint8_t* aPixelBlock, int32_t aBlockSize) { + return YieldPixels(data, length, &bytesRead, aPixelBlock, aBlockSize); + }); if (MOZ_UNLIKELY(bytesRead > length)) { MOZ_ASSERT_UNREACHABLE("Overread?"); diff --git a/image/decoders/nsGIFDecoder2.h b/image/decoders/nsGIFDecoder2.h index d1bf90e507..46ceaa4e94 100644 --- a/image/decoders/nsGIFDecoder2.h +++ b/image/decoders/nsGIFDecoder2.h @@ -61,8 +61,12 @@ private: ColormapIndexToPixel(uint8_t aIndex); /// A generator function that performs LZW decompression and yields pixels. - template NextPixel - YieldPixel(const uint8_t* aData, size_t aLength, size_t* aBytesReadOut); + template Tuple> + YieldPixels(const uint8_t* aData, + size_t aLength, + size_t* aBytesReadOut, + PixelSize* aPixelBlock, + int32_t aBlockSize); /// Checks if we have transparency, either because the header indicates that /// there's alpha, or because the frame rect doesn't cover the entire image. From eac8afce35c85edf3b5c1b66c842f975a0a3ede2 Mon Sep 17 00:00:00 2001 From: Martok Date: Fri, 30 Dec 2022 23:47:18 +0100 Subject: [PATCH 02/11] Issue #2073 - m-c 1343341: Infrastructure necessary to allow discarding of animated images (squashed) Includes squashed changes of: - m-c 1317907: Refactor FrameAnimator::GetCompositedFrame to be a bit simpler - m-c 1351434: bugfix - m-c 686905: Enable the pref image.mem.animated.discardable to allow discarding of animated images --- gfx/thebes/gfxPrefs.h | 1 + image/DynamicImage.cpp | 2 +- image/DynamicImage.h | 2 +- image/FrameAnimator.cpp | 236 ++++++++++++++++++++++++++++-------- image/FrameAnimator.h | 132 +++++++++++++++++--- image/Image.h | 4 +- image/ImageWrapper.cpp | 4 +- image/ImageWrapper.h | 2 +- image/RasterImage.cpp | 63 ++++++++-- image/RasterImage.h | 8 +- image/SurfaceCache.cpp | 2 +- image/VectorImage.cpp | 2 +- image/VectorImage.h | 2 +- modules/libpref/init/all.js | 4 + 14 files changed, 368 insertions(+), 96 deletions(-) diff --git a/gfx/thebes/gfxPrefs.h b/gfx/thebes/gfxPrefs.h index 934bca81e4..c7052b7c6b 100644 --- a/gfx/thebes/gfxPrefs.h +++ b/gfx/thebes/gfxPrefs.h @@ -437,6 +437,7 @@ private: DECL_GFX_PREF(Once, "image.layerize.always", ImageLayerizeAlways, bool, false); DECL_GFX_PREF(Once, "image.mem.decode_bytes_at_a_time", ImageMemDecodeBytesAtATime, uint32_t, 200000); DECL_GFX_PREF(Live, "image.mem.discardable", ImageMemDiscardable, bool, false); + DECL_GFX_PREF(Once, "image.mem.animated.discardable", ImageMemAnimatedDiscardable, bool, false); DECL_GFX_PREF(Once, "image.mem.surfacecache.discard_factor", ImageMemSurfaceCacheDiscardFactor, uint32_t, 1); DECL_GFX_PREF(Once, "image.mem.surfacecache.max_size_kb", ImageMemSurfaceCacheMaxSizeKB, uint32_t, 100 * 1024); DECL_GFX_PREF(Once, "image.mem.surfacecache.min_expiration_ms", ImageMemSurfaceCacheMinExpirationMS, uint32_t, 60*1000); diff --git a/image/DynamicImage.cpp b/image/DynamicImage.cpp index dfdc3e5d81..bef401a6cb 100644 --- a/image/DynamicImage.cpp +++ b/image/DynamicImage.cpp @@ -80,7 +80,7 @@ DynamicImage::OnImageDataComplete(nsIRequest* aRequest, } void -DynamicImage::OnSurfaceDiscarded() +DynamicImage::OnSurfaceDiscarded(const SurfaceKey& aSurfaceKey) { } void diff --git a/image/DynamicImage.h b/image/DynamicImage.h index 751bed82a1..a39a29b8e3 100644 --- a/image/DynamicImage.h +++ b/image/DynamicImage.h @@ -53,7 +53,7 @@ public: nsresult aStatus, bool aLastPart) override; - virtual void OnSurfaceDiscarded() override; + virtual void OnSurfaceDiscarded(const SurfaceKey& aSurfaceKey) override; virtual void SetInnerWindowID(uint64_t aInnerWindowId) override; virtual uint64_t InnerWindowID() const override; diff --git a/image/FrameAnimator.cpp b/image/FrameAnimator.cpp index 8c29b5636a..066da94f40 100644 --- a/image/FrameAnimator.cpp +++ b/image/FrameAnimator.cpp @@ -11,6 +11,7 @@ #include "LookupResult.h" #include "MainThreadUtils.h" #include "RasterImage.h" +#include "gfxPrefs.h" #include "pixman.h" @@ -25,9 +26,80 @@ namespace image { /////////////////////////////////////////////////////////////////////////////// void -AnimationState::SetDoneDecoding(bool aDone) +AnimationState::UpdateState(bool aAnimationFinished, + RasterImage *aImage, + const gfx::IntSize& aSize) { - mDoneDecoding = aDone; + LookupResult result = + SurfaceCache::Lookup(ImageKey(aImage), + RasterSurfaceKey(aSize, + DefaultSurfaceFlags(), + PlaybackType::eAnimated)); + + UpdateStateInternal(result, aAnimationFinished); +} + +void +AnimationState::UpdateStateInternal(LookupResult& aResult, + bool aAnimationFinished) +{ + // Update mDiscarded and mIsCurrentlyDecoded. + if (aResult.Type() == MatchType::NOT_FOUND) { + // no frames, we've either been discarded, or never been decoded before. + mDiscarded = mHasBeenDecoded; + mIsCurrentlyDecoded = false; + } else if (aResult.Type() == MatchType::PENDING) { + // no frames yet, but a decoder is or will be working on it. + mDiscarded = false; + mIsCurrentlyDecoded = false; + } else { + MOZ_ASSERT(aResult.Type() == MatchType::EXACT); + mDiscarded = false; + + // If mHasBeenDecoded is true then we know the true total frame count and + // we can use it to determine if we have all the frames now so we know if + // we are currently fully decoded. + // If mHasBeenDecoded is false then we'll get another UpdateState call + // when the decode finishes. + if (mHasBeenDecoded) { + Maybe frameCount = FrameCount(); + MOZ_ASSERT(frameCount.isSome()); + aResult.Surface().Seek(*frameCount - 1); + if (aResult.Surface() && aResult.Surface()->IsFinished()) { + mIsCurrentlyDecoded = true; + } else { + mIsCurrentlyDecoded = false; + } + } + } + + // Update the value of mCompositedFrameInvalid. + if (mIsCurrentlyDecoded || aAnimationFinished) { + // Animated images that have finished their animation (ie because it is a + // finite length animation) don't have RequestRefresh called on them, and so + // mCompositedFrameInvalid would never get cleared. We clear it here (and + // also in RasterImage::Decode when we create a decoder for an image that + // has finished animated so it can display sooner than waiting until the + // decode completes). We also do it if we are fully decoded. This is safe + // to do for images that aren't finished animating because before we paint + // the refresh driver will call into us to advance to the correct frame, + // and that will succeed because we have all the frames. + mCompositedFrameInvalid = false; + } else if (aResult.Type() == MatchType::NOT_FOUND || + aResult.Type() == MatchType::PENDING) { + if (mHasBeenDecoded) { + MOZ_ASSERT(gfxPrefs::ImageMemAnimatedDiscardable()); + mCompositedFrameInvalid = true; + } + } + // Otherwise don't change the value of mCompositedFrameInvalid, it will be + // updated by RequestRefresh. +} + +void +AnimationState::NotifyDecodeComplete() +{ + mHasBeenDecoded = true; } void @@ -52,7 +124,7 @@ AnimationState::UpdateKnownFrameCount(uint32_t aFrameCount) return; } - MOZ_ASSERT(!mDoneDecoding, "Adding new frames after decoding is finished?"); + MOZ_ASSERT(!mHasBeenDecoded, "Adding new frames after decoding is finished?"); MOZ_ASSERT(aFrameCount <= mFrameCount + 1, "Skipped a frame?"); mFrameCount = aFrameCount; @@ -61,7 +133,7 @@ AnimationState::UpdateKnownFrameCount(uint32_t aFrameCount) Maybe AnimationState::FrameCount() const { - return mDoneDecoding ? Some(mFrameCount) : Nothing(); + return mHasBeenDecoded ? Some(mFrameCount) : Nothing(); } void @@ -98,7 +170,7 @@ AnimationState::LoopLength() const return FrameTimeout::Forever(); } - MOZ_ASSERT(mDoneDecoding, "We know the loop length but decoding isn't done?"); + MOZ_ASSERT(mHasBeenDecoded, "We know the loop length but decoding isn't done?"); // If we're not looping, a single loop time has no meaning. if (mAnimationMode != imgIContainer::kNormalAnimMode) { @@ -113,32 +185,41 @@ AnimationState::LoopLength() const // FrameAnimator implementation. /////////////////////////////////////////////////////////////////////////////// -TimeStamp -FrameAnimator::GetCurrentImgFrameEndTime(AnimationState& aState) const +Maybe +FrameAnimator::GetCurrentImgFrameEndTime(AnimationState& aState, + DrawableSurface& aFrames) const { TimeStamp currentFrameTime = aState.mCurrentAnimationFrameTime; - FrameTimeout timeout = GetTimeoutForFrame(aState.mCurrentAnimationFrameIndex); + Maybe timeout = + GetTimeoutForFrame(aState, aFrames, aState.mCurrentAnimationFrameIndex); - if (timeout == FrameTimeout::Forever()) { + if (timeout.isNothing()) { + MOZ_ASSERT(aState.GetHasBeenDecoded() && !aState.GetIsCurrentlyDecoded()); + return Nothing(); + } + + if (*timeout == FrameTimeout::Forever()) { // We need to return a sentinel value in this case, because our logic // doesn't work correctly if we have an infinitely long timeout. We use one // year in the future as the sentinel because it works with the loop in // RequestRefresh() below. // XXX(seth): It'd be preferable to make our logic work correctly with // infinitely long timeouts. - return TimeStamp::NowLoRes() + - TimeDuration::FromMilliseconds(31536000.0); + return Some(TimeStamp::NowLoRes() + + TimeDuration::FromMilliseconds(31536000.0)); } TimeDuration durationOfTimeout = - TimeDuration::FromMilliseconds(double(timeout.AsMilliseconds())); + TimeDuration::FromMilliseconds(double(timeout->AsMilliseconds())); TimeStamp currentFrameEndTime = currentFrameTime + durationOfTimeout; - return currentFrameEndTime; + return Some(currentFrameEndTime); } RefreshResult -FrameAnimator::AdvanceFrame(AnimationState& aState, TimeStamp aTime) +FrameAnimator::AdvanceFrame(AnimationState& aState, + DrawableSurface& aFrames, + TimeStamp aTime) { NS_ASSERTION(aTime <= TimeStamp::Now(), "Given time appears to be in the future"); @@ -198,7 +279,7 @@ FrameAnimator::AdvanceFrame(AnimationState& aState, TimeStamp aTime) // the appropriate notification on the main thread. Make sure we stay in sync // with AnimationState. MOZ_ASSERT(nextFrameIndex < aState.KnownFrameCount()); - RawAccessFrameRef nextFrame = GetRawFrame(nextFrameIndex); + RawAccessFrameRef nextFrame = GetRawFrame(aFrames, nextFrameIndex); // We should always check to see if we have the next frame even if we have // previously finished decoding. If we needed to redecode (e.g. due to a draw @@ -210,7 +291,11 @@ FrameAnimator::AdvanceFrame(AnimationState& aState, TimeStamp aTime) return ret; } - if (GetTimeoutForFrame(nextFrameIndex) == FrameTimeout::Forever()) { + Maybe nextFrameTimeout = GetTimeoutForFrame(aState, aFrames, nextFrameIndex); + // GetTimeoutForFrame can only return none if frame doesn't exist, + // but we just got it above. + MOZ_ASSERT(nextFrameTimeout.isSome()); + if (*nextFrameTimeout == FrameTimeout::Forever()) { ret.mAnimationFinished = true; } @@ -220,11 +305,13 @@ FrameAnimator::AdvanceFrame(AnimationState& aState, TimeStamp aTime) MOZ_ASSERT(nextFrameIndex == currentFrameIndex + 1); // Change frame - if (!DoBlend(&ret.mDirtyRect, currentFrameIndex, nextFrameIndex)) { + if (!DoBlend(aFrames, &ret.mDirtyRect, currentFrameIndex, nextFrameIndex)) { // something went wrong, move on to next NS_WARNING("FrameAnimator::AdvanceFrame(): Compositing of frame failed"); nextFrame->SetCompositingFailed(true); - aState.mCurrentAnimationFrameTime = GetCurrentImgFrameEndTime(aState); + Maybe currentFrameEndTime = GetCurrentImgFrameEndTime(aState, aFrames); + MOZ_ASSERT(currentFrameEndTime.isSome()); + aState.mCurrentAnimationFrameTime = *currentFrameEndTime; aState.mCurrentAnimationFrameIndex = nextFrameIndex; return ret; @@ -233,7 +320,9 @@ FrameAnimator::AdvanceFrame(AnimationState& aState, TimeStamp aTime) nextFrame->SetCompositingFailed(false); } - aState.mCurrentAnimationFrameTime = GetCurrentImgFrameEndTime(aState); + Maybe currentFrameEndTime = GetCurrentImgFrameEndTime(aState, aFrames); + MOZ_ASSERT(currentFrameEndTime.isSome()); + aState.mCurrentAnimationFrameTime = *currentFrameEndTime; // If we can get closer to the current time by a multiple of the image's loop // time, we should. We can only do this if we're done decoding; otherwise, we @@ -262,41 +351,88 @@ FrameAnimator::AdvanceFrame(AnimationState& aState, TimeStamp aTime) } RefreshResult -FrameAnimator::RequestRefresh(AnimationState& aState, const TimeStamp& aTime) +FrameAnimator::RequestRefresh(AnimationState& aState, + const TimeStamp& aTime, + bool aAnimationFinished) { - // only advance the frame if the current time is greater than or - // equal to the current frame's end time. - TimeStamp currentFrameEndTime = GetCurrentImgFrameEndTime(aState); - // By default, an empty RefreshResult. RefreshResult ret; - while (currentFrameEndTime <= aTime) { - TimeStamp oldFrameEndTime = currentFrameEndTime; + if (aState.IsDiscarded()) { + return ret; + } - RefreshResult frameRes = AdvanceFrame(aState, aTime); + // Get the animation frames once now, and pass them down to callees because + // the surface could be discarded at anytime on a different thread. This is + // must easier to reason about then trying to write code that is safe to + // having the surface disappear at anytime. + LookupResult result = + SurfaceCache::Lookup(ImageKey(mImage), + RasterSurfaceKey(mSize, + DefaultSurfaceFlags(), + PlaybackType::eAnimated)); + + aState.UpdateStateInternal(result, aAnimationFinished); + if (aState.IsDiscarded() || !result) { + return ret; + } + + // only advance the frame if the current time is greater than or + // equal to the current frame's end time. + Maybe currentFrameEndTime = + GetCurrentImgFrameEndTime(aState, result.Surface()); + if (currentFrameEndTime.isNothing()) { + MOZ_ASSERT(gfxPrefs::ImageMemAnimatedDiscardable()); + MOZ_ASSERT(aState.GetHasBeenDecoded() && !aState.GetIsCurrentlyDecoded()); + MOZ_ASSERT(aState.mCompositedFrameInvalid); + // Nothing we can do but wait for our previous current frame to be decoded + // again so we can determine what to do next. + return ret; + } + + while (*currentFrameEndTime <= aTime) { + TimeStamp oldFrameEndTime = *currentFrameEndTime; + + RefreshResult frameRes = AdvanceFrame(aState, result.Surface(), aTime); // Accumulate our result for returning to callers. ret.Accumulate(frameRes); - currentFrameEndTime = GetCurrentImgFrameEndTime(aState); + currentFrameEndTime = GetCurrentImgFrameEndTime(aState, result.Surface()); + // AdvanceFrame can't advance to a frame that doesn't exist yet. + MOZ_ASSERT(currentFrameEndTime.isSome()); // If we didn't advance a frame, and our frame end time didn't change, // then we need to break out of this loop & wait for the frame(s) // to finish downloading. - if (!frameRes.mFrameAdvanced && (currentFrameEndTime == oldFrameEndTime)) { + if (!frameRes.mFrameAdvanced && (*currentFrameEndTime == oldFrameEndTime)) { break; } } + // Advanced to the correct frame, the composited frame is now valid to be drawn. + if (*currentFrameEndTime > aTime) { + aState.mCompositedFrameInvalid = false; + } + + MOZ_ASSERT(!aState.mIsCurrentlyDecoded || !aState.mCompositedFrameInvalid); + return ret; } LookupResult -FrameAnimator::GetCompositedFrame(uint32_t aFrameNum) +FrameAnimator::GetCompositedFrame(AnimationState& aState) { + if (aState.mCompositedFrameInvalid) { + MOZ_ASSERT(gfxPrefs::ImageMemAnimatedDiscardable()); + MOZ_ASSERT(aState.GetHasBeenDecoded()); + MOZ_ASSERT(!aState.GetIsCurrentlyDecoded()); + return LookupResult(MatchType::NOT_FOUND); + } + // If we have a composited version of this frame, return that. - if (mLastCompositedFrameIndex == int32_t(aFrameNum)) { + if (mLastCompositedFrameIndex >= 0 && + (uint32_t(mLastCompositedFrameIndex) == aState.mCurrentAnimationFrameIndex)) { return LookupResult(DrawableSurface(mCompositingFrame->DrawableRef()), MatchType::EXACT); } @@ -314,7 +450,7 @@ FrameAnimator::GetCompositedFrame(uint32_t aFrameNum) // Seek to the appropriate frame. If seeking fails, it means that we couldn't // get the frame we're looking for; treat this as if the lookup failed. - if (NS_FAILED(result.Surface().Seek(aFrameNum))) { + if (NS_FAILED(result.Surface().Seek(aState.mCurrentAnimationFrameIndex))) { return LookupResult(MatchType::NOT_FOUND); } @@ -324,17 +460,19 @@ FrameAnimator::GetCompositedFrame(uint32_t aFrameNum) return result; } -FrameTimeout -FrameAnimator::GetTimeoutForFrame(uint32_t aFrameNum) const +Maybe +FrameAnimator::GetTimeoutForFrame(AnimationState& aState, + DrawableSurface& aFrames, + uint32_t aFrameNum) const { - RawAccessFrameRef frame = GetRawFrame(aFrameNum); + RawAccessFrameRef frame = GetRawFrame(aFrames, aFrameNum); if (frame) { AnimationData data = frame->GetAnimationData(); - return data.mTimeout; + return Some(data.mTimeout); } - NS_WARNING("No frame; called GetTimeoutForFrame too early?"); - return FrameTimeout::FromRawMilliseconds(100); + MOZ_ASSERT(aState.mHasBeenDecoded && !aState.mIsCurrentlyDecoded); + return Nothing(); } static void @@ -382,37 +520,29 @@ FrameAnimator::CollectSizeOfCompositingSurfaces( } RawAccessFrameRef -FrameAnimator::GetRawFrame(uint32_t aFrameNum) const +FrameAnimator::GetRawFrame(DrawableSurface& aFrames, uint32_t aFrameNum) const { - LookupResult result = - SurfaceCache::Lookup(ImageKey(mImage), - RasterSurfaceKey(mSize, - DefaultSurfaceFlags(), - PlaybackType::eAnimated)); - if (!result) { - return RawAccessFrameRef(); - } - // Seek to the frame we want. If seeking fails, it means we couldn't get the // frame we're looking for, so we bail here to avoid returning the wrong frame // to the caller. - if (NS_FAILED(result.Surface().Seek(aFrameNum))) { + if (NS_FAILED(aFrames.Seek(aFrameNum))) { return RawAccessFrameRef(); // Not available yet. } - return result.Surface()->RawAccessRef(); + return aFrames->RawAccessRef(); } //****************************************************************************** // DoBlend gets called when the timer for animation get fired and we have to // update the composited frame of the animation. bool -FrameAnimator::DoBlend(IntRect* aDirtyRect, +FrameAnimator::DoBlend(DrawableSurface& aFrames, + IntRect* aDirtyRect, uint32_t aPrevFrameIndex, uint32_t aNextFrameIndex) { - RawAccessFrameRef prevFrame = GetRawFrame(aPrevFrameIndex); - RawAccessFrameRef nextFrame = GetRawFrame(aNextFrameIndex); + RawAccessFrameRef prevFrame = GetRawFrame(aFrames, aPrevFrameIndex); + RawAccessFrameRef nextFrame = GetRawFrame(aFrames, aNextFrameIndex); MOZ_ASSERT(prevFrame && nextFrame, "Should have frames here"); diff --git a/image/FrameAnimator.h b/image/FrameAnimator.h index da0cb4bf5a..44b5a52e7c 100644 --- a/image/FrameAnimator.h +++ b/image/FrameAnimator.h @@ -15,11 +15,13 @@ #include "nsCOMPtr.h" #include "nsRect.h" #include "SurfaceCache.h" +#include "gfxPrefs.h" namespace mozilla { namespace image { class RasterImage; +class DrawableSurface; class AnimationState { @@ -31,14 +33,62 @@ public: , mLoopCount(-1) , mFirstFrameTimeout(FrameTimeout::FromRawMilliseconds(0)) , mAnimationMode(aAnimationMode) - , mDoneDecoding(false) + , mHasBeenDecoded(false) + , mIsCurrentlyDecoded(false) + , mCompositedFrameInvalid(false) + , mDiscarded(false) { } /** - * Call when this image is finished decoding so we know that there aren't any - * more frames coming. + * Call this whenever a decode completes, a decode starts, or the image is + * discarded. It will update the internal state. Specifically mDiscarded, + * mCompositedFrameInvalid, and mIsCurrentlyDecoded. */ - void SetDoneDecoding(bool aDone); + void UpdateState(bool aAnimationFinished, + RasterImage *aImage, + const gfx::IntSize& aSize); +private: + void UpdateStateInternal(LookupResult& aResult, + bool aAnimationFinished); + +public: + /** + * Call when a decode of this image has been completed. + */ + void NotifyDecodeComplete(); + + /** + * Returns true if this image has been fully decoded before. + */ + bool GetHasBeenDecoded() { return mHasBeenDecoded; } + + /** + * Returns true if this image has been discarded and a decoded has not yet + * been created to redecode it. + */ + bool IsDiscarded() { return mDiscarded; } + + /** + * Sets the composited frame as valid or invalid. + */ + void SetCompositedFrameInvalid(bool aInvalid) { + MOZ_ASSERT(!aInvalid || gfxPrefs::ImageMemAnimatedDiscardable()); + mCompositedFrameInvalid = aInvalid; + } + + /** + * Returns whether the composited frame is valid to draw to the screen. + */ + bool GetCompositedFrameInvalid() { + return mCompositedFrameInvalid; + } + + /** + * Returns whether the image is currently full decoded.. + */ + bool GetIsCurrentlyDecoded() { + return mIsCurrentlyDecoded; + } /** * Call when you need to re-start animating. Ensures we start from the first @@ -140,8 +190,44 @@ private: //! The animation mode of this image. Constants defined in imgIContainer. uint16_t mAnimationMode; - //! Whether this image is done being decoded. - bool mDoneDecoding; + /** + * The following four bools (mHasBeenDecoded, mIsCurrentlyDecoded, + * mCompositedFrameInvalid, mDiscarded) track the state of the image with + * regards to decoding. They all start out false, including mDiscarded, + * because we want to treat being discarded differently from "not yet decoded + * for the first time". + * + * (When we are decoding the image for the first time we want to show the + * image at the speed of data coming in from the network or the speed + * specified in the image file, whichever is slower. But when redecoding we + * want to show nothing until the frame for the current time has been + * decoded. The prevents the user from seeing the image "fast forward" + * to the expected spot.) + * + * When the image is decoded for the first time mHasBeenDecoded and + * mIsCurrentlyDecoded get set to true. When the image is discarded + * mIsCurrentlyDecoded gets set to false, and mCompositedFrameInvalid + * & mDiscarded get set to true. When we create a decoder to redecode the + * image mDiscarded gets set to false. mCompositedFrameInvalid gets set to + * false when we are able to advance to the frame that should be showing + * for the current time. mIsCurrentlyDecoded gets set to true when the + * redecode finishes. + */ + + //! Whether this image has been decoded at least once. + bool mHasBeenDecoded; + + //! Whether this image is currently fully decoded. + bool mIsCurrentlyDecoded; + + //! Whether the composited frame is valid to draw to the screen, note that + //! the composited frame can exist and be filled with image data but not + //! valid to draw to the screen. + bool mCompositedFrameInvalid; + + //! Whether this image is currently discarded. Only set to true after the + //! image has been decoded at least once. + bool mDiscarded; }; /** @@ -197,14 +283,16 @@ public: * Returns the result of that blending, including whether the current frame * changed and what the resulting dirty rectangle is. */ - RefreshResult RequestRefresh(AnimationState& aState, const TimeStamp& aTime); + RefreshResult RequestRefresh(AnimationState& aState, + const TimeStamp& aTime, + bool aAnimationFinished); /** - * If we have a composited frame for @aFrameNum, returns it. Otherwise, - * returns an empty LookupResult. It is an error to call this method with - * aFrameNum == 0, because the first frame is never composited. + * Get the full frame for the current frame of the animation (it may or may + * not have required compositing). It may not be available because it hasn't + * been decoded yet, in which case we return an empty LookupResult. */ - LookupResult GetCompositedFrame(uint32_t aFrameNum); + LookupResult GetCompositedFrame(AnimationState& aState); /** * Collect an accounting of the memory occupied by the compositing surfaces we @@ -227,24 +315,32 @@ private: // methods * @returns a RefreshResult that shows whether the frame was successfully * advanced, and its resulting dirty rect. */ - RefreshResult AdvanceFrame(AnimationState& aState, TimeStamp aTime); + RefreshResult AdvanceFrame(AnimationState& aState, + DrawableSurface& aFrames, + TimeStamp aTime); /** * Get the @aIndex-th frame in the frame index, ignoring results of blending. */ - RawAccessFrameRef GetRawFrame(uint32_t aFrameNum) const; + RawAccessFrameRef GetRawFrame(DrawableSurface& aFrames, + uint32_t aFrameNum) const; - /// @return the given frame's timeout. - FrameTimeout GetTimeoutForFrame(uint32_t aFrameNum) const; + /// @return the given frame's timeout if it is available + Maybe GetTimeoutForFrame(AnimationState& aState, + DrawableSurface& aFrames, + uint32_t aFrameNum) const; /** * Get the time the frame we're currently displaying is supposed to end. * - * In the error case, returns an "infinity" timestamp. + * In the error case (like if the requested frame is not currently + * decoded), returns None(). */ - TimeStamp GetCurrentImgFrameEndTime(AnimationState& aState) const; + Maybe GetCurrentImgFrameEndTime(AnimationState& aState, + DrawableSurface& aFrames) const; - bool DoBlend(gfx::IntRect* aDirtyRect, + bool DoBlend(DrawableSurface& aFrames, + gfx::IntRect* aDirtyRect, uint32_t aPrevFrameIndex, uint32_t aNextFrameIndex); diff --git a/image/Image.h b/image/Image.h index 98c5e8ca54..4aa9b55afd 100644 --- a/image/Image.h +++ b/image/Image.h @@ -211,7 +211,7 @@ public: /** * Called when the SurfaceCache discards a surface belonging to this image. */ - virtual void OnSurfaceDiscarded() = 0; + virtual void OnSurfaceDiscarded(const SurfaceKey& aSurfaceKey) = 0; virtual void SetInnerWindowID(uint64_t aInnerWindowId) = 0; virtual uint64_t InnerWindowID() const = 0; @@ -249,7 +249,7 @@ public: } #endif - virtual void OnSurfaceDiscarded() override { } + virtual void OnSurfaceDiscarded(const SurfaceKey& aSurfaceKey) override { } virtual void SetInnerWindowID(uint64_t aInnerWindowId) override { diff --git a/image/ImageWrapper.cpp b/image/ImageWrapper.cpp index 7d2fbfa363..c593521c9e 100644 --- a/image/ImageWrapper.cpp +++ b/image/ImageWrapper.cpp @@ -88,9 +88,9 @@ ImageWrapper::OnImageDataComplete(nsIRequest* aRequest, } void -ImageWrapper::OnSurfaceDiscarded() +ImageWrapper::OnSurfaceDiscarded(const SurfaceKey& aSurfaceKey) { - return mInnerImage->OnSurfaceDiscarded(); + return mInnerImage->OnSurfaceDiscarded(aSurfaceKey); } void diff --git a/image/ImageWrapper.h b/image/ImageWrapper.h index f60a1c09c3..94cf0948b5 100644 --- a/image/ImageWrapper.h +++ b/image/ImageWrapper.h @@ -45,7 +45,7 @@ public: nsresult aStatus, bool aLastPart) override; - virtual void OnSurfaceDiscarded() override; + virtual void OnSurfaceDiscarded(const SurfaceKey& aSurfaceKey) override; virtual void SetInnerWindowID(uint64_t aInnerWindowId) override; virtual uint64_t InnerWindowID() const override; diff --git a/image/RasterImage.cpp b/image/RasterImage.cpp index 4fd3797bb0..f7dfd0bb1a 100644 --- a/image/RasterImage.cpp +++ b/image/RasterImage.cpp @@ -174,7 +174,7 @@ RasterImage::RequestRefresh(const TimeStamp& aTime) RefreshResult res; if (mAnimationState) { MOZ_ASSERT(mFrameAnimator); - res = mFrameAnimator->RequestRefresh(*mAnimationState, aTime); + res = mFrameAnimator->RequestRefresh(*mAnimationState, aTime, mAnimationFinished); } if (res.mFrameAdvanced) { @@ -275,8 +275,7 @@ RasterImage::LookupFrameInternal(const IntSize& aSize, MOZ_ASSERT(mFrameAnimator); MOZ_ASSERT(ToSurfaceFlags(aFlags) == DefaultSurfaceFlags(), "Can't composite frames with non-default surface flags"); - const size_t index = mAnimationState->GetCurrentAnimationFrameIndex(); - return mFrameAnimator->GetCompositedFrame(index); + return mFrameAnimator->GetCompositedFrame(*mAnimationState); } SurfaceFlags surfaceFlags = ToSurfaceFlags(aFlags); @@ -332,6 +331,7 @@ RasterImage::LookupFrame(const IntSize& aSize, // one. (Or we're sync decoding and the existing decoder hasn't even started // yet.) Trigger decoding so it'll be available next time. MOZ_ASSERT(aPlaybackType != PlaybackType::eAnimated || + gfxPrefs::ImageMemAnimatedDiscardable() || !mAnimationState || mAnimationState->KnownFrameCount() < 1, "Animated frames should be locked"); @@ -399,7 +399,7 @@ RasterImage::WillDrawOpaqueNow() return false; } - if (mAnimationState) { + if (mAnimationState && !gfxPrefs::ImageMemAnimatedDiscardable()) { // We never discard frames of animated images. return true; } @@ -426,11 +426,33 @@ RasterImage::WillDrawOpaqueNow() } void -RasterImage::OnSurfaceDiscarded() +RasterImage::OnSurfaceDiscarded(const SurfaceKey& aSurfaceKey) { MOZ_ASSERT(mProgressTracker); - NS_DispatchToMainThread(NewRunnableMethod(mProgressTracker, &ProgressTracker::OnDiscard)); + bool animatedFramesDiscarded = + mAnimationState && aSurfaceKey.Playback() == PlaybackType::eAnimated; + + RefPtr image = this; + NS_DispatchToMainThread(NS_NewRunnableFunction( + [=]() -> void { + image->OnSurfaceDiscardedInternal(animatedFramesDiscarded); + })); +} + +void +RasterImage::OnSurfaceDiscardedInternal(bool aAnimatedFramesDiscarded) +{ + MOZ_ASSERT(NS_IsMainThread()); + + if (aAnimatedFramesDiscarded && mAnimationState) { + MOZ_ASSERT(gfxPrefs::ImageMemAnimatedDiscardable()); + mAnimationState->UpdateState(mAnimationFinished, this, mSize); + } + + if (mProgressTracker) { + mProgressTracker->OnDiscard(); + } } //****************************************************************************** @@ -706,9 +728,11 @@ RasterImage::SetMetadata(const ImageMetadata& aMetadata, mAnimationState.emplace(mAnimationMode); mFrameAnimator = MakeUnique(this, mSize); - // We don't support discarding animated images (See bug 414259). - // Lock the image and throw away the key. - LockImage(); + if (!gfxPrefs::ImageMemAnimatedDiscardable()) { + // We don't support discarding animated images (See bug 414259). + // Lock the image and throw away the key. + LockImage(); + } if (!aFromMetadataDecode) { // The metadata decode reported that this image isn't animated, but we @@ -1015,11 +1039,16 @@ RasterImage::Discard() { MOZ_ASSERT(NS_IsMainThread()); MOZ_ASSERT(CanDiscard(), "Asked to discard but can't"); - MOZ_ASSERT(!mAnimationState, "Asked to discard for animated image"); + MOZ_ASSERT(!mAnimationState || gfxPrefs::ImageMemAnimatedDiscardable(), + "Asked to discard for animated image"); // Delete all the decoded frames. SurfaceCache::RemoveImage(ImageKey(this)); + if (mAnimationState) { + mAnimationState->UpdateState(mAnimationFinished, this, mSize); + } + // Notify that we discarded. if (mProgressTracker) { mProgressTracker->OnDiscard(); @@ -1028,8 +1057,8 @@ RasterImage::Discard() bool RasterImage::CanDiscard() { - return mHasSourceData && // ...have the source data... - !mAnimationState; // Can never discard animated images + return mHasSourceData && // ...have the source data... + (!mAnimationState || gfxPrefs::ImageMemAnimatedDiscardable()); // Can discard animated images if the pref is set } NS_IMETHODIMP @@ -1158,6 +1187,13 @@ RasterImage::Decode(const IntSize& aSize, task = DecoderFactory::CreateAnimationDecoder(mDecoderType, WrapNotNull(this), mSourceBuffer, mSize, decoderFlags, surfaceFlags); + mAnimationState->UpdateState(mAnimationFinished, this, mSize); + // If the animation is finished we can draw right away because we just draw + // the final frame all the time from now on. See comment in + // AnimationState::UpdateState. + if (mAnimationFinished) { + mAnimationState->SetCompositedFrameInvalid(false); + } } else { task = DecoderFactory::CreateDecoder(mDecoderType, WrapNotNull(this), mSourceBuffer, mSize, aSize, @@ -1597,7 +1633,8 @@ RasterImage::NotifyDecodeComplete(const DecoderFinalStatus& aStatus, mHasBeenDecoded && mAnimationState) { // We've finished a full decode of all animation frames and our AnimationState // has been notified about them all, so let it know not to expect anymore. - mAnimationState->SetDoneDecoding(true); + mAnimationState->NotifyDecodeComplete(); + mAnimationState->UpdateState(mAnimationFinished, this, mSize); } // Only act on errors if we have no usable frames from the decoder. diff --git a/image/RasterImage.h b/image/RasterImage.h index 860983b22e..42af0a4887 100644 --- a/image/RasterImage.h +++ b/image/RasterImage.h @@ -163,7 +163,7 @@ public: virtual nsresult StopAnimation() override; // Methods inherited from Image - virtual void OnSurfaceDiscarded() override; + virtual void OnSurfaceDiscarded(const SurfaceKey& aSurfaceKey) override; virtual size_t SizeOfSourceWithComputedFallback(MallocSizeOf aMallocSizeOf) const override; @@ -321,7 +321,9 @@ private: // never unlock so that animated images always have their lock count >= 1. In // that case we use our animation consumers count as a proxy for lock count. bool IsUnlocked() { - return (mLockCount == 0 || (mAnimationState && mAnimationConsumers == 0)); + return (mLockCount == 0 || + (!gfxPrefs::ImageMemAnimatedDiscardable() && + (mAnimationState && mAnimationConsumers == 0))); } @@ -378,6 +380,8 @@ private: */ void RecoverFromInvalidFrames(const nsIntSize& aSize, uint32_t aFlags); + void OnSurfaceDiscardedInternal(bool aAnimatedFramesDiscarded); + private: // data nsIntSize mSize; Orientation mOrientation; diff --git a/image/SurfaceCache.cpp b/image/SurfaceCache.cpp index 66fdfcca04..f3068fd4c1 100644 --- a/image/SurfaceCache.cpp +++ b/image/SurfaceCache.cpp @@ -503,7 +503,7 @@ public: // If the surface was not a placeholder, tell its image that we discarded it. if (!aSurface->IsPlaceholder()) { - static_cast(imageKey)->OnSurfaceDiscarded(); + static_cast(imageKey)->OnSurfaceDiscarded(aSurface->GetSurfaceKey()); } StopTracking(aSurface, aAutoLock); diff --git a/image/VectorImage.cpp b/image/VectorImage.cpp index 59b31be47e..fb56c4b662 100644 --- a/image/VectorImage.cpp +++ b/image/VectorImage.cpp @@ -1113,7 +1113,7 @@ VectorImage::RequestDiscard() } void -VectorImage::OnSurfaceDiscarded() +VectorImage::OnSurfaceDiscarded(const SurfaceKey& aSurfaceKey) { MOZ_ASSERT(mProgressTracker); diff --git a/image/VectorImage.h b/image/VectorImage.h index bd4d393ed4..471ac7df1f 100644 --- a/image/VectorImage.h +++ b/image/VectorImage.h @@ -49,7 +49,7 @@ public: nsresult aResult, bool aLastPart) override; - virtual void OnSurfaceDiscarded() override; + virtual void OnSurfaceDiscarded(const SurfaceKey& aSurfaceKey) override; /** * Callback for SVGRootRenderingObserver. diff --git a/modules/libpref/init/all.js b/modules/libpref/init/all.js index 209603f1ef..505e7e1663 100644 --- a/modules/libpref/init/all.js +++ b/modules/libpref/init/all.js @@ -4223,6 +4223,10 @@ pref("image.layerize.always", false); // compressed data. pref("image.mem.discardable", true); +// Discards inactive image frames of _animated_ images and re-decodes them on +// demand from compressed data. Has no effect if image.mem.discardable is false. +pref("image.mem.animated.discardable", true); + // Allows image locking of decoded image data in content processes. pref("image.mem.allow_locking_in_content_processes", true); From e96122ede2c3d96208ab39d188d6821369eca4b7 Mon Sep 17 00:00:00 2001 From: Martok Date: Sat, 31 Dec 2022 22:55:46 +0100 Subject: [PATCH 03/11] Issue #2073 - m-c 523950: Discard decoded frames of very large GIF animations (squashed) Controlled by image.animated.decode-on-demand.threshold-kb, default 256MB Includes squashed bugfixes/regressions: - m-c 1444537: Shutting down the decode pool should make animated decoders bail early - m-c 1628606: Make sure to mark the surface cache entry available before sending the frame complete notification - m-c 1502275: Skip recreating the decoder after redecode errors if an animated image is reset - m-c 1443232: Don't insert frames into our AnimationFrameBuffer that we consider in error and unusable --- gfx/thebes/gfxPrefs.h | 2 + image/AnimationFrameBuffer.cpp | 324 +++++++++++++++++++++++++++++ image/AnimationFrameBuffer.h | 204 ++++++++++++++++++ image/AnimationSurfaceProvider.cpp | 237 +++++++++++++++++---- image/AnimationSurfaceProvider.h | 17 +- image/DecodePool.cpp | 14 +- image/DecodePool.h | 4 + image/Decoder.cpp | 4 + image/Decoder.h | 20 ++ image/DecoderFactory.cpp | 29 ++- image/DecoderFactory.h | 13 +- image/FrameAnimator.cpp | 36 +++- image/FrameAnimator.h | 6 + image/ISurfaceProvider.h | 39 ++++ image/RasterImage.cpp | 10 +- image/SourceBuffer.h | 6 + image/decoders/nsBMPDecoder.h | 2 + image/decoders/nsGIFDecoder2.h | 2 + image/decoders/nsICODecoder.h | 1 + image/decoders/nsIconDecoder.h | 2 + image/decoders/nsJPEGDecoder.h | 2 + image/decoders/nsPNGDecoder.h | 2 + image/decoders/nsWebPDecoder.h | 2 + image/moz.build | 1 + modules/libpref/init/all.js | 9 + 25 files changed, 925 insertions(+), 63 deletions(-) create mode 100644 image/AnimationFrameBuffer.cpp create mode 100644 image/AnimationFrameBuffer.h diff --git a/gfx/thebes/gfxPrefs.h b/gfx/thebes/gfxPrefs.h index c7052b7c6b..81c66603c1 100644 --- a/gfx/thebes/gfxPrefs.h +++ b/gfx/thebes/gfxPrefs.h @@ -429,6 +429,8 @@ private: #endif DECL_GFX_PREF(Live, "gl.require-hardware", RequireHardwareGL, bool, false); + DECL_GFX_PREF(Live, "image.animated.decode-on-demand.threshold-kb", ImageAnimatedDecodeOnDemandThresholdKB, uint32_t, 256*1024); + DECL_GFX_PREF(Live, "image.animated.decode-on-demand.batch-size", ImageAnimatedDecodeOnDemandBatchSize, uint32_t, 6); DECL_GFX_PREF(Once, "image.cache.size", ImageCacheSize, int32_t, 5*1024*1024); DECL_GFX_PREF(Once, "image.cache.timeweight", ImageCacheTimeWeight, int32_t, 500); DECL_GFX_PREF(Live, "image.decode-immediately.enabled", ImageDecodeImmediatelyEnabled, bool, false); diff --git a/image/AnimationFrameBuffer.cpp b/image/AnimationFrameBuffer.cpp new file mode 100644 index 0000000000..5d8b672f66 --- /dev/null +++ b/image/AnimationFrameBuffer.cpp @@ -0,0 +1,324 @@ +/* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +#include "AnimationFrameBuffer.h" +#include "mozilla/Move.h" // for Move + +namespace mozilla { +namespace image { + +AnimationFrameBuffer::AnimationFrameBuffer() + : mThreshold(0) + , mBatch(0) + , mPending(0) + , mAdvance(0) + , mInsertIndex(0) + , mGetIndex(0) + , mSizeKnown(false) + , mRedecodeError(false) +{ } + +void +AnimationFrameBuffer::Initialize(size_t aThreshold, + size_t aBatch, + size_t aStartFrame) +{ + MOZ_ASSERT(mThreshold == 0); + MOZ_ASSERT(mBatch == 0); + MOZ_ASSERT(mPending == 0); + MOZ_ASSERT(mAdvance == 0); + MOZ_ASSERT(mFrames.IsEmpty()); + + mThreshold = aThreshold; + mBatch = aBatch; + mAdvance = aStartFrame; + + if (mBatch > SIZE_MAX/4) { + // Batch size is so big, we will just end up decoding the whole animation. + mBatch = SIZE_MAX/4; + } else if (mBatch < 1) { + // Never permit a batch size smaller than 1. We always want to be asking for + // at least one frame to start. + mBatch = 1; + } + + // To simplify the code, we have the assumption that the threshold for + // entering discard-after-display mode is at least twice the batch size (since + // that is the most frames-pending-decode we will request) + 1 for the current + // frame. That way the redecoded frames being inserted will never risk + // overlapping the frames we will discard due to the animation progressing. + // That may cause us to use a little more memory than we want but that is an + // acceptable tradeoff for simplicity. + size_t minThreshold = 2 * mBatch + 1; + if (mThreshold < minThreshold) { + mThreshold = minThreshold; + } + + // The maximum number of frames we should ever have decoded at one time is + // twice the batch. That is a good as number as any to start our decoding at. + mPending = mBatch * 2; +} + +bool +AnimationFrameBuffer::Insert(RawAccessFrameRef&& aFrame) +{ + // We should only insert new frames if we actually asked for them. + MOZ_ASSERT(mPending > 0); + + if (mSizeKnown) { + // We only insert after the size is known if we are repeating the animation + // and we did not keep all of the frames. Replace whatever is there + // (probably an empty frame) with the new frame. + MOZ_ASSERT(MayDiscard()); + + // The first decode produced fewer frames than the redecodes, presumably + // because it hit an out-of-memory error which later attempts avoided. Just + // stop the animation because we can't tell the image that we have more + // frames now. + if (mInsertIndex >= mFrames.Length()) { + mRedecodeError = true; + mPending = 0; + return false; + } + + if (mInsertIndex > 0) { + MOZ_ASSERT(!mFrames[mInsertIndex]); + mFrames[mInsertIndex] = Move(aFrame); + } + } else if (mInsertIndex == mFrames.Length()) { + // We are still on the first pass of the animation decoding, so this is + // the first time we have seen this frame. + mFrames.AppendElement(Move(aFrame)); + + if (mInsertIndex == mThreshold) { + // We just tripped over the threshold for the first time. This is our + // chance to do any clearing of already displayed frames. After this, + // we only need to release as we advance or force a restart. + MOZ_ASSERT(MayDiscard()); + MOZ_ASSERT(mGetIndex < mInsertIndex); + for (size_t i = 1; i < mGetIndex; ++i) { + RawAccessFrameRef discard = Move(mFrames[i]); + } + } + } else if (mInsertIndex > 0) { + // We were forced to restart an animation before we decoded the last + // frame. If we were discarding frames, then we tossed what we had + // except for the first frame. + MOZ_ASSERT(mInsertIndex < mFrames.Length()); + MOZ_ASSERT(!mFrames[mInsertIndex]); + MOZ_ASSERT(MayDiscard()); + mFrames[mInsertIndex] = Move(aFrame); + } else { // mInsertIndex == 0 + // We were forced to restart an animation before we decoded the last + // frame. We don't need the redecoded first frame because we always keep + // the original. + MOZ_ASSERT(MayDiscard()); + } + + MOZ_ASSERT(mFrames[mInsertIndex]); + ++mInsertIndex; + + // Ensure we only request more decoded frames if we actually need them. If we + // need to advance to a certain point in the animation on behalf of the owner, + // then do so. This ensures we keep decoding. If the batch size is really + // small (i.e. 1), it is possible advancing will request the decoder to + // "restart", but we haven't told it to stop yet. Note that we skip the first + // insert because we actually start "advanced" to the first frame anyways. + bool continueDecoding = --mPending > 0; + if (mAdvance > 0 && mInsertIndex > 1) { + continueDecoding |= AdvanceInternal(); + --mAdvance; + } + return continueDecoding; +} + +bool +AnimationFrameBuffer::MarkComplete() +{ + // We may have stopped decoding at a different point in the animation than we + // did previously. That means the decoder likely hit a new error, e.g. OOM. + // This will prevent us from advancing as well, because we are missing the + // required frames to blend. + // + // XXX(aosmond): In an ideal world, we would be generating full frames, and + // the consumer of our data doesn't care about our internal state. It simply + // knows about the first frame, the current frame, and how long to display the + // current frame. + if (NS_WARN_IF(mInsertIndex != mFrames.Length())) { + MOZ_ASSERT(mSizeKnown); + mRedecodeError = true; + mPending = 0; + } + + // We reached the end of the animation, the next frame we get, if we get + // another, will be the first frame again. + mInsertIndex = 0; + + // Since we only request advancing when we want to resume at a certain point + // in the animation, we should never exceed the number of frames. + MOZ_ASSERT(mAdvance == 0); + + if (!mSizeKnown) { + // We just received the last frame in the animation. Compact the frame array + // because we know we won't need to grow beyond here. + mSizeKnown = true; + mFrames.Compact(); + + if (!MayDiscard()) { + // If we did not meet the threshold, then we know we want to keep all of the + // frames. If we also hit the last frame, we don't want to ask for more. + mPending = 0; + } + } + + return mPending > 0; +} + +DrawableFrameRef +AnimationFrameBuffer::Get(size_t aFrame) +{ + // We should not have asked for a frame if we never inserted. + if (mFrames.IsEmpty()) { + MOZ_ASSERT_UNREACHABLE("Calling Get() when we have no frames"); + return DrawableFrameRef(); + } + + // If we don't have that frame, return an empty frame ref. + if (aFrame >= mFrames.Length()) { + return DrawableFrameRef(); + } + + // We've got the requested frame because we are not discarding frames. While + // we typically should have not run out of frames since we ask for more before + // we want them, it is possible the decoder is behind. + if (!mFrames[aFrame]) { + MOZ_ASSERT(MayDiscard()); + return DrawableFrameRef(); + } + + // If we are advancing on behalf of the animation, we don't expect it to be + // getting any frames (besides the first) until we get the desired frame. + MOZ_ASSERT(aFrame == 0 || mAdvance == 0); + return mFrames[aFrame]->DrawableRef(); +} + +bool +AnimationFrameBuffer::AdvanceTo(size_t aExpectedFrame) +{ + // The owner should only be advancing once it has reached the requested frame + // in the animation. + MOZ_ASSERT(mAdvance == 0); + bool restartDecoder = AdvanceInternal(); + // Advancing should always be successful, as it should only happen after the + // owner has accessed the next (now current) frame. + MOZ_ASSERT(mGetIndex == aExpectedFrame); + return restartDecoder; +} + +bool +AnimationFrameBuffer::AdvanceInternal() +{ + // We should not have advanced if we never inserted. + if (mFrames.IsEmpty()) { + MOZ_ASSERT_UNREACHABLE("Calling Advance() when we have no frames"); + return false; + } + + // We only want to change the current frame index if we have advanced. This + // means either a higher frame index, or going back to the beginning. + size_t framesLength = mFrames.Length(); + // We should never have advanced beyond the frame buffer. + MOZ_ASSERT(mGetIndex < framesLength); + // We should never advance if the current frame is null -- it needs to know + // the timeout from it at least to know when to advance. + MOZ_ASSERT(mFrames[mGetIndex]); + if (++mGetIndex == framesLength) { + MOZ_ASSERT(mSizeKnown); + mGetIndex = 0; + } + // The owner should have already accessed the next frame, so it should also + // be available. + MOZ_ASSERT(mFrames[mGetIndex]); + + // If we moved forward, that means we can remove the previous frame, assuming + // that frame is not the first frame. If we looped and are back at the first + // frame, we can remove the last frame. + if (MayDiscard()) { + RawAccessFrameRef discard; + if (mGetIndex > 1) { + discard = Move(mFrames[mGetIndex - 1]); + } else if (mGetIndex == 0) { + MOZ_ASSERT(mSizeKnown && framesLength > 1); + discard = Move(mFrames[framesLength - 1]); + } + } + + if (!mRedecodeError && (!mSizeKnown || MayDiscard())) { + // Calculate how many frames we have requested ahead of the current frame. + size_t buffered = mPending; + if (mGetIndex > mInsertIndex) { + // It wrapped around and we are decoding the beginning again before the + // the display has finished the loop. + MOZ_ASSERT(mSizeKnown); + buffered += mInsertIndex + framesLength - mGetIndex - 1; + } else { + buffered += mInsertIndex - mGetIndex - 1; + } + + if (buffered < mBatch) { + // If we have fewer frames than the batch size, then ask for more. If we + // do not have any pending, then we know that there is no active decoding. + mPending += mBatch; + return mPending == mBatch; + } + } + + return false; +} + +bool +AnimationFrameBuffer::Reset() +{ + // The animation needs to start back at the beginning. + mGetIndex = 0; + mAdvance = 0; + + if (!MayDiscard()) { + // If we haven't crossed the threshold, then we know by definition we have + // not discarded any frames. If we previously requested more frames, but + // it would have been more than we would have buffered otherwise, we can + // stop the decoding after one more frame. + if (mPending > 1 && mInsertIndex - 1 >= mBatch * 2) { + MOZ_ASSERT(!mSizeKnown); + mPending = 1; + } + + // Either the decoder is still running, or we have enough frames already. + // No need for us to restart it. + return false; + } + + // Discard all frames besides the first, because the decoder always expects + // that when it re-inserts a frame, it is not present. (It doesn't re-insert + // the first frame.) + for (size_t i = 1; i < mFrames.Length(); ++i) { + RawAccessFrameRef discard = Move(mFrames[i]); + } + + mInsertIndex = 0; + + // If we hit an error after redecoding, we never want to restart decoding. + if (mRedecodeError) { + MOZ_ASSERT(mPending == 0); + return false; + } + + bool restartDecoder = mPending == 0; + mPending = 2 * mBatch; + return restartDecoder; +} + +} // namespace image +} // namespace mozilla diff --git a/image/AnimationFrameBuffer.h b/image/AnimationFrameBuffer.h new file mode 100644 index 0000000000..aa23327e91 --- /dev/null +++ b/image/AnimationFrameBuffer.h @@ -0,0 +1,204 @@ +/* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +#ifndef mozilla_image_AnimationFrameBuffer_h +#define mozilla_image_AnimationFrameBuffer_h + +#include "ISurfaceProvider.h" + +namespace mozilla { +namespace image { + +/** + * An AnimationFrameBuffer owns the frames outputted by an animated image + * decoder as well as directing its owner on how to drive the decoder, + * whether to produce more or to stop. + * + * Based upon its given configuration parameters, it will retain up to a + * certain number of frames in the buffer before deciding to discard previous + * frames, and relying upon the decoder to recreate older frames when the + * animation loops. It will also request that the decoder stop producing more + * frames when the display of the frames are far behind -- this allows other + * tasks and images which require decoding to take execution priority. + * + * The desire is that smaller animated images should be kept completely in + * memory while larger animated images should only keep a certain number of + * frames to minimize our memory footprint at the cost of CPU. + */ +class AnimationFrameBuffer final +{ +public: + AnimationFrameBuffer(); + + /** + * Configure the frame buffer with a particular threshold and batch size. Note + * that the frame buffer may adjust the given values. + * + * @param aThreshold Maximum number of frames that may be stored in the frame + * buffer before it may discard already displayed frames. + * Once exceeded, it will discard the previous frame to the + * current frame whenever Advance is called. It always + * retains the first frame. + * + * @param aBatch Number of frames we request to be decoded each time it + * decides we need more. + * + * @param aStartFrame The starting frame for the animation. The frame buffer + * will auto-advance (and thus keep the decoding pipeline + * going) until it has reached this frame. Useful when the + * animation was progressing, but the surface was + * discarded, and we had to redecode. + */ + void Initialize(size_t aThreshold, size_t aBatch, size_t aStartFrame); + + /** + * Access a specific frame from the frame buffer. It should generally access + * frames in sequential order, increasing in tandem with AdvanceTo calls. The + * first frame may be accessed at any time. The access order should start with + * the same value as that given in Initialize (aStartFrame). + * + * @param aFrame The frame index to access. + * + * @returns The frame, if available. + */ + DrawableFrameRef Get(size_t aFrame); + + /** + * Inserts a frame into the frame buffer. If it has yet to fully decode the + * animated image yet, then it will append the frame to its internal buffer. + * If it has been fully decoded, it will replace the next frame in its buffer + * with the given frame. + * + * Once we have a sufficient number of frames buffered relative to the + * currently displayed frame, it will return false to indicate the caller + * should stop decoding. + * + * @param aFrame The frame to insert into the buffer. + * + * @returns True if the decoder should decode another frame. + */ + bool Insert(RawAccessFrameRef&& aFrame); + + /** + * This should be called after the last frame has been inserted. If the buffer + * is discarding old frames, it may request more frames to be decoded. In this + * case that means the decoder should start again from the beginning. This + * return value should be used in preference to that of the Insert call. + * + * @returns True if the decoder should decode another frame. + */ + bool MarkComplete(); + + /** + * Advance the currently displayed frame of the frame buffer. If it reaches + * the end, it will loop back to the beginning. It should not be called unless + * a call to Get has returned a valid frame for the next frame index. + * + * As we advance, the number of frames we have buffered ahead of the current + * will shrink. Once that becomes too few, we will request a batch-sized set + * of frames to be decoded from the decoder. + * + * @param aExpectedFrame The frame we expect to have advanced to. This is + * used for confirmation purposes (e.g. asserts). + * + * @returns True if the caller should restart the decoder. + */ + bool AdvanceTo(size_t aExpectedFrame); + + /** + * Resets the currently displayed frame of the frame buffer to the beginning. + * If the buffer is discarding old frames, it will actually discard all frames + * besides the first. + * + * @returns True if the caller should restart the decoder. + */ + bool Reset(); + + /** + * @returns True if frames post-advance may be discarded and redecoded on + * demand, else false. + */ + bool MayDiscard() const { return mFrames.Length() > mThreshold; } + + /** + * @returns True if the frame buffer was ever marked as complete. This implies + * that the total number of frames is known and may be gotten from + * Frames().Length(). + */ + bool SizeKnown() const { return mSizeKnown; } + + /** + * @returns True if encountered an error during redecode which should cause + * the caller to stop inserting frames. + */ + bool HasRedecodeError() const { return mRedecodeError; } + + /** + * @returns The current frame index we have advanced to. + */ + size_t Displayed() const { return mGetIndex; } + + /** + * @returns Outstanding frames desired from the decoder. + */ + size_t PendingDecode() const { return mPending; } + + /** + * @returns Outstanding frames to advance internally. + */ + size_t PendingAdvance() const { return mAdvance; } + + /** + * @returns Number of frames we request to be decoded each time it decides we + * need more. + */ + size_t Batch() const { return mBatch; } + + /** + * @returns Maximum number of frames before we start discarding previous + * frames post-advance. + */ + size_t Threshold() const { return mThreshold; } + + /** + * @returns The frames of this animation, in order. May contain empty indices. + */ + const nsTArray& Frames() const { return mFrames; } + +private: + bool AdvanceInternal(); + + /// The frames of this animation, in order, but may have holes if discarding. + nsTArray mFrames; + + // The maximum number of frames we can have before discarding. + size_t mThreshold; + + // The minimum number of frames that we want buffered ahead of the display. + size_t mBatch; + + // The number of frames to decode before we stop. + size_t mPending; + + // The number of frames we need to auto-advance to synchronize with the caller. + size_t mAdvance; + + // The mFrames index in which to insert the next decoded frame. + size_t mInsertIndex; + + // The mFrames index that we have advanced to. + size_t mGetIndex; + + // True if the total number of frames is known. + bool mSizeKnown; + + // True if we encountered an error while redecoding. + bool mRedecodeError; +}; + +} // namespace image +} // namespace mozilla + +#endif // mozilla_image_AnimationFrameBuffer_h diff --git a/image/AnimationSurfaceProvider.cpp b/image/AnimationSurfaceProvider.cpp index 0dacf25c23..1d76332930 100644 --- a/image/AnimationSurfaceProvider.cpp +++ b/image/AnimationSurfaceProvider.cpp @@ -8,6 +8,7 @@ #include "gfxPrefs.h" #include "nsProxyRelease.h" +#include "DecodePool.h" #include "Decoder.h" using namespace mozilla::gfx; @@ -17,7 +18,8 @@ namespace image { AnimationSurfaceProvider::AnimationSurfaceProvider(NotNull aImage, const SurfaceKey& aSurfaceKey, - NotNull aDecoder) + NotNull aDecoder, + size_t aCurrentFrame) : ISurfaceProvider(ImageKey(aImage.get()), aSurfaceKey, AvailabilityState::StartAsPlaceholder()) , mImage(aImage.get()) @@ -29,6 +31,22 @@ AnimationSurfaceProvider::AnimationSurfaceProvider(NotNull aImage, "Use MetadataDecodingTask for metadata decodes"); MOZ_ASSERT(!mDecoder->IsFirstFrameDecode(), "Use DecodedSurfaceProvider for single-frame image decodes"); + + // We still produce paletted surfaces for GIF which means the frames are + // smaller than one would expect for APNG. This may be removed if/when + // bug 1337111 lands and it is enabled by default. + size_t pixelSize = aDecoder->GetType() == DecoderType::GIF + ? sizeof(uint8_t) : sizeof(uint32_t); + + // Calculate how many frames we need to decode in this animation before we + // enter decode-on-demand mode. + IntSize frameSize = aSurfaceKey.Size(); + size_t threshold = + (size_t(gfxPrefs::ImageAnimatedDecodeOnDemandThresholdKB()) * 1024) / + (pixelSize * frameSize.width * frameSize.height); + size_t batch = gfxPrefs::ImageAnimatedDecodeOnDemandBatchSize(); + + mFrames.Initialize(threshold, batch, aCurrentFrame); } AnimationSurfaceProvider::~AnimationSurfaceProvider() @@ -53,6 +71,75 @@ AnimationSurfaceProvider::DropImageReference() NS_ReleaseOnMainThread(image.forget(), /* aAlwaysProxy = */ true); } +void +AnimationSurfaceProvider::Reset() +{ + // We want to go back to the beginning. + bool mayDiscard; + bool restartDecoder = false; + + { + MutexAutoLock lock(mFramesMutex); + + // If we have not crossed the threshold, we know we haven't discarded any + // frames, and thus we know it is safe move our display index back to the + // very beginning. It would be cleaner to let the frame buffer make this + // decision inside the AnimationFrameBuffer::Reset method, but if we have + // crossed the threshold, we need to hold onto the decoding mutex too. We + // should avoid blocking the main thread on the decoder threads. + mayDiscard = mFrames.MayDiscard(); + if (!mayDiscard) { + restartDecoder = mFrames.Reset(); + } + } + + if (mayDiscard) { + // We are over the threshold and have started discarding old frames. In + // that case we need to seize the decoding mutex. Thankfully we know that + // we are in the process of decoding at most the batch size frames, so + // this should not take too long to acquire. + MutexAutoLock lock(mDecodingMutex); + + // We may have hit an error while redecoding. Because FrameAnimator is + // tightly coupled to our own state, that means we would need to go through + // some heroics to resume animating in those cases. The typical reason for + // a redecode to fail is out of memory, and recycling should prevent most of + // those errors. When image.animated.generate-full-frames has shipped + // enabled on a release or two, we can simply remove the old FrameAnimator + // blending code and simplify this quite a bit -- just always pop the next + // full frame and timeout off the stack. + if (mDecoder) { + mDecoder = DecoderFactory::CloneAnimationDecoder(mDecoder); + MOZ_ASSERT(mDecoder); + + MutexAutoLock lock2(mFramesMutex); + restartDecoder = mFrames.Reset(); + } else { + MOZ_ASSERT(mFrames.HasRedecodeError()); + } + } + + if (restartDecoder) { + DecodePool::Singleton()->AsyncRun(this); + } +} + +void +AnimationSurfaceProvider::Advance(size_t aFrame) +{ + bool restartDecoder; + + { + // Typical advancement of a frame. + MutexAutoLock lock(mFramesMutex); + restartDecoder = mFrames.AdvanceTo(aFrame); + } + + if (restartDecoder) { + DecodePool::Singleton()->AsyncRun(this); + } +} + DrawableFrameRef AnimationSurfaceProvider::DrawableRef(size_t aFrame) { @@ -63,19 +150,7 @@ AnimationSurfaceProvider::DrawableRef(size_t aFrame) return DrawableFrameRef(); } - if (mFrames.IsEmpty()) { - MOZ_ASSERT_UNREACHABLE("Calling DrawableRef() when we have no frames"); - return DrawableFrameRef(); - } - - // If we don't have that frame, return an empty frame ref. - if (aFrame >= mFrames.Length()) { - return DrawableFrameRef(); - } - - // We've got the requested frame. Return it. - MOZ_ASSERT(mFrames[aFrame]); - return mFrames[aFrame]->DrawableRef(); + return mFrames.Get(aFrame); } bool @@ -88,13 +163,20 @@ AnimationSurfaceProvider::IsFinished() const return false; } - if (mFrames.IsEmpty()) { + if (mFrames.Frames().IsEmpty()) { MOZ_ASSERT_UNREACHABLE("Calling IsFinished() when we have no frames"); return false; } // As long as we have at least one finished frame, we're finished. - return mFrames[0]->IsFinished(); + return mFrames.Frames()[0]->IsFinished(); +} + +bool +AnimationSurfaceProvider::IsFullyDecoded() const +{ + MutexAutoLock lock(mFramesMutex); + return mFrames.SizeKnown() && !mFrames.MayDiscard(); } size_t @@ -125,8 +207,10 @@ AnimationSurfaceProvider::AddSizeOfExcludingThis(MallocSizeOf aMallocSizeOf, // that we must be careful to always use the same ordering elsewhere. MutexAutoLock lock(mFramesMutex); - for (const RawAccessFrameRef& frame : mFrames) { - frame->AddSizeOfExcludingThis(aMallocSizeOf, aHeapSizeOut, aNonHeapSizeOut); + for (const RawAccessFrameRef& frame : mFrames.Frames()) { + if (frame) { + frame->AddSizeOfExcludingThis(aMallocSizeOf, aHeapSizeOut, aNonHeapSizeOut); + } } } @@ -135,7 +219,7 @@ AnimationSurfaceProvider::Run() { MutexAutoLock lock(mDecodingMutex); - if (!mDecoder || !mImage) { + if (!mDecoder) { MOZ_ASSERT_UNREACHABLE("Running after decoding finished?"); return; } @@ -150,15 +234,34 @@ AnimationSurfaceProvider::Run() // Since we're not sure, rather than call CheckForNewFrameAtYield() here // we call CheckForNewFrameAtTerminalState(), which handles both of these // possibilities. - CheckForNewFrameAtTerminalState(); - - // We're done! + bool continueDecoding = CheckForNewFrameAtTerminalState(); FinishDecoding(); - return; + + // Even if it is the last frame, we may not have enough frames buffered + // ahead of the current. If we are shutting down, we want to ensure we + // release the thread as soon as possible. The animation may advance even + // during shutdown, which keeps us decoding, and thus blocking the decode + // pool during teardown. + if (!mDecoder || !continueDecoding || + DecodePool::Singleton()->IsShuttingDown()) { + return; + } + // Restart from the very beginning because the decoder was recreated. + continue; + } + + // If there is output available we want to change the entry in the surface + // cache from a placeholder to an actual surface now before NotifyProgress + // call below so that when consumers get the frame complete notification + // from the NotifyProgress they can actually get a surface from the surface + // cache. + bool checkForNewFrameAtYieldResult = false; + if (result == LexerResult(Yield::OUTPUT_AVAILABLE)) { + checkForNewFrameAtYieldResult = CheckForNewFrameAtYield(); } // Notify for the progress we've made so far. - if (mDecoder->HasProgress()) { + if (mImage && mDecoder->HasProgress()) { NotifyProgress(WrapNotNull(mImage), WrapNotNull(mDecoder)); } @@ -168,38 +271,52 @@ AnimationSurfaceProvider::Run() return; } - // There's new output available - a new frame! Grab it. + // There's new output available - a new frame! Grab it. If we don't need any + // more for the moment we can break out of the loop. If we are shutting + // down, we want to ensure we release the thread as soon as possible. The + // animation may advance even during shutdown, which keeps us decoding, and + // thus blocking the decode pool during teardown. MOZ_ASSERT(result == LexerResult(Yield::OUTPUT_AVAILABLE)); - CheckForNewFrameAtYield(); + if (!checkForNewFrameAtYieldResult || + DecodePool::Singleton()->IsShuttingDown()) { + return; + } } } -void +bool AnimationSurfaceProvider::CheckForNewFrameAtYield() { mDecodingMutex.AssertCurrentThreadOwns(); MOZ_ASSERT(mDecoder); bool justGotFirstFrame = false; + bool continueDecoding; { MutexAutoLock lock(mFramesMutex); // Try to get the new frame from the decoder. RawAccessFrameRef frame = mDecoder->GetCurrentFrameRef(); + MOZ_ASSERT(mDecoder->HasFrameToTake()); + mDecoder->ClearHasFrameToTake(); + if (!frame) { MOZ_ASSERT_UNREACHABLE("Decoder yielded but didn't produce a frame?"); - return; + return true; } // We should've gotten a different frame than last time. - MOZ_ASSERT_IF(!mFrames.IsEmpty(), - mFrames.LastElement().get() != frame.get()); + MOZ_ASSERT_IF(!mFrames.Frames().IsEmpty(), + mFrames.Frames().LastElement().get() != frame.get()); // Append the new frame to the list. - mFrames.AppendElement(Move(frame)); + continueDecoding = mFrames.Insert(Move(frame)); - if (mFrames.Length() == 1) { + // We only want to handle the first frame if it is the first pass for the + // animation decoder. The owning image will be cleared after that. + size_t frameCount = mFrames.Frames().Length(); + if (frameCount == 1 && mImage) { justGotFirstFrame = true; } } @@ -207,32 +324,49 @@ AnimationSurfaceProvider::CheckForNewFrameAtYield() if (justGotFirstFrame) { AnnounceSurfaceAvailable(); } + + return continueDecoding; } -void +bool AnimationSurfaceProvider::CheckForNewFrameAtTerminalState() { mDecodingMutex.AssertCurrentThreadOwns(); MOZ_ASSERT(mDecoder); bool justGotFirstFrame = false; + bool continueDecoding; { MutexAutoLock lock(mFramesMutex); + // The decoder may or may not have a new frame for us at this point. Avoid + // reinserting the same frame again. RawAccessFrameRef frame = mDecoder->GetCurrentFrameRef(); - if (!frame) { - return; + + // If the decoder didn't finish a new frame (ie if, after starting the + // frame, it got an error and aborted the frame and the rest of the decode) + // that means it won't be reporting it to the image or FrameAnimator so we + // should ignore it too, that's what HasFrameToTake tracks basically. + if (!mDecoder->HasFrameToTake()) { + frame = RawAccessFrameRef(); + } else { + MOZ_ASSERT(frame); + mDecoder->ClearHasFrameToTake(); } - if (!mFrames.IsEmpty() && mFrames.LastElement().get() == frame.get()) { - return; // We already have this one. + if (!frame || (!mFrames.Frames().IsEmpty() && + mFrames.Frames().LastElement().get() == frame.get())) { + return mFrames.MarkComplete(); } // Append the new frame to the list. - mFrames.AppendElement(Move(frame)); + mFrames.Insert(Move(frame)); + continueDecoding = mFrames.MarkComplete(); - if (mFrames.Length() == 1) { + // We only want to handle the first frame if it is the first pass for the + // animation decoder. The owning image will be cleared after that. + if (mFrames.Frames().Length() == 1 && mImage) { justGotFirstFrame = true; } } @@ -240,6 +374,8 @@ AnimationSurfaceProvider::CheckForNewFrameAtTerminalState() if (justGotFirstFrame) { AnnounceSurfaceAvailable(); } + + return continueDecoding; } void @@ -260,14 +396,27 @@ void AnimationSurfaceProvider::FinishDecoding() { mDecodingMutex.AssertCurrentThreadOwns(); - MOZ_ASSERT(mImage); MOZ_ASSERT(mDecoder); - // Send notifications. - NotifyDecodeComplete(WrapNotNull(mImage), WrapNotNull(mDecoder)); + if (mImage) { + // Send notifications. + NotifyDecodeComplete(WrapNotNull(mImage), WrapNotNull(mDecoder)); + } - // Destroy our decoder; we don't need it anymore. - mDecoder = nullptr; + // Determine if we need to recreate the decoder, in case we are discarding + // frames and need to loop back to the beginning. + bool recreateDecoder; + { + MutexAutoLock lock(mFramesMutex); + recreateDecoder = !mFrames.HasRedecodeError() && mFrames.MayDiscard(); + } + + if (recreateDecoder) { + mDecoder = DecoderFactory::CloneAnimationDecoder(mDecoder); + MOZ_ASSERT(mDecoder); + } else { + mDecoder = nullptr; + } // We don't need a reference to our image anymore, either, and we don't want // one. We may be stored in the surface cache for a long time after decoding diff --git a/image/AnimationSurfaceProvider.h b/image/AnimationSurfaceProvider.h index bf87f37ac4..720cae57e7 100644 --- a/image/AnimationSurfaceProvider.h +++ b/image/AnimationSurfaceProvider.h @@ -13,6 +13,7 @@ #include "FrameAnimator.h" #include "IDecodingTask.h" #include "ISurfaceProvider.h" +#include "AnimationFrameBuffer.h" namespace mozilla { namespace image { @@ -31,7 +32,8 @@ public: AnimationSurfaceProvider(NotNull aImage, const SurfaceKey& aSurfaceKey, - NotNull aDecoder); + NotNull aDecoder, + size_t aCurrentFrame); ////////////////////////////////////////////////////////////////////////////// @@ -44,10 +46,13 @@ public: DrawableSurface Surface() override { return DrawableSurface(WrapNotNull(this)); } bool IsFinished() const override; + bool IsFullyDecoded() const override; size_t LogicalSizeInBytes() const override; void AddSizeOfExcludingThis(MallocSizeOf aMallocSizeOf, size_t& aHeapSizeOut, size_t& aNonHeapSizeOut) override; + void Reset() override; + void Advance(size_t aFrame) override; protected: DrawableFrameRef DrawableRef(size_t aFrame) override; @@ -77,11 +82,15 @@ private: virtual ~AnimationSurfaceProvider(); void DropImageReference(); - void CheckForNewFrameAtYield(); - void CheckForNewFrameAtTerminalState(); void AnnounceSurfaceAvailable(); void FinishDecoding(); + // @returns Whether or not we should continue decoding. + bool CheckForNewFrameAtYield(); + + // @returns Whether or not we should restart decoding. + bool CheckForNewFrameAtTerminalState(); + /// The image associated with our decoder. RefPtr mImage; @@ -95,7 +104,7 @@ private: mutable Mutex mFramesMutex; /// The frames of this animation, in order. - nsTArray mFrames; + AnimationFrameBuffer mFrames; }; } // namespace image diff --git a/image/DecodePool.cpp b/image/DecodePool.cpp index a8c4cbecc0..0aeed61547 100644 --- a/image/DecodePool.cpp +++ b/image/DecodePool.cpp @@ -87,6 +87,12 @@ public: mMonitor.NotifyAll(); } + bool IsShuttingDown() const + { + MonitorAutoLock lock(mMonitor); + return mShuttingDown; + } + /// Pushes a new decode work item. void PushWork(IDecodingTask* aTask) { @@ -150,7 +156,7 @@ private: nsThreadPoolNaming mThreadNaming; // mMonitor guards the queues and mShuttingDown. - Monitor mMonitor; + mutable Monitor mMonitor; nsTArray> mHighPriorityQueue; nsTArray> mLowPriorityQueue; bool mShuttingDown; @@ -299,6 +305,12 @@ DecodePool::Observe(nsISupports*, const char* aTopic, const char16_t*) return NS_OK; } +bool +DecodePool::IsShuttingDown() const +{ + return mImpl->IsShuttingDown(); +} + void DecodePool::AsyncRun(IDecodingTask* aTask) { diff --git a/image/DecodePool.h b/image/DecodePool.h index 9d62731e50..fd56f0bfa1 100644 --- a/image/DecodePool.h +++ b/image/DecodePool.h @@ -54,6 +54,10 @@ public: /// same as the number of decoding threads we're actually using. static uint32_t NumberOfCores(); + /// True if the DecodePool is being shutdown. This may only be called by + /// threads from the pool to check if they should keep working or not. + bool IsShuttingDown() const; + /// Ask the DecodePool to run @aTask asynchronously and return immediately. void AsyncRun(IDecodingTask* aTask); diff --git a/image/Decoder.cpp b/image/Decoder.cpp index 5d39080928..9d0647a4a3 100644 --- a/image/Decoder.cpp +++ b/image/Decoder.cpp @@ -36,6 +36,7 @@ Decoder::Decoder(RasterImage* aImage) , mHaveExplicitOutputSize(false) , mInFrame(false) , mFinishedNewFrame(false) + , mHasFrameToTake(false) , mReachedTerminalState(false) , mDecodeDone(false) , mError(false) @@ -254,6 +255,8 @@ Decoder::AllocateFrame(const gfx::IntSize& aOutputSize, mCurrentFrame.get()); if (mCurrentFrame) { + mHasFrameToTake = true; + // Gather the raw pointers the decoders will use. mCurrentFrame->GetImageData(&mImageData, &mImageDataLength); mCurrentFrame->GetPaletteData(&mColormap, &mColormapSize); @@ -474,6 +477,7 @@ Decoder::PostError() mCurrentFrame->Abort(); mInFrame = false; --mFrameCount; + mHasFrameToTake = false; } } diff --git a/image/Decoder.h b/image/Decoder.h index c0f4a20a67..87dfb00508 100644 --- a/image/Decoder.h +++ b/image/Decoder.h @@ -200,6 +200,11 @@ public: mIterator.emplace(Move(aIterator)); } + SourceBuffer* GetSourceBuffer() const + { + return mIterator->Owner(); + } + /** * Should this decoder send partial invalidations? */ @@ -244,6 +249,12 @@ public: /// Are we in the middle of a frame right now? Used for assertions only. bool InFrame() const { return mInFrame; } + /// Type of decoder. + virtual DecoderType GetType() const + { + return DecoderType::UNKNOWN; + } + enum DecodeStyle { PROGRESSIVE, // produce intermediate frames representing the partial // state of the image @@ -339,6 +350,11 @@ public: : RawAccessFrameRef(); } + bool HasFrameToTake() const { return mHasFrameToTake; } + void ClearHasFrameToTake() { + MOZ_ASSERT(mHasFrameToTake); + mHasFrameToTake = false; + } protected: friend class nsICODecoder; @@ -493,6 +509,10 @@ private: bool mInFrame : 1; bool mFinishedNewFrame : 1; // True if PostFrameStop() has been called since // the last call to TakeCompleteFrameCount(). + // Has a new frame that AnimationSurfaceProvider can take. Unfortunately this + // has to be separate from mFinishedNewFrame because the png decoder yields a + // new frame before calling PostFrameStop(). + bool mHasFrameToTake : 1; bool mReachedTerminalState : 1; bool mDecodeDone : 1; bool mError : 1; diff --git a/image/DecoderFactory.cpp b/image/DecoderFactory.cpp index dffe4dc211..33d2b56558 100644 --- a/image/DecoderFactory.cpp +++ b/image/DecoderFactory.cpp @@ -181,7 +181,8 @@ DecoderFactory::CreateAnimationDecoder(DecoderType aType, NotNull aSourceBuffer, const IntSize& aIntrinsicSize, DecoderFlags aDecoderFlags, - SurfaceFlags aSurfaceFlags) + SurfaceFlags aSurfaceFlags, + size_t aCurrentFrame) { if (aType == DecoderType::UNKNOWN) { return nullptr; @@ -213,7 +214,8 @@ DecoderFactory::CreateAnimationDecoder(DecoderType aType, NotNull> provider = WrapNotNull(new AnimationSurfaceProvider(aImage, surfaceKey, - WrapNotNull(decoder))); + WrapNotNull(decoder), + aCurrentFrame)); // Attempt to insert the surface provider into the surface cache right away so // we won't trigger any more decoders with the same parameters. @@ -226,6 +228,29 @@ DecoderFactory::CreateAnimationDecoder(DecoderType aType, return task.forget(); } +/* static */ already_AddRefed +DecoderFactory::CloneAnimationDecoder(Decoder* aDecoder) +{ + MOZ_ASSERT(aDecoder); + MOZ_ASSERT(aDecoder->HasAnimation()); + + RefPtr decoder = GetDecoder(aDecoder->GetType(), nullptr, + /* aIsRedecode = */ true); + MOZ_ASSERT(decoder, "Should have a decoder now"); + + // Initialize the decoder. + decoder->SetMetadataDecode(false); + decoder->SetIterator(aDecoder->GetSourceBuffer()->Iterator()); + decoder->SetDecoderFlags(aDecoder->GetDecoderFlags()); + decoder->SetSurfaceFlags(aDecoder->GetSurfaceFlags()); + + if (NS_FAILED(decoder->Init())) { + return nullptr; + } + + return decoder.forget(); +} + /* static */ already_AddRefed DecoderFactory::CreateMetadataDecoder(DecoderType aType, NotNull aImage, diff --git a/image/DecoderFactory.h b/image/DecoderFactory.h index 5638789ff6..58b5709aa1 100644 --- a/image/DecoderFactory.h +++ b/image/DecoderFactory.h @@ -92,6 +92,7 @@ public: * @param aDecoderFlags Flags specifying the behavior of this decoder. * @param aSurfaceFlags Flags specifying the type of output this decoder * should produce. + * @param aCurrentFrame The current frame the decoder should auto advance to. */ static already_AddRefed CreateAnimationDecoder(DecoderType aType, @@ -99,7 +100,17 @@ public: NotNull aSourceBuffer, const gfx::IntSize& aIntrinsicSize, DecoderFlags aDecoderFlags, - SurfaceFlags aSurfaceFlags); + SurfaceFlags aSurfaceFlags, + size_t aCurrentFrame); + + /** + * Creates and initializes a decoder for animated images, cloned from the + * given decoder. + * + * @param aDecoder Decoder to clone. + */ + static already_AddRefed + CloneAnimationDecoder(Decoder* aDecoder); /** * Creates and initializes a metadata decoder of type @aType. This decoder diff --git a/image/FrameAnimator.cpp b/image/FrameAnimator.cpp index 066da94f40..b9a4aec4a8 100644 --- a/image/FrameAnimator.cpp +++ b/image/FrameAnimator.cpp @@ -64,12 +64,7 @@ AnimationState::UpdateStateInternal(LookupResult& aResult, if (mHasBeenDecoded) { Maybe frameCount = FrameCount(); MOZ_ASSERT(frameCount.isSome()); - aResult.Surface().Seek(*frameCount - 1); - if (aResult.Surface() && aResult.Surface()->IsFinished()) { - mIsCurrentlyDecoded = true; - } else { - mIsCurrentlyDecoded = false; - } + mIsCurrentlyDecoded = aResult.Surface().IsFullyDecoded(); } } @@ -286,8 +281,12 @@ FrameAnimator::AdvanceFrame(AnimationState& aState, // failure) we would have discarded all the old frames and may not yet have // the new ones. if (!nextFrame || !nextFrame->IsFinished()) { - // Uh oh, the frame we want to show is currently being decoded (partial) - // Wait until the next refresh driver tick and try again + // Uh oh, the frame we want to show is currently being decoded (partial). + // Similar to the above case, we could be blocked by network or decoding, + // and so we should advance our current time rather than risk jumping + // through the animation. We will wait until the next refresh driver tick + // and try again. + aState.mCurrentAnimationFrameTime = aTime; return ret; } @@ -313,6 +312,7 @@ FrameAnimator::AdvanceFrame(AnimationState& aState, MOZ_ASSERT(currentFrameEndTime.isSome()); aState.mCurrentAnimationFrameTime = *currentFrameEndTime; aState.mCurrentAnimationFrameIndex = nextFrameIndex; + aFrames.Advance(nextFrameIndex); return ret; } @@ -343,6 +343,7 @@ FrameAnimator::AdvanceFrame(AnimationState& aState, // Set currentAnimationFrameIndex at the last possible moment aState.mCurrentAnimationFrameIndex = nextFrameIndex; + aFrames.Advance(nextFrameIndex); // If we're here, we successfully advanced the frame. ret.mFrameAdvanced = true; @@ -350,6 +351,25 @@ FrameAnimator::AdvanceFrame(AnimationState& aState, return ret; } +void +FrameAnimator::ResetAnimation(AnimationState& aState) +{ + aState.ResetAnimation(); + + // Our surface provider is synchronized to our state, so we need to reset its + // state as well, if we still have one. + LookupResult result = + SurfaceCache::Lookup(ImageKey(mImage), + RasterSurfaceKey(mSize, + DefaultSurfaceFlags(), + PlaybackType::eAnimated)); + if (!result) { + return; + } + + result.Surface().Reset(); +} + RefreshResult FrameAnimator::RequestRefresh(AnimationState& aState, const TimeStamp& aTime, diff --git a/image/FrameAnimator.h b/image/FrameAnimator.h index 44b5a52e7c..fd331da9eb 100644 --- a/image/FrameAnimator.h +++ b/image/FrameAnimator.h @@ -276,6 +276,12 @@ public: MOZ_COUNT_DTOR(FrameAnimator); } + /** + * Call when you need to re-start animating. Ensures we start from the first + * frame. + */ + void ResetAnimation(AnimationState& aState); + /** * Re-evaluate what frame we're supposed to be on, and do whatever blending * is necessary to get us to that frame. diff --git a/image/ISurfaceProvider.h b/image/ISurfaceProvider.h index 80e1f8e9b0..b4aaf89f9b 100644 --- a/image/ISurfaceProvider.h +++ b/image/ISurfaceProvider.h @@ -54,6 +54,12 @@ public: /// @return true if DrawableRef() will return a completely decoded surface. virtual bool IsFinished() const = 0; + /// @return true if the underlying decoder is currently fully decoded. For + /// animated images, this means that at least every frame has been decoded + /// at least once. It does not guarantee that all of the frames are present, + /// as the surface provider has the option to discard as it deems necessary. + virtual bool IsFullyDecoded() const { return IsFinished(); } + /// @return the number of bytes of memory this ISurfaceProvider is expected to /// require. Optimizations may result in lower real memory usage. Trivial /// overhead is ignored. Because this value is used in bookkeeping, it's @@ -75,6 +81,9 @@ public: ref->AddSizeOfExcludingThis(aMallocSizeOf, aHeapSizeOut, aNonHeapSizeOut); } + virtual void Reset() { } + virtual void Advance(size_t aFrame) { } + /// @return the availability state of this ISurfaceProvider, which indicates /// whether DrawableRef() could successfully return a surface. Should only be /// called from SurfaceCache code as it relies on SurfaceCache for @@ -189,6 +198,36 @@ public: return mDrawableRef ? NS_OK : NS_ERROR_FAILURE; } + void Reset() + { + if (!mProvider) { + MOZ_ASSERT_UNREACHABLE("Trying to reset a static DrawableSurface?"); + return; + } + + mProvider->Reset(); + } + + void Advance(size_t aFrame) + { + if (!mProvider) { + MOZ_ASSERT_UNREACHABLE("Trying to advance a static DrawableSurface?"); + return; + } + + mProvider->Advance(aFrame); + } + + bool IsFullyDecoded() const + { + if (!mProvider) { + MOZ_ASSERT_UNREACHABLE("Trying to check decoding state of a static DrawableSurface?"); + return false; + } + + return mProvider->IsFullyDecoded(); + } + explicit operator bool() const { return mHaveSurface; } imgFrame* operator->() { return DrawableRef().get(); } diff --git a/image/RasterImage.cpp b/image/RasterImage.cpp index f7dfd0bb1a..4c6cce891b 100644 --- a/image/RasterImage.cpp +++ b/image/RasterImage.cpp @@ -853,7 +853,8 @@ RasterImage::ResetAnimation() } MOZ_ASSERT(mAnimationState, "Should have AnimationState"); - mAnimationState->ResetAnimation(); + MOZ_ASSERT(mFrameAnimator, "Should have FrameAnimator"); + mFrameAnimator->ResetAnimation(*mAnimationState); NotifyProgress(NoProgress, mAnimationState->FirstFrameRefreshArea()); @@ -1183,10 +1184,13 @@ RasterImage::Decode(const IntSize& aSize, // Create a decoder. RefPtr task; - if (mAnimationState && aPlaybackType == PlaybackType::eAnimated) { + bool animated = mAnimationState && aPlaybackType == PlaybackType::eAnimated; + if (animated) { + size_t currentFrame = mAnimationState->GetCurrentAnimationFrameIndex(); task = DecoderFactory::CreateAnimationDecoder(mDecoderType, WrapNotNull(this), mSourceBuffer, mSize, - decoderFlags, surfaceFlags); + decoderFlags, surfaceFlags, + currentFrame); mAnimationState->UpdateState(mAnimationFinished, this, mSize); // If the animation is finished we can draw right away because we just draw // the final frame all the time from now on. See comment in diff --git a/image/SourceBuffer.h b/image/SourceBuffer.h index 6f2c74d33b..e5aff1dcdd 100644 --- a/image/SourceBuffer.h +++ b/image/SourceBuffer.h @@ -187,6 +187,12 @@ public: /// @return a count of the bytes in all chunks we've advanced through. size_t ByteCount() const { return mByteCount; } + /// @return the source buffer which owns the iterator. + SourceBuffer* Owner() const { + MOZ_ASSERT(mOwner); + return mOwner; + } + private: friend class SourceBuffer; diff --git a/image/decoders/nsBMPDecoder.h b/image/decoders/nsBMPDecoder.h index 26724d44e6..793ebd1558 100644 --- a/image/decoders/nsBMPDecoder.h +++ b/image/decoders/nsBMPDecoder.h @@ -124,6 +124,8 @@ class nsBMPDecoder : public Decoder public: ~nsBMPDecoder(); + DecoderType GetType() const override { return DecoderType::BMP; } + /// Obtains the internal output image buffer. uint32_t* GetImageData() { return reinterpret_cast(mImageData); } diff --git a/image/decoders/nsGIFDecoder2.h b/image/decoders/nsGIFDecoder2.h index 46ceaa4e94..235bdf7c3d 100644 --- a/image/decoders/nsGIFDecoder2.h +++ b/image/decoders/nsGIFDecoder2.h @@ -24,6 +24,8 @@ class nsGIFDecoder2 : public Decoder public: ~nsGIFDecoder2(); + DecoderType GetType() const override { return DecoderType::GIF; } + protected: LexerResult DoDecode(SourceBufferIterator& aIterator, IResumable* aOnResume) override; diff --git a/image/decoders/nsICODecoder.h b/image/decoders/nsICODecoder.h index 46e1377aad..e33550fc54 100644 --- a/image/decoders/nsICODecoder.h +++ b/image/decoders/nsICODecoder.h @@ -69,6 +69,7 @@ public: /// @return The offset from the beginning of the ICO to the first resource. size_t FirstResourceOffset() const; + DecoderType GetType() const override { return DecoderType::ICO; } LexerResult DoDecode(SourceBufferIterator& aIterator, IResumable* aOnResume) override; nsresult FinishInternal() override; diff --git a/image/decoders/nsIconDecoder.h b/image/decoders/nsIconDecoder.h index 69198315b1..82402f305a 100644 --- a/image/decoders/nsIconDecoder.h +++ b/image/decoders/nsIconDecoder.h @@ -37,6 +37,8 @@ class nsIconDecoder : public Decoder public: virtual ~nsIconDecoder(); + DecoderType GetType() const override { return DecoderType::ICON; } + LexerResult DoDecode(SourceBufferIterator& aIterator, IResumable* aOnResume) override; diff --git a/image/decoders/nsJPEGDecoder.h b/image/decoders/nsJPEGDecoder.h index 7df89318c9..25177cef91 100644 --- a/image/decoders/nsJPEGDecoder.h +++ b/image/decoders/nsJPEGDecoder.h @@ -52,6 +52,8 @@ class nsJPEGDecoder : public Decoder public: virtual ~nsJPEGDecoder(); + DecoderType GetType() const override { return DecoderType::JPEG; } + virtual void SetSampleSize(int aSampleSize) override { mSampleSize = aSampleSize; diff --git a/image/decoders/nsPNGDecoder.h b/image/decoders/nsPNGDecoder.h index 7e677d40ab..c4ec93227f 100644 --- a/image/decoders/nsPNGDecoder.h +++ b/image/decoders/nsPNGDecoder.h @@ -25,6 +25,8 @@ public: /// @return true if this PNG is a valid ICO resource. bool IsValidICO() const; + DecoderType GetType() const override { return DecoderType::PNG; } + protected: nsresult InitInternal() override; LexerResult DoDecode(SourceBufferIterator& aIterator, diff --git a/image/decoders/nsWebPDecoder.h b/image/decoders/nsWebPDecoder.h index cdd2849f30..21df5279b6 100644 --- a/image/decoders/nsWebPDecoder.h +++ b/image/decoders/nsWebPDecoder.h @@ -21,6 +21,8 @@ class nsWebPDecoder final : public Decoder public: virtual ~nsWebPDecoder(); + DecoderType GetType() const override { return DecoderType::WEBP; } + protected: LexerResult DoDecode(SourceBufferIterator& aIterator, IResumable* aOnResume) override; diff --git a/image/moz.build b/image/moz.build index b73c5ae519..04582e9ef8 100644 --- a/image/moz.build +++ b/image/moz.build @@ -47,6 +47,7 @@ EXPORTS += [ ] UNIFIED_SOURCES += [ + 'AnimationFrameBuffer.cpp', 'AnimationSurfaceProvider.cpp', 'ClippedImage.cpp', 'DecodedSurfaceProvider.cpp', diff --git a/modules/libpref/init/all.js b/modules/libpref/init/all.js index 505e7e1663..e9d26dc6c0 100644 --- a/modules/libpref/init/all.js +++ b/modules/libpref/init/all.js @@ -4187,6 +4187,15 @@ pref("toolkit.zoomManager.zoomValues", ".3,.5,.67,.8,.9,1,1.1,1.2,1.33,1.5,1.7,2 // Image-related prefs // +// The maximum size (in kB) that the aggregate frames of an animation can use +// before it starts to discard already displayed frames and redecode them as +// necessary. +pref("image.animated.decode-on-demand.threshold-kb", 262144); + +// The minimum number of frames we want to have buffered ahead of an +// animation's currently displayed frame. +pref("image.animated.decode-on-demand.batch-size", 6); + // The maximum size, in bytes, of the decoded images we cache pref("image.cache.size", 5242880); From 03a4a17ccffd0865dc948a767185ac2f6f23d836 Mon Sep 17 00:00:00 2001 From: Martok Date: Tue, 3 Jan 2023 22:55:13 +0100 Subject: [PATCH 04/11] Issue #2073 - m-c 1383404: make SourceBuffer::Compact more efficient (squashed) The first part also means that Compact no longer needs the SurfaceCache lock (used to be via CreateChunk->CanHold), which avoids potential deadlocks during shutdown that m-c 523950 would otherwise cause --- image/ImageFactory.cpp | 47 +++++++++++-------- image/ImageFactory.h | 3 +- image/SourceBuffer.cpp | 31 ++++++------ image/SourceBuffer.h | 33 ++++++++++--- image/imgTools.cpp | 19 ++++---- .../places/PageIconProtocolHandler.js | 1 + 6 files changed, 80 insertions(+), 54 deletions(-) diff --git a/image/ImageFactory.cpp b/image/ImageFactory.cpp index 428be1424e..343f7b582b 100644 --- a/image/ImageFactory.cpp +++ b/image/ImageFactory.cpp @@ -111,8 +111,32 @@ BadImage(const char* aMessage, RefPtr& aImage) return aImage.forget(); } +static void +SetSourceSizeHint(RasterImage* aImage, uint32_t aSize) +{ + // Pass anything usable on so that the RasterImage can preallocate + // its source buffer. + if (aSize == 0) { + return; + } + + // Bound by something reasonable + uint32_t sizeHint = std::min(aSize, 20000000); + nsresult rv = aImage->SetSourceSizeHint(sizeHint); + if (NS_FAILED(rv)) { + // Flush memory, try to get some back, and try again. + rv = nsMemory::HeapMinimize(true); + nsresult rv2 = aImage->SetSourceSizeHint(sizeHint); + // If we've still failed at this point, things are going downhill. + if (NS_FAILED(rv) || NS_FAILED(rv2)) { + NS_WARNING("About to hit OOM in imagelib!"); + } + } +} + /* static */ already_AddRefed -ImageFactory::CreateAnonymousImage(const nsCString& aMimeType) +ImageFactory::CreateAnonymousImage(const nsCString& aMimeType, + uint32_t aSizeHint /* = 0 */) { nsresult rv; @@ -127,6 +151,7 @@ ImageFactory::CreateAnonymousImage(const nsCString& aMimeType) return BadImage("RasterImage::Init failed", newImage); } + SetSourceSizeHint(newImage, aSizeHint); return newImage.forget(); } @@ -231,25 +256,7 @@ ImageFactory::CreateRasterImage(nsIRequest* aRequest, newImage->SetInnerWindowID(aInnerWindowId); - uint32_t len = GetContentSize(aRequest); - - // Pass anything usable on so that the RasterImage can preallocate - // its source buffer. - if (len > 0) { - // Bound by something reasonable - uint32_t sizeHint = std::min(len, 20000000); - rv = newImage->SetSourceSizeHint(sizeHint); - if (NS_FAILED(rv)) { - // Flush memory, try to get some back, and try again. - rv = nsMemory::HeapMinimize(true); - nsresult rv2 = newImage->SetSourceSizeHint(sizeHint); - // If we've still failed at this point, things are going downhill. - if (NS_FAILED(rv) || NS_FAILED(rv2)) { - NS_WARNING("About to hit OOM in imagelib!"); - } - } - } - + SetSourceSizeHint(newImage, GetContentSize(aRequest)); return newImage.forget(); } diff --git a/image/ImageFactory.h b/image/ImageFactory.h index 6c2e0f5045..fdd6460ec1 100644 --- a/image/ImageFactory.h +++ b/image/ImageFactory.h @@ -51,9 +51,10 @@ public: * the usual image loading mechanism. * * @param aMimeType The mimetype of the image. + * @param aSizeHint The length of the source data for the image. */ static already_AddRefed - CreateAnonymousImage(const nsCString& aMimeType); + CreateAnonymousImage(const nsCString& aMimeType, uint32_t aSizeHint = 0); /** * Creates a new multipart/x-mixed-replace image wrapper, and initializes it diff --git a/image/SourceBuffer.cpp b/image/SourceBuffer.cpp index de066e29fc..5961cc66ff 100644 --- a/image/SourceBuffer.cpp +++ b/image/SourceBuffer.cpp @@ -204,30 +204,27 @@ SourceBuffer::Compact() return NS_OK; } - Maybe newChunk = CreateChunk(length, /* aRoundUp = */ false); - if (MOZ_UNLIKELY(!newChunk || newChunk->AllocationFailed())) { - NS_WARNING("Failed to allocate chunk for SourceBuffer compacting - OOM?"); + Chunk& mergeChunk = mChunks[0]; + if (MOZ_UNLIKELY(!mergeChunk.SetCapacity(length))) { + NS_WARNING("Failed to reallocate chunk for SourceBuffer compacting - OOM?"); return NS_OK; } - // Copy our old chunks into the new chunk. - for (uint32_t i = 0 ; i < mChunks.Length() ; ++i) { - size_t offset = newChunk->Length(); - MOZ_ASSERT(offset < newChunk->Capacity()); - MOZ_ASSERT(offset + mChunks[i].Length() <= newChunk->Capacity()); + // Copy our old chunks into the newly reallocated first chunk. + for (uint32_t i = 1 ; i < mChunks.Length() ; ++i) { + size_t offset = mergeChunk.Length(); + MOZ_ASSERT(offset < mergeChunk.Capacity()); + MOZ_ASSERT(offset + mChunks[i].Length() <= mergeChunk.Capacity()); - memcpy(newChunk->Data() + offset, mChunks[i].Data(), mChunks[i].Length()); - newChunk->AddLength(mChunks[i].Length()); + memcpy(mergeChunk.Data() + offset, mChunks[i].Data(), mChunks[i].Length()); + mergeChunk.AddLength(mChunks[i].Length()); } - MOZ_ASSERT(newChunk->Length() == newChunk->Capacity(), + MOZ_ASSERT(mergeChunk.Length() == mergeChunk.Capacity(), "Compacted chunk has slack space"); - // Replace the old chunks with the new, compact chunk. - mChunks.Clear(); - if (MOZ_UNLIKELY(NS_FAILED(AppendChunk(Move(newChunk))))) { - return HandleError(NS_ERROR_OUT_OF_MEMORY); - } + // Remove the redundant chunks. + mChunks.RemoveElementsAt(1, mChunks.Length() - 1); mChunks.Compact(); return NS_OK; @@ -317,7 +314,7 @@ SourceBuffer::ExpectLength(size_t aExpectedLength) return NS_OK; } - if (MOZ_UNLIKELY(NS_FAILED(AppendChunk(CreateChunk(aExpectedLength))))) { + if (MOZ_UNLIKELY(NS_FAILED(AppendChunk(CreateChunk(aExpectedLength, /* aRoundUp */ false))))) { return HandleError(NS_ERROR_OUT_OF_MEMORY); } diff --git a/image/SourceBuffer.h b/image/SourceBuffer.h index e5aff1dcdd..6e3ef7a538 100644 --- a/image/SourceBuffer.h +++ b/image/SourceBuffer.h @@ -358,7 +358,7 @@ private: // Chunk type and chunk-related methods. ////////////////////////////////////////////////////////////////////////////// - class Chunk + class Chunk final { public: explicit Chunk(size_t aCapacity) @@ -366,13 +366,18 @@ private: , mLength(0) { MOZ_ASSERT(aCapacity > 0, "Creating zero-capacity chunk"); - mData.reset(new (fallible) char[mCapacity]); + mData = static_cast(malloc(mCapacity)); + } + + ~Chunk() + { + free(mData); } Chunk(Chunk&& aOther) : mCapacity(aOther.mCapacity) , mLength(aOther.mLength) - , mData(Move(aOther.mData)) + , mData(aOther.mData) { aOther.mCapacity = aOther.mLength = 0; aOther.mData = nullptr; @@ -380,9 +385,10 @@ private: Chunk& operator=(Chunk&& aOther) { + free(mData); mCapacity = aOther.mCapacity; mLength = aOther.mLength; - mData = Move(aOther.mData); + mData = aOther.mData; aOther.mCapacity = aOther.mLength = 0; aOther.mData = nullptr; return *this; @@ -395,7 +401,7 @@ private: char* Data() const { MOZ_ASSERT(mData, "Allocation failed but nobody checked for it"); - return mData.get(); + return mData; } void AddLength(size_t aAdditionalLength) @@ -404,13 +410,26 @@ private: mLength += aAdditionalLength; } + bool SetCapacity(size_t aCapacity) + { + MOZ_ASSERT(mData, "Allocation failed but nobody checked for it"); + char* data = static_cast(realloc(mData, aCapacity)); + if (!data) { + return false; + } + + mData = data; + mCapacity = aCapacity; + return true; + } + private: Chunk(const Chunk&) = delete; Chunk& operator=(const Chunk&) = delete; size_t mCapacity; size_t mLength; - UniquePtr mData; + char* mData; }; nsresult AppendChunk(Maybe&& aChunk); @@ -454,7 +473,7 @@ private: mutable Mutex mMutex; /// The data in this SourceBuffer, stored as a series of Chunks. - FallibleTArray mChunks; + AutoTArray mChunks; /// Consumers which are waiting to be notified when new data is available. nsTArray> mWaitingConsumers; diff --git a/image/imgTools.cpp b/image/imgTools.cpp index 29905c1ab3..3ac31102e5 100644 --- a/image/imgTools.cpp +++ b/image/imgTools.cpp @@ -67,15 +67,6 @@ imgTools::DecodeImage(nsIInputStream* aInStr, NS_ENSURE_ARG_POINTER(aInStr); - // Create a new image container to hold the decoded data. - nsAutoCString mimeType(aMimeType); - RefPtr image = ImageFactory::CreateAnonymousImage(mimeType); - RefPtr tracker = image->GetProgressTracker(); - - if (image->HasError()) { - return NS_ERROR_FAILURE; - } - // Prepare the input stream. nsCOMPtr inStream = aInStr; if (!NS_InputStreamIsBuffered(aInStr)) { @@ -92,6 +83,16 @@ imgTools::DecodeImage(nsIInputStream* aInStr, NS_ENSURE_SUCCESS(rv, rv); NS_ENSURE_TRUE(length <= UINT32_MAX, NS_ERROR_FILE_TOO_BIG); + // Create a new image container to hold the decoded data. + nsAutoCString mimeType(aMimeType); + RefPtr image = + ImageFactory::CreateAnonymousImage(mimeType, uint32_t(length)); + RefPtr tracker = image->GetProgressTracker(); + + if (image->HasError()) { + return NS_ERROR_FAILURE; + } + // Send the source data to the Image. rv = image->OnImageDataAvailable(nullptr, nullptr, inStream, 0, uint32_t(length)); diff --git a/toolkit/components/places/PageIconProtocolHandler.js b/toolkit/components/places/PageIconProtocolHandler.js index 05e43ccf3e..30bd8f1857 100644 --- a/toolkit/components/places/PageIconProtocolHandler.js +++ b/toolkit/components/places/PageIconProtocolHandler.js @@ -92,6 +92,7 @@ PageIconProtocolHandler.prototype = { try { channel.contentType = mime; + channel.contentLength = len; // Pass the icon data to the output stream. let stream = Cc["@mozilla.org/binaryoutputstream;1"] .createInstance(Ci.nsIBinaryOutputStream); From 9a39001cc30dc04d53f506c86e4c4c8a28ae281b Mon Sep 17 00:00:00 2001 From: Martok Date: Tue, 3 Jan 2023 23:30:46 +0100 Subject: [PATCH 05/11] Issue #2073 - m-c 1651587: Make image::Image release efficient on main thread --- image/AnimationSurfaceProvider.cpp | 10 ++---- image/DecodedSurfaceProvider.cpp | 3 +- image/Decoder.cpp | 2 +- image/RasterImage.cpp | 3 +- image/SurfaceCache.cpp | 58 +++++++++++++++++++++++++++++- image/SurfaceCache.h | 12 +++++++ 6 files changed, 75 insertions(+), 13 deletions(-) diff --git a/image/AnimationSurfaceProvider.cpp b/image/AnimationSurfaceProvider.cpp index 1d76332930..16c872ec5e 100644 --- a/image/AnimationSurfaceProvider.cpp +++ b/image/AnimationSurfaceProvider.cpp @@ -61,14 +61,8 @@ AnimationSurfaceProvider::DropImageReference() return; // Nothing to do. } - // RasterImage objects need to be destroyed on the main thread. We also need - // to destroy them asynchronously, because if our surface cache entry is - // destroyed and we were the only thing keeping |mImage| alive, RasterImage's - // destructor may call into the surface cache while whatever code caused us to - // get evicted is holding the surface cache lock, causing deadlock. - RefPtr image = mImage; - mImage = nullptr; - NS_ReleaseOnMainThread(image.forget(), /* aAlwaysProxy = */ true); + // RasterImage objects need to be destroyed on the main thread. + SurfaceCache::ReleaseImageOnMainThread(mImage.forget()); } void diff --git a/image/DecodedSurfaceProvider.cpp b/image/DecodedSurfaceProvider.cpp index 45b4849d29..d24380a375 100644 --- a/image/DecodedSurfaceProvider.cpp +++ b/image/DecodedSurfaceProvider.cpp @@ -49,7 +49,8 @@ DecodedSurfaceProvider::DropImageReference() // get evicted is holding the surface cache lock, causing deadlock. RefPtr image = mImage; mImage = nullptr; - NS_ReleaseOnMainThread(image.forget(), /* aAlwaysProxy = */ true); + SurfaceCache::ReleaseImageOnMainThread(image.forget(), + /* aAlwaysProxy = */ true); } DrawableFrameRef diff --git a/image/Decoder.cpp b/image/Decoder.cpp index 9d0647a4a3..8aaeb3127a 100644 --- a/image/Decoder.cpp +++ b/image/Decoder.cpp @@ -54,7 +54,7 @@ Decoder::~Decoder() if (mImage && !NS_IsMainThread()) { // Dispatch mImage to main thread to prevent it from being destructed by the // decode thread. - NS_ReleaseOnMainThread(mImage.forget()); + SurfaceCache::ReleaseImageOnMainThread(mImage.forget()); } } diff --git a/image/RasterImage.cpp b/image/RasterImage.cpp index 4c6cce891b..bb591920ef 100644 --- a/image/RasterImage.cpp +++ b/image/RasterImage.cpp @@ -434,8 +434,7 @@ RasterImage::OnSurfaceDiscarded(const SurfaceKey& aSurfaceKey) mAnimationState && aSurfaceKey.Playback() == PlaybackType::eAnimated; RefPtr image = this; - NS_DispatchToMainThread(NS_NewRunnableFunction( - [=]() -> void { + NS_DispatchToMainThread(NS_NewRunnableFunction([=]() -> void { image->OnSurfaceDiscardedInternal(animatedFramesDiscarded); })); } diff --git a/image/SurfaceCache.cpp b/image/SurfaceCache.cpp index f3068fd4c1..856ba162dc 100644 --- a/image/SurfaceCache.cpp +++ b/image/SurfaceCache.cpp @@ -835,6 +835,32 @@ public: } } + void ReleaseImageOnMainThread(already_AddRefed&& aImage, + const StaticMutexAutoLock& aAutoLock) { + RefPtr image = aImage; + if (!image) { + return; + } + + bool needsDispatch = mReleasingImagesOnMainThread.IsEmpty(); + mReleasingImagesOnMainThread.AppendElement(image); + + if (!needsDispatch) { + // There is already a ongoing task for ClearReleasingImages(). + return; + } + + NS_DispatchToMainThread(NS_NewRunnableFunction([]() -> void { + SurfaceCache::ClearReleasingImages(); + })); + } + + void TakeReleasingImages(nsTArray>& aImage, + const StaticMutexAutoLock& aAutoLock) { + MOZ_ASSERT(NS_IsMainThread()); + aImage.SwapElements(mReleasingImagesOnMainThread); + } + private: already_AddRefed GetImageCache(const ImageKey aImageKey) { @@ -942,7 +968,8 @@ private: nsRefPtrHashtable, ImageSurfaceCache> mImageCaches; SurfaceTracker mExpirationTracker; - RefPtr mMemoryPressureObserver; + RefPtr mMemoryPressureObserver; + nsTArray> mReleasingImagesOnMainThread; const uint32_t mDiscardFactor; const Cost mMaxCost; Cost mAvailableCost; @@ -1160,5 +1187,34 @@ SurfaceCache::MaximumCapacity() return sInstance->MaximumCapacity(); } +/* static */ +void SurfaceCache::ReleaseImageOnMainThread( + already_AddRefed aImage, bool aAlwaysProxy) { + if (NS_IsMainThread() && !aAlwaysProxy) { + RefPtr image = std::move(aImage); + return; + } + + StaticMutexAutoLock lock(sInstanceMutex); + if (sInstance) { + sInstance->ReleaseImageOnMainThread(std::move(aImage), lock); + } else { + NS_ReleaseOnMainThread(std::move(aImage), /* aAlwaysProxy */ true); + } +} + +/* static */ +void SurfaceCache::ClearReleasingImages() { + MOZ_ASSERT(NS_IsMainThread()); + + nsTArray> images; + { + StaticMutexAutoLock lock(sInstanceMutex); + if (sInstance) { + sInstance->TakeReleasingImages(images, lock); + } + } +} + } // namespace image } // namespace mozilla diff --git a/image/SurfaceCache.h b/image/SurfaceCache.h index e0c22c999c..518b589d34 100644 --- a/image/SurfaceCache.h +++ b/image/SurfaceCache.h @@ -417,6 +417,18 @@ struct SurfaceCache */ static size_t MaximumCapacity(); + /** + * Release image on main thread. + * The function uses SurfaceCache to release pending releasing images quickly. + */ + static void ReleaseImageOnMainThread(already_AddRefed aImage, + bool aAlwaysProxy = false); + + /** + * Clear all pending releasing images. + */ + static void ClearReleasingImages(); + private: virtual ~SurfaceCache() = 0; // Forbid instantiation. }; From a6a420259c0a0af3ab90c07c64ccfcc8979df80c Mon Sep 17 00:00:00 2001 From: Martok Date: Sun, 1 Jan 2023 13:46:00 +0100 Subject: [PATCH 06/11] Issue #2073 - m-c 1546500: Avoid dispatching synchronous thread shutdown runnables --- dom/cache/Manager.cpp | 2 +- dom/indexedDB/ActorsParent.cpp | 2 +- image/DecodePool.cpp | 2 +- netwerk/sctp/datachannel/DataChannel.cpp | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/dom/cache/Manager.cpp b/dom/cache/Manager.cpp index f294beb544..d2e63a6213 100644 --- a/dom/cache/Manager.cpp +++ b/dom/cache/Manager.cpp @@ -1770,7 +1770,7 @@ Manager::~Manager() // Don't spin the event loop in the destructor waiting for the thread to // shutdown. Defer this to the main thread, instead. - MOZ_ALWAYS_SUCCEEDS(NS_DispatchToMainThread(NewRunnableMethod(ioThread, &nsIThread::Shutdown))); + MOZ_ALWAYS_SUCCEEDS(NS_DispatchToMainThread(NewRunnableMethod(ioThread, &nsIThread::AsyncShutdown))); } void diff --git a/dom/indexedDB/ActorsParent.cpp b/dom/indexedDB/ActorsParent.cpp index 52f221d78a..0e6944e8ef 100644 --- a/dom/indexedDB/ActorsParent.cpp +++ b/dom/indexedDB/ActorsParent.cpp @@ -12487,7 +12487,7 @@ ConnectionPool::ShutdownThread(ThreadInfo& aThreadInfo) NS_DISPATCH_NORMAL)); MOZ_ALWAYS_SUCCEEDS(NS_DispatchToMainThread( - NewRunnableMethod(thread, &nsIThread::Shutdown))); + NewRunnableMethod(thread, &nsIThread::AsyncShutdown))); mTotalThreadCount--; } diff --git a/image/DecodePool.cpp b/image/DecodePool.cpp index 0aeed61547..f9d72df168 100644 --- a/image/DecodePool.cpp +++ b/image/DecodePool.cpp @@ -72,7 +72,7 @@ public: { // Threads have to be shut down from another thread, so we'll ask the // main thread to do it for us. - NS_DispatchToMainThread(NewRunnableMethod(aThisThread, &nsIThread::Shutdown)); + NS_DispatchToMainThread(NewRunnableMethod(aThisThread, &nsIThread::AsyncShutdown)); } /** diff --git a/netwerk/sctp/datachannel/DataChannel.cpp b/netwerk/sctp/datachannel/DataChannel.cpp index dfc993b108..99e0502dc9 100644 --- a/netwerk/sctp/datachannel/DataChannel.cpp +++ b/netwerk/sctp/datachannel/DataChannel.cpp @@ -238,7 +238,7 @@ DataChannelConnection::~DataChannelConnection() // Avoid spinning the event thread from here (which if we're mainthread // is in the event loop already) NS_DispatchToMainThread(WrapRunnable(nsCOMPtr(mInternalIOThread), - &nsIThread::Shutdown), + &nsIThread::AsyncShutdown), NS_DISPATCH_NORMAL); } } else { From 845411a7adc723e87910b15934eb3ab603495918 Mon Sep 17 00:00:00 2001 From: Martok Date: Sun, 1 Jan 2023 01:18:27 +0100 Subject: [PATCH 07/11] Issue #2073 - m-c 1454149: Do not advance animated images which are not displayed --- gfx/thebes/gfxPrefs.h | 1 + image/FrameAnimator.cpp | 33 +++++++++++++++++++++++++++++++++ image/FrameAnimator.h | 12 ++++++++++++ modules/libpref/init/all.js | 4 ++++ 4 files changed, 50 insertions(+) diff --git a/gfx/thebes/gfxPrefs.h b/gfx/thebes/gfxPrefs.h index 81c66603c1..c2b28eee64 100644 --- a/gfx/thebes/gfxPrefs.h +++ b/gfx/thebes/gfxPrefs.h @@ -431,6 +431,7 @@ private: DECL_GFX_PREF(Live, "image.animated.decode-on-demand.threshold-kb", ImageAnimatedDecodeOnDemandThresholdKB, uint32_t, 256*1024); DECL_GFX_PREF(Live, "image.animated.decode-on-demand.batch-size", ImageAnimatedDecodeOnDemandBatchSize, uint32_t, 6); + DECL_GFX_PREF(Live, "image.animated.resume-from-last-displayed", ImageAnimatedResumeFromLastDisplayed, bool, false); DECL_GFX_PREF(Once, "image.cache.size", ImageCacheSize, int32_t, 5*1024*1024); DECL_GFX_PREF(Once, "image.cache.timeweight", ImageCacheTimeWeight, int32_t, 500); DECL_GFX_PREF(Live, "image.decode-immediately.enabled", ImageDecodeImmediatelyEnabled, bool, false); diff --git a/image/FrameAnimator.cpp b/image/FrameAnimator.cpp index b9a4aec4a8..0bdecbb346 100644 --- a/image/FrameAnimator.cpp +++ b/image/FrameAnimator.cpp @@ -151,6 +151,21 @@ AnimationState::SetAnimationFrameTime(const TimeStamp& aTime) mCurrentAnimationFrameTime = aTime; } +bool +AnimationState::MaybeAdvanceAnimationFrameTime(const TimeStamp& aTime) +{ + if (!gfxPrefs::ImageAnimatedResumeFromLastDisplayed() || + mCurrentAnimationFrameTime >= aTime) { + return false; + } + + // We are configured to stop an animation when it is out of view, and restart + // it from the same point when it comes back into view. The same applies if it + // was discarded while out of view. + mCurrentAnimationFrameTime = aTime; + return true; +} + uint32_t AnimationState::GetCurrentAnimationFrameIndex() const { @@ -312,6 +327,7 @@ FrameAnimator::AdvanceFrame(AnimationState& aState, MOZ_ASSERT(currentFrameEndTime.isSome()); aState.mCurrentAnimationFrameTime = *currentFrameEndTime; aState.mCurrentAnimationFrameIndex = nextFrameIndex; + aState.mCompositedFrameRequested = false; aFrames.Advance(nextFrameIndex); return ret; @@ -343,6 +359,7 @@ FrameAnimator::AdvanceFrame(AnimationState& aState, // Set currentAnimationFrameIndex at the last possible moment aState.mCurrentAnimationFrameIndex = nextFrameIndex; + aState.mCompositedFrameRequested = false; aFrames.Advance(nextFrameIndex); // If we're here, we successfully advanced the frame. @@ -379,6 +396,7 @@ FrameAnimator::RequestRefresh(AnimationState& aState, RefreshResult ret; if (aState.IsDiscarded()) { + aState.MaybeAdvanceAnimationFrameTime(aTime); return ret; } @@ -394,6 +412,10 @@ FrameAnimator::RequestRefresh(AnimationState& aState, aState.UpdateStateInternal(result, aAnimationFinished); if (aState.IsDiscarded() || !result) { + aState.MaybeAdvanceAnimationFrameTime(aTime); + if (!ret.mDirtyRect.IsEmpty()) { + ret.mFrameAdvanced = true; + } return ret; } @@ -407,6 +429,15 @@ FrameAnimator::RequestRefresh(AnimationState& aState, MOZ_ASSERT(aState.mCompositedFrameInvalid); // Nothing we can do but wait for our previous current frame to be decoded // again so we can determine what to do next. + aState.MaybeAdvanceAnimationFrameTime(aTime); + return ret; + } + + // If nothing has accessed the composited frame since the last time we + // advanced, then there is no point in continuing to advance the animation. + // This has the effect of freezing the animation while not in view. + if (!aState.mCompositedFrameRequested && + aState.MaybeAdvanceAnimationFrameTime(aTime)) { return ret; } @@ -443,6 +474,8 @@ FrameAnimator::RequestRefresh(AnimationState& aState, LookupResult FrameAnimator::GetCompositedFrame(AnimationState& aState) { + aState.mCompositedFrameRequested = true; + if (aState.mCompositedFrameInvalid) { MOZ_ASSERT(gfxPrefs::ImageMemAnimatedDiscardable()); MOZ_ASSERT(aState.GetHasBeenDecoded()); diff --git a/image/FrameAnimator.h b/image/FrameAnimator.h index fd331da9eb..e245f3797a 100644 --- a/image/FrameAnimator.h +++ b/image/FrameAnimator.h @@ -36,6 +36,7 @@ public: , mHasBeenDecoded(false) , mIsCurrentlyDecoded(false) , mCompositedFrameInvalid(false) + , mCompositedFrameRequested(false) , mDiscarded(false) { } @@ -131,6 +132,13 @@ public: */ void SetAnimationFrameTime(const TimeStamp& aTime); + /** + * Set the animation frame time to @aTime if we are configured to stop the + * animation when not visible and aTime is later than the current time. + * Returns true if the time was updated, else false. + */ + bool MaybeAdvanceAnimationFrameTime(const TimeStamp& aTime); + /** * The current frame we're on, from 0 to (numFrames - 1). */ @@ -225,6 +233,10 @@ private: //! valid to draw to the screen. bool mCompositedFrameInvalid; + //! Whether the composited frame was requested from the animator since the + //! last time we advanced the animation. + bool mCompositedFrameRequested; + //! Whether this image is currently discarded. Only set to true after the //! image has been decoded at least once. bool mDiscarded; diff --git a/modules/libpref/init/all.js b/modules/libpref/init/all.js index e9d26dc6c0..a57f1000e7 100644 --- a/modules/libpref/init/all.js +++ b/modules/libpref/init/all.js @@ -4196,6 +4196,10 @@ pref("image.animated.decode-on-demand.threshold-kb", 262144); // animation's currently displayed frame. pref("image.animated.decode-on-demand.batch-size", 6); +// Resume an animated image from the last displayed frame rather than +// advancing when out of view. +pref("image.animated.resume-from-last-displayed", true); + // The maximum size, in bytes, of the decoded images we cache pref("image.cache.size", 5242880); From db3ce13f2897cc76bb083f93ebb60d07e567a7d7 Mon Sep 17 00:00:00 2001 From: FranklinDM Date: Fri, 6 Jan 2023 21:07:37 +0800 Subject: [PATCH 08/11] Issue #2084 - Part 1: Remove CSSUnprefixingService.js and associated code It's effectively dead code since it's been supplanted by built-in webkit-prefixed-CSS support (landed before fork point in Firefox 49). Based on https://bugzilla.mozilla.org/show_bug.cgi?id=1259348 --- caps/BasePrincipal.h | 2 - caps/nsIPrincipal.idl | 11 +- caps/nsPrincipal.cpp | 204 --------- caps/nsPrincipal.h | 3 - layout/style/CSSUnprefixingService.js | 341 --------------- layout/style/CSSUnprefixingService.manifest | 2 - layout/style/moz.build | 11 - layout/style/nsCSSParser.cpp | 244 +---------- layout/style/nsICSSUnprefixingService.idl | 76 ---- layout/style/test/mochitest.ini | 4 - .../style/test/test_unprefixing_service.html | 93 ----- .../test/test_unprefixing_service_prefs.html | 132 ------ .../test/unprefixing_service_iframe.html | 394 ------------------ .../style/test/unprefixing_service_utils.js | 87 ---- modules/libpref/init/all.js | 10 - 15 files changed, 24 insertions(+), 1590 deletions(-) delete mode 100644 layout/style/CSSUnprefixingService.js delete mode 100644 layout/style/CSSUnprefixingService.manifest delete mode 100644 layout/style/nsICSSUnprefixingService.idl delete mode 100644 layout/style/test/test_unprefixing_service.html delete mode 100644 layout/style/test/test_unprefixing_service_prefs.html delete mode 100644 layout/style/test/unprefixing_service_iframe.html delete mode 100644 layout/style/test/unprefixing_service_utils.js diff --git a/caps/BasePrincipal.h b/caps/BasePrincipal.h index f6a179fa8f..b1f151521f 100644 --- a/caps/BasePrincipal.h +++ b/caps/BasePrincipal.h @@ -287,8 +287,6 @@ public: virtual bool AddonHasPermission(const nsAString& aPerm); - virtual bool IsOnCSSUnprefixingWhitelist() override { return false; } - virtual bool IsCodebasePrincipal() const { return false; }; static BasePrincipal* Cast(nsIPrincipal* aPrin) { return static_cast(aPrin); } diff --git a/caps/nsIPrincipal.idl b/caps/nsIPrincipal.idl index d278decdb4..1d58dcb7fa 100644 --- a/caps/nsIPrincipal.idl +++ b/caps/nsIPrincipal.idl @@ -21,7 +21,7 @@ interface nsIDOMDocument; [ptr] native JSPrincipals(JSPrincipals); [ptr] native PrincipalArray(nsTArray >); -[scriptable, builtinclass, uuid(3da7b133-f1a0-4de9-a2bc-5c49014c1077)] +[scriptable, builtinclass, uuid(f75f502d-79fd-48be-a079-e5a7b8f80c8b)] interface nsIPrincipal : nsISerializable { /** @@ -333,15 +333,6 @@ interface nsIPrincipal : nsISerializable * Returns true iff this is the system principal. */ [infallible] readonly attribute boolean isSystemPrincipal; - - /** - * Returns true if this principal's origin is recognized as being on the - * whitelist of sites that can use the CSS Unprefixing Service. - * - * (This interface provides a trivial implementation, just returning false; - * subclasses can implement something more complex as-needed.) - */ - [noscript,notxpcom,nostdcall] bool IsOnCSSUnprefixingWhitelist(); }; /** diff --git a/caps/nsPrincipal.cpp b/caps/nsPrincipal.cpp index 05d00c80a3..c9e66fef7a 100644 --- a/caps/nsPrincipal.cpp +++ b/caps/nsPrincipal.cpp @@ -35,7 +35,6 @@ using namespace mozilla; -static bool gIsWhitelistingTestDomains = false; static bool gCodeBasePrincipalSupport = false; static bool URIIsImmutable(nsIURI* aURI) @@ -61,10 +60,6 @@ NS_IMPL_CI_INTERFACE_GETTER(nsPrincipal, /* static */ void nsPrincipal::InitializeStatics() { - Preferences::AddBoolVarCache( - &gIsWhitelistingTestDomains, - "layout.css.unprefixing-service.include-test-domains"); - Preferences::AddBoolVarCache(&gCodeBasePrincipalSupport, "signed.applets.codebase_principal_support", false); @@ -483,196 +478,6 @@ nsPrincipal::Write(nsIObjectOutputStream* aStream) return NS_OK; } -// Helper-function to indicate whether the CSS Unprefixing Service -// whitelist should include dummy domains that are only intended for -// use in testing. (Controlled by a pref.) -static inline bool -IsWhitelistingTestDomains() -{ - return gIsWhitelistingTestDomains; -} - -// Checks if the given URI's host is on our "full domain" whitelist -// (i.e. if it's an exact match against a domain that needs unprefixing) -static bool -IsOnFullDomainWhitelist(nsIURI* aURI) -{ - nsAutoCString hostStr; - nsresult rv = aURI->GetHost(hostStr); - NS_ENSURE_SUCCESS(rv, false); - - // NOTE: This static whitelist is expected to be short. If that changes, - // we should consider a different representation; e.g. hash-set, prefix tree. - static const nsLiteralCString sFullDomainsOnWhitelist[] = { - // 0th entry only active when testing: - NS_LITERAL_CSTRING("test1.example.org"), - NS_LITERAL_CSTRING("map.baidu.com"), - NS_LITERAL_CSTRING("3g.163.com"), - NS_LITERAL_CSTRING("3glogo.gtimg.com"), // for 3g.163.com - NS_LITERAL_CSTRING("info.3g.qq.com"), // for 3g.qq.com - NS_LITERAL_CSTRING("3gimg.qq.com"), // for 3g.qq.com - NS_LITERAL_CSTRING("img.m.baidu.com"), // for [shucheng|ks].baidu.com - NS_LITERAL_CSTRING("m.mogujie.com"), - NS_LITERAL_CSTRING("touch.qunar.com"), - NS_LITERAL_CSTRING("mjs.sinaimg.cn"), // for sina.cn - NS_LITERAL_CSTRING("static.qiyi.com"), // for m.iqiyi.com - NS_LITERAL_CSTRING("cdn.kuaidi100.com"), // for m.kuaidi100.com - NS_LITERAL_CSTRING("m.pc6.com"), - NS_LITERAL_CSTRING("m.haosou.com"), - NS_LITERAL_CSTRING("m.mi.com"), - NS_LITERAL_CSTRING("wappass.baidu.com"), - NS_LITERAL_CSTRING("m.video.baidu.com"), - NS_LITERAL_CSTRING("m.video.baidu.com"), - NS_LITERAL_CSTRING("imgcache.gtimg.cn"), // for m.v.qq.com - NS_LITERAL_CSTRING("s.tabelog.jp"), - NS_LITERAL_CSTRING("s.yimg.jp"), // for s.tabelog.jp - NS_LITERAL_CSTRING("i.yimg.jp"), // for *.yahoo.co.jp - NS_LITERAL_CSTRING("ai.yimg.jp"), // for *.yahoo.co.jp - NS_LITERAL_CSTRING("m.finance.yahoo.co.jp"), - NS_LITERAL_CSTRING("daily.c.yimg.jp"), // for sp.daily.co.jp - NS_LITERAL_CSTRING("stat100.ameba.jp"), // for ameblo.jp - NS_LITERAL_CSTRING("user.ameba.jp"), // for ameblo.jp - NS_LITERAL_CSTRING("www.goo.ne.jp"), - NS_LITERAL_CSTRING("x.gnst.jp"), // for mobile.gnavi.co.jp - NS_LITERAL_CSTRING("c.x.gnst.jp"), // for mobile.gnavi.co.jp - NS_LITERAL_CSTRING("www.smbc-card.com"), - NS_LITERAL_CSTRING("static.card.jp.rakuten-static.com"), // for rakuten-card.co.jp - NS_LITERAL_CSTRING("img.travel.rakuten.co.jp"), // for travel.rakuten.co.jp - NS_LITERAL_CSTRING("img.mixi.net"), // for mixi.jp - NS_LITERAL_CSTRING("girlschannel.net"), - NS_LITERAL_CSTRING("www.fancl.co.jp"), - NS_LITERAL_CSTRING("s.cosme.net"), - NS_LITERAL_CSTRING("www.sapporobeer.jp"), - NS_LITERAL_CSTRING("www.mapion.co.jp"), - NS_LITERAL_CSTRING("touch.navitime.co.jp"), - NS_LITERAL_CSTRING("sp.mbga.jp"), - NS_LITERAL_CSTRING("ava-a.sp.mbga.jp"), // for sp.mbga.jp - NS_LITERAL_CSTRING("www.ntv.co.jp"), - NS_LITERAL_CSTRING("mobile.suntory.co.jp"), // for suntory.jp - NS_LITERAL_CSTRING("www.aeonsquare.net"), - NS_LITERAL_CSTRING("mw.nikkei.com"), - NS_LITERAL_CSTRING("www.nhk.or.jp"), - NS_LITERAL_CSTRING("www.tokyo-sports.co.jp"), - NS_LITERAL_CSTRING("www.bellemaison.jp"), - NS_LITERAL_CSTRING("www.kuronekoyamato.co.jp"), - NS_LITERAL_CSTRING("formassist.jp"), // for orico.jp - NS_LITERAL_CSTRING("sp.m.reuters.co.jp"), - NS_LITERAL_CSTRING("www.atre.co.jp"), - NS_LITERAL_CSTRING("www.jtb.co.jp"), - NS_LITERAL_CSTRING("www.sharp.co.jp"), - NS_LITERAL_CSTRING("www.biccamera.com"), - NS_LITERAL_CSTRING("weathernews.jp"), - NS_LITERAL_CSTRING("cache.ymail.jp"), // for www.yamada-denkiweb.com - }; - static const size_t sNumFullDomainsOnWhitelist = - MOZ_ARRAY_LENGTH(sFullDomainsOnWhitelist); - - // Skip 0th (dummy) entry in whitelist, unless a pref is enabled. - const size_t firstWhitelistIdx = IsWhitelistingTestDomains() ? 0 : 1; - - for (size_t i = firstWhitelistIdx; i < sNumFullDomainsOnWhitelist; ++i) { - if (hostStr == sFullDomainsOnWhitelist[i]) { - return true; - } - } - return false; -} - -// Checks if the given URI's host is on our "base domain" whitelist -// (i.e. if it's a subdomain of some host that we've whitelisted as needing -// unprefixing for all its subdomains) -static bool -IsOnBaseDomainWhitelist(nsIURI* aURI) -{ - static const nsLiteralCString sBaseDomainsOnWhitelist[] = { - // 0th entry only active when testing: - NS_LITERAL_CSTRING("test2.example.org"), - NS_LITERAL_CSTRING("tbcdn.cn"), // for m.taobao.com - NS_LITERAL_CSTRING("alicdn.com"), // for m.taobao.com - NS_LITERAL_CSTRING("dpfile.com"), // for m.dianping.com - NS_LITERAL_CSTRING("hao123img.com"), // for hao123.com - NS_LITERAL_CSTRING("tabelog.k-img.com"), // for s.tabelog.com - NS_LITERAL_CSTRING("tsite.jp"), // for *.tsite.jp - }; - static const size_t sNumBaseDomainsOnWhitelist = - MOZ_ARRAY_LENGTH(sBaseDomainsOnWhitelist); - - nsCOMPtr tldService = - do_GetService(NS_EFFECTIVETLDSERVICE_CONTRACTID); - - if (tldService) { - // Skip 0th test-entry in whitelist, unless the testing pref is enabled. - const size_t firstWhitelistIdx = IsWhitelistingTestDomains() ? 0 : 1; - - // Right now, the test base-domain "test2.example.org" is the only entry in - // its whitelist with a nonzero "depth". So we'll only bother going beyond - // 0 depth (to 1) if that entry is enabled. (No point in slowing down the - // normal codepath, for the benefit of a disabled test domain.) If we add a - // "real" base-domain with a depth of >= 1 to our whitelist, we can get rid - // of this conditional & just make this a static variable. - const uint32_t maxSubdomainDepth = IsWhitelistingTestDomains() ? 1 : 0; - - for (uint32_t subdomainDepth = 0; - subdomainDepth <= maxSubdomainDepth; ++subdomainDepth) { - - // Get the base domain (to depth |subdomainDepth|) from passed-in URI: - nsAutoCString baseDomainStr; - nsresult rv = tldService->GetBaseDomain(aURI, subdomainDepth, - baseDomainStr); - if (NS_FAILED(rv)) { - // aURI doesn't have |subdomainDepth| levels of subdomains. If we got - // here without a match yet, then aURI is not on our whitelist. - return false; - } - - // Compare the base domain against each entry in our whitelist: - for (size_t i = firstWhitelistIdx; i < sNumBaseDomainsOnWhitelist; ++i) { - if (baseDomainStr == sBaseDomainsOnWhitelist[i]) { - return true; - } - } - } - } - - return false; -} - -// The actual (non-cached) implementation of IsOnCSSUnprefixingWhitelist(): -static bool -IsOnCSSUnprefixingWhitelistImpl(nsIURI* aURI) -{ - // Check scheme, so we can drop any non-HTTP/HTTPS URIs right away - nsAutoCString schemeStr; - nsresult rv = aURI->GetScheme(schemeStr); - NS_ENSURE_SUCCESS(rv, false); - - // Only proceed if scheme is "http" or "https" - if (!(StringBeginsWith(schemeStr, NS_LITERAL_CSTRING("http")) && - (schemeStr.Length() == 4 || - (schemeStr.Length() == 5 && schemeStr[4] == 's')))) { - return false; - } - - return (IsOnFullDomainWhitelist(aURI) || - IsOnBaseDomainWhitelist(aURI)); -} - - -bool -nsPrincipal::IsOnCSSUnprefixingWhitelist() -{ - if (mIsOnCSSUnprefixingWhitelist.isNothing()) { - // Value not cached -- perform our lazy whitelist-check. - // (NOTE: If our URI is mutable, we just assume it's not on the whitelist, - // since our caching strategy won't work. This isn't expected to be common.) - mIsOnCSSUnprefixingWhitelist.emplace( - mCodebaseImmutable && - IsOnCSSUnprefixingWhitelistImpl(mCodebase)); - } - - return *mIsOnCSSUnprefixingWhitelist; -} - /************************************************************************************************************************/ NS_IMPL_CLASSINFO(nsExpandedPrincipal, nullptr, nsIClassInfo::MAIN_THREAD_ONLY, @@ -837,15 +642,6 @@ nsExpandedPrincipal::AddonHasPermission(const nsAString& aPerm) return false; } -bool -nsExpandedPrincipal::IsOnCSSUnprefixingWhitelist() -{ - // CSS Unprefixing Whitelist is a per-origin thing; doesn't really make sense - // for an expanded principal. (And probably shouldn't be needed.) - return false; -} - - nsresult nsExpandedPrincipal::GetScriptLocation(nsACString& aStr) { diff --git a/caps/nsPrincipal.h b/caps/nsPrincipal.h index d20d81ee3b..c122952c70 100644 --- a/caps/nsPrincipal.h +++ b/caps/nsPrincipal.h @@ -25,7 +25,6 @@ public: NS_IMETHOD GetDomain(nsIURI** aDomain) override; NS_IMETHOD SetDomain(nsIURI* aDomain) override; NS_IMETHOD GetBaseDomain(nsACString& aBaseDomain) override; - virtual bool IsOnCSSUnprefixingWhitelist() override; bool IsCodebasePrincipal() const override { return true; } nsresult GetOriginInternal(nsACString& aOrigin) override; @@ -55,7 +54,6 @@ public: bool mCodebaseImmutable; bool mDomainImmutable; bool mInitialized; - mozilla::Maybe mIsOnCSSUnprefixingWhitelist; // Lazily-computed protected: virtual ~nsPrincipal(); @@ -81,7 +79,6 @@ public: NS_IMETHOD SetDomain(nsIURI* aDomain) override; NS_IMETHOD GetBaseDomain(nsACString& aBaseDomain) override; virtual bool AddonHasPermission(const nsAString& aPerm) override; - virtual bool IsOnCSSUnprefixingWhitelist() override; virtual nsresult GetScriptLocation(nsACString &aStr) override; nsresult GetOriginInternal(nsACString& aOrigin) override; diff --git a/layout/style/CSSUnprefixingService.js b/layout/style/CSSUnprefixingService.js deleted file mode 100644 index f6c63a0237..0000000000 --- a/layout/style/CSSUnprefixingService.js +++ /dev/null @@ -1,341 +0,0 @@ -/* -*- indent-tabs-mode: nil; js-indent-level: 2 -*- / -/* This Source Code Form is subject to the terms of the Mozilla Public - * License, v. 2.0. If a copy of the MPL was not distributed with this - * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ - -/* Implementation of a service that converts certain vendor-prefixed CSS - properties to their unprefixed equivalents, for sites on a whitelist. */ - -"use strict"; - -const Cc = Components.classes; -const Ci = Components.interfaces; -const Cu = Components.utils; - -Cu.import("resource://gre/modules/XPCOMUtils.jsm"); - -function CSSUnprefixingService() { -} - -CSSUnprefixingService.prototype = { - // Boilerplate: - classID: Components.ID("{f0729490-e15c-4a2f-a3fb-99e1cc946b42}"), - _xpcom_factory: XPCOMUtils.generateSingletonFactory(CSSUnprefixingService), - QueryInterface: XPCOMUtils.generateQI([Ci.nsICSSUnprefixingService]), - - // See documentation in nsICSSUnprefixingService.idl - generateUnprefixedDeclaration: function(aPropName, aRightHalfOfDecl, - aUnprefixedDecl /*out*/) { - - // Convert our input strings to lower-case, for easier string-matching. - // (NOTE: If we ever need to add support for unprefixing properties that - // have case-sensitive parts, then we should do these toLowerCase() - // conversions in a more targeted way, to avoid breaking those properties.) - aPropName = aPropName.toLowerCase(); - aRightHalfOfDecl = aRightHalfOfDecl.toLowerCase(); - - // We have several groups of supported properties: - // FIRST GROUP: Properties that can just be handled as aliases: - // ============================================================ - const propertiesThatAreJustAliases = { - "-webkit-background-size": "background-size", - "-webkit-box-flex": "flex-grow", - "-webkit-box-ordinal-group": "order", - "-webkit-box-sizing": "box-sizing", - "-webkit-transform": "transform", - "-webkit-transform-origin": "transform-origin", - }; - - let unprefixedPropName = propertiesThatAreJustAliases[aPropName]; - if (unprefixedPropName !== undefined) { - aUnprefixedDecl.value = unprefixedPropName + ":" + aRightHalfOfDecl; - return true; - } - - // SECOND GROUP: Properties that take a single keyword, where the - // unprefixed version takes a different (but analogous) set of keywords: - // ===================================================================== - const propertiesThatNeedKeywordMapping = { - "-webkit-box-align" : { - unprefixedPropName : "align-items", - valueMap : { - "start" : "flex-start", - "center" : "center", - "end" : "flex-end", - "baseline" : "baseline", - "stretch" : "stretch" - } - }, - "-webkit-box-orient" : { - unprefixedPropName : "flex-direction", - valueMap : { - "horizontal" : "row", - "inline-axis" : "row", - "vertical" : "column", - "block-axis" : "column" - } - }, - "-webkit-box-pack" : { - unprefixedPropName : "justify-content", - valueMap : { - "start" : "flex-start", - "center" : "center", - "end" : "flex-end", - "justify" : "space-between" - } - }, - }; - - let propInfo = propertiesThatNeedKeywordMapping[aPropName]; - if (typeof(propInfo) != "undefined") { - // Regexp for parsing the right half of a declaration, for keyword-valued - // properties. Divides the right half of the declaration into: - // 1) any leading whitespace - // 2) the property value (one or more alphabetical character or hyphen) - // 3) anything after that (e.g. "!important", ";") - // Then we can look up the appropriate unprefixed-property value for the - // value (part 2), and splice that together with the other parts and with - // the unprefixed property-name to make the final declaration. - const keywordValuedPropertyRegexp = /^(\s*)([a-z\-]+)(.*)/; - let parts = keywordValuedPropertyRegexp.exec(aRightHalfOfDecl); - if (!parts) { - // Failed to parse a keyword out of aRightHalfOfDecl. (It probably has - // no alphabetical characters.) - return false; - } - - let mappedKeyword = propInfo.valueMap[parts[2]]; - if (mappedKeyword === undefined) { - // We found a keyword in aRightHalfOfDecl, but we don't have a mapping - // to an equivalent keyword for the unprefixed version of the property. - return false; - } - - aUnprefixedDecl.value = propInfo.unprefixedPropName + ":" + - parts[1] + // any leading whitespace - mappedKeyword + - parts[3]; // any trailing text (e.g. !important, semicolon, etc) - - return true; - } - - // THIRD GROUP: Properties that may need arbitrary string-replacement: - // =================================================================== - const propertiesThatNeedStringReplacement = { - // "-webkit-transition" takes a multi-part value. If "-webkit-transform" - // appears as part of that value, replace it w/ "transform". - // And regardless, we unprefix the "-webkit-transition" property-name. - // (We could handle other prefixed properties in addition to 'transform' - // here, but in practice "-webkit-transform" is the main one that's - // likely to be transitioned & that we're concerned about supporting.) - "-webkit-transition": { - unprefixedPropName : "transition", - stringMap : { - "-webkit-transform" : "transform", - } - }, - }; - - propInfo = propertiesThatNeedStringReplacement[aPropName]; - if (typeof(propInfo) != "undefined") { - let newRightHalf = aRightHalfOfDecl; - for (let strToReplace in propInfo.stringMap) { - let replacement = propInfo.stringMap[strToReplace]; - newRightHalf = newRightHalf.split(strToReplace).join(replacement); - } - aUnprefixedDecl.value = propInfo.unprefixedPropName + ":" + newRightHalf; - - return true; - } - - // No known mapping for property aPropName. - return false; - }, - - // See documentation in nsICSSUnprefixingService.idl - generateUnprefixedGradientValue: function(aPrefixedFuncName, - aPrefixedFuncBody, - aUnprefixedFuncName, /*[out]*/ - aUnprefixedFuncBody /*[out]*/) { - var unprefixedFuncName, newValue; - if (aPrefixedFuncName == "-webkit-gradient") { - // Create expression for oldGradientParser: - var parts = this.oldGradientParser(aPrefixedFuncBody); - var type = parts[0].name; - newValue = this.standardizeOldGradientArgs(type, parts.slice(1)); - unprefixedFuncName = type + "-gradient"; - }else{ // we're dealing with more modern syntax - should be somewhat easier, at least for linear gradients. - // Fix three things: remove -webkit-, add 'to ' before reversed top/bottom keywords (linear) or 'at ' before position keywords (radial), recalculate deg-values - // -webkit-linear-gradient( [ [ | [top | bottom] || [left | right] ],]? [, ]+); - if (aPrefixedFuncName != "-webkit-linear-gradient" && - aPrefixedFuncName != "-webkit-radial-gradient") { - // Unrecognized prefixed gradient type - return false; - } - unprefixedFuncName = aPrefixedFuncName.replace(/-webkit-/, ''); - - // Keywords top, bottom, left, right: can be stand-alone or combined pairwise but in any order ('top left' or 'left top') - // These give the starting edge or corner in the -webkit syntax. The standardised equivalent is 'to ' plus opposite values for linear gradients, 'at ' plus same values for radial gradients - if(unprefixedFuncName.indexOf('linear') > -1){ - newValue = aPrefixedFuncBody.replace(/(top|bottom|left|right)+\s*(top|bottom|left|right)*/, function(str){ - var words = str.split(/\s+/); - for(var i=0; iIsOnCSSUnprefixingWhitelist(); -} - -bool -CSSParserImpl::ParsePropertyWithUnprefixingService( - const nsAString& aPropertyName, - css::Declaration* aDeclaration, - uint32_t aFlags, - bool aMustCallValueAppended, - bool* aChanged, - nsCSSContextType aContext) -{ - MOZ_ASSERT(ShouldUseUnprefixingService(), - "Caller should've checked ShouldUseUnprefixingService()"); - - nsCOMPtr unprefixingSvc = - do_GetService(NS_CSSUNPREFIXINGSERVICE_CONTRACTID); - NS_ENSURE_TRUE(unprefixingSvc, false); - - // Save the state so we can jump back to this spot if our unprefixing fails - // (so we can behave as if we didn't even try to unprefix). - nsAutoCSSParserInputStateRestorer parserStateBeforeTryingToUnprefix(this); - - // Caller has already parsed the first half of the declaration -- - // aPropertyName and the ":". Now, we record the rest of the CSS declaration - // (the part after ':') into rightHalfOfDecl. (This is the property value, - // plus anything else up to the end of the declaration -- maybe "!important", - // maybe trailing junk characters, maybe a semicolon, maybe a trailing "}".) - bool checkForBraces = (aFlags & eParseDeclaration_InBraces) != 0; - nsAutoString rightHalfOfDecl; - mScanner->StartRecording(); - SkipDeclaration(checkForBraces); - mScanner->StopRecording(rightHalfOfDecl); - - // Try to unprefix: - bool success; - nsAutoString unprefixedDecl; - nsresult rv = - unprefixingSvc->GenerateUnprefixedDeclaration(aPropertyName, - rightHalfOfDecl, - unprefixedDecl, &success); - if (NS_FAILED(rv) || !success) { - return false; - } - - // Attempt to parse the unprefixed declaration: - nsAutoScannerChanger scannerChanger(this, unprefixedDecl); - success = ParseDeclaration(aDeclaration, - aFlags | eParseDeclaration_FromUnprefixingSvc, - aMustCallValueAppended, aChanged, aContext); - if (success) { - // We succeeded, so we'll leave the parser pointing at the end of - // the declaration; don't restore it to the pre-recording position. - parserStateBeforeTryingToUnprefix.DoNotRestore(); - } - - return success; -} - -bool -CSSParserImpl::ParseWebkitPrefixedGradientWithService( - nsAString& aPrefixedFuncName, - nsCSSValue& aValue) -{ - MOZ_ASSERT(ShouldUseUnprefixingService(), - "Should only call if we're allowed to use unprefixing service"); - - // Record the body of the "-webkit-*gradient" function into a string. - // Note: we're already just after the opening "(". - nsAutoString prefixedFuncBody; - mScanner->StartRecording(); - bool gotCloseParen = SkipUntil(')'); - mScanner->StopRecording(prefixedFuncBody); - if (gotCloseParen) { - // Strip off trailing close-paren, so that the value we pass to the - // unprefixing service is *just* the function-body (no parens). - prefixedFuncBody.Truncate(prefixedFuncBody.Length() - 1); - } - - // NOTE: Even if we fail, we'll be leaving the parser's cursor just after - // the close of the "-webkit-*gradient(...)" expression. This is the same - // behavior that the other Parse*Gradient functions have in their failure - // cases -- they call "SkipUntil(')') before returning false. So this is - // probably what we want. - nsCOMPtr unprefixingSvc = - do_GetService(NS_CSSUNPREFIXINGSERVICE_CONTRACTID); - NS_ENSURE_TRUE(unprefixingSvc, false); - - bool success; - nsAutoString unprefixedFuncName; - nsAutoString unprefixedFuncBody; - nsresult rv = - unprefixingSvc->GenerateUnprefixedGradientValue(aPrefixedFuncName, - prefixedFuncBody, - unprefixedFuncName, - unprefixedFuncBody, - &success); - - if (NS_FAILED(rv) || !success) { - return false; - } - - // JS service thinks it successfully converted the gradient! Now let's try - // to parse the resulting string. - - // First, add a close-paren if we originally recorded one (so that what we're - // about to put into the CSS parser is a faithful representation of what it - // would've seen if it were just parsing the original input stream): - if (gotCloseParen) { - unprefixedFuncBody.Append(char16_t(')')); - } - - nsAutoScannerChanger scannerChanger(this, unprefixedFuncBody); - if (unprefixedFuncName.EqualsLiteral("linear-gradient")) { - return ParseLinearGradient(aValue, 0); - } - if (unprefixedFuncName.EqualsLiteral("radial-gradient")) { - return ParseRadialGradient(aValue, 0); - } - - NS_ERROR("CSSUnprefixingService returned an unrecognized type of " - "gradient function"); - - return false; -} - //---------------------------------------------------------------------- bool @@ -7434,19 +7266,7 @@ CSSParserImpl::ParseDeclaration(css::Declaration* aDeclaration, (aContext == eCSSContext_Page && !nsCSSProps::PropHasFlags(propID, CSS_PROPERTY_APPLIES_TO_PAGE_RULE))) { // unknown property - if (NonMozillaVendorIdentifier(propertyName)) { - if (!mInSupportsCondition && - aContext == eCSSContext_General && - !(aFlags & eParseDeclaration_FromUnprefixingSvc) && // no recursion - ShouldUseUnprefixingService()) { - if (ParsePropertyWithUnprefixingService(propertyName, - aDeclaration, aFlags, - aMustCallValueAppended, - aChanged, aContext)) { - return true; - } - } - } else { + if (!NonMozillaVendorIdentifier(propertyName)) { REPORT_UNEXPECTED_P(PEUnknownProperty, propertyName); REPORT_UNEXPECTED(PEDeclDropped); OUTPUT_ERROR(); @@ -8074,18 +7894,6 @@ CSSParserImpl::ParseVariant(nsCSSValue& aValue, } return CSSParseResult::Ok; } - - if (ShouldUseUnprefixingService() && - !gradientFlags && - StringBeginsWith(tmp, NS_LITERAL_STRING("-webkit-"))) { - // Copy 'tmp' into a string on the stack, since as soon as we - // start parsing, its backing store (in "tk") will be overwritten - nsAutoString prefixedFuncName(tmp); - if (!ParseWebkitPrefixedGradientWithService(prefixedFuncName, aValue)) { - return CSSParseResult::Error; - } - return CSSParseResult::Ok; - } } if ((aVariantMask & VARIANT_IMAGE_RECT) != 0 && eCSSToken_Function == tk->mType && @@ -10866,7 +10674,7 @@ CSSParserImpl::ParseWebkitGradientRadius(float& aRadius) // (either a percentage or a number between 0 and 1.0), and a color (any // valid CSS color). In addition the shorthand functions from and to are // supported. These functions only require a color argument and are -// equivalent to color-stop(0, ...) and color-stop(1.0, …) respectively. +// equivalent to color-stop(0, ...) and color-stop(1.0, ?? respectively. bool CSSParserImpl::ParseWebkitGradientColorStop(nsCSSValueGradient* aGradient) { @@ -12412,7 +12220,7 @@ CSSParserImpl::IsFunctionTokenValidForImageLayerImage( funcName.LowerCaseEqualsLiteral("-moz-repeating-radial-gradient") || funcName.LowerCaseEqualsLiteral("-moz-image-rect") || funcName.LowerCaseEqualsLiteral("-moz-element") || - ((sWebkitPrefixedAliasesEnabled || ShouldUseUnprefixingService()) && + (sWebkitPrefixedAliasesEnabled && (funcName.LowerCaseEqualsLiteral("-webkit-gradient") || funcName.LowerCaseEqualsLiteral("-webkit-linear-gradient") || funcName.LowerCaseEqualsLiteral("-webkit-radial-gradient") || @@ -15324,17 +15132,17 @@ CSSParserImpl::ParseFontFeatureSettings(nsCSSValue& aValue) return true; } -bool -CSSParserImpl::ParseFontVariationSettings(nsCSSValue& aValue) -{ - // TODO: Actually implement this. - - // This stub is here because websites insist on considering this - // very hardware-dependent and O.S.-variable low-level font-control - // as a "critical feature" which it isn't as there is 0 guarantee - // that font variation settings are supported or honored by any - // operating system used by the client. - return true; +bool +CSSParserImpl::ParseFontVariationSettings(nsCSSValue& aValue) +{ + // TODO: Actually implement this. + + // This stub is here because websites insist on considering this + // very hardware-dependent and O.S.-variable low-level font-control + // as a "critical feature" which it isn't as there is 0 guarantee + // that font variation settings are supported or honored by any + // operating system used by the client. + return true; } bool @@ -17989,12 +17797,6 @@ nsCSSParser::Startup() "layout.css.prefixes.webkit"); Preferences::AddBoolVarCache(&sWebkitDevicePixelRatioEnabled, "layout.css.prefixes.device-pixel-ratio-webkit"); - Preferences::AddBoolVarCache(&sUnprefixingServiceEnabled, - "layout.css.unprefixing-service.enabled"); -#ifdef NIGHTLY_BUILD - Preferences::AddBoolVarCache(&sUnprefixingServiceGloballyWhitelisted, - "layout.css.unprefixing-service.globally-whitelisted"); -#endif Preferences::AddBoolVarCache(&sMozGradientsEnabled, "layout.css.prefixes.gradients"); Preferences::AddBoolVarCache(&sControlCharVisibility, diff --git a/layout/style/nsICSSUnprefixingService.idl b/layout/style/nsICSSUnprefixingService.idl deleted file mode 100644 index 11c3bf43f1..0000000000 --- a/layout/style/nsICSSUnprefixingService.idl +++ /dev/null @@ -1,76 +0,0 @@ -/* -*- Mode: IDL; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */ -/* This Source Code Form is subject to the terms of the Mozilla Public - * License, v. 2.0. If a copy of the MPL was not distributed with this - * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ - -/* interface for a service that converts certain vendor-prefixed CSS properties - to their unprefixed equivalents */ - -#include "nsISupports.idl" - -[scriptable, uuid(a5d6e2f4-d3ec-11e4-b002-782bcbaebb28)] -interface nsICSSUnprefixingService : nsISupports -{ - /** - * This function helps to convert unsupported vendor-prefixed CSS into - * supported unprefixed CSS. Given a vendor-prefixed property name and a - * value (or e.g. value + trailing junk like " !important;}"), this function - * will attempt to produce an equivalent CSS declaration that uses a - * supported unprefixed CSS property. - * - * @param aPropName - * The vendor-prefixed property name. - * - * @param aRightHalfOfDecl - * Everything after the ":" in the CSS declaration. This includes - * the property's value, along with possibly some leading whitespace - * and trailing text like "!important", and possibly a ';' and/or - * '}' (along with any other bogus text the author happens to - * include before those, which will probably make the decl invalid). - * - * @param aUnprefixedDecl[out] - * The resulting unprefixed declaration, if we return true. - * - * @return true if we were able to unprefix -- i.e. if we were able to - * convert the property to a known unprefixed equivalent, and we also - * performed any known-to-be-necessary fixup on the value, and we put - * the result in aUnprefixedDecl. - * Otherwise, this function returns false. - */ - boolean generateUnprefixedDeclaration(in AString aPropName, - in AString aRightHalfOfDecl, - out AString aUnprefixedDecl); - - /** - * @param aPrefixedFuncName - * The webkit-prefixed gradient function: either - * "-webkit-gradient", "-webkit-linear-gradient", or - * "-webkit-radial-gradient". - * - * @param aPrefixedFuncBody - * The body of the gradient function, inside (& not including) the - * parenthesis. - * - * @param aUnprefixedFuncName[out] - * The resulting unprefixed gradient function name: - * either "linear-gradient" or "radial-gradient". - * - * @param aUnprefixedFuncBody[out] - * The resulting unprefixed gradient function body, suitable for - * including in a "linear-gradient(...)" or "radial-gradient(...)" - * expression. - * - * @returns true if we were able to successfully parse aWebkitGradientStr - * and populate the outparams accordingly; false otherwise. - * - */ - boolean generateUnprefixedGradientValue(in AString aPrefixedFuncName, - in AString aPrefixedFuncBody, - out AString aUnprefixedFuncName, - out AString aUnprefixedFuncBody); -}; - -%{C++ -#define NS_CSSUNPREFIXINGSERVICE_CONTRACTID \ - "@mozilla.org/css-unprefixing-service;1" -%} diff --git a/layout/style/test/mochitest.ini b/layout/style/test/mochitest.ini index 8b36e3e1e8..2f0ab87a28 100644 --- a/layout/style/test/mochitest.ini +++ b/layout/style/test/mochitest.ini @@ -273,10 +273,6 @@ support-files = ../../reftests/fonts/markA.woff ../../reftests/fonts/markB.woff [test_units_frequency.html] [test_units_length.html] [test_units_time.html] -[test_unprefixing_service.html] -support-files = unprefixing_service_iframe.html unprefixing_service_utils.js -[test_unprefixing_service_prefs.html] -support-files = unprefixing_service_iframe.html unprefixing_service_utils.js [test_value_cloning.html] [test_value_computation.html] [test_value_storage.html] diff --git a/layout/style/test/test_unprefixing_service.html b/layout/style/test/test_unprefixing_service.html deleted file mode 100644 index c489e2ac01..0000000000 --- a/layout/style/test/test_unprefixing_service.html +++ /dev/null @@ -1,93 +0,0 @@ - - - - - - Test for Bug 1107378 - - - - - -Mozilla Bug 1107378 -
- -
-
-
-
- - diff --git a/layout/style/test/test_unprefixing_service_prefs.html b/layout/style/test/test_unprefixing_service_prefs.html deleted file mode 100644 index 329dce2a63..0000000000 --- a/layout/style/test/test_unprefixing_service_prefs.html +++ /dev/null @@ -1,132 +0,0 @@ - - - - - - Test for Bug 1132743 - - - - - -Mozilla Bug 1132743 -
- -
-
-
-
- - diff --git a/layout/style/test/unprefixing_service_iframe.html b/layout/style/test/unprefixing_service_iframe.html deleted file mode 100644 index 8edeb20dce..0000000000 --- a/layout/style/test/unprefixing_service_iframe.html +++ /dev/null @@ -1,394 +0,0 @@ - - - - - Helper file for testing CSS Unprefixing Service - - - - -
-
-
- - - - diff --git a/layout/style/test/unprefixing_service_utils.js b/layout/style/test/unprefixing_service_utils.js deleted file mode 100644 index cd17d20d08..0000000000 --- a/layout/style/test/unprefixing_service_utils.js +++ /dev/null @@ -1,87 +0,0 @@ -/* This Source Code Form is subject to the terms of the Mozilla Public - * License, v. 2.0. If a copy of the MPL was not distributed with this - * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ - -// Shared data & functionality used in tests for CSS Unprefixing Service. - -// Whitelisted hosts: -// (per implementation of nsPrincipal::IsOnCSSUnprefixingWhitelist()) -var gWhitelistedHosts = [ - // test1.example.org is on the whitelist. - "test1.example.org", - // test2.example.org is on the "allow all subdomains" whitelist. - "test2.example.org", - "sub1.test2.example.org", - "sub2.test2.example.org" -]; - -// *NOT* whitelisted hosts: -var gNotWhitelistedHosts = [ - // Though test1.example.org is on the whitelist, its subdomains are not. - "sub1.test1.example.org", - // mochi.test is not on the whitelist. - "mochi.test:8888" -]; - -// Names of prefs: -const PREF_UNPREFIXING_SERVICE = - "layout.css.unprefixing-service.enabled"; -const PREF_INCLUDE_TEST_DOMAINS = - "layout.css.unprefixing-service.include-test-domains"; - -// Helper-function to make unique URLs in testHost(): -var gCounter = 0; -function getIncreasingCounter() { - return gCounter++; -} - -// This function tests a particular host in our iframe. -// @param aHost The host to be tested -// @param aExpectEnabled Should we expect unprefixing to be enabled for host? -function testHost(aHost, aExpectEnabled) { - // Build the URL: - let url = window.location.protocol; // "http:" or "https:" - url += "//"; - url += aHost; - - // Append the path-name, up to the actual filename (the final "/"): - const re = /(.*\/).*/; - url += window.location.pathname.replace(re, "$1"); - url += IFRAME_TESTFILE; - // In case this is the same URL as last time, we add "?N" for some unique N, - // to make each URL different, so that the iframe actually (re)loads: - url += "?" + getIncreasingCounter(); - // We give the URL a #suffix to indicate to the test whether it should expect - // that unprefixing is enabled or disabled: - url += (aExpectEnabled ? "#expectEnabled" : "#expectDisabled"); - - let iframe = document.getElementById("testIframe"); - iframe.contentWindow.location = url; - // The iframe will report its results back via postMessage. - // Our caller had better have set up a postMessage listener. -} - -// Register a postMessage() handler, to allow our cross-origin iframe to -// communicate back to the main page's mochitest functionality. -// The handler expects postMessage to be called with an object like: -// { type: ["is"|"ok"|"testComplete"], ... } -// The "is" and "ok" types will trigger the corresponding function to be -// called in the main page, with named arguments provided in the payload. -// The "testComplete" type will trigger the passed-in aTestCompleteCallback -// function to be invoked (e.g. to advance to the next testcase, or to finish -// the overall test, as-appropriate). -function registerPostMessageListener(aTestCompleteCallback) { - let receiveMessage = function(event) { - if (event.data.type === "is") { - is(event.data.actual, event.data.expected, event.data.desc); - } else if (event.data.type === "ok") { - ok(event.data.condition, event.data.desc); - } else if (event.data.type === "testComplete") { - aTestCompleteCallback(); - } else { - ok(false, "unrecognized data in postMessage call"); - } - }; - - window.addEventListener("message", receiveMessage, false); -} diff --git a/modules/libpref/init/all.js b/modules/libpref/init/all.js index a57f1000e7..6e4cff6092 100644 --- a/modules/libpref/init/all.js +++ b/modules/libpref/init/all.js @@ -2551,16 +2551,6 @@ pref("layout.css.prefixes.webkit", true); // pref is set to false.) pref("layout.css.prefixes.device-pixel-ratio-webkit", false); -// Is the CSS Unprefixing Service enabled? (This service emulates support -// for certain vendor-prefixed properties & values, for sites on a "fixlist".) -pref("layout.css.unprefixing-service.enabled", true); -#ifdef NIGHTLY_BUILD -// Is the CSS Unprefixing Service whitelisted for all domains? -// (This pref is only honored in Nightly builds and can be removed when -// Bug 1177263 is fixed.) -pref("layout.css.unprefixing-service.globally-whitelisted", false); -#endif - // Is support for the :scope selector enabled? pref("layout.css.scope-pseudo.enabled", true); From 56e636d8ec60acacfe2ede3ebd58291cca2945bc Mon Sep 17 00:00:00 2001 From: FranklinDM Date: Fri, 6 Jan 2023 21:14:29 +0800 Subject: [PATCH 09/11] Issue #2084 - Part 2: Simplify logic in CSSParserImpl::LookupKeywordPrefixAware Based on https://bugzilla.mozilla.org/show_bug.cgi?id=1259348 --- layout/style/nsCSSParser.cpp | 73 +++++++++++------------------------- 1 file changed, 22 insertions(+), 51 deletions(-) diff --git a/layout/style/nsCSSParser.cpp b/layout/style/nsCSSParser.cpp index bc48838c9f..801e9356d5 100644 --- a/layout/style/nsCSSParser.cpp +++ b/layout/style/nsCSSParser.cpp @@ -7112,65 +7112,36 @@ CSSParserImpl::LookupKeywordPrefixAware(nsAString& aKeywordStr, { nsCSSKeyword keyword = nsCSSKeywords::LookupKeyword(aKeywordStr); + if (!sWebkitPrefixedAliasesEnabled) { + // Not accepting webkit-prefixed keywords -> don't do anything special. + return keyword; + } + if (aKeywordTable == nsCSSProps::kDisplayKTable) { - // NOTE: This code will be considerably simpler once we can do away with - // all Unprefixing Service code, in bug 1259348. But for the time being, we - // have to support two different strategies for handling -webkit-box here: - // (1) "Native support" (sWebkitPrefixedAliasesEnabled): we assume that - // -webkit-box will parse correctly (via an entry in kDisplayKTable), - // and we simply make a note that we've parsed it (so that we can we - // can give later "-moz-box" styling special handling as noted below). - // (2) "Unprefixing Service support" (ShouldUseUnprefixingService): we - // convert "-webkit-box" directly to modern "flex" (& do the same for - // any later "-moz-box" styling). - // - // Note that sWebkitPrefixedAliasesEnabled and - // ShouldUseUnprefixingService() are mutually exlusive, because the latter - // explicitly defers to the former. if ((keyword == eCSSKeyword__webkit_box || keyword == eCSSKeyword__webkit_inline_box)) { - const bool usingUnprefixingService = false; - // XXXdholbert This bool^ will be removed & this whole function will be - // simplified in the next patch in this series. - if (sWebkitPrefixedAliasesEnabled || usingUnprefixingService) { - // Make a note that we're accepting some "-webkit-{inline-}box" styling, - // so we can give special treatment to subsequent "-moz-{inline}-box". - // (See special treatment below.) - if (mWebkitBoxUnprefixState == eHaveNotUnprefixed) { - mWebkitBoxUnprefixState = eHaveUnprefixed; - } - if (usingUnprefixingService) { - // When we're using the unprefixing service, we treat - // "display:-webkit-box" as if it were "display:flex" - // (and "-webkit-inline-box" as "inline-flex"). - return (keyword == eCSSKeyword__webkit_box) ? - eCSSKeyword_flex : eCSSKeyword_inline_flex; - } + // Make a note that we're accepting some "-webkit-{inline-}box" styling, + // so we can give special treatment to subsequent "-moz-{inline}-box". + // (See special treatment below.) + if (mWebkitBoxUnprefixState == eHaveNotUnprefixed) { + mWebkitBoxUnprefixState = eHaveUnprefixed; } - } - - // If we've seen "display: -webkit-box" (or "-webkit-inline-box") in an - // earlier declaration and we honored it, then we have to watch out for - // later "display: -moz-box" (and "-moz-inline-box") declarations; they're - // likely just a halfhearted attempt at compatibility, and they actually - // end up stomping on our emulation of the earlier -webkit-box - // display-value, via the CSS cascade. To prevent this problem, we treat - // "display: -moz-box" & "-moz-inline-box" as if they were simply a - // repetition of the webkit equivalent that we already parsed. - if (mWebkitBoxUnprefixState == eHaveUnprefixed && - (keyword == eCSSKeyword__moz_box || - keyword == eCSSKeyword__moz_inline_box)) { + } else if (mWebkitBoxUnprefixState == eHaveUnprefixed && + (keyword == eCSSKeyword__moz_box || + keyword == eCSSKeyword__moz_inline_box)) { + // If we've seen "display: -webkit-box" (or "-webkit-inline-box") in an + // earlier declaration and we honored it, then we have to watch out for + // later "display: -moz-box" (and "-moz-inline-box") declarations; they're + // likely just a halfhearted attempt at compatibility, and they actually + // end up stomping on our emulation of the earlier -webkit-box + // display-value, via the CSS cascade. To prevent this problem, we treat + // "display: -moz-box" & "-moz-inline-box" as if they were simply a + // repetition of the webkit equivalent that we already parsed. MOZ_ASSERT(sWebkitPrefixedAliasesEnabled, "The only way mWebkitBoxUnprefixState can be eHaveUnprefixed " "is if we're supporting webkit-prefixed aliases"); - if (sWebkitPrefixedAliasesEnabled) { - return (keyword == eCSSKeyword__moz_box) ? - eCSSKeyword__webkit_box : eCSSKeyword__webkit_inline_box; - } - // (If we get here, we're using the Unprefixing Service, which means - // we're unprefixing all the way to modern flexbox display values.) return (keyword == eCSSKeyword__moz_box) ? - eCSSKeyword_flex : eCSSKeyword_inline_flex; + eCSSKeyword__webkit_box : eCSSKeyword__webkit_inline_box; } } From 2f7f622cd4110249f681922bc7fcdac64773de66 Mon Sep 17 00:00:00 2001 From: FranklinDM Date: Wed, 4 Jan 2023 21:35:58 +0800 Subject: [PATCH 10/11] No issue - Fix invalid neq check on assert in RegExpParser --- js/src/irregexp/RegExpParser.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/js/src/irregexp/RegExpParser.cpp b/js/src/irregexp/RegExpParser.cpp index ed86fe2464..7c6ddb1130 100644 --- a/js/src/irregexp/RegExpParser.cpp +++ b/js/src/irregexp/RegExpParser.cpp @@ -1238,7 +1238,7 @@ RegExpParser::CreateNamedCaptureAtIndex(const CharacterVector* name, int index) { MOZ_ASSERT(0 < index && index <= captures_started_); - MOZ_ASSERT(name !== nullptr); + MOZ_ASSERT(name != nullptr); RegExpCapture* capture = GetCapture(index); MOZ_ASSERT(capture->name() == nullptr); From efeb0e3e9724461bf5a2ac19b525e193f97f0736 Mon Sep 17 00:00:00 2001 From: Moonchild Date: Mon, 9 Jan 2023 10:52:49 +0100 Subject: [PATCH 11/11] Issue #2087 - Don't throw on lacking PresShell in SetFontInternal In CanvasRenderingContext2D::SetFontInternal, we should not throw if there is no PresShell due to (sandboxed/hidden) iframe use that has not initialized its presentation yet at the time of property manipulation. This removes the throwing of the error and just silently fails. Resolves #2087 --- dom/canvas/CanvasRenderingContext2D.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/dom/canvas/CanvasRenderingContext2D.cpp b/dom/canvas/CanvasRenderingContext2D.cpp index f83fdcb5bf..f56f2bdf53 100644 --- a/dom/canvas/CanvasRenderingContext2D.cpp +++ b/dom/canvas/CanvasRenderingContext2D.cpp @@ -3758,7 +3758,11 @@ CanvasRenderingContext2D::SetFontInternal(const nsAString& aFont, nsCOMPtr presShell = GetPresShell(); if (!presShell) { - aError.Throw(NS_ERROR_FAILURE); + // Do not throw here. We may be in a situation where we're loading in an iframe + // that is sandboxed, and/or initially hidden with display:none, in which case + // we don't want to throw an error but silently fail. + // If we don't do this, JS trying to set context.font to something will abort, + // breaking e.g. third party serviced graphs. return false; }