fix(observer): check if observer is associated with obj on remove fn (#7727)

Co-authored-by: Gabor Kiss-Vamosi <kisvegabor@gmail.com>

src/others/observer/lv_observer.c

@@ -300,11 +300,6 @@ void lv_subject_deinit(lv_subject_t * subject)
     while(observer) {
         lv_observer_t * observer_next = lv_ll_get_next(&subject->subs_ll, observer);
 
-        if(observer->for_obj) {
-            lv_obj_remove_event_cb(observer->target, unsubscribe_on_delete_cb);
-            lv_obj_remove_event_cb_with_user_data(observer->target, NULL, subject);
-        }
-
         lv_observer_remove(observer);
         observer = observer_next;
     }
@@ -394,6 +389,11 @@ void lv_observer_remove(lv_observer_t * observer)
 {
     LV_ASSERT_NULL(observer);
 
+    if(observer->for_obj && observer->target) {
+        lv_obj_remove_event_cb_with_user_data(observer->target, unsubscribe_on_delete_cb, observer);
+        lv_obj_remove_event_cb_with_user_data(observer->target, NULL, observer->subject);
+    }
+
     observer->subject->notify_restart_query = 1;
 
     lv_ll_remove(&(observer->subject->subs_ll), observer);
@@ -406,6 +406,14 @@ void lv_observer_remove(lv_observer_t * observer)
 
 void lv_obj_remove_from_subject(lv_obj_t * obj, lv_subject_t * subject)
 {
+    LV_ASSERT_NULL(obj);
+    /*
+     * Look for the `observer` that connects `obj` and `subject`
+     * Since the obj is associated with the subject,
+     *  the `obj` will have an LV_EVENT_REMOVE event with the `unsubscribe_on_delete_cb` callback
+     *  associated.
+     * From the event we can then find the observer in the event's `user_data` field
+     */
     int32_t i;
     int32_t event_cnt = (int32_t)(obj->spec_attr ? lv_event_get_count(&obj->spec_attr->event_list) : 0);
     for(i = event_cnt - 1; i >= 0; i--) {
@@ -413,8 +421,8 @@ void lv_obj_remove_from_subject(lv_obj_t * obj, lv_subject_t * subject)
         if(event_dsc->cb == unsubscribe_on_delete_cb) {
             lv_observer_t * observer = event_dsc->user_data;
             if(subject == NULL || subject == observer->subject) {
+                /* lv_observer_remove handles the deletion of all possible event callbacks */
                 lv_observer_remove(observer);
-                lv_obj_remove_event(obj, i);
             }
         }
     }

tests/src/test_cases/test_observer.c

@@ -59,6 +59,93 @@ void test_observer_add_remove(void)
     TEST_ASSERT_EQUAL_PTR(NULL, observer); /*The observer must be NULL*/
 }
 
+void test_object_observer_add_remove(void)
+{
+
+    lv_obj_t * obj = lv_obj_create(lv_screen_active());
+    static lv_subject_t subject;
+    lv_subject_init_int(&subject, 1);
+
+    lv_observer_t * observer = lv_obj_bind_flag_if_eq(obj, &subject, LV_OBJ_FLAG_HIDDEN, 5);
+
+    TEST_ASSERT_EQUAL(false, lv_obj_has_flag(obj, LV_OBJ_FLAG_HIDDEN));
+    lv_subject_set_int(&subject, 5);
+    TEST_ASSERT_EQUAL(true, lv_obj_has_flag(obj, LV_OBJ_FLAG_HIDDEN));
+    lv_observer_remove(observer);
+    lv_subject_set_int(&subject, 1);
+
+    /* This shouldn't get updated */
+    TEST_ASSERT_EQUAL(true, lv_obj_has_flag(obj, LV_OBJ_FLAG_HIDDEN));
+    lv_obj_delete(obj);
+    /* We shouldn't crash here */
+}
+
+static lv_event_dsc_t * get_event_delete_from_obj(lv_obj_t * obj)
+{
+
+    /* The remove event is a event callback using the observer as the user data */
+    uint32_t event_cnt = lv_event_get_count(&obj->spec_attr->event_list);
+    for(uint32_t i = 0; i < event_cnt; i++) {
+        lv_event_dsc_t * event = lv_obj_get_event_dsc(obj, i);
+        TEST_ASSERT_NOT_NULL(event);
+        if(event->filter == LV_EVENT_DELETE) {
+            return event;
+        }
+    }
+    return NULL;
+}
+void test_obj_remove_from_subject_removes_delete_event(void)
+{
+
+    lv_obj_t * obj = lv_obj_create(lv_screen_active());
+    static lv_subject_t subject;
+    lv_subject_init_int(&subject, 1);
+    (void)lv_subject_add_observer_obj(&subject, observer_basic, obj, NULL);
+
+    {
+        /*
+         * We expect the event delete to be added to the object allowing the observer
+         * to be deleted when the object is deleted
+         */
+        TEST_ASSERT_NOT_NULL(obj->spec_attr);
+        TEST_ASSERT_EQUAL(lv_event_get_count(&obj->spec_attr->event_list), 1);
+        lv_event_dsc_t * delete_event = get_event_delete_from_obj(obj);
+        TEST_ASSERT_NOT_NULL(delete_event);
+    }
+    {
+        /* Removing the object from the subject should remove the delete event entry */
+        lv_obj_remove_from_subject(obj, &subject);
+        lv_event_dsc_t * delete_event  = get_event_delete_from_obj(obj);
+        TEST_ASSERT_NULL(delete_event);
+    }
+
+}
+
+void test_observer_remove_removes_obj_callback(void)
+{
+    lv_obj_t * obj = lv_obj_create(lv_screen_active());
+    static lv_subject_t subject;
+    lv_subject_init_int(&subject, 1);
+    lv_observer_t * observer = lv_subject_add_observer_obj(&subject, observer_basic, obj, NULL);
+
+    {
+        /*
+         * We expect the event delete to be added to the object allowing the observer
+         * to be deleted when the object is deleted
+         */
+        TEST_ASSERT_NOT_NULL(obj->spec_attr);
+        TEST_ASSERT_EQUAL(lv_event_get_count(&obj->spec_attr->event_list), 1);
+        lv_event_dsc_t * delete_event = get_event_delete_from_obj(obj);
+        TEST_ASSERT_NOT_NULL(delete_event);
+    }
+    {
+        /* Removing the observer associated with the object should remove the delete event entry */
+        lv_observer_remove(observer);
+        lv_event_dsc_t * delete_event  = get_event_delete_from_obj(obj);
+        TEST_ASSERT_NULL(delete_event);
+    }
+}
+
 void test_observer_int(void)
 {
     static lv_subject_t subject;