Merge lp:~aacid/qmenumodel/batch_insert_remove_from_menu into lp:qmenumodel

[Albert Astals Cid]

Batch add and removes into the model

Works much better with Qt Quick views to have all the insert/removes at once

Batch add and removes into the model

[lp:~aacid/qmenumodel/batch_insert_remove_from_menu]

130. By Albert Astals Cid on 2017-02-28

valign

[Lukáš Tinkl (lukas-kde)]

Crashes after a few popup menus opened:

http://paste.ubuntu.com/24182582/

review: Needs Fixing

[lp:~aacid/qmenumodel/batch_insert_remove_from_menu]

131. By Albert Astals Cid on 2017-03-16

Fix c&p, this ref should be unref

[Albert Astals Cid (aacid)]

> Crashes after a few popup menus opened:
>
> http://paste.ubuntu.com/24182582/

Should be fixed, an ref should had been an unref (meh copy&paste)

[Lukáš Tinkl (lukas-kde)]

Works fine, no more crashes. I'd like someone with more glib/gobject knowledge to have a look at the code as well though

review: Approve

[Charles Kerr (charlesk)]

It's a little twisty, but but at the glib/gobject level everything seems to balance out.

review: Approve

=== modified file 'libqmenumodel/src/gtk/gtkmenutracker.c'
--- libqmenumodel/src/gtk/gtkmenutracker.c	2013-06-04 19:35:59 +0000
+++ libqmenumodel/src/gtk/gtkmenutracker.c	2017-03-16 15:38:21 +0000
@@ -85,7 +85,8 @@
                                                                  GMenuModel            *model,
                                                                  gboolean               with_separators,
                                                                  gint                   offset,
-                                                                 const gchar           *action_namespace);
+                                                                 const gchar           *action_namespace,
+                                                                 GPtrArray             *items_already_created);
 static void                    gtk_menu_tracker_section_free    (GtkMenuTrackerSection *section);
 
 static GtkMenuTrackerSection *
@@ -195,9 +196,12 @@
     {
       /* Add a separator */
       GtkMenuTrackerItem *item;
+      GPtrArray *items = g_ptr_array_new ();
 
       item = _gtk_menu_tracker_item_new (tracker->observable, parent_model, parent_index, NULL, TRUE);
-      (* tracker->insert_func) (item, offset, tracker->user_data);
+      g_ptr_array_add (items, (gpointer) item);
+      (* tracker->insert_func) (items, offset, tracker->user_data);
+      g_ptr_array_unref (items);
       g_object_unref (item);
 
       section->has_separator = TRUE;
@@ -205,7 +209,7 @@
   else if (should_have_separator < section->has_separator)
     {
       /* Remove a separator */
-      (* tracker->remove_func) (offset, tracker->user_data);
+      (* tracker->remove_func) (offset, 1, tracker->user_data);
       section->has_separator = FALSE;
     }
 
@@ -241,6 +245,7 @@
                                gint             n_items)
 {
   gint i;
+  gint n_total_items = 0;
 
   for (i = 0; i < n_items; i++)
     {
@@ -253,8 +258,12 @@
       n = gtk_menu_tracker_section_measure (subsection);
       gtk_menu_tracker_section_free (subsection);
 
-      while (n--)
-        (* tracker->remove_func) (offset, tracker->user_data);
+      n_total_items += n;
+    }
+
+  if (n_total_items)
+    {
+      (* tracker->remove_func) (offset, n_total_items, tracker->user_data);
     }
 }
 
@@ -265,8 +274,19 @@
                             gint                    offset,
                             GMenuModel             *model,
                             gint                    position,
