From 5a79c69b0216f070b07d8238e4c2c7e3420faeec Mon Sep 17 00:00:00 2001
From: Andreas Kling <kling@serenityos.org>
Date: Sat, 20 Nov 2021 14:29:33 +0100
Subject: [PATCH] LibGfx: Make ImageDecoderPlugin::frame() return ErrorOr<>

This is a first step towards better error propagation from image codecs.
---
 Meta/Lagom/Fuzzers/FuzzGIFLoader.cpp          |  7 ++++---
 Tests/LibGfx/TestImageDecoder.cpp             | 17 ++++++++--------
 .../Libraries/LibGUI/FileIconProvider.cpp     |  6 +++++-
 Userland/Libraries/LibGUI/ImageWidget.cpp     |  6 +++---
 Userland/Libraries/LibGfx/BMPLoader.cpp       | 12 +++++------
 Userland/Libraries/LibGfx/BMPLoader.h         |  2 +-
 Userland/Libraries/LibGfx/Bitmap.cpp          |  3 ++-
 Userland/Libraries/LibGfx/DDSLoader.cpp       | 12 +++++------
 Userland/Libraries/LibGfx/DDSLoader.h         |  2 +-
 Userland/Libraries/LibGfx/GIFLoader.cpp       | 20 +++++++------------
 Userland/Libraries/LibGfx/GIFLoader.h         |  2 +-
 Userland/Libraries/LibGfx/ICOLoader.cpp       | 17 ++++++++--------
 Userland/Libraries/LibGfx/ICOLoader.h         |  2 +-
 Userland/Libraries/LibGfx/ImageDecoder.h      |  4 ++--
 Userland/Libraries/LibGfx/JPGLoader.cpp       | 12 +++++------
 Userland/Libraries/LibGfx/JPGLoader.h         |  2 +-
 Userland/Libraries/LibGfx/PBMLoader.cpp       | 12 +++++------
 Userland/Libraries/LibGfx/PBMLoader.h         |  2 +-
 Userland/Libraries/LibGfx/PGMLoader.cpp       | 12 +++++------
 Userland/Libraries/LibGfx/PGMLoader.h         |  2 +-
 Userland/Libraries/LibGfx/PNGLoader.cpp       | 12 +++++------
 Userland/Libraries/LibGfx/PNGLoader.h         |  2 +-
 Userland/Libraries/LibGfx/PPMLoader.cpp       | 12 +++++------
 Userland/Libraries/LibGfx/PPMLoader.h         |  2 +-
 .../ImageDecoder/ClientConnection.cpp         | 10 +++++-----
 Userland/Services/SpiceAgent/SpiceAgent.cpp   |  9 +++++----
 26 files changed, 101 insertions(+), 100 deletions(-)

diff --git a/Meta/Lagom/Fuzzers/FuzzGIFLoader.cpp b/Meta/Lagom/Fuzzers/FuzzGIFLoader.cpp
index c713c2234c1..948214c8f22 100644
--- a/Meta/Lagom/Fuzzers/FuzzGIFLoader.cpp
+++ b/Meta/Lagom/Fuzzers/FuzzGIFLoader.cpp
@@ -14,8 +14,9 @@
 extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size)
 {
     Gfx::GIFImageDecoderPlugin gif_decoder(data, size);
-    auto bitmap = gif_decoder.frame(0).image;
-    if (bitmap) {
+    auto bitmap_or_error = gif_decoder.frame(0);
+    if (!bitmap_or_error.is_error()) {
+        auto const& bitmap = bitmap_or_error.value().image;
         // Looks like a valid GIF. Try to load the other frames:
         dbgln_if(GIF_DEBUG, "bitmap size: {}", bitmap->size());
         dbgln_if(GIF_DEBUG, "codec size: {}", gif_decoder.size());
@@ -24,7 +25,7 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size)
         dbgln_if(GIF_DEBUG, "loop_count: {}", gif_decoder.loop_count());
         dbgln_if(GIF_DEBUG, "frame_count: {}", gif_decoder.frame_count());
         for (size_t i = 0; i < gif_decoder.frame_count(); ++i) {
-            auto ifd = gif_decoder.frame(i);
+            auto ifd = gif_decoder.frame(i).release_value_but_fixme_should_propagate_errors();
             dbgln_if(GIF_DEBUG, "frame #{} size: {}", i, ifd.image->size());
             dbgln_if(GIF_DEBUG, "frame #{} duration: {}", i, ifd.duration);
         }
diff --git a/Tests/LibGfx/TestImageDecoder.cpp b/Tests/LibGfx/TestImageDecoder.cpp
index 7f2a4397866..459bf6a4b54 100644
--- a/Tests/LibGfx/TestImageDecoder.cpp
+++ b/Tests/LibGfx/TestImageDecoder.cpp
@@ -31,7 +31,7 @@ TEST_CASE(test_bmp)
     EXPECT(!bmp.is_animated());
     EXPECT(!bmp.loop_count());
 
-    auto frame = bmp.frame(1);
+    auto frame = bmp.frame(0).release_value_but_fixme_should_propagate_errors();
     EXPECT(frame.duration == 0);
 }
 
@@ -45,7 +45,7 @@ TEST_CASE(test_gif)
     EXPECT(gif.is_animated());
     EXPECT(!gif.loop_count());
 
-    auto frame = gif.frame(1);
+    auto frame = gif.frame(1).release_value_but_fixme_should_propagate_errors();
     EXPECT(frame.duration == 400);
 }
 
