Split off GMenuModel -> GtkMenuBar code

Put this in a separate file and substantially refactor it.

Move handling of submenu creation into gtkmodelmenuitem where it
belongs.

Improve our handling of when to show separators or not.

5 files changed, 322 insertions, 213 deletions

diff --git a/gtk/Makefile.am b/gtk/Makefile.am
index 3088bd0384..d1cafc53e1 100644
--- a/gtk/Makefile.am
+++ b/gtk/Makefile.am
@@ -16,7 +16,7 @@ else
 GTK_PRINT_PREVIEW_COMMAND="evince --unlink-tempfile --preview --print-settings %s %f"
 endif
 
-SUBDIRS = a11y . tests
+SUBDIRS = a11y .
 
 if HAVE_PAPI_CUPS
 GTK_PRINT_BACKENDS=file,papi,cups
@@ -434,6 +434,7 @@ gtk_private_h_sources =		\
 	gtkmenuitemprivate.h	\
 	gtkmenushellprivate.h	\
 	gtkmnemonichash.h	\
+	gtkmodelmenu.h		\
 	gtkmodelmenuitem.h	\
 	gtkmodifierstyle.h	\
 	gtkmodulesprivate.h	\
@@ -636,6 +637,7 @@ gtk_base_c_sources = 		\
 	gtkmessagedialog.c	\
 	gtkmisc.c		\
 	gtkmnemonichash.c	\
+	gtkmodelmenu.c		\
 	gtkmodelmenuitem.c	\
 	gtkmodifierstyle.c	\
 	gtkmodules.c		\
diff --git a/gtk/gtkapplicationwindow.c b/gtk/gtkapplicationwindow.c
index b3ad18cc02..3577842c0d 100644
--- a/gtk/gtkapplicationwindow.c
+++ b/gtk/gtkapplicationwindow.c
@@ -23,10 +23,7 @@
 
 #include "gtkapplicationwindow.h"
 
-#include "gtkseparatormenuitem.h"
-#include "gtkmodelmenuitem.h"
-#include "gtkcheckmenuitem.h"
-#include "gtkmenubar.h"
+#include "gtkmodelmenu.h"
 #include "gactionmuxer.h"
 #include "gtkintl.h"
 
@@ -65,11 +62,6 @@ struct _GtkApplicationWindowPrivate
   gboolean show_menubar;
 };
 
-static GtkWidget *
-gtk_application_window_create_menubar (GMenuModel        *model,
-                                       GActionObservable *actions);
-
-
 static void
 gtk_application_window_update_menubar (GtkApplicationWindow *window)
 {
@@ -103,7 +95,7 @@ gtk_application_window_update_menubar (GtkApplicationWindow *window)
       g_menu_append_section (combined, NULL, G_MENU_MODEL (window->priv->app_menu_section));
       g_menu_append_section (combined, NULL, G_MENU_MODEL (window->priv->menubar_section));
 
-      window->priv->menubar = gtk_application_window_create_menubar (G_MENU_MODEL (combined), G_ACTION_OBSERVABLE (muxer));
+      window->priv->menubar = gtk_model_menu_create_menu_bar (G_MENU_MODEL (combined), G_ACTION_OBSERVABLE (muxer));
       gtk_widget_set_parent (window->priv->menubar, GTK_WIDGET (window));
       gtk_widget_show_all (window->priv->menubar);
       g_object_unref (combined);
@@ -629,205 +621,3 @@ gtk_application_window_set_show_menubar (GtkApplicationWindow *window,
       g_object_notify_by_pspec (G_OBJECT (window), gtk_application_window_properties[PROP_SHOW_MENUBAR]);
     }
 }
