diff --git a/image/MultipartImage.cpp b/image/MultipartImage.cpp index 5f0eeff11d4e..10c6f9028d08 100644 --- a/image/MultipartImage.cpp +++ b/image/MultipartImage.cpp @@ -18,6 +18,44 @@ namespace image { // Helpers /////////////////////////////////////////////////////////////////////////////// +static void FinishPotentialVectorImage(Image* aImage) { + if (!aImage || aImage->GetType() != imgIContainer::TYPE_VECTOR) { + return; + } + // We only want to transition to or away from a part once it has reached + // load complete status, because if we don't then the progress tracker will + // send a fake load event with the lastpart bit set whenever an observer is + // removed without having the load complete progress. That fake load event + // with the lastpart bit set will get out of the multipart image and to the + // imgRequestProxy and further, which will make it look like we got the last + // part of the multipart image and confuse things. If we get here then we + // are still waiting for the load event for this image but we've gotten + // OnDataAvailable for the part after aImage. That means we must have + // gotten OnStopRequest for aImage; OnStopRequest calls + // OnImageDataComplete. For raster images that are part of a multipart + // image OnImageDataComplete will synchronously fire the load event + // because we synchronously perform the metadata decode in that function, + // because parts of multipart images have the transient flag set. So for + // raster images we are assured that the load event has happened by this + // point for aImage. For vector images there is no such luck because + // OnImageDataComplete needs to wait for the load event in the underlying + // svg document, which we can't force to happen synchronously. So we just + // send a fake load event for aImage. This shouldn't confuse anyone because + // the progress tracker won't send out another load event when the real load + // event comes because it's already in the progress. And nobody should be + // caring about load events on the current part of multipart images since + // they might be sent multiple times or might not be sent at all depending + // on timing. So this should be okay. + + RefPtr tracker = aImage->GetProgressTracker(); + if (tracker && !(tracker->GetProgress() & FLAG_LOAD_COMPLETE)) { + Progress loadProgress = + LoadCompleteProgress(/* aLastPart = */ false, /* aError = */ false, + /* aStatus = */ NS_OK); + tracker->SyncNotifyProgress(loadProgress | FLAG_SIZE_AVAILABLE); + } +} + class NextPartObserver : public IProgressObserver { public: MOZ_DECLARE_REFCOUNTED_TYPENAME(NextPartObserver) @@ -42,42 +80,9 @@ class NextPartObserver : public IProgressObserver { mImage->RequestDecodeForSize(gfx::IntSize(0, 0), imgIContainer::FLAG_SYNC_DECODE); - if (mImage && mImage->GetType() == imgIContainer::TYPE_VECTOR) { - // We don't want to make a pending part the current part until it has had - // it's load event because when we transition from the current part to the - // next part we remove the multipart image as an observer of the current - // part and the progress tracker will send a fake load event with the - // lastpart bit set whenever an observer is removed without having the - // load complete progress. That fake load event with the lastpart bit set - // will get out of the multipart image and to the imgRequestProxy and - // further, which will make it look like we got the last part of the - // multipart image and confuse things. If we get here then we are still - // waiting to make mNextPart the current part, but we've gotten - // OnDataAvailable for the part after mNextPart. That means we must have - // gotten OnStopRequest for mNextPart; OnStopRequest calls - // OnImageDataComplete. For raster images that are part of a multipart - // image OnImageDataComplete will synchronously fire the load event - // because we synchronously perform the metadata decode in that function, - // because parts of multipart images have the transient flag set. So for - // raster images we are assured that the load event has happened by this - // point for mNextPart. For vector images there is no such luck because - // OnImageDataComplete needs to wait for the load event in the underlying - // svg document, which we can't force to happen synchronously. So we just - // send a fake load event for mNextPart. We are the only listener for it - // right now so it won't confuse anyone. When the real load event comes, - // progress tracker won't send it out because it's already in the - // progress. And nobody should be caring about load events on the current - // part of multipart images since they might be sent multiple times or - // might not be sent at all depending on timing. So this should be okay. - - RefPtr tracker = mImage->GetProgressTracker(); - if (tracker && !(tracker->GetProgress() & FLAG_LOAD_COMPLETE)) { - Progress loadProgress = - LoadCompleteProgress(/* aLastPart = */ false, /* aError = */ false, - /* aStatus = */ NS_OK); - tracker->SyncNotifyProgress(loadProgress | FLAG_SIZE_AVAILABLE); - } - } + // Vector images need special handling to make sure they are in a complete + // state. + FinishPotentialVectorImage(mImage); // RequestDecodeForSize() should've sent synchronous notifications that // would have caused us to call FinishObserving() (and null out mImage) @@ -245,6 +250,12 @@ void MultipartImage::FinishTransition() { return; } + // Vector images need special handling to make sure they are in a complete + // state. (Only the first part can require this, subsequent parts have + // FinishPotentialVectorImage called on them before they become the current + // part.) + FinishPotentialVectorImage(InnerImage()); + // Stop observing the current part. { RefPtr currentPartTracker = diff --git a/image/test/crashtests/1944122-1.html b/image/test/crashtests/1944122-1.html new file mode 100644 index 000000000000..ab552a7c17c0 --- /dev/null +++ b/image/test/crashtests/1944122-1.html @@ -0,0 +1,15 @@ + + + + diff --git a/image/test/crashtests/1944122-1.svg b/image/test/crashtests/1944122-1.svg new file mode 100644 index 000000000000..435cff38d332 --- /dev/null +++ b/image/test/crashtests/1944122-1.svg @@ -0,0 +1,13 @@ +--BOUNDARYOMG +Content-Type: image/svg+xml; charset=utf-8 + +--BOUNDARYOMG +Content-Type: image/svg+xml; charset=utf-8 + +--BOUNDARYOMG +Content-Type: image/svg+xml; charset=utf-8 + +--BOUNDARYOMG +Content-Type: image/svg+xml; charset=utf-8 + +--BOUNDARYOMG-- \ No newline at end of file diff --git a/image/test/crashtests/1944122-1.svg^headers^ b/image/test/crashtests/1944122-1.svg^headers^ new file mode 100644 index 000000000000..f5e846508e57 --- /dev/null +++ b/image/test/crashtests/1944122-1.svg^headers^ @@ -0,0 +1,3 @@ +HTTP 200 OK +Content-Type: multipart/x-mixed-replace;boundary=BOUNDARYOMG +Cache-Control: no-cache diff --git a/image/test/crashtests/crashtests.list b/image/test/crashtests/crashtests.list index 6ff61e6ef33d..f6c1707c2177 100644 --- a/image/test/crashtests/crashtests.list +++ b/image/test/crashtests/crashtests.list @@ -85,3 +85,4 @@ load 1885209-1.html load 1898606.jpg load 1910211-1.avif load 1943715-1.png +HTTP load 1944122-1.html