From 19cfd37da6f115508594a429a7c4f64697291f46 Mon Sep 17 00:00:00 2001
From: George Kiagiadakis <george.kiagiadakis@collabora.com>
Date: Tue, 20 Jun 2023 16:57:49 +0300
Subject: [PATCH] comp-loader: register the component & feature only if it
 activates successfully

Fixes #470
See also !503
---
 lib/wp/component-loader.c   | 42 ++++++++++++++++++++++---------------
 tests/wp/component-loader.c | 38 ++++++++++++++++++++++++++++++++-
 2 files changed, 62 insertions(+), 18 deletions(-)

diff --git a/lib/wp/component-loader.c b/lib/wp/component-loader.c
index d32d37c9..5b29f7e7 100644
--- a/lib/wp/component-loader.c
+++ b/lib/wp/component-loader.c
@@ -73,6 +73,23 @@ wp_component_loader_load_finish (WpComponentLoader * self, GAsyncResult * res,
   return WP_COMPONENT_LOADER_GET_IFACE (self)->load_finish (self, res, error);
 }
 
+static void
+wp_component_loader_load_task_return (GTask * task, gpointer object)
+{
+  WpCore *core = g_task_get_source_object (task);
+  WpRegistry *reg = wp_core_get_registry (core);
+  gchar *provides = g_task_get_task_data (task);
+
+  /* store object in the registry */
+  if (object)
+    wp_core_register_object (core, g_object_ref (object));
+
+  if (provides)
+    wp_registry_mark_feature_provided (reg, provides);
+
+  g_task_return_boolean (task, TRUE);
+}
+
 static void
 on_object_activated (WpObject * object, GAsyncResult * res, gpointer data)
 {
@@ -84,7 +101,7 @@ on_object_activated (WpObject * object, GAsyncResult * res, gpointer data)
     return;
   }
 
-  g_task_return_boolean (task, TRUE);
+  wp_component_loader_load_task_return (task, object);
 }
 
 static void
@@ -93,35 +110,26 @@ on_component_loader_load_done (WpComponentLoader * cl, GAsyncResult * res,
 {
   g_autoptr (GTask) task = G_TASK (data);
   g_autoptr (GError) error = NULL;
-  g_autoptr (GObject) o = NULL;
-  WpCore *core = g_task_get_source_object (task);
-  WpRegistry *reg = wp_core_get_registry (core);
-  gchar *provides = g_task_get_task_data (task);
+  g_autoptr (GObject) object = NULL;
 
-  o = wp_component_loader_load_finish (cl, res, &error);
+  object = wp_component_loader_load_finish (cl, res, &error);
   if (error) {
     g_task_return_error (task, g_steal_pointer (&error));
     return;
   }
 
-  if (provides)
-    wp_registry_mark_feature_provided (reg, provides);
+  if (object) {
+    wp_trace_object (cl, "loaded object " WP_OBJECT_FORMAT, WP_OBJECT_ARGS (object));
 
-  if (o) {
-    wp_trace_object (cl, "loaded object " WP_OBJECT_FORMAT, WP_OBJECT_ARGS (o));
-
-    /* store object in the registry */
-    wp_core_register_object (core, g_object_ref (o));
-
-    if (WP_IS_OBJECT (o)) {
+    if (WP_IS_OBJECT (object)) {
       /* WpObject needs to be activated */
-      wp_object_activate (WP_OBJECT (o), WP_OBJECT_FEATURES_ALL, NULL,
+      wp_object_activate (WP_OBJECT (object), WP_OBJECT_FEATURES_ALL, NULL,
           (GAsyncReadyCallback) on_object_activated, g_steal_pointer (&task));
       return;
     }
   }
 
-  g_task_return_boolean (task, TRUE);
+  wp_component_loader_load_task_return (task, object);
 }
 
 /*!
diff --git a/tests/wp/component-loader.c b/tests/wp/component-loader.c
index 3c24f8a4..388c7b7b 100644
--- a/tests/wp/component-loader.c
+++ b/tests/wp/component-loader.c
@@ -26,7 +26,13 @@ static void
 wp_test_plugin_enable (WpPlugin * self, WpTransition * transition)
 {
   WP_TEST_PLUGIN (self)->enabled = TRUE;
-  wp_object_update_features (WP_OBJECT (self), WP_PLUGIN_FEATURE_ENABLED, 0);
+
+  if (g_str_equal (wp_plugin_get_name (self), "fail")) {
+    wp_transition_return_error (transition, g_error_new (WP_DOMAIN_LIBRARY,
+        WP_LIBRARY_ERROR_INVALID_ARGUMENT, "fail"));
+  } else {
+    wp_object_update_features (WP_OBJECT (self), WP_PLUGIN_FEATURE_ENABLED, 0);
+  }
 }
 
 static void
@@ -155,6 +161,34 @@ test_load (TestFixture *f, gconstpointer data)
   g_assert_true (wp_core_test_feature (f->base.core, "feature.name123"));
 }
 
+static void
+on_component_failed (WpCore * core, GAsyncResult * res, TestFixture *f)
+{
+  gboolean loaded;
+  GError *error = NULL;
+
+  loaded = wp_core_load_component_finish (core, res, &error);
+  g_assert_error (error, WP_DOMAIN_LIBRARY, WP_LIBRARY_ERROR_INVALID_ARGUMENT);
+  g_assert_false (loaded);
+
+  g_main_loop_quit (f->base.loop);
+}
+
+static void
+test_load_failure (TestFixture *f, gconstpointer data)
+{
+  wp_core_load_component (f->base.core, "fail", "test", NULL,
+      "feature.fail", NULL, (GAsyncReadyCallback) on_component_failed, f);
+  g_main_loop_run (f->base.loop);
+
+  g_assert_cmpuint (f->loader->history->len, ==, 1);
+  g_assert_cmpstr (f->loader->history->pdata[0], ==, "fail");
+
+  g_autoptr (WpPlugin) plugin = wp_plugin_find (f->base.core, "fail");
+  g_assert_null (plugin);
+  g_assert_false (wp_core_test_feature (f->base.core, "feature.fail"));
+}
+
 static void
 test_dependencies_setup (TestFixture *f, gconstpointer data)
 {
@@ -204,6 +238,8 @@ main (gint argc, gchar *argv[])
 
   g_test_add ("/wp/comploader/load", TestFixture, NULL,
       test_setup, test_load, test_teardown);
+  g_test_add ("/wp/comploader/load_failure", TestFixture, NULL,
+      test_setup, test_load_failure, test_teardown);
   g_test_add ("/wp/comploader/dependencies", TestFixture, NULL,
       test_dependencies_setup, test_dependencies, test_teardown);