@@ -60,8 +60,7 @@ TEST_CASE(test_ico)
     EXPECT(!ico.is_animated());
     EXPECT(!ico.loop_count());
 
-    auto frame = ico.frame(1);
-    EXPECT(frame.duration == 0);
+    EXPECT(ico.frame(0).is_error());
 }
 
 TEST_CASE(test_jpg)
@@ -74,7 +73,7 @@ TEST_CASE(test_jpg)
     EXPECT(!jpg.is_animated());
     EXPECT(!jpg.loop_count());
 
-    auto frame = jpg.frame(1);
+    auto frame = jpg.frame(0).release_value_but_fixme_should_propagate_errors();
     EXPECT(frame.duration == 0);
 }
 
@@ -88,7 +87,7 @@ TEST_CASE(test_pbm)
     EXPECT(!pbm.is_animated());
     EXPECT(!pbm.loop_count());
 
-    auto frame = pbm.frame(1);
+    auto frame = pbm.frame(0).release_value_but_fixme_should_propagate_errors();
     EXPECT(frame.duration == 0);
 }
 
@@ -102,7 +101,7 @@ TEST_CASE(test_pgm)
     EXPECT(!pgm.is_animated());
     EXPECT(!pgm.loop_count());
 
-    auto frame = pgm.frame(1);
+    auto frame = pgm.frame(0).release_value_but_fixme_should_propagate_errors();
     EXPECT(frame.duration == 0);
 }
 
@@ -116,7 +115,7 @@ TEST_CASE(test_png)
     EXPECT(!png.is_animated());
     EXPECT(!png.loop_count());
 
-    auto frame = png.frame(1);
+    auto frame = png.frame(0).release_value_but_fixme_should_propagate_errors();
     EXPECT(frame.duration == 0);
 }
 
@@ -130,6 +129,6 @@ TEST_CASE(test_ppm)
     EXPECT(!ppm.is_animated());
     EXPECT(!ppm.loop_count());
 
-    auto frame = ppm.frame(1);
+    auto frame = ppm.frame(0).release_value_but_fixme_should_propagate_errors();
     EXPECT(frame.duration == 0);
 }