-                            gint                    n_items)
+                            gint                    n_items,
+                            GPtrArray              *items_already_created
+                           )
 {
+  GPtrArray *items;
+  if (items_already_created)
+    {
+      items = items_already_created;
+    }
+  else
+    {
+      items = g_ptr_array_new ();
+    }
   while (n_items--)
     {
       GMenuModel *submenu;
@@ -286,11 +306,11 @@
               gchar *namespace;
 
               namespace = g_strjoin (".", section->action_namespace, action_namespace, NULL);
-              subsection = gtk_menu_tracker_section_new (tracker, submenu, FALSE, offset, namespace);
+              subsection = gtk_menu_tracker_section_new (tracker, submenu, FALSE, offset, namespace, items_already_created);
               g_free (namespace);
             }
           else
-            subsection = gtk_menu_tracker_section_new (tracker, submenu, FALSE, offset, section->action_namespace);
+            subsection = gtk_menu_tracker_section_new (tracker, submenu, FALSE, offset, section->action_namespace, items_already_created);
 
           *change_point = g_slist_prepend (*change_point, subsection);
           g_free (action_namespace);
@@ -302,12 +322,21 @@
 
           item = _gtk_menu_tracker_item_new (tracker->observable, model, position + n_items,
                                              section->action_namespace, FALSE);
-          (* tracker->insert_func) (item, offset, tracker->user_data);
-          g_object_unref (item);
+          g_ptr_array_insert (items, 0, (gpointer) item);
 
           *change_point = g_slist_prepend (*change_point, NULL);
         }
     }
+  if (!items_already_created)
+    {
+      if (items->len)
+        {
+          (* tracker->insert_func) (items, offset, tracker->user_data);
+          for (gint i = 0; i < items->len; ++i)
+            g_object_unref(g_ptr_array_index(items, i));
+        }
+      g_ptr_array_unref (items);
+    }
 }
 
 static void
@@ -352,7 +381,7 @@
    * would do by appending.
    */
   gtk_menu_tracker_remove_items (tracker, change_point, offset, removed);
