evas textblock: fix double free issue from user style push/pop and free

Summary: The Textblock Style which is created for user style was managed
application side.  It is created and free'd from application - outside
of Evas Textblock.  Recently, evas_object_textblock_style_user_push/pop
start to call efl_canvas_text_style_set() instead of legacy code. The
problem is efl_canvas_text_style_set() is always going to call free()
when a style is going to be deleted.  It makes conflicts(double free
issue) with application which is used to call
evas_textblock_style_free().  So, the issue will be fixed by this patch.

The patch also revise push/pop/peek code to make clean and avoid
meaningless calculation/events.

@fix

Test Plan:
A test case is Included in this patch.
The test case try to trigger double free.

Reviewers: herdsman, raster, cedric

Subscribers: zmike, woohyun

Tags: #efl

Differential Revision: https://phab.enlightenment.org/D5812

Committer's note: formatted commit summary (80 width).

2 files changed, 64 insertions, 11 deletions

diff --git a/src/lib/evas/canvas/evas_object_textblock.c b/src/lib/evas/canvas/evas_object_textblock.c
index ce6e60edd8..f17ae47975 100644
--- a/src/lib/evas/canvas/evas_object_textblock.c
+++ b/src/lib/evas/canvas/evas_object_textblock.c
@@ -7221,6 +7221,16 @@ _style_by_key_find(Efl_Canvas_Text_Data *o, const char *key)
    return NULL;
 }
 
+static void
+_style_remove_from_obj(Eo *eo_obj, Efl_Canvas_Text_Data *o, Evas_Textblock_Style *ts, Eina_Bool style_free)
+{
+   o->styles = eina_list_remove(o->styles, ts);
+   ts->objects = eina_list_remove(ts->objects, eo_obj);
+
+   if (style_free || (ts->delete_me && !ts->objects))
+     evas_textblock_style_free(ts);
+}
+
 EOLIAN static void
 _efl_canvas_text_style_set(Eo *eo_obj, Efl_Canvas_Text_Data *o, const char *key, const char *style)
 {
@@ -7246,9 +7256,7 @@ _efl_canvas_text_style_set(Eo *eo_obj, Efl_Canvas_Text_Data *o, const char *key,
           }
         else
           {
-             o->styles = eina_list_remove(o->styles, ts);
-             ts->objects = eina_list_remove(ts->objects, eo_obj);
-             evas_textblock_style_free(ts);
+             _style_remove_from_obj(eo_obj, o, ts, EINA_TRUE);
           }
      }
    else if (!ts && style)
@@ -7296,20 +7304,29 @@ EAPI void
 evas_object_textblock_style_user_push(Eo *eo_obj, Evas_Textblock_Style *ts)
 {
    EINA_SAFETY_ON_NULL_RETURN(eo_obj);
-   Efl_Canvas_Text_Data *o = efl_data_scope_get(eo_obj, MY_CLASS);
    Evas_Object_Protected_Data *obj = efl_data_scope_get(eo_obj, EFL_CANVAS_OBJECT_CLASS);
    evas_object_async_block(obj);
+   Efl_Canvas_Text_Data *o = efl_data_scope_get(eo_obj, MY_CLASS);
+   Evas_Textblock_Style *old_ts, *tmp = NULL;
 
-   Evas_Textblock_Style *old_ts = _style_by_key_find(o, _STYLE_USER);
+   old_ts = _style_by_key_find(o, _STYLE_USER);
+
+   if (old_ts == ts) return;
 
    if (old_ts)
+     _style_remove_from_obj(eo_obj, o, old_ts, EINA_FALSE);
+
+   if (ts)
      {
-        efl_canvas_text_style_set(eo_obj, _STYLE_USER, NULL);
+        _textblock_style_generic_set(eo_obj, ts, &tmp);
+        ts->key = eina_stringshare_add(_STYLE_USER);
+        o->styles = eina_list_append(o->styles, ts);
      }
-   Evas_Textblock_Style *tmp = NULL;
-   _textblock_style_generic_set(eo_obj, ts, &tmp);
-   ts->key = eina_stringshare_add(_STYLE_USER);
-   o->styles = eina_list_append(o->styles, ts);
+
+   o->format_changed = EINA_TRUE;
+   _evas_textblock_invalidate_all(o);
+   _evas_textblock_changed(o, eo_obj);
+   efl_event_callback_call(eo_obj, EFL_CANVAS_TEXT_EVENT_CHANGED, NULL);
 }
 
 EAPI const Evas_Textblock_Style*