diff --git a/Userland/Libraries/LibGUI/FileIconProvider.cpp b/Userland/Libraries/LibGUI/FileIconProvider.cpp
index 716e90fc275..90eafe9404b 100644
--- a/Userland/Libraries/LibGUI/FileIconProvider.cpp
+++ b/Userland/Libraries/LibGUI/FileIconProvider.cpp
@@ -188,7 +188,11 @@ Icon FileIconProvider::icon_for_executable(const String& path)
         if (!section.has_value()) {
             bitmap = s_executable_icon.bitmap_for_size(icon_section.image_size);
         } else {
-            bitmap = Gfx::PNGImageDecoderPlugin(reinterpret_cast<u8 const*>(section->raw_data()), section->size()).frame(0).image;
+            // FIXME: Use the ImageDecoder service.
+            auto frame_or_error = Gfx::PNGImageDecoderPlugin(reinterpret_cast<u8 const*>(section->raw_data()), section->size()).frame(0);
+            if (!frame_or_error.is_error()) {
+                bitmap = frame_or_error.value().image;
+            }
         }
 
         if (!bitmap) {
diff --git a/Userland/Libraries/LibGUI/ImageWidget.cpp b/Userland/Libraries/LibGUI/ImageWidget.cpp
index dcf4f16d250..03cc58e2715 100644
--- a/Userland/Libraries/LibGUI/ImageWidget.cpp
+++ b/Userland/Libraries/LibGUI/ImageWidget.cpp
@@ -56,7 +56,7 @@ void ImageWidget::animate()
 {
     m_current_frame_index = (m_current_frame_index + 1) % m_image_decoder->frame_count();
 
-    const auto& current_frame = m_image_decoder->frame(m_current_frame_index);
+    auto current_frame = m_image_decoder->frame(m_current_frame_index).release_value_but_fixme_should_propagate_errors();
     set_bitmap(current_frame.image);
 
     if (current_frame.duration != m_timer->interval()) {
@@ -81,14 +81,14 @@ void ImageWidget::load_from_file(StringView path)
     m_image_decoder = Gfx::ImageDecoder::try_create(mapped_file.bytes());
     VERIFY(m_image_decoder);
 
-    auto frame = m_image_decoder->frame(0);
+    auto frame = m_image_decoder->frame(0).release_value_but_fixme_should_propagate_errors();
     auto bitmap = frame.image;
     VERIFY(bitmap);
 
     set_bitmap(bitmap);
 
     if (m_image_decoder->is_animated() && m_image_decoder->frame_count() > 1) {
-        const auto& first_frame = m_image_decoder->frame(0);
+        auto first_frame = m_image_decoder->frame(0).release_value_but_fixme_should_propagate_errors();
         m_timer->set_interval(first_frame.duration);
         m_timer->on_timeout = [this] { animate(); };
         m_timer->start();
diff --git a/Userland/Libraries/LibGfx/BMPLoader.cpp b/Userland/Libraries/LibGfx/BMPLoader.cpp
index 04e406ac76e..d642117177b 100644
--- a/Userland/Libraries/LibGfx/BMPLoader.cpp
+++ b/Userland/Libraries/LibGfx/BMPLoader.cpp
@@ -1354,19 +1354,19 @@ size_t BMPImageDecoderPlugin::frame_count()
     return 1;
 }
 
-ImageFrameDescriptor BMPImageDecoderPlugin::frame(size_t i)
+ErrorOr<ImageFrameDescriptor> BMPImageDecoderPlugin::frame(size_t index)
 {
-    if (i > 0)
-        return {};
+    if (index > 0)
+        return Error::from_string_literal("BMPImageDecoderPlugin: Invalid frame index"sv);
 
     if (m_context->state == BMPLoadingContext::State::Error)
-        return {};
+        return Error::from_string_literal("BMPImageDecoderPlugin: Decoding failed"sv);
 
     if (m_context->state < BMPLoadingContext::State::PixelDataDecoded && !decode_bmp_pixel_data(*m_context))
-        return {};
+        return Error::from_string_literal("BMPImageDecoderPlugin: Decoding failed"sv);
 
     VERIFY(m_context->bitmap);
-    return { m_context->bitmap, 0 };
+    return ImageFrameDescriptor { m_context->bitmap, 0 };
 }
 
 }
diff --git a/Userland/Libraries/LibGfx/BMPLoader.h b/Userland/Libraries/LibGfx/BMPLoader.h
index f848ac4dd5b..a690f326190 100644
--- a/Userland/Libraries/LibGfx/BMPLoader.h
+++ b/Userland/Libraries/LibGfx/BMPLoader.h
@@ -24,7 +24,7 @@ public:
     virtual bool is_animated() override;
     virtual size_t loop_count() override;
     virtual size_t frame_count() override;
-    virtual ImageFrameDescriptor frame(size_t i) override;
+    virtual ErrorOr<ImageFrameDescriptor> frame(size_t index) override;
 
 private:
     OwnPtr<BMPLoadingContext> m_context;
diff --git a/Userland/Libraries/LibGfx/Bitmap.cpp b/Userland/Libraries/LibGfx/Bitmap.cpp
index 5d2c6e823be..06f2afe4191 100644
--- a/Userland/Libraries/LibGfx/Bitmap.cpp
+++ b/Userland/Libraries/LibGfx/Bitmap.cpp
@@ -136,7 +136,8 @@ ErrorOr<NonnullRefPtr<Bitmap>> Bitmap::try_load_from_fd_and_close(int fd, String
 {
     auto file = TRY(MappedFile::map_from_fd_and_close(fd, path));
     if (auto decoder = ImageDecoder::try_create(file->bytes())) {
-        if (auto bitmap = decoder->frame(0).image)
+        auto frame = TRY(decoder->frame(0));
+        if (auto& bitmap = frame.image)
             return bitmap.release_nonnull();
     }
 
diff --git a/Userland/Libraries/LibGfx/DDSLoader.cpp b/Userland/Libraries/LibGfx/DDSLoader.cpp
index 915990465be..482184ff429 100644
--- a/Userland/Libraries/LibGfx/DDSLoader.cpp
+++ b/Userland/Libraries/LibGfx/DDSLoader.cpp
@@ -997,22 +997,22 @@ size_t DDSImageDecoderPlugin::frame_count()
     return 1;
 }
 
-ImageFrameDescriptor DDSImageDecoderPlugin::frame(size_t i)
+ErrorOr<ImageFrameDescriptor> DDSImageDecoderPlugin::frame(size_t index)
 {
-    if (i > 0)
-        return {};
+    if (index > 0)
+        return Error::from_string_literal("DDSImageDecoderPlugin: Invalid frame index"sv);
 
     if (m_context->state == DDSLoadingContext::State::Error)
-        return {};
+        return Error::from_string_literal("DDSImageDecoderPlugin: Decoding failed"sv);
 
     if (m_context->state < DDSLoadingContext::State::BitmapDecoded) {
         bool success = decode_dds(*m_context);
         if (!success)
-            return {};
+            return Error::from_string_literal("DDSImageDecoderPlugin: Decoding failed"sv);
     }
 
     VERIFY(m_context->bitmap);
-    return { m_context->bitmap, 0 };
+    return ImageFrameDescriptor { m_context->bitmap, 0 };
 }
 
 }
diff --git a/Userland/Libraries/LibGfx/DDSLoader.h b/Userland/Libraries/LibGfx/DDSLoader.h
index 1ba87da25e8..a313d7d3e2b 100644
--- a/Userland/Libraries/LibGfx/DDSLoader.h
+++ b/Userland/Libraries/LibGfx/DDSLoader.h
@@ -245,7 +245,7 @@ public:
     virtual bool is_animated() override;
     virtual size_t loop_count() override;
     virtual size_t frame_count() override;
-    virtual ImageFrameDescriptor frame(size_t i) override;
+    virtual ErrorOr<ImageFrameDescriptor> frame(size_t index) override;
 
 private:
     OwnPtr<DDSLoadingContext> m_context;
diff --git a/Userland/Libraries/LibGfx/GIFLoader.cpp b/Userland/Libraries/LibGfx/GIFLoader.cpp
index 49f7a0b1c83..00892eb61a6 100644
--- a/Userland/Libraries/LibGfx/GIFLoader.cpp
+++ b/Userland/Libraries/LibGfx/GIFLoader.cpp
@@ -691,36 +691,30 @@ size_t GIFImageDecoderPlugin::frame_count()
     return m_context->images.size();
 }
 
-ImageFrameDescriptor GIFImageDecoderPlugin::frame(size_t i)
+ErrorOr<ImageFrameDescriptor> GIFImageDecoderPlugin::frame(size_t index)
 {
     if (m_context->error_state >= GIFLoadingContext::ErrorState::FailedToDecodeAnyFrame) {
-        return {};
+        return Error::from_string_literal("GIFImageDecoderPlugin: Decoding failed"sv);
     }
 
     if (m_context->state < GIFLoadingContext::State::FrameDescriptorsLoaded) {
         if (!load_gif_frame_descriptors(*m_context)) {
             m_context->error_state = GIFLoadingContext::ErrorState::FailedToLoadFrameDescriptors;
-            return {};
+            return Error::from_string_literal("GIFImageDecoderPlugin: Decoding failed"sv);
         }
     }
 
-    if (m_context->error_state == GIFLoadingContext::ErrorState::NoError && !decode_frame(*m_context, i)) {
+    if (m_context->error_state == GIFLoadingContext::ErrorState::NoError && !decode_frame(*m_context, index)) {
         if (m_context->state < GIFLoadingContext::State::FrameComplete || !decode_frame(*m_context, 0)) {
             m_context->error_state = GIFLoadingContext::ErrorState::FailedToDecodeAnyFrame;
-            return {};
+            return Error::from_string_literal("GIFImageDecoderPlugin: Decoding failed"sv);
         }
         m_context->error_state = GIFLoadingContext::ErrorState::FailedToDecodeAllFrames;
     }
 
-    auto image_or_error = m_context->frame_buffer->clone();
-    if (image_or_error.is_error()) {
-        m_context->error_state = GIFLoadingContext::ErrorState::FailedToDecodeAllFrames;
-        return {};
-    }
-
     ImageFrameDescriptor frame {};
-    frame.image = image_or_error.release_value_but_fixme_should_propagate_errors();
-    frame.duration = m_context->images.at(i).duration * 10;
+    frame.image = TRY(m_context->frame_buffer->clone());
+    frame.duration = m_context->images.at(index).duration * 10;
 
     if (frame.duration <= 10) {
         frame.duration = 100;
diff --git a/Userland/Libraries/LibGfx/GIFLoader.h b/Userland/Libraries/LibGfx/GIFLoader.h
index d3960cd8d09..e42de301067 100644
--- a/Userland/Libraries/LibGfx/GIFLoader.h
+++ b/Userland/Libraries/LibGfx/GIFLoader.h
@@ -25,7 +25,7 @@ public:
     virtual bool is_animated() override;
     virtual size_t loop_count() override;
     virtual size_t frame_count() override;
-    virtual ImageFrameDescriptor frame(size_t i) override;
+    virtual ErrorOr<ImageFrameDescriptor> frame(size_t index) override;
 
 private:
     OwnPtr<GIFLoadingContext> m_context;
diff --git a/Userland/Libraries/LibGfx/ICOLoader.cpp b/Userland/Libraries/LibGfx/ICOLoader.cpp
index 4056d7c0f0d..1ff99ad6c49 100644
--- a/Userland/Libraries/LibGfx/ICOLoader.cpp
+++ b/Userland/Libraries/LibGfx/ICOLoader.cpp
@@ -263,11 +263,12 @@ static bool load_ico_bitmap(ICOLoadingContext& context, Optional<size_t> index)
 
     PNGImageDecoderPlugin png_decoder(context.data + desc.offset, desc.size);
     if (png_decoder.sniff()) {
-        desc.bitmap = png_decoder.frame(0).image;
-        if (!desc.bitmap) {
+        auto decoded_png_frame = png_decoder.frame(0);
+        if (!decoded_png_frame.is_error() || !decoded_png_frame.value().image) {
             dbgln_if(ICO_DEBUG, "load_ico_bitmap: failed to load PNG encoded image index: {}", real_index);
             return false;
         }
+        desc.bitmap = decoded_png_frame.value().image;
         return true;
     } else {
         if (!load_ico_bmp(context, desc)) {
@@ -338,26 +339,26 @@ size_t ICOImageDecoderPlugin::frame_count()
     return 1;
 }
 
-ImageFrameDescriptor ICOImageDecoderPlugin::frame(size_t i)
+ErrorOr<ImageFrameDescriptor> ICOImageDecoderPlugin::frame(size_t index)
 {
-    if (i > 0)
-        return {};
+    if (index > 0)
+        return Error::from_string_literal("ICOImageDecoderPlugin: Invalid frame index"sv);
 
     if (m_context->state == ICOLoadingContext::State::Error)
-        return {};
+        return Error::from_string_literal("ICOImageDecoderPlugin: Decoding failed"sv);
 
     if (m_context->state < ICOLoadingContext::State::BitmapDecoded) {
         // NOTE: This forces the chunk decoding to happen.
         bool success = load_ico_bitmap(*m_context, {});
         if (!success) {
             m_context->state = ICOLoadingContext::State::Error;
-            return {};
+            return Error::from_string_literal("ICOImageDecoderPlugin: Decoding failed"sv);
         }
         m_context->state = ICOLoadingContext::State::BitmapDecoded;
     }
 
     VERIFY(m_context->images[m_context->largest_index].bitmap);
-    return { m_context->images[m_context->largest_index].bitmap, 0 };
+    return ImageFrameDescriptor { m_context->images[m_context->largest_index].bitmap, 0 };
 }
 
 }
diff --git a/Userland/Libraries/LibGfx/ICOLoader.h b/Userland/Libraries/LibGfx/ICOLoader.h
index 0340b6249e8..b99afcd6624 100644
--- a/Userland/Libraries/LibGfx/ICOLoader.h
+++ b/Userland/Libraries/LibGfx/ICOLoader.h
@@ -24,7 +24,7 @@ public:
     virtual bool is_animated() override;
     virtual size_t loop_count() override;
     virtual size_t frame_count() override;
-    virtual ImageFrameDescriptor frame(size_t i) override;
+    virtual ErrorOr<ImageFrameDescriptor> frame(size_t index) override;
 
 private:
     OwnPtr<ICOLoadingContext> m_context;
diff --git a/Userland/Libraries/LibGfx/ImageDecoder.h b/Userland/Libraries/LibGfx/ImageDecoder.h
index 7641402b991..62a164921dc 100644
--- a/Userland/Libraries/LibGfx/ImageDecoder.h
+++ b/Userland/Libraries/LibGfx/ImageDecoder.h
@@ -39,7 +39,7 @@ public:
     virtual bool is_animated() = 0;
     virtual size_t loop_count() = 0;
     virtual size_t frame_count() = 0;
-    virtual ImageFrameDescriptor frame(size_t i) = 0;
+    virtual ErrorOr<ImageFrameDescriptor> frame(size_t index) = 0;
 
 protected:
     ImageDecoderPlugin() { }
@@ -59,7 +59,7 @@ public:
     bool is_animated() const { return m_plugin->is_animated(); }
     size_t loop_count() const { return m_plugin->loop_count(); }
     size_t frame_count() const { return m_plugin->frame_count(); }
-    ImageFrameDescriptor frame(size_t i) const { return m_plugin->frame(i); }
+    ErrorOr<ImageFrameDescriptor> frame(size_t index) const { return m_plugin->frame(index); }
 
 private:
     explicit ImageDecoder(NonnullOwnPtr<ImageDecoderPlugin>);
diff --git a/Userland/Libraries/LibGfx/JPGLoader.cpp b/Userland/Libraries/LibGfx/JPGLoader.cpp
index c32dac6cec5..650684bf88b 100644
--- a/Userland/Libraries/LibGfx/JPGLoader.cpp
+++ b/Userland/Libraries/LibGfx/JPGLoader.cpp
@@ -1276,23 +1276,23 @@ size_t JPGImageDecoderPlugin::frame_count()
     return 1;
 }
 
-ImageFrameDescriptor JPGImageDecoderPlugin::frame(size_t i)
+ErrorOr<ImageFrameDescriptor> JPGImageDecoderPlugin::frame(size_t index)
 {
-    if (i > 0)
-        return {};
+    if (index > 0)
+        return Error::from_string_literal("JPGImageDecoderPlugin: Invalid frame index"sv);
 
     if (m_context->state == JPGLoadingContext::State::Error)
-        return {};
+        return Error::from_string_literal("JPGImageDecoderPlugin: Decoding failed"sv);
 
     if (m_context->state < JPGLoadingContext::State::BitmapDecoded) {
         if (!decode_jpg(*m_context)) {
             m_context->state = JPGLoadingContext::State::Error;
-            return {};
+            return Error::from_string_literal("JPGImageDecoderPlugin: Decoding failed"sv);
         }
         m_context->state = JPGLoadingContext::State::BitmapDecoded;
     }
 
-    return { m_context->bitmap, 0 };
+    return ImageFrameDescriptor { m_context->bitmap, 0 };
 }
 
 }
diff --git a/Userland/Libraries/LibGfx/JPGLoader.h b/Userland/Libraries/LibGfx/JPGLoader.h
index 13573ea0eea..08133191776 100644
--- a/Userland/Libraries/LibGfx/JPGLoader.h
+++ b/Userland/Libraries/LibGfx/JPGLoader.h
@@ -23,7 +23,7 @@ public:
     virtual bool is_animated() override;
     virtual size_t loop_count() override;
     virtual size_t frame_count() override;
-    virtual ImageFrameDescriptor frame(size_t i) override;
+    virtual ErrorOr<ImageFrameDescriptor> frame(size_t index) override;
 
 private:
     OwnPtr<JPGLoadingContext> m_context;
diff --git a/Userland/Libraries/LibGfx/PBMLoader.cpp b/Userland/Libraries/LibGfx/PBMLoader.cpp
index 4b2f45c8b36..8f2ff976b95 100644
--- a/Userland/Libraries/LibGfx/PBMLoader.cpp
+++ b/Userland/Libraries/LibGfx/PBMLoader.cpp
@@ -162,22 +162,22 @@ size_t PBMImageDecoderPlugin::frame_count()
     return 1;
 }
 
-ImageFrameDescriptor PBMImageDecoderPlugin::frame(size_t i)
+ErrorOr<ImageFrameDescriptor> PBMImageDecoderPlugin::frame(size_t index)
 {
-    if (i > 0)
-        return {};
+    if (index > 0)
+        return Error::from_string_literal("PBMImageDecoderPlugin: Invalid frame index"sv);
 
     if (m_context->state == PBMLoadingContext::State::Error)
-        return {};
+        return Error::from_string_literal("PBMImageDecoderPlugin: Decoding failed"sv);
 
     if (m_context->state < PBMLoadingContext::State::Decoded) {
         bool success = decode(*m_context);
         if (!success)
-            return {};
+            return Error::from_string_literal("PBMImageDecoderPlugin: Decoding failed"sv);
     }
 
     VERIFY(m_context->bitmap);
-    return { m_context->bitmap, 0 };
+    return ImageFrameDescriptor { m_context->bitmap, 0 };
 }
 
 }
diff --git a/Userland/Libraries/LibGfx/PBMLoader.h b/Userland/Libraries/LibGfx/PBMLoader.h
index ce7c26558bc..29e9da93108 100644
--- a/Userland/Libraries/LibGfx/PBMLoader.h
+++ b/Userland/Libraries/LibGfx/PBMLoader.h
@@ -27,7 +27,7 @@ public:
     virtual bool is_animated() override;
     virtual size_t loop_count() override;
     virtual size_t frame_count() override;
-    virtual ImageFrameDescriptor frame(size_t i) override;
+    virtual ErrorOr<ImageFrameDescriptor> frame(size_t index) override;
 
 private:
     OwnPtr<PBMLoadingContext> m_context;
diff --git a/Userland/Libraries/LibGfx/PGMLoader.cpp b/Userland/Libraries/LibGfx/PGMLoader.cpp
index 837563e27bd..9280f7f5284 100644
--- a/Userland/Libraries/LibGfx/PGMLoader.cpp
+++ b/Userland/Libraries/LibGfx/PGMLoader.cpp
@@ -165,22 +165,22 @@ size_t PGMImageDecoderPlugin::frame_count()
     return 1;
 }
 
