viewport: Do not use evas_object_event_callback_del()

For short, the `evas_object_event_callback_del_full()` should be used
instead of `evas_object_event_callback_del()`.

The crash happened as the backtrace shown below, and it seems that the
crash was due to the misuse of `evas_object_event_callback_del()`.
This patch also log more about the viewport just in case the patch may
not fix the crash.

backtrace:
(gdb)bt

  #0  0x000d6e98 in e_object_is_del (obj=0x6) at /usr/src/debug/enlightenment-0.20.0/src/bin/e_object.c:106
  #1  0x00086088 in _e_comp_wl_viewport_cb_parent_show(data=0x1d50c70, e=<optimized out>, obj=<optimized out>, event_info=<optimized out>) at /usr/src/debug/enlightenment-0.20.0/src/bin/e_comp_wl_viewport.c:572
  #2 0xb5d91df6 in _eo_evas_object_cb (data=0x207e4c8, event=<optimized out>) at /usr/src/debug/efl-1.25.1/builddir/../src/lib/evas/canvas/evas_callbacks.c:181
  ...

The evas_object_event_callback_del() is to remove the most recently
added callback from the object. Having used it in `e_comp_wl_viewport.c`
could have caused unintentional removal of another callback depending on
the timing of creation and destruction of several viewports, and this
eventually could result in a crash.

I couldn't find the clear evidence that this really led the crash, but
let me give you an example what situation can make this crash.

1. There are 2 viewports created with the same parent. So, 2 viewports
would add a callback for its own. I will call both viewport as `A` and
`B`, and `A` is created before `B`.

2. The viewport `A` try to delete its callback from the parent evas
object because of the change of subsurface parent by calling
`evas_object_event_callback_del()`.  And this will unintentionally
delete the callback of `B`, not `A` because the callback of `B` is the
most recently added one.

3. Let's say the viewport `A` gets deleted by `tizen_viewport.destroy`,
and it does not call the `evas_object_event_callback_del_full()` because
`viewport->epc` must already be null.  And now, the data for the
callback `A` becomes freed memory, but the callback `A` is still in
callback list of parent object.

4. Event if the viewport `B` gets deleted by `tizen_viewport.destroy`,
it won't be able to remove any callbacks with
`evas_object_event_callback_del_full()` because the callback of `B` has
already been removed by the process 2.

As I mentioned, I'm not sure this really led the crash but I believe and
hope that this will fix the crash.

Change-Id: Ie92900df80d2351bbf9054fd3c4e9e6184b67d57

diff --git a/src/bin/e_comp_wl_viewport.c b/src/bin/e_comp_wl_viewport.c
index f2279d0..41b31da 100644 (file)
--- a/src/bin/e_comp_wl_viewport.c
+++ b/src/bin/e_comp_wl_viewport.c
@@ -8,17 +8,14 @@
 #include <scaler-server-protocol.h>
 #include <tizen-extension-server-protocol.h>
 
-#define PER(fmt,arg...) ELOGF("VIEWPORT", "<ERR> window(0x%08"PRIxPTR") epc(%p): " \
-                              fmt, viewport->ec, \
-                              viewport->window, viewport->epc, ##arg)
-#define PWR(fmt,arg...) ELOGF("VIEWPORT", "<WRN> window(0x%08"PRIxPTR") epc(%p): " \
-                              fmt, viewport->ec, \
-                              viewport->window, viewport->epc, ##arg)
-#define PIN(fmt,arg...) ELOGF("VIEWPORT", "<INF> window(0x%08"PRIxPTR") epc(%p): " \
-                              fmt, viewport->ec, \
-                              viewport->window, viewport->epc, ##arg)
-#define PDB(fmt,arg...) DBG("window(0x%08"PRIxPTR") ec(%p) epc(%p): "fmt, \
-                            viewport->window, viewport->ec, viewport->epc, ##arg)
+#define PER(fmt,arg...) ELOGF("VIEWPORT<E>", "viewport(%p) epc(%p): " \
+                              fmt, viewport->ec, viewport, viewport->epc, ##arg)
+#define PWR(fmt,arg...) ELOGF("VIEWPORT<W>", "viewport(%p) epc(%p): " \
+                              fmt, viewport->ec, viewport, viewport->epc, ##arg)
+#define PIN(fmt,arg...) ELOGF("VIEWPORT<I>", "viewport(%p) epc(%p): " \
+                              fmt, viewport->ec, viewport, viewport->epc, ##arg)
+#define PDB(fmt,arg...) DBG("viewport(%p) window(0x%08"PRIxPTR") ec(%p) epc(%p): "fmt, \
+                            viewport, viewport->window, viewport->ec, viewport->epc, ##arg)
 
 #undef SWAP
 #define SWAP(a, b)  ({double t; t = a; a = b; b = t;})
@@ -101,9 +98,9 @@ typedef struct _E_Viewport
 } E_Viewport;
 
 static E_Viewport* _e_comp_wl_viewport_get_viewport(struct wl_resource *resource);