-
-/* GtkMenu construction {{{1 */
-
-static void populate_menu_from_model (GtkMenuShell *menu,
-                                      GMenuModel   *model,
-                                      GActionObservable *actions);
-
-static void
-append_items_from_model (GtkMenuShell      *menu,
-                         GMenuModel        *model,
-                         GActionObservable *actions,
-                         gboolean          *need_separator,
-                         const gchar       *heading)
-{
-  gint n;
-  gint i;
-  GtkWidget *w;
-  GtkMenuItem *menuitem;
-  GtkWidget *submenu;
-  GMenuModel *m;
-  gchar *label;
-
-  n = g_menu_model_get_n_items (model);
-
-  if (!GTK_IS_MENU_BAR (menu) && *need_separator && n > 0)
-    {
-      w = gtk_separator_menu_item_new ();
-      gtk_widget_show (w);
-      gtk_menu_shell_append (menu, w);
-      *need_separator = FALSE;
-    }
-
-  if (heading != NULL)
-    {
-      w = gtk_menu_item_new_with_label (heading);
-      gtk_widget_show (w);
-      gtk_widget_set_sensitive (w, FALSE);
-      gtk_menu_shell_append (GTK_MENU_SHELL (menu), w);
-    }
-
-  for (i = 0; i < n; i++)
-    {
-      if ((m = g_menu_model_get_item_link (model, i, G_MENU_LINK_SECTION)))
-        {
-          label = NULL;
-          g_menu_model_get_item_attribute (model, i, G_MENU_ATTRIBUTE_LABEL, "s", &label);
-          append_items_from_model (menu, m, actions, need_separator, label);
-          g_object_unref (m);
-          g_free (label);
-
-          if (!GTK_IS_MENU_BAR (menu)  && *need_separator)
-            {
-              w = gtk_separator_menu_item_new ();
-              gtk_widget_show (w);
-              gtk_menu_shell_append (menu, w);
-              *need_separator = FALSE;
-            }
-
-          continue;
-        }
-
-      menuitem = gtk_model_menu_item_new (model, i, actions);
-
-      if ((m = g_menu_model_get_item_link (model, i, G_MENU_LINK_SUBMENU)))
-        {
-          submenu = gtk_menu_new ();
-          populate_menu_from_model (GTK_MENU_SHELL (submenu), m, actions);
-          gtk_menu_item_set_submenu (menuitem, submenu);
-          g_object_unref (m);
-        }
-
-      gtk_widget_show (GTK_WIDGET (menuitem));
-      gtk_menu_shell_append (menu, GTK_WIDGET (menuitem));
-
-      *need_separator = TRUE;
-    }
-}
-
-static void
-populate_menu_from_model (GtkMenuShell      *menu,
-                          GMenuModel        *model,
-                          GActionObservable *actions)
-{
-  gboolean need_separator;
-
-  need_separator = FALSE;
-  append_items_from_model (menu, model, actions, &need_separator, NULL);
-}
-
-typedef struct {
-  GActionObservable *actions;
-  GMenuModel        *model;
-  GtkMenuShell      *menu;
-  guint              update_idle;
-  GHashTable        *connected;
-} ItemsChangedData;
-
-static void
-free_items_changed_data (gpointer data)
-{
-  ItemsChangedData *d = data;
-
-  g_object_unref (d->actions);
-  g_object_unref (d->model);
-
-  if (d->update_idle != 0)
-    g_source_remove (d->update_idle);
-
-  g_hash_table_unref (d->connected);
-
-  g_free (d);
-}
-
-static gboolean
-repopulate_menu (gpointer data)
-{
-  ItemsChangedData *d = data;
-  GList *children, *l;
-  GtkWidget *child;
-
-  /* remove current children */
-  children = gtk_container_get_children (GTK_CONTAINER (d->menu));
-  for (l = children; l; l = l->next)
-    {
-      child = l->data;
-      gtk_container_remove (GTK_CONTAINER (d->menu), child);
-    }
-  g_list_free (children);
-
-  populate_menu_from_model (d->menu, d->model, d->actions);
-  d->update_idle = 0;
-
-  return FALSE;
-}
-
-static void
-connect_to_items_changed (GMenuModel *model,
-                          GCallback   callback,
-                          gpointer    data)
-{
-  ItemsChangedData *d = data;
-  gint i;
-  GMenuModel *m;
-  GMenuLinkIter *iter;
-
-  if (!g_hash_table_lookup (d->connected, model))
-    {
-      g_signal_connect (model, "items-changed", callback, data);
-      g_hash_table_insert (d->connected, model, model);
-    }
-
-  for (i = 0; i < g_menu_model_get_n_items (model); i++)
-    {
-      iter = g_menu_model_iterate_item_links (model, i);
-      while (g_menu_link_iter_next (iter))
-        {
-          m = g_menu_link_iter_get_value (iter);
-          connect_to_items_changed (m, callback, data);
-          g_object_unref (m);
-        }
-      g_object_unref (iter);
-    }
-}
-
-static void
-items_changed (GMenuModel *model,
-               gint        position,
-               gint        removed,
-               gint        added,
-               gpointer    data)
-{
-  ItemsChangedData *d = data;
-
-  if (d->update_idle == 0)
-    d->update_idle = gdk_threads_add_idle (repopulate_menu, data);
-  connect_to_items_changed (model, G_CALLBACK (items_changed), data);
-}
-
-static GtkWidget *
-gtk_application_window_create_menubar (GMenuModel        *model,
-                                       GActionObservable *actions)
-{
-  ItemsChangedData *data;
-  GtkWidget *menubar;
-
-  menubar = gtk_menu_bar_new ();
-
-  data = g_new (ItemsChangedData, 1);
-  data->model = g_object_ref (model);
-  data->actions = g_object_ref (actions);
-  data->menu = GTK_MENU_SHELL (menubar);
-  data->update_idle = 0;
-  data->connected = g_hash_table_new (NULL, NULL);
-
-  g_object_set_data_full (G_OBJECT (menubar), "gtk-application-menu-data",
-                          data, free_items_changed_data);
-
-  connect_to_items_changed (model, G_CALLBACK (items_changed), data);
-  repopulate_menu (data);
-
-  return menubar;
-}
diff --git a/gtk/gtkmodelmenu.c b/gtk/gtkmodelmenu.c
new file mode 100644
index 0000000000..0d42bd60b4
--- /dev/null
+++ b/gtk/gtkmodelmenu.c
@@ -0,0 +1,265 @@
+/*
+ * Copyright © 2011 Red Hat, Inc.
+ * Copyright © 2011 Canonical Limited
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the licence, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the
+ * Free Software Foundation, Inc., 59 Temple Place - Suite 330,
+ * Boston, MA 02111-1307, USA.
+ *
+ * Author: Matthias Clasen <mclasen@redhat.com>
+ *         Ryan Lortie <desrt@desrt.ca>
+ */
+
+#include "config.h"
+
+#include "gtkmodelmenu.h"
+
+#include "gtkseparatormenuitem.h"
+#include "gtkmodelmenuitem.h"
+
+typedef struct {
+  GActionObservable *actions;
+  GMenuModel        *model;
+  GtkMenuShell      *shell;
+  guint              update_idle;
+  GSList            *connected;
+  gboolean           with_separators;
+  gint               n_items;
+} GtkModelMenuBinding;
+
+static void
+gtk_model_menu_binding_items_changed (GMenuModel *model,
+                                      gint        position,
+                                      gint        removed,
+                                      gint        added,
+                                      gpointer    user_data);
+static void gtk_model_menu_binding_append_model (GtkModelMenuBinding *binding,
+                                                 GMenuModel *model,
+                                                 gboolean with_separators);
+
+static void
+gtk_model_menu_binding_free (gpointer data)
+{
+  GtkModelMenuBinding *binding = data;
+
+  /* disconnect all existing signal handlers */
+  while (binding->connected)
+    {
+      g_signal_handlers_disconnect_by_func (binding->connected->data, gtk_model_menu_binding_items_changed, binding);
+      g_object_unref (binding->connected->data);
+
+      binding->connected = g_slist_delete_link (binding->connected, binding->connected);
+    }
+
+  g_object_unref (binding->actions);
+  g_object_unref (binding->model);
+}
+
+static void
+gtk_model_menu_binding_append_item (GtkModelMenuBinding  *binding,
+                                    GMenuModel           *model,
+                                    gint                  item_index,
+                                    gchar               **heading)
+{
+  GMenuModel *section;
+
+  if ((section = g_menu_model_get_item_link (model, item_index, "section")))
+    {
+      g_menu_model_get_item_attribute (model, item_index, "label", "s", &heading);
+      gtk_model_menu_binding_append_model (binding, section, FALSE);
+    }
+  else
+    {
+      GtkMenuItem *item;
+
+      item = gtk_model_menu_item_new (model, item_index, binding->actions);
+      gtk_menu_shell_append (binding->shell, GTK_WIDGET (item));
+      gtk_widget_show (GTK_WIDGET (item));
+      binding->n_items++;
+    }
+}
+
+static void
+gtk_model_menu_binding_append_model (GtkModelMenuBinding *binding,
+                                     GMenuModel          *model,
+                                     gboolean             with_separators)
+{
+  gint n, i;
+
+  g_signal_connect (model, "items-changed", G_CALLBACK (gtk_model_menu_binding_items_changed), binding);
+  binding->connected = g_slist_prepend (binding->connected, g_object_ref (model));
+
+  /* Deciding if we should show a separator is a bit difficult.  There
+   * are two types of separators:
+   *
+   *  - section headings (when sections have 'label' property)
+   *
+   *  - normal separators automatically put between sections
+   *
+   * The easiest way to think about it is that a section usually has a
+   * separator (or heading) immediately before it.
+   *
+   * There are three exceptions to this general rule:
+   *
+   *  - empty sections don't get separators or headings
+   *
+   *  - sections only get separators and headings at the toplevel of a
+   *    menu (ie: no separators on nested sections or in menubars)
+   *
+   *  - the first section in the menu doesn't get a normal separator,
+   *    but it can get a header (if it's not empty)
+   *
+   * Unfortunately, we cannot simply check the size of the section in
+   * order to determine if we should place a header: the section may
+   * contain other sections that are themselves empty.  Instead, we need
+   * to append the section, and check if we ended up with any actual
+   * content.  If we did, then we need to insert before that content.
+   * We use 'our_position' to keep track of this.
+   */
+
+  n = g_menu_model_get_n_items (model);
+
+  for (i = 0; i < n; i++)
+    {
+      gint our_position = binding->n_items;
+      gchar *heading = NULL;
+
+      gtk_model_menu_binding_append_item (binding, model, i, &heading);
+
+      if (with_separators && our_position < binding->n_items)
+        {
+          GtkWidget *separator = NULL;
+
+          if (heading)
+            {
+              separator = gtk_menu_item_new_with_label (heading);
+              gtk_widget_set_sensitive (separator, FALSE);
+            }
+          else if (our_position > 0)
+            separator = gtk_separator_menu_item_new ();
+
+          if (separator)
+            {
+              gtk_menu_shell_insert (binding->shell, separator, our_position);
+              gtk_widget_show (separator);
+              binding->n_items++;
+            }
+        }
+
+      g_free (heading);
+    }
+}
+
+static void
+gtk_model_menu_binding_populate (GtkModelMenuBinding *binding)
+{
+  GList *children;
+
+  /* remove current children */
+  children = gtk_container_get_children (GTK_CONTAINER (binding->shell));
+  while (children)
+    {
+      gtk_container_remove (GTK_CONTAINER (binding->shell), children->data);
+      children = g_list_delete_link (children, children);
+    }
+
+  binding->n_items = 0;
+
+  /* add new items from the model */
+  gtk_model_menu_binding_append_model (binding, binding->model, binding->with_separators);
+}
+
+static gboolean
+gtk_model_menu_binding_handle_changes (gpointer user_data)
+{
+  GtkModelMenuBinding *binding = user_data;
+
+  /* disconnect all existing signal handlers */
+  while (binding->connected)
+    {
+      g_signal_handlers_disconnect_by_func (binding->connected->data, gtk_model_menu_binding_items_changed, binding);
+      g_object_unref (binding->connected->data);
+
+      binding->connected = g_slist_delete_link (binding->connected, binding->connected);
+    }
+
+  gtk_model_menu_binding_populate (binding);
+
+  binding->update_idle = 0;
+
+  g_object_unref (binding->shell);
+
+  return G_SOURCE_REMOVE;
+}
+
+static void
+gtk_model_menu_binding_items_changed (GMenuModel *model,
+                                      gint        position,
+                                      gint        removed,
+                                      gint        added,
+                                      gpointer    user_data)
+{
+  GtkModelMenuBinding *binding = user_data;
+
+  if (binding->update_idle == 0)
+    {
+      binding->update_idle = gdk_threads_add_idle (gtk_model_menu_binding_handle_changes, user_data);
+      g_object_ref (binding->shell);
+    }
+}
+
+void
+gtk_model_menu_bind (GtkMenuShell      *shell,
+                     GMenuModel        *model,
+                     GActionObservable *actions,
+                     gboolean           with_separators)
+{
+  GtkModelMenuBinding *binding;
+
+  binding = g_slice_new (GtkModelMenuBinding);
+  binding->model = g_object_ref (model);
+  binding->actions = g_object_ref (actions);
+  binding->shell = shell;
+  binding->update_idle = 0;
+  binding->connected = NULL;
+  binding->with_separators = with_separators;
+
+  g_object_set_data_full (G_OBJECT (shell), "gtk-model-menu-binding", binding, gtk_model_menu_binding_free);
+  gtk_model_menu_binding_populate (binding);
+}
+
+GtkWidget *
+gtk_model_menu_create_menu (GMenuModel        *model,
+                            GActionObservable *actions)
+{
+  GtkWidget *menu;
+
+  menu = gtk_menu_new ();
+  gtk_model_menu_bind (GTK_MENU_SHELL (menu), model, actions, TRUE);
+
+  return menu;
+}
+
+GtkWidget *
+gtk_model_menu_create_menu_bar (GMenuModel        *model,
+                                GActionObservable *actions)
+{
+  GtkWidget *menubar;
+
+  menubar = gtk_menu_bar_new ();
+  gtk_model_menu_bind (GTK_MENU_SHELL (menubar), model, actions, FALSE);
+
+  return menubar;
+}
+
diff --git a/gtk/gtkmodelmenu.h b/gtk/gtkmodelmenu.h
new file mode 100644
index 0000000000..5a9d7112b0
--- /dev/null
+++ b/gtk/gtkmodelmenu.h
@@ -0,0 +1,43 @@
+/*
+ * Copyright © 2011 Canonical Limited
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the licence, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the
+ * Free Software Foundation, Inc., 59 Temple Place - Suite 330,
+ * Boston, MA 02111-1307, USA.
+ *
+ * Author: Ryan Lortie <desrt@desrt.ca>
+ */
+
+#ifndef __GTK_MODEL_MENU_H__
+#define __GTK_MODEL_MENU_H__
+
+#include <gtk/gactionobservable.h>
+#include <gtk/gtkmenubar.h>
+#include <gtk/gtkmenu.h>
+
+G_GNUC_INTERNAL
+void                    gtk_model_menu_bind                             (GtkMenuShell      *shell,
+                                                                         GMenuModel        *model,
+                                                                         GActionObservable *actions,
+                                                                         gboolean           with_separators);
+
+G_GNUC_INTERNAL
+GtkWidget *             gtk_model_menu_create_menu_bar                  (GMenuModel        *model,
+                                                                         GActionObservable *actions);
+
+G_GNUC_INTERNAL
+GtkWidget *             gtk_model_menu_create_menu                      (GMenuModel        *model,
+                                                                         GActionObservable *actions);
+
+#endif /* __GTK_MODEL_MENU_H__ */
diff --git a/gtk/gtkmodelmenuitem.c b/gtk/gtkmodelmenuitem.c
index 08a3ac1db2..30ca3f4fc0 100644
--- a/gtk/gtkmodelmenuitem.c
+++ b/gtk/gtkmodelmenuitem.c
@@ -23,6 +23,8 @@
 
 #include "gtkmodelmenuitem.h"
 
+#include "gtkmodelmenu.h"
+
 struct _GtkModelMenuItem
 {
   GtkCheckMenuItem parent_instance;
@@ -193,9 +195,16 @@ gtk_model_menu_item_setup (GtkModelMenuItem  *item,
                            GActionObservable *actions)
 {
   GMenuAttributeIter *iter;
+  GMenuModel *submenu;
   const gchar *key;
   GVariant *value;
 
+  if ((submenu = g_menu_model_get_item_link (model, item_index, "submenu")))
+    {
+      gtk_menu_item_set_submenu (GTK_MENU_ITEM (item), gtk_model_menu_create_menu (submenu, actions));
+      g_object_unref (submenu);
+    }
+
   iter = g_menu_model_iterate_item_attributes (model, item_index);
   while (g_menu_attribute_iter_get_next (iter, &key, &value))
     {