PixelPaint: Use ErrorOr<T> for Image and Layer creation helpers

7 files changed, 76 insertions, 113 deletions

diff --git a/Userland/Applications/PixelPaint/Image.cpp b/Userland/Applications/PixelPaint/Image.cpp
index e24e9a9e1e..dbbbdce429 100644
--- a/Userland/Applications/PixelPaint/Image.cpp
+++ b/Userland/Applications/PixelPaint/Image.cpp
@@ -25,15 +25,14 @@
 
 namespace PixelPaint {
 
-RefPtr<Image> Image::try_create_with_size(Gfx::IntSize const& size)
+ErrorOr<NonnullRefPtr<Image>> Image::try_create_with_size(Gfx::IntSize const& size)
 {
-    if (size.is_empty())
-        return nullptr;
+    VERIFY(!size.is_empty());
 
     if (size.width() > 16384 || size.height() > 16384)
-        return nullptr;
+        return Error::from_string_literal("Image size too large"sv);
 
-    return adopt_ref(*new Image(size));
+    return adopt_nonnull_ref_or_enomem(new (nothrow) Image(size));
 }
 
 Image::Image(Gfx::IntSize const& size)
@@ -56,42 +55,34 @@ void Image::paint_into(GUI::Painter& painter, Gfx::IntRect const& dest_rect) con
     }
 }
 
-RefPtr<Gfx::Bitmap> Image::try_decode_bitmap(ReadonlyBytes const& bitmap_data)
+ErrorOr<NonnullRefPtr<Gfx::Bitmap>> Image::try_decode_bitmap(ReadonlyBytes bitmap_data)
 {
     // Spawn a new ImageDecoder service process and connect to it.
     auto client = ImageDecoderClient::Client::construct();
 
     // FIXME: Find a way to avoid the memory copying here.
-    auto decoded_image_or_error = client->decode_image(bitmap_data);
-    if (!decoded_image_or_error.has_value())
-        return nullptr;
+    auto maybe_decoded_image = client->decode_image(bitmap_data);
+    if (!maybe_decoded_image.has_value())
+        return Error::from_string_literal("Image decode failed"sv);
 
     // FIXME: Support multi-frame images?
-    auto decoded_image = decoded_image_or_error.release_value();
+    auto decoded_image = maybe_decoded_image.release_value();
     if (decoded_image.frames.is_empty())
-        return nullptr;
-    return move(decoded_image.frames[0].bitmap);
+        return Error::from_string_literal("Image decode failed (no frames)"sv);
+    return decoded_image.frames[0].bitmap.release_nonnull();
 }
 
-RefPtr<Image> Image::try_create_from_bitmap(NonnullRefPtr<Gfx::Bitmap> bitmap)
+ErrorOr<NonnullRefPtr<Image>> Image::try_create_from_bitmap(NonnullRefPtr<Gfx::Bitmap> bitmap)
 {
-    auto image = try_create_with_size({ bitmap->width(), bitmap->height() });
-    if (!image)
-        return nullptr;
-
-    auto layer = Layer::try_create_with_bitmap(*image, *bitmap, "Background");
-    if (!layer)
-        return nullptr;
-
-    image->add_layer(layer.release_nonnull());
+    auto image = TRY(try_create_with_size({ bitmap->width(), bitmap->height() }));
+    auto layer = TRY(Layer::try_create_with_bitmap(*image, *bitmap, "Background"));
+    image->add_layer(move(layer));
     return image;
 }
 
