FileManager: Propagate errors from PropertiesWindow initialization

Userland/Applications/FileManager/PropertiesWindow.cpp

@@ -1,6 +1,6 @@
 /*
  * Copyright (c) 2018-2020, Andreas Kling <kling@serenityos.org>
- * Copyright (c) 2022, the SerenityOS developers.
+ * Copyright (c) 2022-2023, the SerenityOS developers.
  *
  * SPDX-License-Identifier: BSD-2-Clause
  */
@@ -10,6 +10,7 @@
 #include <AK/NumberFormat.h>
 #include <Applications/FileManager/DirectoryView.h>
 #include <Applications/FileManager/PropertiesWindowGeneralTabGML.h>
+#include <LibCore/System.h>
 #include <LibDesktop/Launcher.h>
 #include <LibGUI/BoxLayout.h>
 #include <LibGUI/CheckBox.h>
@@ -26,34 +27,43 @@
 #include <string.h>
 #include <unistd.h>
 
-PropertiesWindow::PropertiesWindow(DeprecatedString const& path, bool disable_rename, Window* parent_window)
+ErrorOr<NonnullRefPtr<PropertiesWindow>> PropertiesWindow::try_create(DeprecatedString const& path, bool disable_rename, Window* parent)
+{
+    auto window = TRY(adopt_nonnull_ref_or_enomem(new (nothrow) PropertiesWindow(path, parent)));
+    window->set_icon(TRY(Gfx::Bitmap::load_from_file("/res/icons/16x16/properties.png"sv)));
+    TRY(window->create_widgets(disable_rename));
+    return window;
+}
+
+PropertiesWindow::PropertiesWindow(DeprecatedString const& path, Window* parent_window)
     : Window(parent_window)
 {
     auto lexical_path = LexicalPath(path);
 
-    auto main_widget = set_main_widget<GUI::Widget>().release_value_but_fixme_should_propagate_errors();
-    main_widget->set_layout<GUI::VerticalBoxLayout>();
-    main_widget->layout()->set_spacing(6);
-    main_widget->layout()->set_margins(4);
-    main_widget->set_fill_with_background_color(true);
+    m_name = lexical_path.basename();
+    m_path = lexical_path.string();
+    m_parent_path = lexical_path.dirname();
 
     set_rect({ 0, 0, 360, 420 });
     set_resizable(false);
+}
 