@@ -7320,6 +7337,7 @@ evas_object_textblock_style_user_peek(const Eo *eo_obj)
    evas_object_async_block(obj);
    Efl_Canvas_Text_Data *o = efl_data_scope_get(eo_obj, MY_CLASS);
    Evas_Textblock_Style *ts = _style_by_key_find(o, _STYLE_USER);
+
    return ts;
 }
 
@@ -7329,8 +7347,18 @@ evas_object_textblock_style_user_pop(Eo *eo_obj)
    EINA_SAFETY_ON_NULL_RETURN(eo_obj);
    Evas_Object_Protected_Data *obj = efl_data_scope_get(eo_obj, EFL_CANVAS_OBJECT_CLASS);
    evas_object_async_block(obj);
+   Efl_Canvas_Text_Data *o = efl_data_scope_get(eo_obj, MY_CLASS);
+   Evas_Textblock_Style *ts = _style_by_key_find(o, _STYLE_USER);
 
-   efl_canvas_text_style_set(eo_obj, _STYLE_USER, NULL);
+   if (ts)
+     {
+        _style_remove_from_obj(eo_obj, o, ts, EINA_FALSE);
+
+        o->format_changed = EINA_TRUE;
+        _evas_textblock_invalidate_all(o);
+        _evas_textblock_changed(o, eo_obj);
+        efl_event_callback_call(eo_obj, EFL_CANVAS_TEXT_EVENT_CHANGED, NULL);
+     }
 }
 
 EAPI void
diff --git a/src/tests/evas/evas_test_textblock.c b/src/tests/evas/evas_test_textblock.c
index d4d39eb598..070117fa38 100644
--- a/src/tests/evas/evas_test_textblock.c
+++ b/src/tests/evas/evas_test_textblock.c
@@ -3705,6 +3705,30 @@ EFL_START_TEST(evas_textblock_style)
 }
 EFL_END_TEST
 
+/* Basic test for style user push/peek/pop. */
+START_TEST(evas_textblock_style_user)
+{
+   Evas_Textblock_Style *user_st;
+
+   START_TB_TEST();
+
+   user_st = evas_textblock_style_new();
+   fail_if(!user_st);
+   evas_textblock_style_set(user_st, "DEFAULT='" TEST_FONT " font_size=50 color=#000'");
+
+   evas_object_textblock_style_user_push(tb, user_st);
+   fail_if(evas_object_textblock_style_user_peek(tb) != user_st);
+
+   evas_object_textblock_style_user_pop(tb);
+   fail_if(evas_object_textblock_style_user_peek(tb) != NULL);
+
+   /* new/free should be handled from outside of Evas Textblock. */
+   evas_textblock_style_free(user_st);
+
+   END_TB_TEST();
+}
+END_TEST
+
 /* Various setters and getters */
 EFL_START_TEST(evas_textblock_set_get)
 {
@@ -4472,6 +4496,7 @@ void evas_test_textblock(TCase *tc)
    tcase_add_test(tc, evas_textblock_size);
    tcase_add_test(tc, evas_textblock_editing);
    tcase_add_test(tc, evas_textblock_style);
+   tcase_add_test(tc, evas_textblock_style_user);
    tcase_add_test(tc, evas_textblock_evas);
    tcase_add_test(tc, evas_textblock_text_getters);
    tcase_add_test(tc, evas_textblock_formats);