-Result<NonnullRefPtr<Image>, String> Image::try_create_from_pixel_paint_json(JsonObject const& json)
+ErrorOr<NonnullRefPtr<Image>> Image::try_create_from_pixel_paint_json(JsonObject const& json)
 {
-    auto image = try_create_with_size({ json.get("width").to_i32(), json.get("height").to_i32() });
-    if (!image)
-        return String { "Image memory allocation failed" };
+    auto image = TRY(try_create_with_size({ json.get("width").to_i32(), json.get("height").to_i32() }));
 
     auto layers_value = json.get("layers");
     for (auto& layer_value : layers_value.as_array().values()) {
@@ -101,21 +92,16 @@ Result<NonnullRefPtr<Image>, String> Image::try_create_from_pixel_paint_json(Jso
         auto bitmap_base64_encoded = layer_object.get("bitmap").as_string();
         auto bitmap_data = decode_base64(bitmap_base64_encoded);
         if (!bitmap_data.has_value())
-            return String { "Base64 decode failed"sv };
-
-        auto bitmap = try_decode_bitmap(bitmap_data.value());
-        if (!bitmap)
-            return String { "Layer bitmap decode failed"sv };
+            return Error::from_string_literal("Base64 decode failed"sv);
 
-        auto layer = Layer::try_create_with_bitmap(*image, bitmap.release_nonnull(), name);
-        if (!layer)
-            return String { "Layer allocation failed"sv };
+        auto bitmap = TRY(try_decode_bitmap(bitmap_data.value()));
+        auto layer = TRY(Layer::try_create_with_bitmap(*image, move(bitmap), name));
 
         auto width = layer_object.get("width").to_i32();
         auto height = layer_object.get("height").to_i32();
 
         if (width != layer->size().width() || height != layer->size().height())
-            return String { "Decoded layer bitmap has wrong size"sv };
+            return Error::from_string_literal("Decoded layer bitmap has wrong size"sv);
 
         image->add_layer(*layer);
 
@@ -125,7 +111,7 @@ Result<NonnullRefPtr<Image>, String> Image::try_create_from_pixel_paint_json(Jso
         layer->set_selected(layer_object.get("selected").as_bool());
     }
 
-    return image.release_nonnull();
+    return image;
 }
 
 void Image::serialize_as_json(JsonObjectSerializer<StringBuilder>& json) const
@@ -238,29 +224,24 @@ void Image::add_layer(NonnullRefPtr<Layer> layer)
     did_modify_layer_stack();
 }
 
-RefPtr<Image> Image::take_snapshot() const
+ErrorOr<NonnullRefPtr<Image>> Image::take_snapshot() const
 {
-    auto snapshot = try_create_with_size(m_size);
-    if (!snapshot)
-        return nullptr;
+    auto snapshot = TRY(try_create_with_size(m_size));
     for (const auto& layer : m_layers) {
-        auto layer_snapshot = Layer::try_create_snapshot(*snapshot, layer);
-        if (!layer_snapshot)
-            return nullptr;
-        snapshot->add_layer(layer_snapshot.release_nonnull());
+        auto layer_snapshot = TRY(Layer::try_create_snapshot(*snapshot, layer));
+        snapshot->add_layer(move(layer_snapshot));
     }
     return snapshot;
 }
 
-void Image::restore_snapshot(Image const& snapshot)
+ErrorOr<void> Image::restore_snapshot(Image const& snapshot)
 {
     m_layers.clear();
     select_layer(nullptr);
 
     bool layer_selected = false;
-    for (const auto& snapshot_layer : snapshot.m_layers) {
-        auto layer = Layer::try_create_snapshot(*this, snapshot_layer);
-        VERIFY(layer);
+    for (auto const& snapshot_layer : snapshot.m_layers) {
+        auto layer = TRY(Layer::try_create_snapshot(*this, snapshot_layer));
         if (layer->is_selected()) {
             select_layer(layer.ptr());
             layer_selected = true;
@@ -272,6 +253,7 @@ void Image::restore_snapshot(Image const& snapshot)
         select_layer(&layer(0));
 
     did_modify_layer_stack();
+    return {};
 }
 
 size_t Image::index_of(Layer const& layer) const
@@ -470,14 +452,15 @@ void Image::did_change_rect(Gfx::IntRect const& a_modified_rect)
 }
 
 ImageUndoCommand::ImageUndoCommand(Image& image)
-    : m_snapshot(image.take_snapshot())
+    : m_snapshot(image.take_snapshot().release_value_but_fixme_should_propagate_errors())
     , m_image(image)
 {
 }
 
 void ImageUndoCommand::undo()
 {
-    m_image.restore_snapshot(*m_snapshot);
+    // FIXME: Handle errors.
+    (void)m_image.restore_snapshot(*m_snapshot);
 }
 
 void ImageUndoCommand::redo()
diff --git a/Userland/Applications/PixelPaint/Image.h b/Userland/Applications/PixelPaint/Image.h
index 414874e8d2..5b3e5274ca 100644
--- a/Userland/Applications/PixelPaint/Image.h
+++ b/Userland/Applications/PixelPaint/Image.h
@@ -46,11 +46,11 @@ protected:
 
 class Image : public RefCounted<Image> {
 public:
-    static RefPtr<Image> try_create_with_size(Gfx::IntSize const&);
-    static Result<NonnullRefPtr<Image>, String> try_create_from_pixel_paint_json(JsonObject const&);
-    static RefPtr<Image> try_create_from_bitmap(NonnullRefPtr<Gfx::Bitmap>);
+    static ErrorOr<NonnullRefPtr<Image>> try_create_with_size(Gfx::IntSize const&);
+    static ErrorOr<NonnullRefPtr<Image>> try_create_from_pixel_paint_json(JsonObject const&);
+    static ErrorOr<NonnullRefPtr<Image>> try_create_from_bitmap(NonnullRefPtr<Gfx::Bitmap>);
 
-    static RefPtr<Gfx::Bitmap> try_decode_bitmap(const ReadonlyBytes& bitmap_data);
+    static ErrorOr<NonnullRefPtr<Gfx::Bitmap>> try_decode_bitmap(ReadonlyBytes);
 
     // This generates a new Bitmap with the final image (all layers composed according to their attributes.)
     ErrorOr<NonnullRefPtr<Gfx::Bitmap>> try_compose_bitmap(Gfx::BitmapFormat format) const;
@@ -64,8 +64,8 @@ public:
     Gfx::IntRect rect() const { return { {}, m_size }; }
 
     void add_layer(NonnullRefPtr<Layer>);
-    RefPtr<Image> take_snapshot() const;
-    void restore_snapshot(Image const&);
+    ErrorOr<NonnullRefPtr<Image>> take_snapshot() const;
+    ErrorOr<void> restore_snapshot(Image const&);
 
     void paint_into(GUI::Painter&, Gfx::IntRect const& dest_rect) const;
 
diff --git a/Userland/Applications/PixelPaint/Layer.cpp b/Userland/Applications/PixelPaint/Layer.cpp
index 134737948f..3b1a1e1f8b 100644
--- a/Userland/Applications/PixelPaint/Layer.cpp
+++ b/Userland/Applications/PixelPaint/Layer.cpp
@@ -7,46 +7,41 @@
 #include "Layer.h"
 #include "Image.h"
 #include "Selection.h"
+#include <AK/RefPtr.h>
+#include <AK/Try.h>
 #include <LibGfx/Bitmap.h>
 
 namespace PixelPaint {
 
-RefPtr<Layer> Layer::try_create_with_size(Image& image, Gfx::IntSize const& size, String name)
+ErrorOr<NonnullRefPtr<Layer>> Layer::try_create_with_size(Image& image, Gfx::IntSize const& size, String name)
 {
-    if (size.is_empty())
-        return nullptr;
+    VERIFY(!size.is_empty());
 
     if (size.width() > 16384 || size.height() > 16384)
-        return nullptr;
-
-    auto bitmap_or_error = Gfx::Bitmap::try_create(Gfx::BitmapFormat::BGRA8888, size);
-    if (bitmap_or_error.is_error())
-        return nullptr;
+        return Error::from_string_literal("Layer size too large"sv);
 
-    return adopt_ref(*new Layer(image, bitmap_or_error.release_value_but_fixme_should_propagate_errors(), move(name)));
+    auto bitmap = TRY(Gfx::Bitmap::try_create(Gfx::BitmapFormat::BGRA8888, size));
+    return adopt_nonnull_ref_or_enomem(new (nothrow) Layer(image, move(bitmap), move(name)));
 }
 
-RefPtr<Layer> Layer::try_create_with_bitmap(Image& image, NonnullRefPtr<Gfx::Bitmap> bitmap, String name)
+ErrorOr<NonnullRefPtr<Layer>> Layer::try_create_with_bitmap(Image& image, NonnullRefPtr<Gfx::Bitmap> bitmap, String name)
 {
-    if (bitmap->size().is_empty())
-        return nullptr;
+    VERIFY(!bitmap->size().is_empty());
 
     if (bitmap->size().width() > 16384 || bitmap->size().height() > 16384)
-        return nullptr;
+        return Error::from_string_literal("Layer size too large"sv);
 
-    return adopt_ref(*new Layer(image, bitmap, move(name)));
+    return adopt_nonnull_ref_or_enomem(new (nothrow) Layer(image, bitmap, move(name)));
 }
 
-RefPtr<Layer> Layer::try_create_snapshot(Image& image, Layer const& layer)
+ErrorOr<NonnullRefPtr<Layer>> Layer::try_create_snapshot(Image& image, Layer const& layer)
 {
-    auto new_bitmap_or_error = layer.bitmap().clone();
-    if (new_bitmap_or_error.is_error())
-        return nullptr;
+    auto bitmap = TRY(layer.bitmap().clone());
+    auto snapshot = TRY(try_create_with_bitmap(image, move(bitmap), layer.name()));
 
-    auto snapshot = try_create_with_bitmap(image, new_bitmap_or_error.release_value_but_fixme_should_propagate_errors(), layer.name());
     /*
-        We set these properties directly because calling the setters might 
-        notify the image of an update on the newly created layer, but this 
+        We set these properties directly because calling the setters might
+        notify the image of an update on the newly created layer, but this
         layer has not yet been added to the image.
     */
     snapshot->m_opacity_percent = layer.opacity_percent();
diff --git a/Userland/Applications/PixelPaint/Layer.h b/Userland/Applications/PixelPaint/Layer.h
index 273836b30b..766575cf2b 100644
--- a/Userland/Applications/PixelPaint/Layer.h
+++ b/Userland/Applications/PixelPaint/Layer.h
@@ -25,9 +25,9 @@ class Layer
     AK_MAKE_NONMOVABLE(Layer);
 
 public:
-    static RefPtr<Layer> try_create_with_size(Image&, Gfx::IntSize const&, String name);
-    static RefPtr<Layer> try_create_with_bitmap(Image&, NonnullRefPtr<Gfx::Bitmap>, String name);
-    static RefPtr<Layer> try_create_snapshot(Image&, Layer const&);
+    static ErrorOr<NonnullRefPtr<Layer>> try_create_with_size(Image&, Gfx::IntSize const&, String name);
+    static ErrorOr<NonnullRefPtr<Layer>> try_create_with_bitmap(Image&, NonnullRefPtr<Gfx::Bitmap>, String name);
+    static ErrorOr<NonnullRefPtr<Layer>> try_create_snapshot(Image&, Layer const&);
 
     ~Layer() { }
 
diff --git a/Userland/Applications/PixelPaint/MainWidget.cpp b/Userland/Applications/PixelPaint/MainWidget.cpp
index 00faeccca1..c414c17c0e 100644
--- a/Userland/Applications/PixelPaint/MainWidget.cpp
+++ b/Userland/Applications/PixelPaint/MainWidget.cpp
@@ -94,9 +94,8 @@ void MainWidget::initialize_menubar(GUI::Window& window)
         "&New Image...", { Mod_Ctrl, Key_N }, Gfx::Bitmap::try_load_from_file("/res/icons/16x16/new.png").release_value_but_fixme_should_propagate_errors(), [&](auto&) {
             auto dialog = PixelPaint::CreateNewImageDialog::construct(&window);
             if (dialog->exec() == GUI::Dialog::ExecOK) {
-                auto image = PixelPaint::Image::try_create_with_size(dialog->image_size());
-                auto bg_layer = PixelPaint::Layer::try_create_with_size(*image, image->size(), "Background");
-                VERIFY(bg_layer);
+                auto image = PixelPaint::Image::try_create_with_size(dialog->image_size()).release_value_but_fixme_should_propagate_errors();
+                auto bg_layer = PixelPaint::Layer::try_create_with_size(*image, image->size(), "Background").release_value_but_fixme_should_propagate_errors();
                 image->add_layer(*bg_layer);
                 bg_layer->bitmap().fill(Color::White);
                 auto image_title = dialog->image_name().trim_whitespace();
@@ -219,8 +218,7 @@ void MainWidget::initialize_menubar(GUI::Window& window)
         if (!bitmap)
             return;
 
-        auto layer = PixelPaint::Layer::try_create_with_bitmap(editor->image(), *bitmap, "Pasted layer");
-        VERIFY(layer);
+        auto layer = PixelPaint::Layer::try_create_with_bitmap(editor->image(), *bitmap, "Pasted layer").release_value_but_fixme_should_propagate_errors();
         editor->image().add_layer(*layer);
         editor->set_active_layer(layer);
         editor->selection().clear();
@@ -440,12 +438,12 @@ void MainWidget::initialize_menubar(GUI::Window& window)
                 return;
             auto dialog = PixelPaint::CreateNewLayerDialog::construct(editor->image().size(), &window);
             if (dialog->exec() == GUI::Dialog::ExecOK) {
-                auto layer = PixelPaint::Layer::try_create_with_size(editor->image(), dialog->layer_size(), dialog->layer_name());
-                if (!layer) {
-                    GUI::MessageBox::show_error(&window, String::formatted("Unable to create layer with size {}", dialog->size().to_string()));
+                auto layer_or_error = PixelPaint::Layer::try_create_with_size(editor->image(), dialog->layer_size(), dialog->layer_name());
+                if (layer_or_error.is_error()) {
+                    GUI::MessageBox::show_error(&window, String::formatted("Unable to create layer with size {}", dialog->size()));
                     return;
                 }
-                editor->image().add_layer(layer.release_nonnull());
+                editor->image().add_layer(layer_or_error.release_value());
                 editor->layers_did_change();
                 m_layer_list_widget->select_top_layer();
             }
@@ -529,9 +527,8 @@ void MainWidget::initialize_menubar(GUI::Window& window)
                 auto& next_active_layer = editor->image().layer(active_layer_index > 0 ? active_layer_index - 1 : 0);
                 editor->set_active_layer(&next_active_layer);
             } else {
-                auto layer = PixelPaint::Layer::try_create_with_size(editor->image(), editor->image().size(), "Background");
-                VERIFY(layer);
-                editor->image().add_layer(layer.release_nonnull());
+                auto layer = PixelPaint::Layer::try_create_with_size(editor->image(), editor->image().size(), "Background").release_value_but_fixme_should_propagate_errors();
+                editor->image().add_layer(move(layer));
                 editor->layers_did_change();
                 m_layer_list_widget->select_top_layer();
             }
@@ -741,10 +738,9 @@ void MainWidget::open_image_fd(int fd, String const& path)
 
 void MainWidget::create_default_image()
 {
-    auto image = Image::try_create_with_size({ 480, 360 });
+    auto image = Image::try_create_with_size({ 480, 360 }).release_value_but_fixme_should_propagate_errors();
 
-    auto bg_layer = Layer::try_create_with_size(*image, image->size(), "Background");
-    VERIFY(bg_layer);
+    auto bg_layer = Layer::try_create_with_size(*image, image->size(), "Background").release_value_but_fixme_should_propagate_errors();
     image->add_layer(*bg_layer);
     bg_layer->bitmap().fill(Color::White);
 
diff --git a/Userland/Applications/PixelPaint/ProjectLoader.cpp b/Userland/Applications/PixelPaint/ProjectLoader.cpp
index df48b2747a..768918a364 100644
--- a/Userland/Applications/PixelPaint/ProjectLoader.cpp
+++ b/Userland/Applications/PixelPaint/ProjectLoader.cpp
@@ -16,12 +16,12 @@
 
 namespace PixelPaint {
 
-Result<void, String> ProjectLoader::try_load_from_fd_and_close(int fd, StringView path)
+ErrorOr<void> ProjectLoader::try_load_from_fd_and_close(int fd, StringView path)
 {
     auto file = Core::File::construct();
     file->open(fd, Core::OpenMode::ReadOnly, Core::File::ShouldCloseFileDescriptor::No);
     if (file->has_error())
-        return String { file->error_string() };
+        return Error::from_errno(file->error());
 
     auto contents = file->read_all();
 
@@ -29,18 +29,11 @@ Result<void, String> ProjectLoader::try_load_from_fd_and_close(int fd, StringVie
     if (!json_or_error.has_value()) {
         m_is_raw_image = true;
 
-        auto file_or_error = MappedFile::map_from_fd_and_close(fd, path);
-        if (file_or_error.is_error())
-            return String::formatted("Unable to mmap file {}", file_or_error.error());
+        auto mapped_file = TRY(MappedFile::map_from_fd_and_close(fd, path));
 
-        auto& mapped_file = *file_or_error.value();
         // FIXME: Find a way to avoid the memory copy here.
-        auto bitmap = Image::try_decode_bitmap(mapped_file.bytes());
-        if (!bitmap)
-            return String { "Unable to decode image"sv };
-        auto image = Image::try_create_from_bitmap(bitmap.release_nonnull());
-        if (!image)
-            return String { "Unable to allocate Image"sv };
+        auto bitmap = TRY(Image::try_decode_bitmap(mapped_file->bytes()));
+        auto image = TRY(Image::try_create_from_bitmap(move(bitmap)));
 
         image->set_path(path);
         m_image = image;
@@ -49,12 +42,8 @@ Result<void, String> ProjectLoader::try_load_from_fd_and_close(int fd, StringVie
 
     close(fd);
     auto& json = json_or_error.value().as_object();
-    auto image_or_error = Image::try_create_from_pixel_paint_json(json);
+    auto image = TRY(Image::try_create_from_pixel_paint_json(json));
 
-    if (image_or_error.is_error())
-        return image_or_error.release_error();
-
-    auto image = image_or_error.release_value();
     image->set_path(path);
 
     if (json.has("guides"))
diff --git a/Userland/Applications/PixelPaint/ProjectLoader.h b/Userland/Applications/PixelPaint/ProjectLoader.h
index f7609b911b..3f56340ad2 100644
--- a/Userland/Applications/PixelPaint/ProjectLoader.h
+++ b/Userland/Applications/PixelPaint/ProjectLoader.h
@@ -18,7 +18,7 @@ public:
     ProjectLoader() = default;
     ~ProjectLoader() = default;
 
-    Result<void, String> try_load_from_fd_and_close(int fd, StringView path);
+    ErrorOr<void> try_load_from_fd_and_close(int fd, StringView path);
 
     bool is_raw_image() const { return m_is_raw_image; }
     bool has_image() const { return !m_image.is_null(); }