-    set_icon(Gfx::Bitmap::load_from_file("/res/icons/16x16/properties.png"sv).release_value_but_fixme_should_propagate_errors());
+ErrorOr<void> PropertiesWindow::create_widgets(bool disable_rename)
+{
+    auto main_widget = TRY(set_main_widget<GUI::Widget>());
+    (void)TRY(main_widget->try_set_layout<GUI::VerticalBoxLayout>());
+    main_widget->layout()->set_spacing(6);
+    main_widget->layout()->set_margins(4);
+    main_widget->set_fill_with_background_color(true);
 
-    auto& tab_widget = main_widget->add<GUI::TabWidget>();
+    auto tab_widget = TRY(main_widget->try_add<GUI::TabWidget>());
 
-    auto& general_tab = tab_widget.add_tab<GUI::Widget>("General");
-    general_tab.load_from_gml(properties_window_general_tab_gml).release_value_but_fixme_should_propagate_errors();
+    auto general_tab = TRY(tab_widget->try_add_tab<GUI::Widget>("General"));
+    TRY(general_tab->load_from_gml(properties_window_general_tab_gml));
 
-    m_name = lexical_path.basename();
-    m_path = lexical_path.string();
-    m_parent_path = lexical_path.dirname();
+    m_icon = general_tab->find_descendant_of_type_named<GUI::ImageWidget>("icon");
 
-    m_icon = general_tab.find_descendant_of_type_named<GUI::ImageWidget>("icon");
-
-    m_name_box = general_tab.find_descendant_of_type_named<GUI::TextBox>("name");
+    m_name_box = general_tab->find_descendant_of_type_named<GUI::TextBox>("name");
     m_name_box->set_text(m_name);
     m_name_box->set_mode(disable_rename ? GUI::TextBox::Mode::DisplayOnly : GUI::TextBox::Mode::Editable);
     m_name_box->on_change = [&]() {
@@ -61,11 +71,13 @@ PropertiesWindow::PropertiesWindow(DeprecatedString const& path, bool disable_re
         m_apply_button->set_enabled(m_name_dirty || m_permissions_dirty);
     };
 
-    struct stat st;
-    if (lstat(path.characters(), &st)) {
-        perror("stat");
-        return;
-    }
+    auto* location = general_tab->find_descendant_of_type_named<GUI::LinkLabel>("location");
+    location->set_text(m_path);
+    location->on_click = [this] {
+        Desktop::Launcher::open(URL::create_with_file_scheme(m_parent_path, m_name));
+    };
+
+    auto st = TRY(Core::System::lstat(m_path));
 
     DeprecatedString owner_name;
     DeprecatedString group_name;
@@ -85,22 +97,16 @@ PropertiesWindow::PropertiesWindow(DeprecatedString const& path, bool disable_re
     m_mode = st.st_mode;
     m_old_mode = st.st_mode;
 
-    auto type = general_tab.find_descendant_of_type_named<GUI::Label>("type");
+    auto* type = general_tab->find_descendant_of_type_named<GUI::Label>("type");
     type->set_text(get_description(m_mode));
 
-    auto location = general_tab.find_descendant_of_type_named<GUI::LinkLabel>("location");
-    location->set_text(path);
-    location->on_click = [this] {
-        Desktop::Launcher::open(URL::create_with_file_scheme(m_parent_path, m_name));
-    };
-
     if (S_ISLNK(m_mode)) {
-        auto link_destination_or_error = Core::File::read_link(path);
+        auto link_destination_or_error = Core::File::read_link(m_path);
         if (link_destination_or_error.is_error()) {
             perror("readlink");
         } else {
             auto link_destination = link_destination_or_error.release_value();
-            auto link_location = general_tab.find_descendant_of_type_named<GUI::LinkLabel>("link_location");
+            auto* link_location = general_tab->find_descendant_of_type_named<GUI::LinkLabel>("link_location");
             link_location->set_text(link_destination);
             link_location->on_click = [link_destination] {
                 auto link_directory = LexicalPath(link_destination);
@@ -108,60 +114,63 @@ PropertiesWindow::PropertiesWindow(DeprecatedString const& path, bool disable_re
             };
         }
     } else {
-        auto link_location_widget = general_tab.find_descendant_of_type_named<GUI::Widget>("link_location_widget");
-        general_tab.remove_child(*link_location_widget);
+        auto* link_location_widget = general_tab->find_descendant_of_type_named<GUI::Widget>("link_location_widget");
+        general_tab->remove_child(*link_location_widget);
     }
 
-    auto size = general_tab.find_descendant_of_type_named<GUI::Label>("size");
+    auto* size = general_tab->find_descendant_of_type_named<GUI::Label>("size");
     size->set_text(human_readable_size_long(st.st_size));
 
-    auto owner = general_tab.find_descendant_of_type_named<GUI::Label>("owner");
+    auto* owner = general_tab->find_descendant_of_type_named<GUI::Label>("owner");
     owner->set_text(DeprecatedString::formatted("{} ({})", owner_name, st.st_uid));
 
-    auto group = general_tab.find_descendant_of_type_named<GUI::Label>("group");
+    auto* group = general_tab->find_descendant_of_type_named<GUI::Label>("group");
     group->set_text(DeprecatedString::formatted("{} ({})", group_name, st.st_gid));
 
-    auto created_at = general_tab.find_descendant_of_type_named<GUI::Label>("created_at");
+    auto* created_at = general_tab->find_descendant_of_type_named<GUI::Label>("created_at");
     created_at->set_text(GUI::FileSystemModel::timestamp_string(st.st_ctime));
 
-    auto last_modified = general_tab.find_descendant_of_type_named<GUI::Label>("last_modified");
+    auto* last_modified = general_tab->find_descendant_of_type_named<GUI::Label>("last_modified");
     last_modified->set_text(GUI::FileSystemModel::timestamp_string(st.st_mtime));
 
-    auto owner_read = general_tab.find_descendant_of_type_named<GUI::CheckBox>("owner_read");
-    auto owner_write = general_tab.find_descendant_of_type_named<GUI::CheckBox>("owner_write");
-    auto owner_execute = general_tab.find_descendant_of_type_named<GUI::CheckBox>("owner_execute");
-    setup_permission_checkboxes(*owner_read, *owner_write, *owner_execute, { S_IRUSR, S_IWUSR, S_IXUSR }, m_mode);
+    auto* owner_read = general_tab->find_descendant_of_type_named<GUI::CheckBox>("owner_read");
+    auto* owner_write = general_tab->find_descendant_of_type_named<GUI::CheckBox>("owner_write");
+    auto* owner_execute = general_tab->find_descendant_of_type_named<GUI::CheckBox>("owner_execute");
+    TRY(setup_permission_checkboxes(*owner_read, *owner_write, *owner_execute, { S_IRUSR, S_IWUSR, S_IXUSR }, m_mode));
 
-    auto group_read = general_tab.find_descendant_of_type_named<GUI::CheckBox>("group_read");
-    auto group_write = general_tab.find_descendant_of_type_named<GUI::CheckBox>("group_write");
-    auto group_execute = general_tab.find_descendant_of_type_named<GUI::CheckBox>("group_execute");
-    setup_permission_checkboxes(*group_read, *group_write, *group_execute, { S_IRGRP, S_IWGRP, S_IXGRP }, m_mode);
+    auto* group_read = general_tab->find_descendant_of_type_named<GUI::CheckBox>("group_read");
+    auto* group_write = general_tab->find_descendant_of_type_named<GUI::CheckBox>("group_write");
+    auto* group_execute = general_tab->find_descendant_of_type_named<GUI::CheckBox>("group_execute");
+    TRY(setup_permission_checkboxes(*group_read, *group_write, *group_execute, { S_IRGRP, S_IWGRP, S_IXGRP }, m_mode));
 
-    auto others_read = general_tab.find_descendant_of_type_named<GUI::CheckBox>("others_read");
-    auto others_write = general_tab.find_descendant_of_type_named<GUI::CheckBox>("others_write");
-    auto others_execute = general_tab.find_descendant_of_type_named<GUI::CheckBox>("others_execute");
-    setup_permission_checkboxes(*others_read, *others_write, *others_execute, { S_IROTH, S_IWOTH, S_IXOTH }, m_mode);
+    auto* others_read = general_tab->find_descendant_of_type_named<GUI::CheckBox>("others_read");
+    auto* others_write = general_tab->find_descendant_of_type_named<GUI::CheckBox>("others_write");
+    auto* others_execute = general_tab->find_descendant_of_type_named<GUI::CheckBox>("others_execute");
+    TRY(setup_permission_checkboxes(*others_read, *others_write, *others_execute, { S_IROTH, S_IWOTH, S_IXOTH }, m_mode));
 
-    auto& button_widget = main_widget->add<GUI::Widget>();
-    button_widget.set_layout<GUI::HorizontalBoxLayout>();
-    button_widget.set_fixed_height(22);
-    button_widget.layout()->set_spacing(5);
+    auto button_widget = TRY(main_widget->try_add<GUI::Widget>());
+    (void)TRY(button_widget->try_set_layout<GUI::HorizontalBoxLayout>());
+    button_widget->set_fixed_height(22);
+    button_widget->layout()->set_spacing(5);
 
-    button_widget.layout()->add_spacer();
+    button_widget->layout()->add_spacer();
 
-    make_button("OK", button_widget).on_click = [this](auto) {
+    auto ok_button = TRY(make_button("OK", button_widget));
+    ok_button->on_click = [this](auto) {
         if (apply_changes())
             close();
     };
-    make_button("Cancel", button_widget).on_click = [this](auto) {
+    auto cancel_button = TRY(make_button("Cancel", button_widget));
+    cancel_button->on_click = [this](auto) {
         close();
     };
 
-    m_apply_button = make_button("Apply", button_widget);
+    m_apply_button = TRY(make_button("Apply", button_widget));
     m_apply_button->on_click = [this](auto) { apply_changes(); };
     m_apply_button->set_enabled(false);
 
     update();
+    return {};
 }
 
 void PropertiesWindow::update()
@@ -226,13 +235,9 @@ bool PropertiesWindow::apply_changes()
     return true;
 }
 
-void PropertiesWindow::setup_permission_checkboxes(GUI::CheckBox& box_read, GUI::CheckBox& box_write, GUI::CheckBox& box_execute, PermissionMasks masks, mode_t mode)
+ErrorOr<void> PropertiesWindow::setup_permission_checkboxes(GUI::CheckBox& box_read, GUI::CheckBox& box_write, GUI::CheckBox& box_execute, PermissionMasks masks, mode_t mode)
 {
-    struct stat st;
-    if (lstat(m_path.characters(), &st)) {
-        perror("stat");
-        return;
-    }
+    auto st = TRY(Core::System::lstat(m_path));
 
     auto can_edit_checkboxes = st.st_uid == getuid();
 
@@ -247,11 +252,13 @@ void PropertiesWindow::setup_permission_checkboxes(GUI::CheckBox& box_read, GUI:
     box_execute.set_checked(mode & masks.execute);
     box_execute.on_checked = [&, masks](bool checked) { permission_changed(masks.execute, checked); };
     box_execute.set_enabled(can_edit_checkboxes);
+
+    return {};
 }
 
-GUI::Button& PropertiesWindow::make_button(DeprecatedString text, GUI::Widget& parent)
+ErrorOr<NonnullRefPtr<GUI::Button>> PropertiesWindow::make_button(DeprecatedString text, GUI::Widget& parent)
 {
-    auto& button = parent.add<GUI::Button>(text);
-    button.set_fixed_size(70, 22);
+    auto button = TRY(parent.try_add<GUI::Button>(text));
+    button->set_fixed_size(70, 22);
     return button;
 }

Userland/Applications/FileManager/PropertiesWindow.h

@@ -1,6 +1,6 @@
 /*
  * Copyright (c) 2018-2020, Andreas Kling <kling@serenityos.org>
- * Copyright (c) 2022, the SerenityOS developers.
+ * Copyright (c) 2022-2023, the SerenityOS developers.
  *
  * SPDX-License-Identifier: BSD-2-Clause
  */
@@ -19,10 +19,12 @@ class PropertiesWindow final : public GUI::Window {
     C_OBJECT(PropertiesWindow);
 
 public:
+    static ErrorOr<NonnullRefPtr<PropertiesWindow>> try_create(DeprecatedString const& path, bool disable_rename, Window* parent = nullptr);
     virtual ~PropertiesWindow() override = default;
 
 private:
-    PropertiesWindow(DeprecatedString const& path, bool disable_rename, Window* parent = nullptr);
+    PropertiesWindow(DeprecatedString const& path, Window* parent = nullptr);
+    ErrorOr<void> create_widgets(bool disable_rename);
 
     struct PropertyValuePair {
         DeprecatedString property;
@@ -58,8 +60,8 @@ private:
         return "Unknown";
     }
 
-    GUI::Button& make_button(DeprecatedString, GUI::Widget& parent);
-    void setup_permission_checkboxes(GUI::CheckBox& box_read, GUI::CheckBox& box_write, GUI::CheckBox& box_execute, PermissionMasks masks, mode_t mode);
+    static ErrorOr<NonnullRefPtr<GUI::Button>> make_button(DeprecatedString, GUI::Widget& parent);
+    ErrorOr<void> setup_permission_checkboxes(GUI::CheckBox& box_read, GUI::CheckBox& box_write, GUI::CheckBox& box_execute, PermissionMasks masks, mode_t mode);
     void permission_changed(mode_t mask, bool set);
     bool apply_changes();
     void update();

Userland/Applications/FileManager/main.cpp

@@ -2,7 +2,7 @@
  * Copyright (c) 2018-2021, Andreas Kling <kling@serenityos.org>
  * Copyright (c) 2021-2023, Sam Atkins <atkinssj@serenityos.org>
  * Copyright (c) 2021, Mustafa Quraish <mustafa@cs.toronto.edu>
- * Copyright (c) 2022, the SerenityOS developers.
+ * Copyright (c) 2022-2023, the SerenityOS developers.
  *
  * SPDX-License-Identifier: BSD-2-Clause
  */
@@ -290,12 +290,19 @@ void do_unzip_archive(Vector<DeprecatedString> const& selected_file_paths, GUI::
 
 void show_properties(DeprecatedString const& container_dir_path, DeprecatedString const& path, Vector<DeprecatedString> const& selected, GUI::Window* window)
 {
-    RefPtr<PropertiesWindow> properties;
+    ErrorOr<RefPtr<PropertiesWindow>> properties_or_error = nullptr;
     if (selected.is_empty()) {
-        properties = window->add<PropertiesWindow>(path, true);
+        properties_or_error = window->try_add<PropertiesWindow>(path, true);
     } else {
-        properties = window->add<PropertiesWindow>(selected.first(), access(container_dir_path.characters(), W_OK) != 0);
+        properties_or_error = window->try_add<PropertiesWindow>(selected.first(), access(container_dir_path.characters(), W_OK) != 0);
     }
+
+    if (properties_or_error.is_error()) {
+        GUI::MessageBox::show(window, "Could not show properties"sv, "Properties Error"sv, GUI::MessageBox::Type::Error);
+        return;
+    }
+
+    auto properties = properties_or_error.release_value();
     properties->on_close = [properties = properties.ptr()] {
         properties->remove_from_parent();
     };