LibGfx: Improve ImageDecoder construction

Previously, ImageDecoder::create() would return a NonnullRefPtr and
could not "fail", although the returned decoder may be "invalid" which
you then had to check anyway.

The new interface looks like this:

    static RefPtr<Gfx::ImageDecoder> try_create(ReadonlyBytes);

This simplifies ImageDecoder since it no longer has to worry about its
validity. Client code gets slightly clearer as well.

6 files changed, 77 insertions, 58 deletions

diff --git a/Userland/Libraries/LibGUI/ImageWidget.cpp b/Userland/Libraries/LibGUI/ImageWidget.cpp
index fec51759e9..887fe2938d 100644
--- a/Userland/Libraries/LibGUI/ImageWidget.cpp
+++ b/Userland/Libraries/LibGUI/ImageWidget.cpp
@@ -78,7 +78,9 @@ void ImageWidget::load_from_file(const StringView& path)
         return;
 
     auto& mapped_file = *file_or_error.value();
-    m_image_decoder = Gfx::ImageDecoder::create((const u8*)mapped_file.data(), mapped_file.size());
+    m_image_decoder = Gfx::ImageDecoder::try_create(mapped_file.bytes());
+    VERIFY(m_image_decoder);
+
     auto bitmap = m_image_decoder->bitmap();
     VERIFY(bitmap);
 
diff --git a/Userland/Libraries/LibGfx/ImageDecoder.cpp b/Userland/Libraries/LibGfx/ImageDecoder.cpp
index 3e0690189c..2d7f89554e 100644
--- a/Userland/Libraries/LibGfx/ImageDecoder.cpp
+++ b/Userland/Libraries/LibGfx/ImageDecoder.cpp
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2018-2020, Andreas Kling <kling@serenityos.org>
+ * Copyright (c) 2018-2021, Andreas Kling <kling@serenityos.org>
  *
  * SPDX-License-Identifier: BSD-2-Clause
  */
