From eaa6304aab85fc21345a65522624d129b5a9a7fd Mon Sep 17 00:00:00 2001
From: Tim Ledbetter <timledbetter@gmail.com>
Date: Sun, 1 Oct 2023 19:56:19 +0100
Subject: [PATCH] Userland: Return empty if ImageDecoder client receives an
 invalid frame

This simplifies error checking for all the users of the ImageDecoder
client.
---
 Userland/Applications/ImageViewer/ViewWidget.cpp      |  2 +-
 Userland/Applications/PixelPaint/Image.cpp            |  5 +----
 Userland/Libraries/LibImageDecoderClient/Client.cpp   | 11 ++++++-----
 Userland/Libraries/LibImageDecoderClient/Client.h     |  2 +-
 .../Services/WebContent/ImageCodecPluginSerenity.cpp  |  4 +---
 Userland/Utilities/pixelflut.cpp                      |  2 +-
 6 files changed, 11 insertions(+), 15 deletions(-)

diff --git a/Userland/Applications/ImageViewer/ViewWidget.cpp b/Userland/Applications/ImageViewer/ViewWidget.cpp
index f0f301e846..dea7573f3d 100644
--- a/Userland/Applications/ImageViewer/ViewWidget.cpp
+++ b/Userland/Applications/ImageViewer/ViewWidget.cpp
@@ -246,7 +246,7 @@ ErrorOr<void> ViewWidget::try_open_file(String const& path, Core::File& file)
         frames.ensure_capacity(decoded_image->frames.size());
         for (u32 i = 0; i < decoded_image->frames.size(); i++) {
             auto& frame_data = decoded_image->frames[i];
-            frames.unchecked_append({ BitmapImage::create(*frame_data.bitmap), int(frame_data.duration) });
+            frames.unchecked_append({ BitmapImage::create(frame_data.bitmap), int(frame_data.duration) });
         }
     }
 
diff --git a/Userland/Applications/PixelPaint/Image.cpp b/Userland/Applications/PixelPaint/Image.cpp
index 592fd18b59..1e90c5815d 100644
--- a/Userland/Applications/PixelPaint/Image.cpp
+++ b/Userland/Applications/PixelPaint/Image.cpp
@@ -66,10 +66,7 @@ ErrorOr<NonnullRefPtr<Gfx::Bitmap>> Image::decode_bitmap(ReadonlyBytes bitmap_da
     if (decoded_image.frames.is_empty())
         return Error::from_string_literal("Image decode failed (no frames)");
 
-    auto decoded_bitmap = decoded_image.frames.first().bitmap;
-    if (decoded_bitmap.is_null())
-        return Error::from_string_literal("Image decode failed (no bitmap for frame)");
-    return decoded_bitmap.release_nonnull();
+    return decoded_image.frames.first().bitmap;
 }
 
 ErrorOr<NonnullRefPtr<Image>> Image::create_from_bitmap(NonnullRefPtr<Gfx::Bitmap> const& bitmap)
diff --git a/Userland/Libraries/LibImageDecoderClient/Client.cpp b/Userland/Libraries/LibImageDecoderClient/Client.cpp
index 93b1e59a05..13e92195af 100644
--- a/Userland/Libraries/LibImageDecoderClient/Client.cpp
+++ b/Userland/Libraries/LibImageDecoderClient/Client.cpp
@@ -48,12 +48,13 @@ Optional<DecodedImage> Client::decode_image(ReadonlyBytes encoded_data, Optional
     DecodedImage image;
     image.is_animated = response.is_animated();
     image.loop_count = response.loop_count();
-    image.frames.resize(response.bitmaps().size());
+    image.frames.ensure_capacity(response.bitmaps().size());
     auto bitmaps = response.take_bitmaps();
-    for (size_t i = 0; i < image.frames.size(); ++i) {
-        auto& frame = image.frames[i];
-        frame.bitmap = bitmaps[i].bitmap();
-        frame.duration = response.durations()[i];
+    for (size_t i = 0; i < bitmaps.size(); ++i) {
+        if (!bitmaps[i].is_valid())
+            return {};
+
+        image.frames.empend(*bitmaps[i].bitmap(), response.durations()[i]);
     }
     return image;
 }
diff --git a/Userland/Libraries/LibImageDecoderClient/Client.h b/Userland/Libraries/LibImageDecoderClient/Client.h
index 21328c45ce..d04ac21873 100644
--- a/Userland/Libraries/LibImageDecoderClient/Client.h
+++ b/Userland/Libraries/LibImageDecoderClient/Client.h
@@ -14,7 +14,7 @@
 namespace ImageDecoderClient {
 
 struct Frame {
-    RefPtr<Gfx::Bitmap> bitmap;
+    NonnullRefPtr<Gfx::Bitmap> bitmap;
     u32 duration { 0 };
 };
 
diff --git a/Userland/Services/WebContent/ImageCodecPluginSerenity.cpp b/Userland/Services/WebContent/ImageCodecPluginSerenity.cpp
index 3b04f5a2e1..cc50fdf202 100644
--- a/Userland/Services/WebContent/ImageCodecPluginSerenity.cpp
+++ b/Userland/Services/WebContent/ImageCodecPluginSerenity.cpp
@@ -31,9 +31,7 @@ Optional<Web::Platform::DecodedImage> ImageCodecPluginSerenity::decode_image(Rea
     decoded_image.is_animated = result.is_animated;
     decoded_image.loop_count = result.loop_count;
     for (auto const& frame : result.frames) {
-        if (!frame.bitmap)
-            return {};
-        decoded_image.frames.empend(move(frame.bitmap), frame.duration);
+        decoded_image.frames.empend(frame.bitmap, frame.duration);
     }
 
     return decoded_image;
diff --git a/Userland/Utilities/pixelflut.cpp b/Userland/Utilities/pixelflut.cpp
index a596ba6173..dd5fa69d11 100644
--- a/Userland/Utilities/pixelflut.cpp
+++ b/Userland/Utilities/pixelflut.cpp
@@ -80,7 +80,7 @@ ErrorOr<NonnullRefPtr<Client>> Client::create(StringView image_path, StringView
     if (!maybe_image.has_value())
         return Error::from_string_view("Image could not be read"sv);
 
-    auto image = maybe_image->frames.take_first().bitmap.release_nonnull();
+    auto image = maybe_image->frames.take_first().bitmap;
 
     // Make sure to not draw out of bounds; some servers will disconnect us for that!
     if (image->width() > canvas_size.width()) {