-ImageFrameDescriptor PGMImageDecoderPlugin::frame(size_t i)
+ErrorOr<ImageFrameDescriptor> PGMImageDecoderPlugin::frame(size_t index)
 {
-    if (i > 0)
-        return {};
+    if (index > 0)
+        return Error::from_string_literal("PGMImageDecoderPlugin: Invalid frame index"sv);
 
     if (m_context->state == PGMLoadingContext::State::Error)
-        return {};
+        return Error::from_string_literal("PGMImageDecoderPlugin: Decoding failed"sv);
 
     if (m_context->state < PGMLoadingContext::State::Decoded) {
         bool success = decode(*m_context);
         if (!success)
-            return {};
+            return Error::from_string_literal("PGMImageDecoderPlugin: Decoding failed"sv);
     }
 
     VERIFY(m_context->bitmap);
-    return { m_context->bitmap, 0 };
+    return ImageFrameDescriptor { m_context->bitmap, 0 };
 }
 
 }
diff --git a/Userland/Libraries/LibGfx/PGMLoader.h b/Userland/Libraries/LibGfx/PGMLoader.h
index f3190ef709c..ad97aa80853 100644
--- a/Userland/Libraries/LibGfx/PGMLoader.h
+++ b/Userland/Libraries/LibGfx/PGMLoader.h
@@ -27,7 +27,7 @@ public:
     virtual bool is_animated() override;
     virtual size_t loop_count() override;
     virtual size_t frame_count() override;