@@ -17,45 +17,61 @@
 
 namespace Gfx {
 
-ImageDecoder::ImageDecoder(const u8* data, size_t size)
+RefPtr<ImageDecoder> ImageDecoder::try_create(ReadonlyBytes bytes)
 {
-    m_plugin = make<PNGImageDecoderPlugin>(data, size);
-    if (m_plugin->sniff())
-        return;
+    auto* data = bytes.data();
+    auto size = bytes.size();
 
-    m_plugin = make<GIFImageDecoderPlugin>(data, size);
-    if (m_plugin->sniff())
-        return;
+    auto plugin = [](auto* data, auto size) -> OwnPtr<ImageDecoderPlugin> {
+        OwnPtr<ImageDecoderPlugin> plugin;
 
-    m_plugin = make<BMPImageDecoderPlugin>(data, size);
-    if (m_plugin->sniff())
-        return;
+        plugin = make<PNGImageDecoderPlugin>(data, size);
+        if (plugin->sniff())
+            return plugin;
 
-    m_plugin = make<PBMImageDecoderPlugin>(data, size);
-    if (m_plugin->sniff())
-        return;
+        plugin = make<GIFImageDecoderPlugin>(data, size);
+        if (plugin->sniff())
+            return plugin;
 
-    m_plugin = make<PGMImageDecoderPlugin>(data, size);
-    if (m_plugin->sniff())
-        return;
+        plugin = make<BMPImageDecoderPlugin>(data, size);
+        if (plugin->sniff())
+            return plugin;
 
-    m_plugin = make<PPMImageDecoderPlugin>(data, size);
-    if (m_plugin->sniff())
-        return;
+        plugin = make<PBMImageDecoderPlugin>(data, size);
+        if (plugin->sniff())
+            return plugin;
 
-    m_plugin = make<ICOImageDecoderPlugin>(data, size);
-    if (m_plugin->sniff())
-        return;
+        plugin = make<PGMImageDecoderPlugin>(data, size);
+        if (plugin->sniff())
+            return plugin;
 
-    m_plugin = make<JPGImageDecoderPlugin>(data, size);
-    if (m_plugin->sniff())
-        return;
+        plugin = make<PPMImageDecoderPlugin>(data, size);
+        if (plugin->sniff())
+            return plugin;
 
-    m_plugin = make<DDSImageDecoderPlugin>(data, size);
-    if (m_plugin->sniff())
-        return;
+        plugin = make<ICOImageDecoderPlugin>(data, size);
+        if (plugin->sniff())
+            return plugin;
 
-    m_plugin = nullptr;
+        plugin = make<JPGImageDecoderPlugin>(data, size);
+        if (plugin->sniff())
+            return plugin;
+
+        plugin = make<DDSImageDecoderPlugin>(data, size);
+        if (plugin->sniff())
+            return plugin;
+
+        return {};
+    }(data, size);
+
+    if (!plugin)
+        return {};
+    return adopt_ref_if_nonnull(new (nothrow) ImageDecoder(plugin.release_nonnull()));
+}
+
+ImageDecoder::ImageDecoder(NonnullOwnPtr<ImageDecoderPlugin> plugin)
+    : m_plugin(move(plugin))
+{
 }
 
 ImageDecoder::~ImageDecoder()
@@ -64,8 +80,6 @@ ImageDecoder::~ImageDecoder()
 
 RefPtr<Gfx::Bitmap> ImageDecoder::bitmap() const
 {
-    if (!m_plugin)
-        return nullptr;
     return m_plugin->bitmap();
 }
 
diff --git a/Userland/Libraries/LibGfx/ImageDecoder.h b/Userland/Libraries/LibGfx/ImageDecoder.h
index a22dc3b584..964cdee23f 100644
--- a/Userland/Libraries/LibGfx/ImageDecoder.h
+++ b/Userland/Libraries/LibGfx/ImageDecoder.h
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2018-2020, Andreas Kling <kling@serenityos.org>
+ * Copyright (c) 2018-2021, Andreas Kling <kling@serenityos.org>
  *
  * SPDX-License-Identifier: BSD-2-Clause
  */
@@ -48,32 +48,25 @@ protected:
 
 class ImageDecoder : public RefCounted<ImageDecoder> {
 public:
-    static NonnullRefPtr<ImageDecoder> create(const u8* data, size_t size) { return adopt_ref(*new ImageDecoder(data, size)); }
-    static NonnullRefPtr<ImageDecoder> create(const ByteBuffer& data) { return adopt_ref(*new ImageDecoder(data.data(), data.size())); }
+    static RefPtr<ImageDecoder> try_create(ReadonlyBytes);
     ~ImageDecoder();
 
-    bool is_valid() const { return m_plugin; }
-
-    IntSize size() const { return m_plugin ? m_plugin->size() : IntSize(); }
+    IntSize size() const { return m_plugin->size(); }
     int width() const { return size().width(); }
     int height() const { return size().height(); }
     RefPtr<Gfx::Bitmap> bitmap() const;
-    void set_volatile()
-    {
-        if (m_plugin)
-            m_plugin->set_volatile();
-    }
-    [[nodiscard]] bool set_nonvolatile(bool& was_purged) { return m_plugin ? m_plugin->set_nonvolatile(was_purged) : false; }
-    bool sniff() const { return m_plugin ? m_plugin->sniff() : false; }
-    bool is_animated() const { return m_plugin ? m_plugin->is_animated() : false; }
-    size_t loop_count() const { return m_plugin ? m_plugin->loop_count() : 0; }
-    size_t frame_count() const { return m_plugin ? m_plugin->frame_count() : 0; }
-    ImageFrameDescriptor frame(size_t i) const { return m_plugin ? m_plugin->frame(i) : ImageFrameDescriptor(); }
+    void set_volatile() { m_plugin->set_volatile(); }
+    [[nodiscard]] bool set_nonvolatile(bool& was_purged) { return m_plugin->set_nonvolatile(was_purged); }
+    bool sniff() const { return m_plugin->sniff(); }
+    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); }
 
 private:
