edje: Improve error report with efl_part misuse

This improves a rare error message when a function is called on an
efl_part() that does not implement it. Example: calling a swallow
function on a non-swallow part.

This isn't entirely fool-proof but should already help quite a bit.

This also changes how the efl_part proxies are stored: the variable
is not reset to NULL every time we use it, instead we check it in
the del intercept.

Note: _part_reuse_error() can not be enabled inside
_internal_proxy_get because there are valid use cases such as:

  func1(efl_part(obj, part), func2(efl_part(obj, part), ...), ...)

Here we use two efl_part() at the same time, on the same object,
but we haven't entered "func1" yet when we are reaching the second
call to efl_part(). This is completely valid and there is pretty
much no way to detect this.

I think I will improve this later with a core function on
Efl.Object like "debug_string".

Ref T5584

diff --git a/src/lib/edje/edje_part.c b/src/lib/edje/edje_part.c
index cbfd71e..eb09b93 100644 (file)
--- a/src/lib/edje/edje_part.c
+++ b/src/lib/edje/edje_part.c
@@ -2,9 +2,55 @@
 #include "edje_part_helper.h"
 #define MY_CLASS EFL_CANVAS_LAYOUT_INTERNAL_CLASS
 
-PROXY_IMPLEMENTATION(other, INTERNAL, EINA_FALSE)
+PROXY_IMPLEMENTATION(other, MY_CLASS, EINA_FALSE)
 #undef PROXY_IMPLEMENTATION
 
+void
+_part_reuse_error(Efl_Canvas_Layout_Internal_Data *pd)
+{
+   const char *typestr = "UNKNOWN";
+   Edje *ed;
+
+   // TODO: Enable full debug only for eo_debug?
+   // Don't trust pd->ed as it may be invalid now.
+   ed = efl_data_scope_safe_get(pd->obj, EDJE_OBJECT_CLASS);
+   if (!ed)
+     {
+        ERR("A previous misuse of efl_part has been detected. Handles returned "
+            "by efl_part() are valid for a single function call. Did you call "
+            "a non implemented function? obj: %p has been deleted!", pd->obj);
+        return;
+     }
+
+   switch (pd->rp->part->type)
+     {
+      case EDJE_PART_TYPE_RECTANGLE: typestr = "RECTANGLE"; break;
+      case EDJE_PART_TYPE_TEXT: typestr = "TEXT"; break;
+      case EDJE_PART_TYPE_IMAGE: typestr = "IMAGE"; break;
+      case EDJE_PART_TYPE_SWALLOW: typestr = "SWALLOW"; break;
+      case EDJE_PART_TYPE_TEXTBLOCK: typestr = "TEXTBLOCK"; break;
+      case EDJE_PART_TYPE_GRADIENT: typestr = "GRADIENT"; break;
+      case EDJE_PART_TYPE_GROUP: typestr = "GROUP"; break;
+      case EDJE_PART_TYPE_BOX: typestr = "BOX"; break;
+      case EDJE_PART_TYPE_TABLE: typestr = "TABLE"; break;
+      case EDJE_PART_TYPE_EXTERNAL: typestr = "EXTERNAL"; break;
+      case EDJE_PART_TYPE_PROXY: typestr = "PROXY"; break;
+      case EDJE_PART_TYPE_SPACER: typestr = "SPACER"; break;
+      case EDJE_PART_TYPE_MESH_NODE: typestr = "MESH_NODE"; break;
+      case EDJE_PART_TYPE_LIGHT: typestr = "LIGHT"; break;
+      case EDJE_PART_TYPE_CAMERA: typestr = "CAMERA"; break;
+      case EDJE_PART_TYPE_SNAPSHOT: typestr = "SNAPSHOT"; break;
+      case EDJE_PART_TYPE_VECTOR: typestr = "VECTOR"; break;
+      default: break;
+     }
+
+   ERR("A previous misuse of efl_part has been detected. Handles returned "
+       "by efl_part() are valid for a single function call. Did you call "
+       "a non implemented function? obj: %p group: '%s' part: '%s' type: %s%s",
+       pd->obj, ed->group, pd->part, typestr,
+       ed->delete_me ? ". This object is already deleted." : "");
+}
+
 PROXY_INIT(box)
 PROXY_INIT(table)
 PROXY_INIT(swallow)
@@ -24,20 +70,25 @@ _edje_internal_proxy_shutdown(void)
 }
 
 void
