[gtk+/wip/dboles/combobox-cleanup: 3/4] combobox: Avoid constantly NULL-checking the menu

[Daniel Boles <dboles src gnome org>]

commit 342aa6e14181fe4d7565445ecc04066715604f86
Author: Daniel Boles <dboles src gnome org>
Date:   Fri Jan 20 02:24:16 2017 +0000

    combobox: Avoid constantly NULL-checking the menu
    
    By reordering a few pieces of code, we can ensure that the TreeMenu
    popup_widget is always valid, so we don’t have to constantly check it.
    
    ComboBoxText must be changed to set its model in constructed(), not
    init() – because CB::set_model() uses cell_area, which can be set by a
    ctor property and so is only definitely valid after CB::constructed().
    Benjamin wants to bin those constructors, but until then, this works!

 gtk/gtkcombobox.c     |   97 ++++++++++++++-----------------------------------
 gtk/gtkcomboboxtext.c |   24 +++++++-----
 2 files changed, 41 insertions(+), 80 deletions(-)
---
diff --git a/gtk/gtkcombobox.c b/gtk/gtkcombobox.c
index d6bc2c9..f483925 100644
--- a/gtk/gtkcombobox.c
+++ b/gtk/gtkcombobox.c
@@ -1030,7 +1030,6 @@ gtk_combo_box_init (GtkComboBox *combo_box)
   combo_box->priv = gtk_combo_box_get_instance_private (combo_box);
   priv = combo_box->priv;
 
-  priv->popup_widget = NULL;
   priv->wrap_width = 0;
 
   priv->active = -1;
@@ -1375,12 +1374,9 @@ gtk_combo_box_remove (GtkContainer *container,
 
   gtk_widget_queue_resize (GTK_WIDGET (container));
 
-  if (priv->popup_widget)
-    {
-      gtk_combo_box_menu_destroy (combo_box);
-      gtk_menu_detach (GTK_MENU (priv->popup_widget));
-      priv->popup_widget = NULL;
-    }
+  gtk_combo_box_menu_destroy (combo_box);
+  gtk_menu_detach (GTK_MENU (priv->popup_widget));
+  priv->popup_widget = NULL;
 
   gtk_combo_box_create_child (combo_box);
 
@@ -1725,8 +1721,7 @@ gtk_combo_box_popup_for_device (GtkComboBox *combo_box,
   if (gtk_widget_get_mapped (priv->popup_widget))
     return;
 
-  if (priv->popup_widget)
-    gtk_combo_box_menu_popup (combo_box, priv->trigger_event);
+  gtk_combo_box_menu_popup (combo_box, priv->trigger_event);
 }
 
 static void
@@ -1734,8 +1729,7 @@ gtk_combo_box_real_popup (GtkComboBox *combo_box)
 {
   GtkComboBoxPrivate *priv = combo_box->priv;
 
-  if (priv->popup_widget)
-    gtk_combo_box_menu_popup (combo_box, priv->trigger_event);
+  gtk_combo_box_menu_popup (combo_box, priv->trigger_event);
 }
 
 static gboolean
@@ -1768,8 +1762,7 @@ gtk_combo_box_popdown (GtkComboBox *combo_box)
 
   g_return_if_fail (GTK_IS_COMBO_BOX (combo_box));
 
-  if (priv->popup_widget)
-    gtk_menu_popdown (GTK_MENU (priv->popup_widget));
+  gtk_menu_popdown (GTK_MENU (priv->popup_widget));
 }
 
 static void
@@ -2088,10 +2081,6 @@ gtk_combo_box_menu_setup (GtkComboBox *combo_box)
   g_signal_connect (menu, "key-press-event",
                     G_CALLBACK (gtk_combo_box_menu_key_press), combo_box);
 
