From 9a03e0085d651e0e74d19dbb9c65f317341dd425 Mon Sep 17 00:00:00 2001
From: Mike Blumenkrantz <michael.blumenkrantz@gmail.com>
Date: Thu, 30 Aug 2012 07:04:53 +0000
Subject: [PATCH] fix longstanding-but-unnoticed issue where submenus would not
 always be freed, also related issue where active menus would be freed in the
 wrong order, resulting in double frees in very rare circumstances

SVN revision: 75839
---
 src/bin/e_menu.c | 66 ++++++++++++++++++++++++++++++++++++++----------
 1 file changed, 53 insertions(+), 13 deletions(-)

diff --git a/src/bin/e_menu.c b/src/bin/e_menu.c
index 26fded4e5..8038b6fb0 100644
--- a/src/bin/e_menu.c
+++ b/src/bin/e_menu.c
@@ -91,6 +91,7 @@ static Ecore_X_Window _e_menu_win = 0;
 static Eina_List *_e_active_menus = NULL;
 static Eina_Hash *_e_menu_hash = NULL;
 static E_Menu_Item *_e_active_menu_item = NULL;
+static E_Menu_Item *_e_prev_active_menu_item = NULL;
 /*static Eina_Hash	   *_e_menu_category_items	= NULL;*/
 static Eina_Hash *_e_menu_categories = NULL;
 static Ecore_X_Time _e_menu_activate_time = 0;
@@ -130,12 +131,34 @@ static Eina_List *
 _e_menu_list_free_unref(Eina_List *l)
 {
    E_Object *o;
+   Eina_List *ll, *lll;
 
-   EINA_LIST_FREE(l, o)
-     e_object_unref(o);
+   /* list must be freed in reverse to ensure that submenus
+    * (which are added to the end of the list)
+    * are deleted before their parents
+    */
+   EINA_LIST_REVERSE_FOREACH_SAFE(l, ll, lll, o)
+     {
+        e_object_unref(o);
+        l = eina_list_remove_list(l, ll);
+     }
    return l;
 }
 
+/* macros for debugging menu refcounts */
+#if 0
+#define e_object_ref(X) do {\
+   int xx; \
+   xx = e_object_ref(X); \
+   INF("REF: %p || %d", X, xx); \
+  } while (0)
+#define e_object_unref(X) do {\
+   int xx; \
+   xx = e_object_unref(X); \
+   INF("UNREF: %p || %d", X, xx); \
+  } while (0)
+#endif
+
 /* externally accessible functions */
 EINTERN int
 e_menu_init(void)
@@ -939,7 +962,13 @@ e_menu_item_active_set(E_Menu_Item *mi, int active)
         if (mi->disable) return;
         pmi = _e_menu_item_active_get();
         if (mi == pmi) return;
-        if (pmi) e_menu_item_active_set(pmi, 0);
+        if (pmi)
+          e_menu_item_active_set(pmi, 0);
+        if (_e_prev_active_menu_item && (mi != _e_prev_active_menu_item))
+          {
+             if (_e_prev_active_menu_item != mi->menu->parent_item)
+               _e_menu_submenu_deactivate(_e_prev_active_menu_item);
+          }
         mi->active = 1;
         _e_active_menu_item = mi;
         if (mi->bg_object)
@@ -968,6 +997,7 @@ e_menu_item_active_set(E_Menu_Item *mi, int active)
    else if ((!active) && (mi->active))
      {
         mi->active = 0;
+        _e_prev_active_menu_item = mi;
         _e_active_menu_item = NULL;
         if (mi->bg_object)
           edje_object_signal_emit(mi->bg_object, "e,state,unselected", "e");
@@ -990,7 +1020,6 @@ e_menu_item_active_set(E_Menu_Item *mi, int active)
                }
           }
         edje_object_signal_emit(mi->menu->bg_object, "e,state,unselected", "e");
-        _e_menu_submenu_deactivate(mi);
      }
 }
 
@@ -1208,11 +1237,8 @@ _e_menu_free(E_Menu *m)
              if (cb->free) cb->free(cb->data);
           }
      }
-   if (m->parent_item)
-     {
-        if (m->parent_item->submenu == m)
-          m->parent_item->submenu = NULL;
-     }
+   if (m->parent_item && (m->parent_item->submenu == m))
+     m->parent_item->submenu = NULL;
    _e_menu_unrealize(m);
    E_FREE(m->shape_rects);
    m->shape_rects_num = 0;
@@ -1233,11 +1259,25 @@ static void
 _e_menu_item_free(E_Menu_Item *mi)
 {
    if (mi == _e_active_menu_item) _e_active_menu_item = NULL;
+   if (mi == _e_prev_active_menu_item) _e_prev_active_menu_item = NULL;
    if (mi->submenu)
      {
-        mi->submenu->parent_item = NULL;
-        e_object_unref(E_OBJECT(mi->submenu)); /* added on submenu_set() */
-/*	e_object_del(E_OBJECT(mi->submenu)); */
+        int ref = 0;
+
+        /* parent_item gets unset in a few places, reapply it for use in cleanup */
+        if (!mi->submenu->parent_item)
+          mi->submenu->parent_item = mi;
+        /* menu may not have been deactivated, ensure deactivate callback is called */
+        if (mi->active)
+          _e_menu_submenu_deactivate(mi);
+        /* submenus CANNOT exist without their parent menu+item, so ensure that they get deleted */
+        if (mi->submenu)
+          {
+             ref = e_object_ref_get(E_OBJECT(mi->submenu)) - 1;
+             e_object_unref(E_OBJECT(mi->submenu));
+          }
+       if (ref)
+         WRN("DANGLING SUBMENU FOR %s: REF(%d)||MENU(%p)", mi->label, ref, mi->submenu);
      }
    if (mi->menu->realized) _e_menu_item_unrealize(mi);
    mi->menu->items = eina_list_remove(mi->menu->items, mi);
@@ -2989,7 +3029,7 @@ _e_menu_cb_item_submenu_post_default(void *data __UNUSED__, E_Menu *m __UNUSED__
    if (!mi->submenu) return;
 
    subm = mi->submenu;
-   //e_menu_item_submenu_set(mi, NULL);
+   e_menu_item_submenu_set(mi, NULL);
    e_object_del(E_OBJECT(subm));
 }