-    ImageDecoder(const u8*, size_t);
+    explicit ImageDecoder(NonnullOwnPtr<ImageDecoderPlugin>);
 
-    mutable OwnPtr<ImageDecoderPlugin> m_plugin;
+    NonnullOwnPtr<ImageDecoderPlugin> mutable m_plugin;
 };
 
 }
diff --git a/Userland/Libraries/LibWeb/Loader/FrameLoader.cpp b/Userland/Libraries/LibWeb/Loader/FrameLoader.cpp
index 6db5f9b13c..1fc5755223 100644
--- a/Userland/Libraries/LibWeb/Loader/FrameLoader.cpp
+++ b/Userland/Libraries/LibWeb/Loader/FrameLoader.cpp
@@ -68,7 +68,9 @@ static bool build_text_document(DOM::Document& document, const ByteBuffer& data)
 
 static bool build_image_document(DOM::Document& document, const ByteBuffer& data)
 {
-    auto image_decoder = Gfx::ImageDecoder::create(data.data(), data.size());
+    auto image_decoder = Gfx::ImageDecoder::try_create(data.bytes());
+    if (!image_decoder)
+        return false;
     auto bitmap = image_decoder->bitmap();
     if (!bitmap)
         return false;
@@ -164,7 +166,11 @@ bool FrameLoader::load(const LoadRequest& request, Type type)
             favicon_url,
             [this, favicon_url](auto data, auto&, auto) {
                 dbgln("Favicon downloaded, {} bytes from {}", data.size(), favicon_url);
-                auto decoder = Gfx::ImageDecoder::create(data.data(), data.size());
+                auto decoder = Gfx::ImageDecoder::try_create(data);
+                if (!decoder) {
+                    dbgln("No image decoder plugin for favicon {}", favicon_url);
+                    return;
+                }
                 auto bitmap = decoder->bitmap();
                 if (!bitmap) {
                     dbgln("Could not decode favicon {}", favicon_url);
diff --git a/Userland/Services/ImageDecoder/ClientConnection.cpp b/Userland/Services/ImageDecoder/ClientConnection.cpp
index 08f75b930b..d47c4d46e9 100644
--- a/Userland/Services/ImageDecoder/ClientConnection.cpp
+++ b/Userland/Services/ImageDecoder/ClientConnection.cpp
@@ -37,7 +37,12 @@ Messages::ImageDecoderServer::DecodeImageResponse ClientConnection::decode_image
         return nullptr;
     }
 
-    auto decoder = Gfx::ImageDecoder::create(encoded_buffer.data<u8>(), encoded_buffer.size());
+    auto decoder = Gfx::ImageDecoder::try_create(ReadonlyBytes { encoded_buffer.data<u8>(), encoded_buffer.size() });
+
+    if (!decoder) {
+        dbgln_if(IMAGE_DECODER_DEBUG, "Could not find suitable image decoder plugin for data");
+        return { false, 0, Vector<Gfx::ShareableBitmap> {}, Vector<u32> {} };
+    }
 
     if (!decoder->frame_count()) {
         dbgln_if(IMAGE_DECODER_DEBUG, "Could not decode image from encoded data");
diff --git a/Userland/Utilities/file.cpp b/Userland/Utilities/file.cpp
index e35d676089..64290726de 100644
--- a/Userland/Utilities/file.cpp
+++ b/Userland/Utilities/file.cpp
@@ -28,9 +28,8 @@ static Optional<String> image_details(const String& description, const String& p
         return {};
 
     auto& mapped_file = *file_or_error.value();
-    auto image_decoder = Gfx::ImageDecoder::create((const u8*)mapped_file.data(), mapped_file.size());
-
-    if (!image_decoder->is_valid())
+    auto image_decoder = Gfx::ImageDecoder::try_create(mapped_file.bytes());
+    if (!image_decoder)
         return {};
 
     return String::formatted("{}, {} x {}", description, image_decoder->width(), image_decoder->height());