-  /* Set up the popup menu */
-  if (priv->popup_widget)
-    gtk_menu_detach (GTK_MENU (priv->popup_widget));
-
   priv->popup_widget = menu;
 
   g_signal_connect (menu, "show", G_CALLBACK (gtk_combo_box_menu_show), combo_box);
@@ -2107,10 +2096,14 @@ gtk_combo_box_menu_destroy (GtkComboBox *combo_box)
 {
   GtkComboBoxPrivate *priv = combo_box->priv;
 
+  if (!priv->popup_widget)
+    return;
+
   g_signal_handlers_disconnect_matched (priv->button,
                                         G_SIGNAL_MATCH_DATA,
                                         0, 0, NULL,
                                         gtk_combo_box_menu_button_press, NULL);
+
   g_signal_handlers_disconnect_matched (priv->popup_widget,
                                         G_SIGNAL_MATCH_DATA,
                                         0, 0, NULL,
@@ -2128,8 +2121,7 @@ gtk_combo_box_menu_button_press (GtkWidget      *widget,
   GtkComboBox *combo_box = GTK_COMBO_BOX (user_data);
   GtkComboBoxPrivate *priv = combo_box->priv;
 
-  if (priv->popup_widget &&
-      event->type == GDK_BUTTON_PRESS && event->button == GDK_BUTTON_PRIMARY)
+  if (event->type == GDK_BUTTON_PRESS && event->button == GDK_BUTTON_PRIMARY)
     {
       if (gtk_widget_get_focus_on_click (GTK_WIDGET (combo_box)) &&
           !gtk_widget_has_focus (priv->button))
@@ -2440,10 +2432,7 @@ gtk_combo_box_set_wrap_width (GtkComboBox *combo_box,
   if (width != priv->wrap_width)
     {
       priv->wrap_width = width;
-
-      if (priv->popup_widget)
-        _gtk_tree_menu_set_wrap_width (GTK_TREE_MENU (priv->popup_widget), priv->wrap_width);
-
+      _gtk_tree_menu_set_wrap_width (GTK_TREE_MENU (priv->popup_widget), priv->wrap_width);
       g_object_notify (G_OBJECT (combo_box), "wrap-width");
     }
 }
@@ -2494,10 +2483,7 @@ gtk_combo_box_set_row_span_column (GtkComboBox *combo_box,
   if (row_span != priv->row_column)
     {
       priv->row_column = row_span;
-
-      if (priv->popup_widget)
-        _gtk_tree_menu_set_row_span_column (GTK_TREE_MENU (priv->popup_widget), priv->row_column);
-
+      _gtk_tree_menu_set_row_span_column (GTK_TREE_MENU (priv->popup_widget), priv->row_column);
       g_object_notify (G_OBJECT (combo_box), "row-span-column");
     }
 }
@@ -2548,10 +2534,7 @@ gtk_combo_box_set_column_span_column (GtkComboBox *combo_box,
   if (column_span != priv->col_column)
     {
       priv->col_column = column_span;
-
-      if (priv->popup_widget)
-        _gtk_tree_menu_set_column_span_column (GTK_TREE_MENU (priv->popup_widget), priv->col_column);
-
+      _gtk_tree_menu_set_column_span_column (GTK_TREE_MENU (priv->popup_widget), priv->col_column);
       g_object_notify (G_OBJECT (combo_box), "column-span-column");
     }
 }
@@ -2662,8 +2645,7 @@ gtk_combo_box_set_active_internal (GtkComboBox *combo_box,
 
   if (!path)
     {
-      if (priv->popup_widget)
-        gtk_menu_set_active (GTK_MENU (priv->popup_widget), -1);
+      gtk_menu_set_active (GTK_MENU (priv->popup_widget), -1);
 
       if (priv->cell_view)
         gtk_cell_view_set_displayed_row (GTK_CELL_VIEW (priv->cell_view), NULL);
@@ -2681,9 +2663,8 @@ gtk_combo_box_set_active_internal (GtkComboBox *combo_box,
         gtk_tree_row_reference_new (priv->model, path);
 
       /* FIXME handle nested menus better */
-      if (priv->popup_widget)
-        gtk_menu_set_active (GTK_MENU (priv->popup_widget),
-                             gtk_tree_path_get_indices (path)[0]);
+      gtk_menu_set_active (GTK_MENU (priv->popup_widget),
+                           gtk_tree_path_get_indices (path)[0]);
 
       if (priv->cell_view)
         gtk_cell_view_set_displayed_row (GTK_CELL_VIEW (priv->cell_view),
@@ -2808,9 +2789,8 @@ gtk_combo_box_set_model (GtkComboBox  *combo_box,
                       G_CALLBACK (gtk_combo_box_model_row_changed),
                       combo_box);
 
-  if (priv->popup_widget)
-    _gtk_tree_menu_set_model (GTK_TREE_MENU (priv->popup_widget),
-                              priv->model);
+  _gtk_tree_menu_set_model (GTK_TREE_MENU (priv->popup_widget),
+                            priv->model);
 
   if (priv->cell_view)
     gtk_cell_view_set_model (GTK_CELL_VIEW (priv->cell_view),
@@ -3006,8 +2986,6 @@ gtk_combo_box_destroy (GtkWidget *widget)
       _gtk_bin_set_child (GTK_BIN (combo_box), NULL);
     }
 
-  gtk_combo_box_popdown (combo_box);
-
   if (priv->row_separator_destroy)
     priv->row_separator_destroy (priv->row_separator_data);
 
@@ -3116,9 +3094,7 @@ gtk_combo_box_constructed (GObject *object)
 
   gtk_combo_box_create_child (combo_box);
 
-  /* Create the popup menu, if it doesn’t already exist. */
-  if (!priv->popup_widget)
-    gtk_combo_box_menu_setup (combo_box);
+  gtk_combo_box_menu_setup (combo_box);
 
   if (priv->has_entry)
     {
@@ -3137,12 +3113,9 @@ gtk_combo_box_dispose(GObject* object)
   GtkComboBox *combo_box = GTK_COMBO_BOX (object);
   GtkComboBoxPrivate *priv = combo_box->priv;
 
-  if (priv->popup_widget)
-    {
-      gtk_combo_box_menu_destroy (combo_box);
-      gtk_menu_detach (GTK_MENU (priv->popup_widget));
-      priv->popup_widget = NULL;
-    }
+  gtk_combo_box_menu_destroy (combo_box);
+  gtk_menu_detach (GTK_MENU (priv->popup_widget));
+  priv->popup_widget = NULL;
 
   if (priv->area)
     {
@@ -3249,7 +3222,6 @@ gtk_combo_box_set_popup_fixed_width (GtkComboBox *combo_box,
   if (priv->popup_fixed_width != fixed)
     {
       priv->popup_fixed_width = fixed;
-
       g_object_notify (G_OBJECT (combo_box), "popup-fixed-width");
     }
 }
@@ -3291,20 +3263,9 @@ gtk_combo_box_get_popup_fixed_width (GtkComboBox *combo_box)
 AtkObject*
 gtk_combo_box_get_popup_accessible (GtkComboBox *combo_box)
 {
-  GtkComboBoxPrivate *priv;
-  AtkObject *atk_obj;
-
   g_return_val_if_fail (GTK_IS_COMBO_BOX (combo_box), NULL);
 
-  priv = combo_box->priv;
-
-  if (priv->popup_widget)
-    {
-      atk_obj = gtk_widget_get_accessible (priv->popup_widget);
-      return atk_obj;
-    }
-
-  return NULL;
+  return gtk_widget_get_accessible (combo_box->priv->popup_widget);
 }
 
 /**
@@ -3357,12 +3318,9 @@ gtk_combo_box_set_row_separator_func (GtkComboBox                 *combo_box,
   priv->row_separator_data = data;
   priv->row_separator_destroy = destroy;
 
-  /* Provoke the underlying menu to rebuild themselves with the new separator func */
-  if (priv->popup_widget)
-    {
-      _gtk_tree_menu_set_model (GTK_TREE_MENU (priv->popup_widget), NULL);
-      _gtk_tree_menu_set_model (GTK_TREE_MENU (priv->popup_widget), priv->model);
-    }
+  /* Rebuild the menu with the new separator function */
+  _gtk_tree_menu_set_model (GTK_TREE_MENU (priv->popup_widget), NULL);
+  _gtk_tree_menu_set_model (GTK_TREE_MENU (priv->popup_widget), priv->model);
 
   gtk_widget_queue_draw (GTK_WIDGET (combo_box));
 }
@@ -3392,7 +3350,6 @@ gtk_combo_box_set_button_sensitivity (GtkComboBox        *combo_box,
     {
       priv->button_sensitivity = sensitivity;
       gtk_combo_box_update_sensitivity (combo_box);
-
       g_object_notify (G_OBJECT (combo_box), "button-sensitivity");
     }
 }
diff --git a/gtk/gtkcomboboxtext.c b/gtk/gtkcomboboxtext.c
index 4b15c04..491d739 100644
--- a/gtk/gtkcomboboxtext.c
+++ b/gtk/gtkcomboboxtext.c
@@ -109,20 +109,29 @@ G_DEFINE_TYPE_WITH_CODE (GtkComboBoxText, gtk_combo_box_text, GTK_TYPE_COMBO_BOX
 static void
 gtk_combo_box_text_constructed (GObject *object)
 {
+  GtkComboBox *combo_box = (GtkComboBox *)object;
   const gint text_column = 0;
+  GtkListStore *store;
 
   G_OBJECT_CLASS (gtk_combo_box_text_parent_class)->constructed (object);
 
-  gtk_combo_box_set_entry_text_column (GTK_COMBO_BOX (object), text_column);
-  gtk_combo_box_set_id_column (GTK_COMBO_BOX (object), 1);
+  /* Do this here, not in init(), because combo_box_set_model() calls
+   * combo_box_menu_setup(), which relies on ::cell area being valid, and the
+   * area is not guaranteed to be set until after ComboBox::constructed(). */
+  store = gtk_list_store_new (2, G_TYPE_STRING, G_TYPE_STRING);
+  gtk_combo_box_set_model (combo_box, GTK_TREE_MODEL (store));
+  g_object_unref (store);
+
+  gtk_combo_box_set_entry_text_column (combo_box, text_column);
+  gtk_combo_box_set_id_column (combo_box, 1);
 
-  if (!gtk_combo_box_get_has_entry (GTK_COMBO_BOX (object)))
+  if (!gtk_combo_box_get_has_entry (combo_box))
     {
       GtkCellRenderer *cell;
 
       cell = gtk_cell_renderer_text_new ();
-      gtk_cell_layout_pack_start (GTK_CELL_LAYOUT (object), cell, TRUE);
-      gtk_cell_layout_set_attributes (GTK_CELL_LAYOUT (object), cell,
+      gtk_cell_layout_pack_start (GTK_CELL_LAYOUT (combo_box), cell, TRUE);
+      gtk_cell_layout_set_attributes (GTK_CELL_LAYOUT (combo_box), cell,
                                       "text", text_column,
                                       NULL);
     }
@@ -131,11 +140,6 @@ gtk_combo_box_text_constructed (GObject *object)
 static void
 gtk_combo_box_text_init (GtkComboBoxText *combo_box)
 {
-  GtkListStore *store;
-
-  store = gtk_list_store_new (2, G_TYPE_STRING, G_TYPE_STRING);
-  gtk_combo_box_set_model (GTK_COMBO_BOX (combo_box), GTK_TREE_MODEL (store));
-  g_object_unref (store);
 }
 
 static void