-static void _e_comp_wl_viewport_cb_parent_show(void *data, Evas *e, Evas_Object *obj, void *event_info);
-static void _e_comp_wl_viewport_cb_parent_resize(void *data, Evas *e, Evas_Object *obj, void *event_info);
 static void _e_comp_wl_viewport_parent_check(E_Viewport *viewport);
+static void _e_comp_wl_viewport_parent_set(E_Viewport *viewport, E_Client *epc);
+static void _e_comp_wl_viewport_parent_unset(E_Viewport *viewport);
 
 static void
 _destroy_viewport(E_Viewport *viewport)
@@ -112,18 +109,13 @@ _destroy_viewport(E_Viewport *viewport)
 
    if (!viewport) return;
 
+   PIN("destroy E_Viewport");
+
    ec = viewport->ec;
 
    ecore_event_handler_del(viewport->topmost_rotate_hdl);
 
-   if (viewport->epc)
-     {
-        evas_object_event_callback_del_full(viewport->epc->frame, EVAS_CALLBACK_SHOW,
-                                            _e_comp_wl_viewport_cb_parent_show, viewport);
-        evas_object_event_callback_del_full(viewport->epc->frame, EVAS_CALLBACK_RESIZE,
-                                            _e_comp_wl_viewport_cb_parent_resize, viewport);
-        viewport->epc = NULL;
-     }
+   _e_comp_wl_viewport_parent_unset(viewport);
 
    e_client_hook_del(viewport->client_hook_del);
    e_client_hook_del(viewport->client_hook_move);
@@ -151,8 +143,6 @@ _destroy_viewport(E_Viewport *viewport)
         ec->comp_data->pending.buffer_viewport.changed = 1;
      }
 
-   PIN("tizen_viewport@%d destroy. viewport(%p)", wl_resource_get_id(viewport->resource), viewport);
-
    free(viewport);
 }
 
@@ -197,12 +187,9 @@ _client_cb_del(void *data, E_Client *ec)
    viewport = data;
    if (viewport->epc != ec) return;
 
-   evas_object_event_callback_del(viewport->epc->frame, EVAS_CALLBACK_SHOW,
-                                  _e_comp_wl_viewport_cb_parent_show);
-   evas_object_event_callback_del(viewport->epc->frame, EVAS_CALLBACK_RESIZE,
-                                  _e_comp_wl_viewport_cb_parent_resize);
    PIN("epc del");
-   viewport->epc = NULL;
+
+   _e_comp_wl_viewport_parent_unset(viewport);
 }
 
 static void
@@ -247,33 +234,9 @@ _e_comp_wl_viewport_parent_check(E_Viewport *viewport)
    if (e_comp_wl_subsurface_check(ec))
      new_parent = e_comp_wl_subsurface_parent_get(ec);
 
-   if (viewport->epc == new_parent) return;
-
-   if (viewport->epc)
-     {
-        evas_object_event_callback_del(viewport->epc->frame,
-                                       EVAS_CALLBACK_SHOW,
-                                       _e_comp_wl_viewport_cb_parent_show);
-        evas_object_event_callback_del(viewport->epc->frame,
-                                       EVAS_CALLBACK_RESIZE,
-                                       _e_comp_wl_viewport_cb_parent_resize);
-     }
-
-   viewport->epc = new_parent;
+   PIN("parent_check: parent(%p)", new_parent);
 
-   PIN("epc(%p)", viewport->epc);
-
-   if (viewport->epc)
-     {
-        evas_object_event_callback_add(viewport->epc->frame,
-                                       EVAS_CALLBACK_SHOW,
-                                       _e_comp_wl_viewport_cb_parent_show,
-                                       viewport);
-        evas_object_event_callback_add(viewport->epc->frame,
-                                       EVAS_CALLBACK_RESIZE,
-                                       _e_comp_wl_viewport_cb_parent_resize,
-                                       viewport);
-     }
+   _e_comp_wl_viewport_parent_set(viewport, new_parent);
 }
 
 static void
@@ -564,52 +527,6 @@ static const struct tizen_destination_mode_interface _e_comp_wl_destination_mode
 };
 
 static void