-  gtk_menu_tracker_add_items (tracker, section, change_point, offset, model, position, added);
+  gtk_menu_tracker_add_items (tracker, section, change_point, offset, model, position, added, NULL);
 
   /* The offsets for insertion/removal of separators will be all over
    * the place, however...
@@ -378,7 +407,8 @@
                               GMenuModel     *model,
                               gboolean        with_separators,
                               gint            offset,
-                              const gchar    *action_namespace)
+                              const gchar    *action_namespace,
+                              GPtrArray      *items_already_created)
 {
   GtkMenuTrackerSection *section;
 
@@ -387,7 +417,7 @@
   section->with_separators = with_separators;
   section->action_namespace = g_strdup (action_namespace);
 
-  gtk_menu_tracker_add_items (tracker, section, &section->items, offset, model, 0, g_menu_model_get_n_items (model));
+  gtk_menu_tracker_add_items (tracker, section, &section->items, offset, model, 0, g_menu_model_get_n_items (model), items_already_created);
   section->handler = g_signal_connect (model, "items-changed", G_CALLBACK (gtk_menu_tracker_model_changed), tracker);
 
   return section;
@@ -461,7 +491,7 @@
   tracker->remove_func = remove_func;
   tracker->user_data = user_data;
 
-  tracker->toplevel = gtk_menu_tracker_section_new (tracker, model, with_separators, 0, action_namespace);
+  tracker->toplevel = gtk_menu_tracker_section_new (tracker, model, with_separators, 0, action_namespace, NULL);
   gtk_menu_tracker_section_sync_separators (tracker->toplevel, tracker, 0, FALSE, NULL, 0);
 
   return tracker;

=== modified file 'libqmenumodel/src/gtk/gtkmenutracker.h'
--- libqmenumodel/src/gtk/gtkmenutracker.h	2013-06-04 19:35:59 +0000
+++ libqmenumodel/src/gtk/gtkmenutracker.h	2017-03-16 15:38:21 +0000
@@ -26,11 +26,12 @@
 
 typedef struct _GtkMenuTracker GtkMenuTracker;
 
-typedef void         (* GtkMenuTrackerInsertFunc)                       (GtkMenuTrackerItem       *item,
+typedef void         (* GtkMenuTrackerInsertFunc)                       (GPtrArray                *items,
                                                                          gint                      position,
                                                                          gpointer                  user_data);
 
 typedef void         (* GtkMenuTrackerRemoveFunc)                       (gint                      position,
+                                                                         gint                      n_items,
                                                                          gpointer                  user_data);
 
 

=== modified file 'libqmenumodel/src/unitymenumodel.cpp'
--- libqmenumodel/src/unitymenumodel.cpp	2016-12-09 17:06:25 +0000
+++ libqmenumodel/src/unitymenumodel.cpp	2017-03-16 15:38:21 +0000
@@ -89,8 +89,8 @@
 
     static void nameAppeared(GDBusConnection *connection, const gchar *name, const gchar *owner, gpointer user_data);
     static void nameVanished(GDBusConnection *connection, const gchar *name, gpointer user_data);
-    static void menuItemInserted(GtkMenuTrackerItem *item, gint position, gpointer user_data);
-    static void menuItemRemoved(gint position, gpointer user_data);
+    static void menuItemInserted(GPtrArray *items, gint position, gpointer user_data);
+    static void menuItemRemoved(gint position, gint n_items, gpointer user_data);
     static void menuItemChanged(GObject *object, GParamSpec *pspec, gpointer user_data);
 
     static void registeredActionAdded(GtkSimpleActionObserver    *observer_item,
@@ -250,19 +250,19 @@
     priv->clearName();
 }
 
-void UnityMenuModelPrivate::menuItemInserted(GtkMenuTrackerItem *item, gint position, gpointer user_data)
+void UnityMenuModelPrivate::menuItemInserted(GPtrArray *items, gint position, gpointer user_data)
 {
     UnityMenuModelPrivate *priv = (UnityMenuModelPrivate *)user_data;
 
-    UnityMenuModelAddRowEvent ummare(item, position);
+    UnityMenuModelAddRowEvent ummare(items, position);
     QCoreApplication::sendEvent(priv->model, &ummare);
 }
 
-void UnityMenuModelPrivate::menuItemRemoved(gint position, gpointer user_data)
+void UnityMenuModelPrivate::menuItemRemoved(gint position, gint n_items, gpointer user_data)
 {
     UnityMenuModelPrivate *priv = (UnityMenuModelPrivate *)user_data;
 
-    UnityMenuModelRemoveRowEvent ummrre(position);
+    UnityMenuModelRemoveRowEvent ummrre(position, n_items);
     QCoreApplication::sendEvent(priv->model, &ummrre);
 }
 
@@ -797,26 +797,29 @@
         GSequenceIter *it;
         it = g_sequence_get_iter_at_pos (priv->items, ummrce->position);
 
-        beginInsertRows(QModelIndex(), ummrce->position, ummrce->position);
+        beginInsertRows(QModelIndex(), ummrce->position, ummrce->position + ummrce->items->len - 1);
 
-        it = g_sequence_insert_before (it, g_object_ref (ummrce->item));
-        g_object_set_qdata (G_OBJECT (ummrce->item), unity_menu_model_quark (), this);
-        g_signal_connect (ummrce->item, "notify", G_CALLBACK (UnityMenuModelPrivate::menuItemChanged), it);
+        for (gint i = ummrce->items->len - 1; i >= 0; --i) {
+            GtkMenuTrackerItem *item = (GtkMenuTrackerItem*)g_ptr_array_index(ummrce->items, i);
+            it = g_sequence_insert_before (it, g_object_ref (item));
+            g_object_set_qdata (G_OBJECT (item), unity_menu_model_quark (), this);
+            g_signal_connect (item, "notify", G_CALLBACK (UnityMenuModelPrivate::menuItemChanged), it);
+        }
 
         endInsertRows();
         return true;
     } else if (e->type() == UnityMenuModelRemoveRowEvent::eventType) {
         UnityMenuModelRemoveRowEvent *ummrre = static_cast<UnityMenuModelRemoveRowEvent*>(e);
 
-        GSequenceIter *it;
-        it = g_sequence_get_iter_at_pos (priv->items, ummrre->position);
-        if (!g_sequence_iter_is_end (it)) {
-            beginRemoveRows(QModelIndex(), ummrre->position, ummrre->position);
-
-            g_sequence_remove (it);
-
-            endRemoveRows();
+        beginRemoveRows(QModelIndex(), ummrre->position, ummrre->position + ummrre->nItems - 1);
+        for (int i = 0; i < ummrre->nItems; ++i) {
+            GSequenceIter *it = g_sequence_get_iter_at_pos (priv->items, ummrre->position);
+            if (!g_sequence_iter_is_end (it)) {
+                g_sequence_remove (it);
+            }
         }
+        endRemoveRows();
+
         return true;
     } else if (e->type() == UnityMenuModelDataChangeEvent::eventType) {
         UnityMenuModelDataChangeEvent *ummdce = static_cast<UnityMenuModelDataChangeEvent*>(e);

=== modified file 'libqmenumodel/src/unitymenumodelevents.cpp'
--- libqmenumodel/src/unitymenumodelevents.cpp	2013-08-09 09:50:12 +0000
+++ libqmenumodel/src/unitymenumodelevents.cpp	2017-03-16 15:38:21 +0000
@@ -35,26 +35,30 @@
       reset(_reset)
 {}
 
-UnityMenuModelAddRowEvent::UnityMenuModelAddRowEvent(GtkMenuTrackerItem *_item, int _position)
+UnityMenuModelAddRowEvent::UnityMenuModelAddRowEvent(GPtrArray *_items, int _position)
     : QEvent(UnityMenuModelAddRowEvent::eventType),
-      item(_item),
+      items(_items),
       position(_position)
 {
-    if (item) {
-        g_object_ref(item);
+    if (items) {
+        for (gint i = 0; i < items->len; ++i)
+            g_object_ref(g_ptr_array_index(items, i));
+        g_ptr_array_ref(items);
     }
 }
 
 UnityMenuModelAddRowEvent::~UnityMenuModelAddRowEvent()
 {
-    if (item) {
-        g_object_unref(item);
+    if (items) {
+        for (gint i = 0; i < items->len; ++i)
+            g_object_unref(g_ptr_array_index(items, i));
+        g_ptr_array_unref(items);
     }
 }
 
-UnityMenuModelRemoveRowEvent::UnityMenuModelRemoveRowEvent(int _position)
+UnityMenuModelRemoveRowEvent::UnityMenuModelRemoveRowEvent(int _position, int _nItems)
     : QEvent(UnityMenuModelRemoveRowEvent::eventType),
-      position(_position)
+      position(_position), nItems(_nItems)
 {}
 
 UnityMenuModelDataChangeEvent::UnityMenuModelDataChangeEvent(int _position)

=== modified file 'libqmenumodel/src/unitymenumodelevents.h'
--- libqmenumodel/src/unitymenumodelevents.h	2013-08-09 09:50:12 +0000
+++ libqmenumodel/src/unitymenumodelevents.h	2017-03-16 15:38:21 +0000
@@ -21,6 +21,7 @@
 #define UNITYMENUMODELEVENTS_H
 
 #include <QEvent>
+#include <glib.h>
 
 typedef struct _GtkMenuTrackerItem GtkMenuTrackerItem;
 
@@ -39,10 +40,10 @@
 {
 public:
     static const QEvent::Type eventType;
-    UnityMenuModelAddRowEvent(GtkMenuTrackerItem *item, int position);
+    UnityMenuModelAddRowEvent(GPtrArray *_items, int position);
     ~UnityMenuModelAddRowEvent();
 
-    GtkMenuTrackerItem *item;
+    GPtrArray *items;
     int position;
 };
 
@@ -51,9 +52,10 @@
 {
 public:
     static const QEvent::Type eventType;
-    UnityMenuModelRemoveRowEvent(int position);
+    UnityMenuModelRemoveRowEvent(int position, int nItems);
 
     int position;
+    int nItems;
 };
 
 /* Event for a row data change for unitymenumodel */