session-item: introduce a rollback() virtual method

+ replace calling execute_step(..., STEP_ERROR) with rollback
+ implement deactivate internally using rollback

This unifies deactivation steps, which are common between deactivate()
and calling execute_step() with WP_TRANSITION_STEP_ERROR at the
end of a failed activation transition.

lib/wp/session-item.c

@@ -25,6 +25,7 @@ struct _WpSiTransition
       guint step);
   void (*execute_step) (WpSessionItem * self, WpTransition * transition,
       guint step);
+  void (*rollback) (WpSessionItem * self);
 };
 
 G_DECLARE_FINAL_TYPE (WpSiTransition, wp_si_transition,
@@ -54,7 +55,11 @@ wp_si_transition_execute_step (WpTransition * transition, guint step)
 {
   WpSiTransition *self = WP_SI_TRANSITION (transition);
   WpSessionItem *item = wp_transition_get_source_object (transition);
-  self->execute_step (item, transition, step);
+
+  if (step != WP_TRANSITION_STEP_ERROR)
+    self->execute_step (item, transition, step);
+  else if (self->rollback)
+    self->rollback (item);
 }
 
 static void
@@ -172,32 +177,6 @@ wp_session_item_default_get_next_step (WpSessionItem * self,
   return WP_TRANSITION_STEP_NONE;
 }
 
-static void
-wp_session_item_default_execute_step (WpSessionItem * self,
-    WpTransition * transition, guint step)
-{
-  switch (step) {
-  case WP_TRANSITION_STEP_NONE:
-  case WP_TRANSITION_STEP_ERROR:
-    break;
-  default:
-    g_return_if_reached ();
-  }
-}
-
-static void
-wp_session_item_default_deactivate (WpSessionItem * self)
-{
-  WpSessionItemPrivate *priv = wp_session_item_get_instance_private (self);
-  static const guint flags =
-      (WP_SI_FLAG_ACTIVATING | WP_SI_FLAG_ACTIVE | WP_SI_FLAG_IN_ERROR);
-
-  if (priv->flags & flags) {
-    priv->flags &= ~flags;
-    g_signal_emit (self, signals[SIGNAL_FLAGS_CHANGED], 0, priv->flags);
-  }
-}
-
 enum {
   EXPORT_STEP_ENDPOINT = WP_TRANSITION_STEP_CUSTOM_START,
   EXPORT_STEP_STREAMS,
@@ -340,16 +319,23 @@ default_export_execute_step (WpSessionItem * self, WpTransition * transition,
     wp_transition_advance (transition);
     break;
 
-  case WP_TRANSITION_STEP_ERROR:
-    g_clear_pointer (&priv->impl_streams, g_hash_table_unref);
-    g_clear_object (&priv->impl_proxy);
-    g_weak_ref_set (&priv->session, NULL);
+  default:
+    g_return_if_reached ();
+  }
+}
+static void
+default_export_rollback (WpSessionItem * self)
+{
+  WpSessionItemPrivate *priv = wp_session_item_get_instance_private (self);
+  static const guint flags = (WP_SI_FLAG_EXPORTING | WP_SI_FLAG_EXPORTED);
+
+  g_clear_pointer (&priv->impl_streams, g_hash_table_unref);
+  g_clear_object (&priv->impl_proxy);
+  g_weak_ref_set (&priv->session, NULL);
 
-    if (priv->flags & WP_SI_FLAG_EXPORTING) {
-      priv->flags &= ~WP_SI_FLAG_EXPORTING;
-      g_signal_emit (self, signals[SIGNAL_FLAGS_CHANGED], 0, priv->flags);
-    }
-    break;
+  if (priv->flags & flags) {
+    priv->flags &= ~flags;
+    g_signal_emit (self, signals[SIGNAL_FLAGS_CHANGED], 0, priv->flags);
   }
 }
 
@@ -369,6 +355,7 @@ wp_session_item_default_export (WpSessionItem * self,
 
   WP_SI_TRANSITION (transition)->get_next_step = default_export_get_next_step;
   WP_SI_TRANSITION (transition)->execute_step = default_export_execute_step;
+  WP_SI_TRANSITION (transition)->rollback = default_export_rollback;
   wp_transition_advance (transition);
 }
 
@@ -385,17 +372,7 @@ wp_session_item_default_export_finish (WpSessionItem * self,
 static void
 wp_session_item_default_unexport (WpSessionItem * self)
 {
-  WpSessionItemPrivate *priv = wp_session_item_get_instance_private (self);
-  static const guint flags = (WP_SI_FLAG_EXPORTING | WP_SI_FLAG_EXPORTED);
-
-  g_clear_pointer (&priv->impl_streams, g_hash_table_unref);
-  g_clear_object (&priv->impl_proxy);
-  g_weak_ref_set (&priv->session, NULL);
-
-  if (priv->flags & flags) {
-    priv->flags &= ~flags;
-    g_signal_emit (self, signals[SIGNAL_FLAGS_CHANGED], 0, priv->flags);
-  }
+  default_export_rollback (self);
 }
 
 static void
@@ -409,8 +386,6 @@ wp_session_item_class_init (WpSessionItemClass * klass)
   klass->reset = wp_session_item_default_reset;
   klass->get_associated_proxy = wp_session_item_default_get_associated_proxy;
   klass->get_next_step = wp_session_item_default_get_next_step;
-  klass->execute_step = wp_session_item_default_execute_step;
-  klass->deactivate = wp_session_item_default_deactivate;
   klass->export = wp_session_item_default_export;
   klass->export_finish = wp_session_item_default_export_finish;
   klass->unexport = wp_session_item_default_unexport;
@@ -656,6 +631,8 @@ wp_session_item_activate (WpSessionItem * self,
       WP_SESSION_ITEM_GET_CLASS (self)->get_next_step;
   WP_SI_TRANSITION (transition)->execute_step =
       WP_SESSION_ITEM_GET_CLASS (self)->execute_step;
+  WP_SI_TRANSITION (transition)->rollback =
+      WP_SESSION_ITEM_GET_CLASS (self)->rollback;
   wp_transition_advance (transition);
 }
 
@@ -677,7 +654,7 @@ wp_session_item_activate_finish (WpSessionItem * self, GAsyncResult * res,
 }
 
 /**
- * wp_session_item_deactivate: (virtual deactivate)
+ * wp_session_item_deactivate:
  * @self: the session item
  *
  * De-activates the item and/or cancels any ongoing activation operation.
@@ -688,11 +665,21 @@ void
 wp_session_item_deactivate (WpSessionItem * self)
 {
   g_return_if_fail (WP_IS_SESSION_ITEM (self));
-  g_return_if_fail (WP_SESSION_ITEM_GET_CLASS (self)->deactivate);
+
+  WpSessionItemPrivate *priv = wp_session_item_get_instance_private (self);
+  static const guint flags =
+      (WP_SI_FLAG_ACTIVATING | WP_SI_FLAG_ACTIVE | WP_SI_FLAG_IN_ERROR);
 
   //TODO cancel job if ACTIVATING
 
-  WP_SESSION_ITEM_GET_CLASS (self)->deactivate (self);
+  if (priv->flags & WP_SI_FLAG_ACTIVE &&
+      WP_SESSION_ITEM_GET_CLASS (self)->rollback)
+    WP_SESSION_ITEM_GET_CLASS (self)->rollback (self);
+
+  if (priv->flags & flags) {
+    priv->flags &= ~flags;
+    g_signal_emit (self, signals[SIGNAL_FLAGS_CHANGED], 0, priv->flags);
+  }
 }
 
 /**

lib/wp/session-item.h

@@ -72,7 +72,7 @@ typedef enum {
  *   transition of wp_session_item_activate()
  * @execute_step: Implements #WpTransitionClass.execute_step() for the
  *   transition of wp_session_item_activate()
- * @deactivate: See wp_session_item_deactivate()
+ * @rollback:
  * @export: See wp_session_item_export()
  * @export_finish: See wp_session_item_export_finish()
  * @unexport: See wp_session_item_unexport()
@@ -92,7 +92,7 @@ struct _WpSessionItemClass
       guint step);
   void (*execute_step) (WpSessionItem * self, WpTransition * transition,
       guint step);
-  void (*deactivate) (WpSessionItem * self);
+  void (*rollback) (WpSessionItem * self);
 
   void (*export) (WpSessionItem * self,
       WpSession * session, GCancellable * cancellable,

modules/module-si-adapter.c

@@ -70,14 +70,12 @@ si_adapter_reset (WpSessionItem * item)
 }
 
 static void
-si_adapter_deactivate (WpSessionItem * item)
+si_adapter_rollback (WpSessionItem * item)
 {
   WpSiAdapter *self = WP_SI_ADAPTER (item);
 
   g_clear_object (&self->ports_om);
   wp_session_item_clear_flag (item, WP_SI_FLAG_CONFIGURED);
-
-  WP_SESSION_ITEM_CLASS (si_adapter_parent_class)->deactivate (item);
 }
 
 static gpointer
@@ -354,9 +352,7 @@ si_adapter_execute_step (WpSessionItem * item, WpTransition * transition,
       break;
     }
     default:
-      WP_SESSION_ITEM_CLASS (si_adapter_parent_class)->execute_step (item,
-          transition, step);
-      break;
+      g_return_if_reached ();
   }
 }
 
@@ -371,7 +367,7 @@ si_adapter_class_init (WpSiAdapterClass * klass)
   si_class->get_configuration = si_adapter_get_configuration;
   si_class->get_next_step = si_adapter_get_next_step;
   si_class->execute_step = si_adapter_execute_step;
-  si_class->deactivate = si_adapter_deactivate;
+  si_class->rollback = si_adapter_rollback;
 }
 
 static guint

modules/module-si-standard-link.c

@@ -374,14 +374,12 @@ si_standard_link_execute_step (WpSessionItem * item, WpTransition * transition,
     break;
   }
   default:
-    WP_SESSION_ITEM_CLASS (si_standard_link_parent_class)->execute_step (
-          item, transition, step);
-      break;
+    g_return_if_reached ();
   }
 }
 
 static void
-si_standard_link_deactivate (WpSessionItem * item)
+si_standard_link_rollback (WpSessionItem * item)
 {
   WpSiStandardLink *self = WP_SI_STANDARD_LINK (item);
   WpSiEndpoint *out_endpoint, *in_endpoint;
@@ -402,8 +400,6 @@ si_standard_link_deactivate (WpSessionItem * item)
   }
 
   g_clear_pointer (&self->node_links, g_ptr_array_unref);
-
-  WP_SESSION_ITEM_CLASS (si_standard_link_parent_class)->deactivate (item);
 }
 
 static void
@@ -416,7 +412,7 @@ si_standard_link_class_init (WpSiStandardLinkClass * klass)
   si_class->get_configuration = si_standard_link_get_configuration;
   si_class->get_next_step = si_standard_link_get_next_step;
   si_class->execute_step = si_standard_link_execute_step;
-  si_class->deactivate = si_standard_link_deactivate;
+  si_class->rollback = si_standard_link_rollback;
 }
 
 static GVariant *

tests/wp/session-item.c

@@ -111,29 +111,19 @@ si_dummy_execute_step (WpSessionItem * item, WpTransition * transition,
       wp_transition_advance (transition);
       break;
 
-    case WP_TRANSITION_STEP_ERROR:
-      self->cleaned_up = TRUE;
-      self->step_1_done = FALSE;
-      self->step_2_done = FALSE;
-      WP_SESSION_ITEM_CLASS (si_dummy_parent_class)->execute_step (item,
-          transition, step);
-      break;
-
     default:
       g_assert_not_reached ();
   }
 }
 
 static void
-si_dummy_deactivate (WpSessionItem * item)
+si_dummy_rollback (WpSessionItem * item)
 {
   TestSiDummy *self = TEST_SI_DUMMY (item);
 
-  self->cleaned_up = FALSE;
+  self->cleaned_up = TRUE;
   self->step_1_done = FALSE;
   self->step_2_done = FALSE;
-
-  WP_SESSION_ITEM_CLASS (si_dummy_parent_class)->deactivate (item);
 }
 
 static void
@@ -145,7 +135,7 @@ si_dummy_class_init (TestSiDummyClass * klass)
   si_class->get_configuration = si_dummy_get_configuration;
   si_class->get_next_step = si_dummy_get_next_step;
   si_class->execute_step = si_dummy_execute_step;
-  si_class->deactivate = si_dummy_deactivate;
+  si_class->rollback = si_dummy_rollback;
 }
 
 static void
@@ -256,7 +246,7 @@ test_activation (void)
   g_assert_cmpint (signalled_flags, ==, 0);
   g_assert_false (dummy->step_1_done);
   g_assert_false (dummy->step_2_done);
-  g_assert_false (dummy->cleaned_up);
+  g_assert_true (dummy->cleaned_up);
 }
 
 
@@ -314,6 +304,7 @@ test_activation_error (void)
   g_assert_false (dummy->step_2_done);
   g_assert_true (dummy->cleaned_up);
 
+  dummy->cleaned_up = FALSE;
   wp_session_item_deactivate (item);
 
   g_assert_cmpint (wp_session_item_get_flags (item), ==, WP_SI_FLAG_CONFIGURED);