-_edje_real_part_set(Eo *obj, void *ed, void *rp, const char *part)
+_edje_real_part_set(Eo *obj, Edje *ed, Edje_Real_Part *rp, const char *part)
 {
-   PROXY_DATA_GET(obj, pd);
+   Efl_Canvas_Layout_Internal_Data *pd;
+
+   pd = efl_data_scope_get(obj, EFL_CANVAS_LAYOUT_INTERNAL_CLASS);
+   pd->obj = ed->obj;
    pd->ed = ed;
    pd->rp = rp;
    pd->part = part;
    pd->temp = 1;
-   efl_parent_set(obj, pd->ed->obj);
+   pd->in_call = 0;
+   efl_parent_set(obj, ed->obj);
 }
 
 EOLIAN static Efl_Object *
 _efl_canvas_layout_internal_efl_object_finalize(Eo *obj, Efl_Canvas_Layout_Internal_Data *pd)
 {
    EINA_SAFETY_ON_FALSE_RETURN_VAL(pd->rp && pd->ed && pd->part, NULL);
+   // Do not use RETURN_VAL() here!
    return efl_finalize(efl_super(obj, MY_CLASS));
 }
 
@@ -45,6 +96,7 @@ EOLIAN void
 _efl_canvas_layout_internal_efl_gfx_geometry_get(Eo *obj, Efl_Canvas_Layout_Internal_Data *pd, int *x, int *y, int *w, int *h)
 {
    Edje_Real_Part *rp = pd->rp;
+   PROXY_CALL_BEGIN(pd);
 
    _edje_recalc_do(pd->ed);
    if (!rp)
@@ -53,6 +105,7 @@ _efl_canvas_layout_internal_efl_gfx_geometry_get(Eo *obj, Efl_Canvas_Layout_Inte
         if (y) *y = 0;
         if (w) *w = 0;
         if (h) *h = 0;
+        RETURN_VOID;
      }
 
    if (x) *x = rp->x;
@@ -66,8 +119,10 @@ EOLIAN static void
 _efl_canvas_layout_internal_state_get(Eo *obj, Efl_Canvas_Layout_Internal_Data *pd, const char **name, double *val)
 {
    const char *str;
+   PROXY_CALL_BEGIN(pd);
 
-   if (!name && !val) return;
+   if (!name && !val)
+     RETURN_VOID;
    str = _edje_object_part_state_get(pd->ed, pd->part, val);
    if (name) *name = str;
    RETURN_VOID;
@@ -76,66 +131,77 @@ _efl_canvas_layout_internal_state_get(Eo *obj, Efl_Canvas_Layout_Internal_Data *
 EOLIAN static Eina_Bool
 _efl_canvas_layout_internal_efl_ui_drag_drag_value_set(Eo *obj, Efl_Canvas_Layout_Internal_Data *pd, double dx, double dy)
 {
+   PROXY_CALL_BEGIN(pd);
    RETURN_VAL(_edje_object_part_drag_value_set(pd->ed, pd->part, dx, dy));
 }
 
 EOLIAN static Eina_Bool
 _efl_canvas_layout_internal_efl_ui_drag_drag_value_get(Eo *obj, Efl_Canvas_Layout_Internal_Data *pd, double *dx, double *dy)
 {
+   PROXY_CALL_BEGIN(pd);
    RETURN_VAL(_edje_object_part_drag_value_get(pd->ed, pd->part, dx, dy));
 }
 
 EOLIAN static Eina_Bool
 _efl_canvas_layout_internal_efl_ui_drag_drag_size_set(Eo *obj, Efl_Canvas_Layout_Internal_Data *pd, double dw, double dh)
 {
+   PROXY_CALL_BEGIN(pd);
    RETURN_VAL(_edje_object_part_drag_size_set(pd->ed, pd->part, dw, dh));
 }
 
 EOLIAN static Eina_Bool
 _efl_canvas_layout_internal_efl_ui_drag_drag_size_get(Eo *obj, Efl_Canvas_Layout_Internal_Data *pd, double *dw, double *dh)
 {
+   PROXY_CALL_BEGIN(pd);
    RETURN_VAL(_edje_object_part_drag_size_get(pd->ed, pd->part, dw, dh));
 }
 
 EOLIAN static Efl_Ui_Drag_Dir
 _efl_canvas_layout_internal_efl_ui_drag_drag_dir_get(Eo *obj, Efl_Canvas_Layout_Internal_Data *pd)
 {
+   PROXY_CALL_BEGIN(pd);
    RETURN_VAL(_edje_object_part_drag_dir_get(pd->ed, pd->part));
 }
 
 EOLIAN static Eina_Bool
 _efl_canvas_layout_internal_efl_ui_drag_drag_step_set(Eo *obj, Efl_Canvas_Layout_Internal_Data *pd, double dx, double dy)
 {
+   PROXY_CALL_BEGIN(pd);
    RETURN_VAL(_edje_object_part_drag_step_set(pd->ed, pd->part, dx, dy));
 }
 
 EOLIAN static Eina_Bool
 _efl_canvas_layout_internal_efl_ui_drag_drag_step_get(Eo *obj, Efl_Canvas_Layout_Internal_Data *pd, double *dx, double *dy)
 {
+   PROXY_CALL_BEGIN(pd);
    RETURN_VAL(_edje_object_part_drag_step_get(pd->ed, pd->part, dx, dy));
 }
 
 EOLIAN static Eina_Bool
 _efl_canvas_layout_internal_efl_ui_drag_drag_step_move(Eo *obj, Efl_Canvas_Layout_Internal_Data *pd, double dx, double dy)
 {
+   PROXY_CALL_BEGIN(pd);
    RETURN_VAL(_edje_object_part_drag_step(pd->ed, pd->part, dx, dy));
 }
 
 EOLIAN static Eina_Bool
 _efl_canvas_layout_internal_efl_ui_drag_drag_page_set(Eo *obj, Efl_Canvas_Layout_Internal_Data *pd, double dx, double dy)
 {
+   PROXY_CALL_BEGIN(pd);
    RETURN_VAL(_edje_object_part_drag_page_set(pd->ed, pd->part, dx, dy));
 }
 
 EOLIAN static Eina_Bool
 _efl_canvas_layout_internal_efl_ui_drag_drag_page_get(Eo *obj, Efl_Canvas_Layout_Internal_Data *pd, double *dx, double *dy)
 {
+   PROXY_CALL_BEGIN(pd);
    RETURN_VAL(_edje_object_part_drag_page_get(pd->ed, pd->part, dx, dy));
 }
 
 EOLIAN static Eina_Bool
 _efl_canvas_layout_internal_efl_ui_drag_drag_page_move(Eo *obj, Efl_Canvas_Layout_Internal_Data *pd, double dx, double dy)
 {
+   PROXY_CALL_BEGIN(pd);
    RETURN_VAL(_edje_object_part_drag_page(pd->ed, pd->part, dx, dy));
 }
 

diff --git a/src/lib/edje/edje_part_box.c b/src/lib/edje/edje_part_box.c
index f2ae4b6..6e537fe 100644 (file)
--- a/src/lib/edje/edje_part_box.c
+++ b/src/lib/edje/edje_part_box.c
@@ -5,7 +5,7 @@
 
 #include "../evas/canvas/evas_box.eo.h"
 
-PROXY_IMPLEMENTATION(box, INTERNAL_BOX, EINA_FALSE)
+PROXY_IMPLEMENTATION(box, MY_CLASS, EINA_FALSE)
 #undef PROXY_IMPLEMENTATION
 
 typedef struct _Part_Item_Iterator Part_Item_Iterator;

diff --git a/src/lib/edje/edje_part_external.c b/src/lib/edje/edje_part_external.c
index 156d13f..1ffc416 100644 (file)
--- a/src/lib/edje/edje_part_external.c
+++ b/src/lib/edje/edje_part_external.c
@@ -5,7 +5,7 @@
 
 static void _external_compose(Eo *obj, Edje *ed, const char *part);
 
-PROXY_IMPLEMENTATION(external, EXTERNAL, EINA_TRUE, _external_compose(proxy, ed, rp->part->name))
+PROXY_IMPLEMENTATION(external, MY_CLASS, EINA_TRUE, _external_compose(proxy, ed, rp->part->name))
 #undef PROXY_IMPLEMENTATION
 
 static void

diff --git a/src/lib/edje/edje_part_helper.h b/src/lib/edje/edje_part_helper.h
index 4ae1ae0..90f0185 100644 (file)
--- a/src/lib/edje/edje_part_helper.h
+++ b/src/lib/edje/edje_part_helper.h
@@ -8,7 +8,8 @@ struct _Efl_Canvas_Layout_Internal_Data
    Edje           *ed;
    Edje_Real_Part *rp;
    const char     *part;
-   unsigned char   temp;
+   Eo             *obj;
+   unsigned char   temp, in_call;
 };
 
 struct _Part_Item_Iterator
@@ -19,26 +20,53 @@ struct _Part_Item_Iterator
    Eo            *object;
 };
 
+void _part_reuse_error(Efl_Canvas_Layout_Internal_Data *pd);
+
+#define PROXY_CALL_BEGIN(pd) do { pd->in_call = 1; } while (0)
+#define PROXY_CALL_END(pd) do { pd->in_call = 0; } while (0)
 #define PROXY_REF(obj, pd) do { if (!(pd->temp++)) efl_ref(obj); } while(0)
 #define PROXY_UNREF(obj, pd) do { if (pd->temp) { if (!(--pd->temp)) efl_del(obj); } } while(0)
-#define RETURN_VAL(a) do { typeof(a) _ret = a; PROXY_UNREF(obj, pd); return _ret; } while(0)
-#define RETURN_VOID do { PROXY_UNREF(obj, pd); return; } while(0)
+#define RETURN_VAL(a) do { typeof(a) _ret = a; PROXY_CALL_END(pd); PROXY_UNREF(obj, pd); return _ret; } while(0)
+#define RETURN_VOID do { PROXY_CALL_END(pd); PROXY_UNREF(obj, pd); return; } while(0)
 #define PROXY_CALL(a) ({ PROXY_REF(obj, pd); a; })
+#define PROXY_STATIC_VAR(type) _##type##_proxy
 
 #ifndef PROXY_ADD_EXTRA_OP
 # define PROXY_ADD_EXTRA_OP
 #endif
 
-void _edje_real_part_set(Eo *obj, void *ed, void *rp, const char *part);
+void _edje_real_part_set(Eo *obj, Edje *ed, Edje_Real_Part *rp, const char *part);
+
+static inline void
+_part_proxy_del_cb(Eo *proxy, Eo **static_var)
+{
+   if (*static_var)
+     {
+        // FIXME: Enable debug checks only in debug mode
+        Efl_Canvas_Layout_Internal_Data *pd = efl_data_scope_get
+              (*static_var, EFL_CANVAS_LAYOUT_INTERNAL_CLASS);
+        if (pd && pd->temp && !pd->in_call)
+          _part_reuse_error(pd);
+        if (*static_var != proxy)
+          efl_del_intercept_set(*static_var, NULL);
+     }
+   if (efl_parent_get(proxy))
+     {
+        efl_ref(proxy);
+        efl_parent_set(proxy, NULL);
+     }
+   efl_reuse(proxy);
+   *static_var = proxy;
+}
 
 /* ugly macros to avoid code duplication */
 
 #define PROXY_RESET(type) \
-   do { if (_ ## type ## _proxy) \
+   do { if (PROXY_STATIC_VAR(type)) \
      { \
-        efl_del_intercept_set(_ ## type ## _proxy, NULL); \
-        efl_del(_ ## type ## _proxy); \
-        _ ## type ## _proxy = NULL; \
+        efl_del_intercept_set(PROXY_STATIC_VAR(type), NULL); \
+        efl_del(PROXY_STATIC_VAR(type)); \
+        PROXY_STATIC_VAR(type) = NULL; \
      } } while (0)
 
 #define PROXY_INIT(type) \
@@ -46,27 +74,16 @@ void \
 _ ## type ## _shutdown(void); \
 
 #define PROXY_DATA_GET(obj, pd) \
-   Efl_Canvas_Layout_Internal_Data *pd = efl_data_scope_get(obj, EFL_CANVAS_LAYOUT_INTERNAL_CLASS)
+   Efl_Canvas_Layout_Internal_Data *pd = efl_data_scope_get(obj, EFL_CANVAS_LAYOUT_INTERNAL_CLASS); \
+   PROXY_CALL_BEGIN(pd)
 
-#define PROXY_IMPLEMENTATION(type, TYPE, no_del_cb, ...) \
-static Eo * _ ## type ## _proxy = NULL; \
+#define PROXY_IMPLEMENTATION(type, KLASS, no_del_cb, ...) \
+static Eo * PROXY_STATIC_VAR(type) = NULL; \
 \
 static void \
 _ ## type ## _del_cb(Eo *proxy) \
 { \
-   if (_ ## type ## _proxy) \
-     { \
-        efl_del_intercept_set(proxy, NULL); \
-        efl_del(proxy); \
-        return; \
-     } \
-   if (efl_parent_get(proxy)) \
-     { \
-        efl_ref(proxy); \
-        efl_parent_set(proxy, NULL); \
-     } \
-   efl_reuse(proxy); \
-   _ ## type ## _proxy = proxy; \
+   _part_proxy_del_cb(proxy, &(PROXY_STATIC_VAR(type))); \
 } \
 \
 void \
@@ -81,32 +98,27 @@ _edje_ ## type ## _internal_proxy_get(Edje_Object *obj EINA_UNUSED, Edje *ed, Ed
    Efl_Canvas_Layout_Internal_Data *pd; \
    Eo *proxy; \
    \
-   pd = efl_data_scope_get(_ ## type ## _proxy, EFL_CANVAS_LAYOUT_INTERNAL_CLASS); \
+   pd = PROXY_STATIC_VAR(type) ? efl_data_scope_get(PROXY_STATIC_VAR(type), EFL_CANVAS_LAYOUT_INTERNAL_CLASS) : NULL; \
    if (!pd) \
      { \
-        if (_ ## type ## _proxy) \
-          { \
-             ERR("Found invalid handle for efl_part. Reset."); \
-             _ ## type ## _proxy = NULL; \
-          } \
-        proxy = efl_add(EFL_CANVAS_LAYOUT_##TYPE##_CLASS, ed->obj, \
-                        _edje_real_part_set(efl_added, ed, rp, rp->part->name)); \
-        __VA_ARGS__; \
-        if (!no_del_cb) efl_del_intercept_set(proxy, _ ## type ## _del_cb); \
-        return proxy; \
+        if (PROXY_STATIC_VAR(type)) \
+          ERR("Found invalid handle for efl_part. Reset."); \
+        PROXY_STATIC_VAR(type) = efl_add(KLASS, ed->obj, _edje_real_part_set(efl_added, ed, rp, rp->part->name)); \
+        goto end ; \
      } \
    \
    if (EINA_UNLIKELY(pd->temp)) \
      { \
-        /* warn about misuse, since non-implemented functions may trigger this \
-         * misuse by accident. */ \
-        ERR("Misuse of efl_part detected. Handles returned by efl_part() are " \
-            "valid for a single function call! Did you call a non implemented " \
-            "function?"); \
+        /* if (!pd->in_call) _part_reuse_error(pd); */ \
+        PROXY_STATIC_VAR(type) = efl_add(KLASS, ed->obj, _edje_real_part_set(efl_added, ed, rp, rp->part->name)); \
      } \
-   proxy = _ ## type ## _proxy; \
-   _ ## type ## _proxy = NULL; \
-   _edje_real_part_set(proxy, ed, rp, rp->part->name); \
+   else \
+     { \
+        _edje_real_part_set(PROXY_STATIC_VAR(type), ed, rp, rp->part->name); \
+     } \
+   \
+end: \
+   proxy = PROXY_STATIC_VAR(type); \
    __VA_ARGS__; \
    if (!no_del_cb) efl_del_intercept_set(proxy, _ ## type ## _del_cb); \
    return proxy; \

diff --git a/src/lib/edje/edje_part_swallow.c b/src/lib/edje/edje_part_swallow.c
index a85fdb9..4e9564d 100644 (file)
--- a/src/lib/edje/edje_part_swallow.c
+++ b/src/lib/edje/edje_part_swallow.c
@@ -3,7 +3,7 @@
 #include "efl_canvas_layout_internal_swallow.eo.h"
 #define MY_CLASS EFL_CANVAS_LAYOUT_INTERNAL_SWALLOW_CLASS
 
-PROXY_IMPLEMENTATION(swallow, INTERNAL_SWALLOW, EINA_FALSE)
+PROXY_IMPLEMENTATION(swallow, MY_CLASS, EINA_FALSE)
 #undef PROXY_IMPLEMENTATION
 
 /* Swallow parts */

diff --git a/src/lib/edje/edje_part_table.c b/src/lib/edje/edje_part_table.c
index c1bc0b4..211dce1 100644 (file)
--- a/src/lib/edje/edje_part_table.c
+++ b/src/lib/edje/edje_part_table.c
@@ -5,7 +5,7 @@
 
 #include "../evas/canvas/evas_table.eo.h"
 
-PROXY_IMPLEMENTATION(table, INTERNAL_TABLE, EINA_FALSE)
+PROXY_IMPLEMENTATION(table, MY_CLASS, EINA_FALSE)
 #undef PROXY_IMPLEMENTATION
 
 typedef struct _Part_Item_Iterator Part_Item_Iterator;

diff --git a/src/lib/edje/edje_part_text.c b/src/lib/edje/edje_part_text.c
index 027863a..bc848fc 100644 (file)
--- a/src/lib/edje/edje_part_text.c
+++ b/src/lib/edje/edje_part_text.c
@@ -3,7 +3,7 @@
 #include "efl_canvas_layout_internal_text.eo.h"
 #define MY_CLASS EFL_CANVAS_LAYOUT_INTERNAL_TEXT_CLASS
 
-PROXY_IMPLEMENTATION(text, INTERNAL_TEXT, EINA_FALSE)
+PROXY_IMPLEMENTATION(text, MY_CLASS, EINA_FALSE)
 #undef PROXY_IMPLEMENTATION
 
 EOLIAN static void