Browse Source

LibCore+LibGUI: Add fallible versions of Widget::load_from_gml()

The existing `load_from_gml()` methods look the same as before from the
outside. Inside though, they now forward to `try_load_from_gml()` which
returns Error when things go wrong. It also now calls the `try_create()`
factory method for Objects instead of the `construct()` one.
Sam Atkins 2 năm trước cách đây
mục cha
commit
b32f5dbcff

+ 1 - 1
Userland/Libraries/LibCore/Object.cpp

@@ -263,7 +263,7 @@ static HashMap<StringView, ObjectClassRegistration*>& object_classes()
     return s_map;
 }
 
-ObjectClassRegistration::ObjectClassRegistration(StringView class_name, Function<RefPtr<Object>()> factory, ObjectClassRegistration* parent_class)
+ObjectClassRegistration::ObjectClassRegistration(StringView class_name, Function<ErrorOr<NonnullRefPtr<Object>>()> factory, ObjectClassRegistration* parent_class)
     : m_class_name(class_name)
     , m_factory(move(factory))
     , m_parent_class(parent_class)

+ 13 - 13
Userland/Libraries/LibCore/Object.h

@@ -22,18 +22,18 @@
 
 namespace Core {
 
-#define REGISTER_ABSTRACT_CORE_OBJECT(namespace_, class_name)                                                                     \
-    namespace Core {                                                                                                              \
-    namespace Registration {                                                                                                      \
-    Core::ObjectClassRegistration registration_##class_name(#namespace_ "::" #class_name##sv, []() { return RefPtr<Object>(); }); \
-    }                                                                                                                             \
+#define REGISTER_ABSTRACT_CORE_OBJECT(namespace_, class_name)                                                                                                                             \
+    namespace Core {                                                                                                                                                                      \
+    namespace Registration {                                                                                                                                                              \
+    Core::ObjectClassRegistration registration_##class_name(#namespace_ "::" #class_name##sv, []() { return Error::from_string_literal("Attempted to construct an abstract object."); }); \
+    }                                                                                                                                                                                     \
     }
 
-#define REGISTER_CORE_OBJECT(namespace_, class_name)                                                                                                 \
-    namespace Core {                                                                                                                                 \
-    namespace Registration {                                                                                                                         \
-    Core::ObjectClassRegistration registration_##class_name(#namespace_ "::" #class_name##sv, []() { return namespace_::class_name::construct(); }); \
-    }                                                                                                                                                \
+#define REGISTER_CORE_OBJECT(namespace_, class_name)                                                                                                  \
+    namespace Core {                                                                                                                                  \
+    namespace Registration {                                                                                                                          \
+    Core::ObjectClassRegistration registration_##class_name(#namespace_ "::" #class_name##sv, []() { return namespace_::class_name::try_create(); }); \
+    }                                                                                                                                                 \
     }
 
 class ObjectClassRegistration {
@@ -41,12 +41,12 @@ class ObjectClassRegistration {
     AK_MAKE_NONMOVABLE(ObjectClassRegistration);
 
 public:
-    ObjectClassRegistration(StringView class_name, Function<RefPtr<Object>()> factory, ObjectClassRegistration* parent_class = nullptr);
+    ObjectClassRegistration(StringView class_name, Function<ErrorOr<NonnullRefPtr<Object>>()> factory, ObjectClassRegistration* parent_class = nullptr);
     ~ObjectClassRegistration() = default;
 
     StringView class_name() const { return m_class_name; }
     ObjectClassRegistration const* parent_class() const { return m_parent_class; }
-    RefPtr<Object> construct() const { return m_factory(); }
+    ErrorOr<NonnullRefPtr<Object>> construct() const { return m_factory(); }
     bool is_derived_from(ObjectClassRegistration const& base_class) const;
 
     static void for_each(Function<void(ObjectClassRegistration const&)>);
@@ -54,7 +54,7 @@ public:
 
 private:
     StringView m_class_name;
-    Function<RefPtr<Object>()> m_factory;
+    Function<ErrorOr<NonnullRefPtr<Object>>()> m_factory;
     ObjectClassRegistration* m_parent_class { nullptr };
 };
 

+ 5 - 5
Userland/Libraries/LibGUI/GML/AST.h

@@ -202,18 +202,18 @@ public:
         }
     }
 
-    // Uses IterationDecision to allow the callback to interrupt the iteration, like a for-loop break.
-    template<IteratorFunction<NonnullRefPtr<Object>> Callback>
-    void for_each_child_object_interruptible(Callback callback)
+    template<FallibleFunction<NonnullRefPtr<Object>> Callback>
+    ErrorOr<void> try_for_each_child_object(Callback callback)
     {
         for (NonnullRefPtr<Node> child : m_sub_objects) {
             // doesn't capture layout as intended, as that's behind a kv-pair
             if (is<Object>(child.ptr())) {
                 auto object = static_ptr_cast<Object>(child);
-                if (callback(object) == IterationDecision::Break)
-                    return;
+                TRY(callback(object));
             }
         }
+
+        return {};
     }
 
     RefPtr<Object> layout_object() const

+ 2 - 2
Userland/Libraries/LibGUI/GML/AutocompleteProvider.cpp

@@ -143,8 +143,8 @@ void AutocompleteProvider::provide_completions(Function<void(Vector<CodeComprehe
         // FIXME: Don't show properties that are already specified in the scope.
         auto registration = Core::ObjectClassRegistration::find(class_name);
         if (registration && (registration->is_derived_from(widget_class) || registration->is_derived_from(layout_class))) {
-            if (auto instance = registration->construct()) {
-                for (auto& it : instance->properties()) {
+            if (auto instance = registration->construct(); !instance.is_error()) {
+                for (auto& it : instance.value()->properties()) {
                     if (!it.value->is_readonly() && it.key.matches(pattern))
                         identifier_entries.empend(DeprecatedString::formatted("{}: ", it.key), partial_input_length, CodeComprehension::Language::Unspecified, it.key);
                 }

+ 7 - 10
Userland/Libraries/LibGUI/ScrollableContainerWidget.cpp

@@ -102,7 +102,7 @@ void ScrollableContainerWidget::set_widget(GUI::Widget* widget)
     update_widget_position();
 }
 
-bool ScrollableContainerWidget::load_from_gml_ast(NonnullRefPtr<GUI::GML::Node> ast, RefPtr<Core::Object> (*unregistered_child_handler)(DeprecatedString const&))
+ErrorOr<void> ScrollableContainerWidget::load_from_gml_ast(NonnullRefPtr<GUI::GML::Node> ast, RefPtr<Core::Object> (*unregistered_child_handler)(DeprecatedString const&))
 {
     if (is<GUI::GML::GMLFile>(ast.ptr()))
         return load_from_gml_ast(static_ptr_cast<GUI::GML::GMLFile>(ast)->main_class(), unregistered_child_handler);
@@ -116,15 +116,13 @@ bool ScrollableContainerWidget::load_from_gml_ast(NonnullRefPtr<GUI::GML::Node>
 
     auto content_widget_value = object->get_property("content_widget"sv);
     if (!content_widget_value.is_null() && !is<GUI::GML::Object>(content_widget_value.ptr())) {
-        dbgln("content widget is not an object");
-        return false;
+        return Error::from_string_literal("ScrollableContainerWidget content_widget is not an object");
     }
 
     auto has_children = false;
     object->for_each_child_object([&](auto) { has_children = true; });
     if (has_children) {
-        dbgln("children specified for ScrollableContainerWidget, but only 1 widget as content_widget is supported");
-        return false;
+        return Error::from_string_literal("Children specified for ScrollableContainerWidget, but only 1 widget as content_widget is supported");
     }
 
     if (!content_widget_value.is_null() && is<GUI::GML::Object>(content_widget_value.ptr())) {
@@ -133,19 +131,18 @@ bool ScrollableContainerWidget::load_from_gml_ast(NonnullRefPtr<GUI::GML::Node>
 
         RefPtr<Core::Object> child;
         if (auto* registration = Core::ObjectClassRegistration::find(class_name)) {
-            child = registration->construct();
+            child = TRY(registration->construct());
         } else {
             child = unregistered_child_handler(class_name);
         }
         if (!child)
-            return false;
+            return Error::from_string_literal("Unable to construct a Widget class for ScrollableContainerWidget content_widget property");
         auto widget_ptr = verify_cast<GUI::Widget>(child.ptr());
         set_widget(widget_ptr);
-        static_ptr_cast<Widget>(child)->load_from_gml_ast(content_widget.release_nonnull(), unregistered_child_handler);
-        return true;
+        TRY(static_ptr_cast<Widget>(child)->load_from_gml_ast(content_widget.release_nonnull(), unregistered_child_handler));
     }
 
-    return true;
+    return {};
 }
 
 }

+ 1 - 1
Userland/Libraries/LibGUI/ScrollableContainerWidget.h

@@ -30,7 +30,7 @@ private:
     void update_widget_size();
     void update_widget_position();
     void update_widget_min_size();
-    virtual bool load_from_gml_ast(NonnullRefPtr<GUI::GML::Node> ast, RefPtr<Core::Object> (*unregistered_child_handler)(DeprecatedString const&)) override;
+    virtual ErrorOr<void> load_from_gml_ast(NonnullRefPtr<GUI::GML::Node> ast, RefPtr<Core::Object> (*unregistered_child_handler)(DeprecatedString const&)) override;
 
     ScrollableContainerWidget();
 

+ 31 - 28
Userland/Libraries/LibGUI/Widget.cpp

@@ -1145,26 +1145,31 @@ void Widget::set_override_cursor(AK::Variant<Gfx::StandardCursor, NonnullRefPtr<
     }
 }
 
-bool Widget::load_from_gml(StringView gml_string)
+ErrorOr<void> Widget::try_load_from_gml(StringView gml_string)
 {
-    return load_from_gml(gml_string, [](DeprecatedString const& class_name) -> RefPtr<Core::Object> {
+    return try_load_from_gml(gml_string, [](DeprecatedString const& class_name) -> RefPtr<Core::Object> {
         dbgln("Class '{}' not registered", class_name);
         return nullptr;
     });
 }
 
+ErrorOr<void> Widget::try_load_from_gml(StringView gml_string, RefPtr<Core::Object> (*unregistered_child_handler)(DeprecatedString const&))
+{
+    auto value = TRY(GML::parse_gml(gml_string));
+    return load_from_gml_ast(value, unregistered_child_handler);
+}
+
+bool Widget::load_from_gml(StringView gml_string)
+{
+    return !try_load_from_gml(gml_string).is_error();
+}
+
 bool Widget::load_from_gml(StringView gml_string, RefPtr<Core::Object> (*unregistered_child_handler)(DeprecatedString const&))
 {
-    auto value = GML::parse_gml(gml_string);
-    if (value.is_error()) {
-        // FIXME: We don't report the error, so at least print it.
-        dbgln("Error while parsing GML: {}", value.error());
-        return false;
-    }
-    return load_from_gml_ast(value.release_value(), unregistered_child_handler);
+    return !try_load_from_gml(gml_string, unregistered_child_handler).is_error();
 }
 
-bool Widget::load_from_gml_ast(NonnullRefPtr<GUI::GML::Node> ast, RefPtr<Core::Object> (*unregistered_child_handler)(DeprecatedString const&))
+ErrorOr<void> Widget::load_from_gml_ast(NonnullRefPtr<GUI::GML::Node> ast, RefPtr<Core::Object> (*unregistered_child_handler)(DeprecatedString const&))
 {
     if (is<GUI::GML::GMLFile>(ast.ptr()))
         return load_from_gml_ast(static_ptr_cast<GUI::GML::GMLFile>(ast)->main_class(), unregistered_child_handler);
@@ -1180,21 +1185,20 @@ bool Widget::load_from_gml_ast(NonnullRefPtr<GUI::GML::Node> ast, RefPtr<Core::O
     if (!layout.is_null()) {
         auto class_name = layout->name();
         if (class_name.is_null()) {
-            dbgln("Invalid layout class name");
-            return false;
+            return Error::from_string_literal("Invalid layout class name");
         }
 
         auto& layout_class = *Core::ObjectClassRegistration::find("GUI::Layout"sv);
         if (auto* registration = Core::ObjectClassRegistration::find(class_name)) {
-            auto layout = registration->construct();
-            if (!layout || !registration->is_derived_from(layout_class)) {
+            auto layout = TRY(registration->construct());
+            if (!registration->is_derived_from(layout_class)) {
                 dbgln("Invalid layout class: '{}'", class_name.to_deprecated_string());
-                return false;
+                return Error::from_string_literal("Invalid layout class");
             }
-            set_layout(static_ptr_cast<Layout>(layout).release_nonnull());
+            set_layout(static_ptr_cast<Layout>(layout));
         } else {
             dbgln("Unknown layout class: '{}'", class_name.to_deprecated_string());
-            return false;
+            return Error::from_string_literal("Unknown layout class");
         }
 
         layout->for_each_property([&](auto key, auto value) {
@@ -1204,33 +1208,32 @@ bool Widget::load_from_gml_ast(NonnullRefPtr<GUI::GML::Node> ast, RefPtr<Core::O
 
     auto& widget_class = *Core::ObjectClassRegistration::find("GUI::Widget"sv);
     bool is_tab_widget = is<TabWidget>(*this);
-    object->for_each_child_object_interruptible([&](auto child_data) {
+    TRY(object->try_for_each_child_object([&](auto child_data) -> ErrorOr<void> {
         auto class_name = child_data->name();
 
         // It is very questionable if this pseudo object should exist, but it works fine like this for now.
         if (class_name == "GUI::Layout::Spacer") {
             if (!this->layout()) {
-                dbgln("Specified GUI::Layout::Spacer in GML, but the parent has no Layout.");
-                return IterationDecision::Break;
+                return Error::from_string_literal("Specified GUI::Layout::Spacer in GML, but the parent has no Layout.");
             }
             this->layout()->add_spacer();
         } else {
             RefPtr<Core::Object> child;
             if (auto* registration = Core::ObjectClassRegistration::find(class_name)) {
-                child = registration->construct();
-                if (!child || !registration->is_derived_from(widget_class)) {
+                child = TRY(registration->construct());
+                if (!registration->is_derived_from(widget_class)) {
                     dbgln("Invalid widget class: '{}'", class_name);
-                    return IterationDecision::Break;
+                    return Error::from_string_literal("Invalid widget class");
                 }
             } else {
                 child = unregistered_child_handler(class_name);
             }
             if (!child)
-                return IterationDecision::Break;
+                return Error::from_string_literal("Unable to construct a Widget class for child");
             add_child(*child);
 
             // This is possible as we ensure that Widget is a base class above.
-            static_ptr_cast<Widget>(child)->load_from_gml_ast(child_data, unregistered_child_handler);
+            TRY(static_ptr_cast<Widget>(child)->load_from_gml_ast(child_data, unregistered_child_handler));
 
             if (is_tab_widget) {
                 // FIXME: We need to have the child added before loading it so that it can access us. But the TabWidget logic requires the child to not be present yet.
@@ -1239,10 +1242,10 @@ bool Widget::load_from_gml_ast(NonnullRefPtr<GUI::GML::Node> ast, RefPtr<Core::O
             }
         }
 
-        return IterationDecision::Continue;
-    });
+        return {};
+    }));
 
-    return true;
+    return {};
 }
 
 bool Widget::has_focus_within() const

+ 12 - 8
Userland/Libraries/LibGUI/Widget.h

@@ -32,12 +32,12 @@ extern Core::ObjectClassRegistration registration_Widget;
 }
 }
 
-#define REGISTER_WIDGET(namespace_, class_name)                                                                                                       \
-    namespace Core {                                                                                                                                  \
-    namespace Registration {                                                                                                                          \
-    Core::ObjectClassRegistration registration_##class_name(                                                                                          \
-        #namespace_ "::" #class_name##sv, []() { return static_ptr_cast<Core::Object>(namespace_::class_name::construct()); }, &registration_Widget); \
-    }                                                                                                                                                 \
+#define REGISTER_WIDGET(namespace_, class_name)                                                                                                                                                     \
+    namespace Core {                                                                                                                                                                                \
+    namespace Registration {                                                                                                                                                                        \
+    Core::ObjectClassRegistration registration_##class_name(                                                                                                                                        \
+        #namespace_ "::" #class_name##sv, []() -> ErrorOr<NonnullRefPtr<Core::Object>> { return static_ptr_cast<Core::Object>(TRY(namespace_::class_name::try_create())); }, &registration_Widget); \
+    }                                                                                                                                                                                               \
     }
 
 namespace GUI {
@@ -353,7 +353,11 @@ public:
     AK::Variant<Gfx::StandardCursor, NonnullRefPtr<Gfx::Bitmap>> const& override_cursor() const { return m_override_cursor; }
     void set_override_cursor(AK::Variant<Gfx::StandardCursor, NonnullRefPtr<Gfx::Bitmap>>);
 
-    bool load_from_gml(StringView);
+    ErrorOr<void> try_load_from_gml(StringView);
+    ErrorOr<void> try_load_from_gml(StringView, RefPtr<Core::Object> (*unregistered_child_handler)(DeprecatedString const&));
+
+    // FIXME: Replace all uses of load_from_gml() with try_load_from_gml()
+    bool load_from_gml(StringView gml_string);
     bool load_from_gml(StringView, RefPtr<Core::Object> (*unregistered_child_handler)(DeprecatedString const&));
 
     // FIXME: remove this when all uses of shrink_to_fit are eliminated
@@ -363,7 +367,7 @@ public:
     bool has_pending_drop() const;
 
     // In order for others to be able to call this, it needs to be public.
-    virtual bool load_from_gml_ast(NonnullRefPtr<GUI::GML::Node> ast, RefPtr<Core::Object> (*unregistered_child_handler)(DeprecatedString const&));
+    virtual ErrorOr<void> load_from_gml_ast(NonnullRefPtr<GUI::GML::Node> ast, RefPtr<Core::Object> (*unregistered_child_handler)(DeprecatedString const&));
 
 protected:
     Widget();