-    virtual ImageFrameDescriptor frame(size_t i) override;
+    virtual ErrorOr<ImageFrameDescriptor> frame(size_t index) override;
 
 private:
     OwnPtr<PGMLoadingContext> m_context;
diff --git a/Userland/Libraries/LibGfx/PNGLoader.cpp b/Userland/Libraries/LibGfx/PNGLoader.cpp
index 13220dfe531..eb0557f3ba7 100644
--- a/Userland/Libraries/LibGfx/PNGLoader.cpp
+++ b/Userland/Libraries/LibGfx/PNGLoader.cpp
@@ -964,23 +964,23 @@ size_t PNGImageDecoderPlugin::frame_count()
     return 1;
 }
 
-ImageFrameDescriptor PNGImageDecoderPlugin::frame(size_t i)
+ErrorOr<ImageFrameDescriptor> PNGImageDecoderPlugin::frame(size_t index)
 {
-    if (i > 0)
-        return {};
+    if (index > 0)
+        return Error::from_string_literal("PNGImageDecoderPlugin: Invalid frame index"sv);
 
     if (m_context->state == PNGLoadingContext::State::Error)
-        return {};
+        return Error::from_string_literal("PNGImageDecoderPlugin: Decoding failed"sv);
 
     if (m_context->state < PNGLoadingContext::State::BitmapDecoded) {
         // NOTE: This forces the chunk decoding to happen.
         bool success = decode_png_bitmap(*m_context);
         if (!success)
-            return {};
+            return Error::from_string_literal("PNGImageDecoderPlugin: Decoding failed"sv);
     }
 
     VERIFY(m_context->bitmap);
-    return { m_context->bitmap, 0 };
+    return ImageFrameDescriptor { m_context->bitmap, 0 };
 }
 
 }