-_e_comp_wl_viewport_cb_parent_show(void *data, Evas *e EINA_UNUSED, Evas_Object *obj EINA_UNUSED, void *event_info EINA_UNUSED)
-{
-   E_Viewport *viewport;
-
-   viewport = data;
-   if (e_object_is_del(E_OBJECT(viewport->ec)))
-     return;
-
-   if (e_comp_wl_viewport_is_changed(viewport->ec))
-     {
-        PIN("Parent gets visible and viewport has changed, "
-            "try to apply viewport to ec(%p)", viewport->ec);
-        e_comp_wl_viewport_apply(viewport->ec);
-     }
-}
-
-static void
-_e_comp_wl_viewport_cb_parent_resize(void *data, Evas *e EINA_UNUSED, Evas_Object *obj EINA_UNUSED, void *event_info EINA_UNUSED)
-{
-   E_Viewport *viewport = data;
-   Evas_Coord old_w, old_h;
-
-   if (e_object_is_del(E_OBJECT(viewport->epc))) return;
-
-   if (viewport->query_parent_size)
-     {
-        old_w = viewport->parent_size.w;
-        old_h = viewport->parent_size.h;
-
-        evas_object_geometry_get(viewport->epc->frame,
-                                 &viewport->parent_size.x,
-                                 &viewport->parent_size.y,
-                                 &viewport->parent_size.w,
-                                 &viewport->parent_size.h);
-
-        if ((old_w != viewport->parent_size.w) ||
-            (old_h != viewport->parent_size.h))
-          tizen_viewport_send_parent_size(viewport->resource,
-                                          viewport->parent_size.w,
-                                          viewport->parent_size.h);
-     }
-
-   e_comp_wl_viewport_apply(viewport->ec);
-}
-
-static void
 _e_comp_wl_viewport_destroy(struct wl_resource *resource)
 {
    E_Viewport *viewport;
@@ -1848,7 +1765,7 @@ e_comp_wl_viewport_create(struct wl_resource *resource,
    viewport->surface_destroy_listener.notify = _e_comp_wl_viewport_cb_surface_destroy;
    wl_resource_add_destroy_listener(ec->comp_data->surface, &viewport->surface_destroy_listener);
 
-   PIN("tizen_viewport@%d viewport(%p) created", id, viewport);
+   PIN("create viewport");
 
    return EINA_TRUE;
 }
@@ -2233,3 +2150,92 @@ e_comp_wl_viewport_shutdown(void)
 {
    e_info_server_hook_set("viewport", NULL, NULL);
 }
+
+static void
+_e_comp_wl_viewport_cb_parent_show(void *data, Evas *e EINA_UNUSED, Evas_Object *obj EINA_UNUSED, void *event_info EINA_UNUSED)
+{
+   E_Viewport *viewport = data;
+
+   if (e_object_is_del(E_OBJECT(viewport->ec))) return;
+
+   if (e_comp_wl_viewport_is_changed(viewport->ec))
+     {
+        PIN("Parent gets visible and viewport has changed, "
+            "try to apply viewport to ec(%p)", viewport->ec);
+        e_comp_wl_viewport_apply(viewport->ec);
+     }
+}
+
+static void
+_e_comp_wl_viewport_cb_parent_resize(void *data, Evas *e EINA_UNUSED, Evas_Object *obj EINA_UNUSED, void *event_info EINA_UNUSED)
+{
+   E_Viewport *viewport = data;
+   Evas_Coord old_w, old_h;
+
+   if (e_object_is_del(E_OBJECT(viewport->epc))) return;
+
+   if (viewport->query_parent_size)
+     {
+        old_w = viewport->parent_size.w;
+        old_h = viewport->parent_size.h;
+
+        evas_object_geometry_get(viewport->epc->frame,
+                                 &viewport->parent_size.x,
+                                 &viewport->parent_size.y,
+                                 &viewport->parent_size.w,
+                                 &viewport->parent_size.h);
+
+        if ((old_w != viewport->parent_size.w) ||
+            (old_h != viewport->parent_size.h))
+          tizen_viewport_send_parent_size(viewport->resource,
+                                          viewport->parent_size.w,
+                                          viewport->parent_size.h);
+     }
+
+   e_comp_wl_viewport_apply(viewport->ec);
+}
+
+static void
+_e_comp_wl_viewport_parent_set(E_Viewport *viewport, E_Client *epc)
+{
+   if (viewport->epc == epc) return;
+
+   PIN("parent_set: new_parent(%p)", epc);
+
+   _e_comp_wl_viewport_parent_unset(viewport);
+
+   if (!epc)
+     return;
+
+   viewport->epc = epc;
+
+   EINA_SAFETY_ON_NULL_RETURN(epc->frame);
+
+   evas_object_event_callback_add(epc->frame, EVAS_CALLBACK_SHOW,
+                                  _e_comp_wl_viewport_cb_parent_show,
+                                  viewport);
+   evas_object_event_callback_add(epc->frame, EVAS_CALLBACK_RESIZE,
+                                  _e_comp_wl_viewport_cb_parent_resize,
+                                  viewport);
+}
+
+static void
+_e_comp_wl_viewport_parent_unset(E_Viewport *viewport)
+{
+   if (!viewport->epc) return;
+
+   PIN("parent_unset");
+
+   EINA_SAFETY_ON_NULL_GOTO(viewport->epc->frame, end);
+
+   evas_object_event_callback_del_full(viewport->epc->frame,
+                                       EVAS_CALLBACK_SHOW,
+                                       _e_comp_wl_viewport_cb_parent_show,
+                                       viewport);
+   evas_object_event_callback_del_full(viewport->epc->frame,
+                                       EVAS_CALLBACK_RESIZE,
+                                       _e_comp_wl_viewport_cb_parent_resize,
+                                       viewport);
+end:
+   viewport->epc = NULL;
+}