diff --git a/Userland/Libraries/LibGfx/PNGLoader.h b/Userland/Libraries/LibGfx/PNGLoader.h
index 77c4b19c3d1..9f7c0e7b6c4 100644
--- a/Userland/Libraries/LibGfx/PNGLoader.h
+++ b/Userland/Libraries/LibGfx/PNGLoader.h
@@ -24,7 +24,7 @@ public:
     virtual bool is_animated() override;
     virtual size_t loop_count() override;
     virtual size_t frame_count() override;
-    virtual ImageFrameDescriptor frame(size_t i) override;
+    virtual ErrorOr<ImageFrameDescriptor> frame(size_t index) override;
 
 private:
     OwnPtr<PNGLoadingContext> m_context;
diff --git a/Userland/Libraries/LibGfx/PPMLoader.cpp b/Userland/Libraries/LibGfx/PPMLoader.cpp
index 5316c27ab04..c05789f344e 100644
--- a/Userland/Libraries/LibGfx/PPMLoader.cpp
+++ b/Userland/Libraries/LibGfx/PPMLoader.cpp
@@ -169,22 +169,22 @@ size_t PPMImageDecoderPlugin::frame_count()
     return 1;
 }
 
-ImageFrameDescriptor PPMImageDecoderPlugin::frame(size_t i)
+ErrorOr<ImageFrameDescriptor> PPMImageDecoderPlugin::frame(size_t index)
 {
-    if (i > 0)
-        return {};
+    if (index > 0)
+        return Error::from_string_literal("PPMImageDecoderPlugin: Invalid frame index"sv);
 
     if (m_context->state == PPMLoadingContext::State::Error)
-        return {};
+        return Error::from_string_literal("PGMImageDecoderPlugin: Decoding failed"sv);
 
     if (m_context->state < PPMLoadingContext::State::Decoded) {
         bool success = decode(*m_context);
         if (!success)
-            return {};
+            return Error::from_string_literal("PGMImageDecoderPlugin: Decoding failed"sv);
     }
 
     VERIFY(m_context->bitmap);
-    return { m_context->bitmap, 0 };
+    return ImageFrameDescriptor { m_context->bitmap, 0 };
 }
 
 }
diff --git a/Userland/Libraries/LibGfx/PPMLoader.h b/Userland/Libraries/LibGfx/PPMLoader.h
index 0a9b876fba7..19dc9e7ed8e 100644
--- a/Userland/Libraries/LibGfx/PPMLoader.h
+++ b/Userland/Libraries/LibGfx/PPMLoader.h
@@ -27,7 +27,7 @@ public:
     virtual bool is_animated() override;
     virtual size_t loop_count() override;
     virtual size_t frame_count() override;
-    virtual ImageFrameDescriptor frame(size_t i) override;
+    virtual ErrorOr<ImageFrameDescriptor> frame(size_t index) override;
 
 private:
     OwnPtr<PPMLoadingContext> m_context;
diff --git a/Userland/Services/ImageDecoder/ClientConnection.cpp b/Userland/Services/ImageDecoder/ClientConnection.cpp
index 3469a4ac83d..d1804333802 100644
--- a/Userland/Services/ImageDecoder/ClientConnection.cpp
+++ b/Userland/Services/ImageDecoder/ClientConnection.cpp
@@ -52,12 +52,12 @@ Messages::ImageDecoderServer::DecodeImageResponse ClientConnection::decode_image
     Vector<Gfx::ShareableBitmap> bitmaps;
     Vector<u32> durations;
     for (size_t i = 0; i < decoder->frame_count(); ++i) {
-        auto frame = decoder->frame(i);
-        if (frame.image)
-            bitmaps.append(frame.image->to_shareable_bitmap());
-        else
+        auto frame_or_error = decoder->frame(i);
+        if (frame_or_error.is_error() || !frame_or_error.value().image)
             bitmaps.append(Gfx::ShareableBitmap {});
-        durations.append(frame.duration);
+        else
+            bitmaps.append(frame_or_error.value().image->to_shareable_bitmap());
+        durations.append(frame_or_error.value().duration);
     }
 
     return { decoder->is_animated(), static_cast<u32>(decoder->loop_count()), bitmaps, durations };
diff --git a/Userland/Services/SpiceAgent/SpiceAgent.cpp b/Userland/Services/SpiceAgent/SpiceAgent.cpp
index ce158cb31bb..5d7f362c6a8 100644
--- a/Userland/Services/SpiceAgent/SpiceAgent.cpp
+++ b/Userland/Services/SpiceAgent/SpiceAgent.cpp
@@ -140,17 +140,18 @@ void SpiceAgent::on_message_received()
             m_clipboard_connection.async_set_clipboard_data(anon_buffer, "text/plain", {});
             return;
         } else {
-            RefPtr<Gfx::Bitmap> bitmap;
+            ErrorOr<Gfx::ImageFrameDescriptor> frame_or_error = Gfx::ImageFrameDescriptor {};
             if (type == ClipboardType::PNG) {
-                bitmap = Gfx::PNGImageDecoderPlugin(data_buffer.data(), data_buffer.size()).frame(0).image;
+                frame_or_error = Gfx::PNGImageDecoderPlugin(data_buffer.data(), data_buffer.size()).frame(0);
             } else if (type == ClipboardType::BMP) {
-                bitmap = Gfx::BMPImageDecoderPlugin(data_buffer.data(), data_buffer.size()).frame(0).image;
+                frame_or_error = Gfx::BMPImageDecoderPlugin(data_buffer.data(), data_buffer.size()).frame(0);
             } else if (type == ClipboardType::JPG) {
-                bitmap = Gfx::JPGImageDecoderPlugin(data_buffer.data(), data_buffer.size()).frame(0).image;
+                frame_or_error = Gfx::JPGImageDecoderPlugin(data_buffer.data(), data_buffer.size()).frame(0);
             } else {
                 dbgln("Unknown clipboard type: {}", (u32)type);
                 return;
             }
+            auto const& bitmap = frame_or_error.value().image;
             m_clipboard_connection.set_bitmap(*bitmap);
         }
         break;