Don't reload the current folder unnecessarily on ::map().

2005-09-27  Federico Mena Quintero  <federico@ximian.com>

	Don't reload the current folder unnecessarily on ::map().

	* gtk/gtkfilechooserprivate.h (ReloadState): New enum to represent
	the reloading state.
	(struct _GtkFileChooserDefault): Added a "reload_state" field.

	* gtk/gtkfilechooserdefault.c (gtk_file_chooser_default_init):
	Initialize impl->reload_state.
	(gtk_file_chooser_default_map): Check the impl->reload_state; load
	a default folder if no folder has been set, or reload the current
	one only if we had been unmapped first.
	(gtk_file_chooser_default_update_current_folder): Set the
	reload_state to RELOAD_HAS_FOLDER.
	(gtk_file_chooser_default_unmap): Implement, and set the
	reload_state to RELOAD_WAS_UNMAPPED.
	(shortcuts_model_create): Don't call shortcuts_add_bookmarks()
	here; they'll get (re)loaded on ::map() anyway.

	* gtk/gtkfilechooserwidget.c
	(gtk_file_chooser_widget_constructor): Don't set a default folder here.

	* tests/autotestfilechooser.c (test_action_widgets): Don't take in
	a dialog; build it ourselves.
	(test_reload): New test to ensure that we don't load the default
	folder more than once, and that we reload it when
	unmapping/remapping.
	(get_impl_from_dialog): New utility function.
	(test_widgets_for_current_action): Use get_impl_from_dialog().

5 files changed, 238 insertions, 23 deletions

diff --git a/ChangeLog b/ChangeLog
index 5cb1fc6e45..2645cd0606 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,36 @@
 2005-09-27  Federico Mena Quintero  <federico@ximian.com>
 
+	Don't reload the current folder unnecessarily on ::map().
+
+	* gtk/gtkfilechooserprivate.h (ReloadState): New enum to represent
+	the reloading state.
+	(struct _GtkFileChooserDefault): Added a "reload_state" field.
+
+	* gtk/gtkfilechooserdefault.c (gtk_file_chooser_default_init):
+	Initialize impl->reload_state.
+	(gtk_file_chooser_default_map): Check the impl->reload_state; load
+	a default folder if no folder has been set, or reload the current
+	one only if we had been unmapped first.
+	(gtk_file_chooser_default_update_current_folder): Set the
+	reload_state to RELOAD_HAS_FOLDER.
+	(gtk_file_chooser_default_unmap): Implement, and set the
+	reload_state to RELOAD_WAS_UNMAPPED.
+	(shortcuts_model_create): Don't call shortcuts_add_bookmarks()
+	here; they'll get (re)loaded on ::map() anyway.
+
+	* gtk/gtkfilechooserwidget.c
+	(gtk_file_chooser_widget_constructor): Don't set a default folder here.
+
+	* tests/autotestfilechooser.c (test_action_widgets): Don't take in
+	a dialog; build it ourselves.
+	(test_reload): New test to ensure that we don't load the default
+	folder more than once, and that we reload it when
+	unmapping/remapping.
+	(get_impl_from_dialog): New utility function.
+	(test_widgets_for_current_action): Use get_impl_from_dialog().
+
+2005-09-27  Federico Mena Quintero  <federico@ximian.com>
+
 	* gtk/gtkfilechooserdefault.c
 	(gtk_file_chooser_default_constructor): Mark the entry/exit of
 	this function for profiling.
diff --git a/ChangeLog.pre-2-10 b/ChangeLog.pre-2-10
index 5cb1fc6e45..2645cd0606 100644
--- a/ChangeLog.pre-2-10
+++ b/ChangeLog.pre-2-10
@@ -1,5 +1,36 @@
 2005-09-27  Federico Mena Quintero  <federico@ximian.com>
 
+	Don't reload the current folder unnecessarily on ::map().
+
+	* gtk/gtkfilechooserprivate.h (ReloadState): New enum to represent
+	the reloading state.
+	(struct _GtkFileChooserDefault): Added a "reload_state" field.
+
+	* gtk/gtkfilechooserdefault.c (gtk_file_chooser_default_init):
+	Initialize impl->reload_state.
+	(gtk_file_chooser_default_map): Check the impl->reload_state; load
+	a default folder if no folder has been set, or reload the current
+	one only if we had been unmapped first.
+	(gtk_file_chooser_default_update_current_folder): Set the
+	reload_state to RELOAD_HAS_FOLDER.
+	(gtk_file_chooser_default_unmap): Implement, and set the
+	reload_state to RELOAD_WAS_UNMAPPED.
+	(shortcuts_model_create): Don't call shortcuts_add_bookmarks()
+	here; they'll get (re)loaded on ::map() anyway.
+
+	* gtk/gtkfilechooserwidget.c
+	(gtk_file_chooser_widget_constructor): Don't set a default folder here.
+
+	* tests/autotestfilechooser.c (test_action_widgets): Don't take in
+	a dialog; build it ourselves.
+	(test_reload): New test to ensure that we don't load the default
+	folder more than once, and that we reload it when
+	unmapping/remapping.
+	(get_impl_from_dialog): New utility function.
+	(test_widgets_for_current_action): Use get_impl_from_dialog().
+
+2005-09-27  Federico Mena Quintero  <federico@ximian.com>
+
 	* gtk/gtkfilechooserdefault.c
 	(gtk_file_chooser_default_constructor): Mark the entry/exit of
 	this function for profiling.
diff --git a/gtk/gtkfilechooserdefault.c b/gtk/gtkfilechooserdefault.c
index 41efc5b595..4d5cdbd46f 100644
--- a/gtk/gtkfilechooserdefault.c
+++ b/gtk/gtkfilechooserdefault.c
@@ -258,6 +258,7 @@ static void     gtk_file_chooser_default_get_property (GObject               *ob
 static void     gtk_file_chooser_default_dispose      (GObject               *object);
 static void     gtk_file_chooser_default_show_all       (GtkWidget             *widget);
 static void     gtk_file_chooser_default_map            (GtkWidget             *widget);
+static void     gtk_file_chooser_default_unmap          (GtkWidget             *widget);
 static void     gtk_file_chooser_default_hierarchy_changed (GtkWidget          *widget,
 							    GtkWidget          *previous_toplevel);
 static void     gtk_file_chooser_default_style_set      (GtkWidget             *widget,
@@ -500,6 +501,7 @@ gtk_file_chooser_default_class_init (GtkFileChooserDefaultClass *class)
 
   widget_class->show_all = gtk_file_chooser_default_show_all;
   widget_class->map = gtk_file_chooser_default_map;
+  widget_class->unmap = gtk_file_chooser_default_unmap;
   widget_class->hierarchy_changed = gtk_file_chooser_default_hierarchy_changed;
   widget_class->style_set = gtk_file_chooser_default_style_set;
   widget_class->screen_changed = gtk_file_chooser_default_screen_changed;
@@ -634,6 +636,7 @@ gtk_file_chooser_default_init (GtkFileChooserDefault *impl)
   impl->show_hidden = FALSE;
   impl->icon_size = FALLBACK_ICON_SIZE;
   impl->load_state = LOAD_EMPTY;
+  impl->reload_state = RELOAD_EMPTY;
   impl->pending_select_paths = NULL;
 
   gtk_box_set_spacing (GTK_BOX (impl), 12);
@@ -1749,7 +1752,6 @@ shortcuts_model_create (GtkFileChooserDefault *impl)
       shortcuts_append_home (impl);
       shortcuts_append_desktop (impl);
       shortcuts_add_volumes (impl);
-      shortcuts_add_bookmarks (impl);
     }
 
   impl->shortcuts_filter_model = shortcuts_model_filter_new (impl,
@@ -4656,6 +4658,7 @@ static void
 gtk_file_chooser_default_map (GtkWidget *widget)
 {
   GtkFileChooserDefault *impl;
+  char *current_working_dir;
 
   profile_start ("start", NULL);
 
@@ -4663,10 +4666,29 @@ gtk_file_chooser_default_map (GtkWidget *widget)
 
   GTK_WIDGET_CLASS (parent_class)->map (widget);
 
-  if (impl->current_folder)
+  switch (impl->reload_state)
     {
+    case RELOAD_EMPTY:
+      /* The user didn't explicitly give us a folder to display, so we'll use the cwd */
+      current_working_dir = g_get_current_dir ();
+      gtk_file_chooser_set_current_folder (GTK_FILE_CHOOSER (impl), current_working_dir);
+      g_free (current_working_dir);
+      break;
+
+    case RELOAD_HAS_FOLDER:
+      /* Nothing; we are already loading or loaded, so we don't need to reload */
+      break;
+
+    case RELOAD_WAS_UNMAPPED:
+      /* Just reload the current folder */
+      g_assert (impl->current_folder != NULL);
+
       pending_select_paths_store_selection (impl);
       change_folder_and_display_error (impl, impl->current_folder);
+      break;
+
+    default:
+      g_assert_not_reached ();
     }
 
   bookmarks_changed_cb (impl->file_system, impl);
@@ -4674,6 +4696,19 @@ gtk_file_chooser_default_map (GtkWidget *widget)
   profile_end ("end", NULL);
 }
 
+/* GtkWidget::unmap method */
+static void
+gtk_file_chooser_default_unmap (GtkWidget *widget)
+{
+  GtkFileChooserDefault *impl;
+
+  impl = GTK_FILE_CHOOSER_DEFAULT (widget);
+
+  GTK_WIDGET_CLASS (parent_class)->unmap (widget);
+
+  impl->reload_state = RELOAD_WAS_UNMAPPED;
+}
+
 static gboolean
 list_model_filter_func (GtkFileSystemModel *model,
 			GtkFilePath        *path,
@@ -5283,6 +5318,8 @@ gtk_file_chooser_default_update_current_folder (GtkFileChooser    *chooser,
 	gtk_file_path_free (impl->current_folder);
 
       impl->current_folder = gtk_file_path_copy (path);
+
+      impl->reload_state = RELOAD_HAS_FOLDER;
     }
 
   /* Update the widgets that may trigger a folder change themselves.  */
diff --git a/gtk/gtkfilechooserwidget.c b/gtk/gtkfilechooserwidget.c
index c065f67f65..fa4e3f2d71 100644
--- a/gtk/gtkfilechooserwidget.c
+++ b/gtk/gtkfilechooserwidget.c
@@ -139,7 +139,6 @@ gtk_file_chooser_widget_constructor (GType                  type,
 {
   GtkFileChooserWidgetPrivate *priv;
   GObject *object;
-  gchar *current_folder;
   
   object = parent_class->constructor (type,
 				      n_construct_properties,
@@ -153,10 +152,6 @@ gtk_file_chooser_widget_constructor (GType                  type,
   gtk_box_pack_start (GTK_BOX (object), priv->impl, TRUE, TRUE, 0);
   gtk_widget_show (priv->impl);
 
-  current_folder = g_get_current_dir ();
-  gtk_file_chooser_set_current_folder (GTK_FILE_CHOOSER (priv->impl), current_folder);
-  g_free (current_folder);
-
   _gtk_file_chooser_set_delegate (GTK_FILE_CHOOSER (object),
 				  GTK_FILE_CHOOSER (priv->impl));
 
diff --git a/tests/autotestfilechooser.c b/tests/autotestfilechooser.c
index 0ba2683333..bc9002ff68 100644
--- a/tests/autotestfilechooser.c
+++ b/tests/autotestfilechooser.c
@@ -63,20 +63,17 @@ get_action_name (GtkFileChooserAction action)
   return enum_value->value_name;
 }
 
-static gboolean
-test_widgets_for_current_action (GtkFileChooserDialog *dialog,
-				 GtkFileChooserAction  expected_action)
+static GtkFileChooserDefault *
+get_impl_from_dialog (GtkWidget *dialog)
 {
+  GtkFileChooserDialog *d;
   GtkFileChooserDialogPrivate *dialog_priv;
   GtkFileChooserWidget *chooser_widget;
   GtkFileChooserWidgetPrivate *widget_priv;
   GtkFileChooserDefault *impl;
-  gboolean passed;
 
-  if (gtk_file_chooser_get_action (GTK_FILE_CHOOSER (dialog)) != expected_action)
-    return FALSE;
-
-  dialog_priv = dialog->priv;
+  d = GTK_FILE_CHOOSER_DIALOG (dialog);
+  dialog_priv = d->priv;
   chooser_widget = GTK_FILE_CHOOSER_WIDGET (dialog_priv->widget);
   if (!chooser_widget)
     g_error ("BUG: dialog_priv->widget is not a GtkFileChooserWidget");
@@ -86,6 +83,21 @@ test_widgets_for_current_action (GtkFileChooserDialog *dialog,
   if (!impl)
     g_error ("BUG: widget_priv->impl is not a GtkFileChooserDefault");
 
+  return impl;
+}
+
+static gboolean
+test_widgets_for_current_action (GtkFileChooserDialog *dialog,
+				 GtkFileChooserAction  expected_action)
+{
+  GtkFileChooserDefault *impl;
+  gboolean passed;
+
+  if (gtk_file_chooser_get_action (GTK_FILE_CHOOSER (dialog)) != expected_action)
+    return FALSE;
+
+  impl = get_impl_from_dialog (GTK_WIDGET (dialog));
+
   g_assert (impl->action == expected_action);
 
   passed = TRUE;
@@ -208,31 +220,45 @@ switch_from_action_cb (GtkFileChooserDialog *dialog,
 }
 
 static gboolean
-test_action_widgets (GtkFileChooserDialog *dialog)
+test_action_widgets (void)
 {
+  GtkWidget *dialog;
   GtkFileChooserAction action;
   gboolean passed;
 
+  dialog = gtk_file_chooser_dialog_new ("Test file chooser",
+					NULL,
+					GTK_FILE_CHOOSER_ACTION_OPEN,
+					GTK_STOCK_CANCEL,
+					GTK_RESPONSE_CANCEL,
+					GTK_STOCK_OK,
+					GTK_RESPONSE_ACCEPT,
+					NULL);
+  gtk_widget_show (dialog);
+
   action = gtk_file_chooser_get_action (GTK_FILE_CHOOSER (dialog));
 
-  passed = test_widgets_for_current_action (dialog, action);
+  passed = test_widgets_for_current_action (GTK_FILE_CHOOSER_DIALOG (dialog), action);
   log_test (passed, "test_action_widgets(): widgets for initial action %s", get_action_name (action));
   if (!passed)
     return FALSE;
 
-  passed = foreach_action (dialog, switch_from_action_cb, NULL);
+  passed = foreach_action (GTK_FILE_CHOOSER_DIALOG (dialog), switch_from_action_cb, NULL);
   log_test (passed, "test_action_widgets(): all transitions through property change");
 
+  gtk_widget_destroy (dialog);
+
   return passed;
 }
 
-
-int
-main (int argc, char **argv)
+static gboolean
+test_reload_sequence (gboolean set_folder_before_map)
 {
   GtkWidget *dialog;
+  GtkFileChooserDefault *impl;
+  gboolean passed;
 
-  gtk_init (&argc, &argv);
+  passed = TRUE;
 
   dialog = gtk_file_chooser_dialog_new ("Test file chooser",
 					NULL,
@@ -242,11 +268,106 @@ main (int argc, char **argv)
 					GTK_STOCK_OK,
 					GTK_RESPONSE_ACCEPT,
 					NULL);
+  impl = get_impl_from_dialog (dialog);
+
+  if (set_folder_before_map)
+    {
+      gtk_file_chooser_set_current_folder (GTK_FILE_CHOOSER (dialog), g_get_home_dir ());
+
+      passed = (impl->current_folder != NULL
+		&& impl->browse_files_model != NULL
+		&& (impl->load_state == LOAD_PRELOAD || impl->load_state == LOAD_LOADING || impl->load_state == LOAD_FINISHED)
+		&& impl->reload_state == RELOAD_HAS_FOLDER
+		&& (impl->load_state == LOAD_PRELOAD ? (impl->load_timeout_id != 0) : TRUE)
+		&& ((impl->load_state == LOAD_LOADING || impl->load_state == LOAD_FINISHED)
+		    ? (impl->load_timeout_id == 0 && impl->sort_model != NULL)
+		    : TRUE));
+    }
+  else
+    {
+      /* Initially, no folder is not loaded or pending */
+      passed = passed && (impl->current_folder == NULL
+			  && impl->sort_model == NULL
+			  && impl->browse_files_model == NULL
+			  && impl->load_state == LOAD_EMPTY
+			  && impl->reload_state == RELOAD_EMPTY
+			  && impl->load_timeout_id == 0);
+    }
+
+  if (!passed)
+    return FALSE;
+
+  /* After mapping, it is loading some folder, either the one that was explicitly set or the default one */
+
   gtk_widget_show (dialog);
 
-  test_action_widgets (GTK_FILE_CHOOSER_DIALOG (dialog));
+  passed = (impl->current_folder != NULL
+	    && impl->browse_files_model != NULL
+	    && (impl->load_state == LOAD_PRELOAD || impl->load_state == LOAD_LOADING || impl->load_state == LOAD_FINISHED)
+	    && impl->reload_state == RELOAD_HAS_FOLDER
+	    && (impl->load_state == LOAD_PRELOAD ? (impl->load_timeout_id != 0) : TRUE)
+	    && ((impl->load_state == LOAD_LOADING || impl->load_state == LOAD_FINISHED)
+		? (impl->load_timeout_id == 0 && impl->sort_model != NULL)
+		: TRUE));
+  if (!passed)
+    return FALSE;
+
+  /* Unmap it; we should still have a folder */
+
+  gtk_widget_hide (dialog);
+
+  passed = (impl->current_folder != NULL
+	    && impl->browse_files_model != NULL
+	    && (impl->load_state == LOAD_PRELOAD || impl->load_state == LOAD_LOADING || impl->load_state == LOAD_FINISHED)
+	    && impl->reload_state == RELOAD_WAS_UNMAPPED
+	    && (impl->load_state == LOAD_PRELOAD ? (impl->load_timeout_id != 0) : TRUE)
+	    && ((impl->load_state == LOAD_LOADING || impl->load_state == LOAD_FINISHED)
+		? (impl->load_timeout_id == 0 && impl->sort_model != NULL)
+		: TRUE));
+
+  /* Map it again! */
+
+  gtk_widget_show (dialog);
+  
+  passed = (impl->current_folder != NULL
+	    && impl->browse_files_model != NULL
+	    && (impl->load_state == LOAD_PRELOAD || impl->load_state == LOAD_LOADING || impl->load_state == LOAD_FINISHED)
+	    && impl->reload_state == RELOAD_HAS_FOLDER
+	    && (impl->load_state == LOAD_PRELOAD ? (impl->load_timeout_id != 0) : TRUE)
+	    && ((impl->load_state == LOAD_LOADING || impl->load_state == LOAD_FINISHED)
+		? (impl->load_timeout_id == 0 && impl->sort_model != NULL)
+		: TRUE));
+  if (!passed)
+    return FALSE;
 
   gtk_widget_destroy (dialog);
 
+  return passed;
+}
+
+static gboolean
+test_reload (void)
+{
+  gboolean passed;
+
+  passed = test_reload_sequence (FALSE);
+  log_test (passed, "test_reload(): create and use the default folder");
+  if (!passed)
+    return FALSE;
+
+  passed = test_reload_sequence (TRUE);
+  log_test (passed, "test_reload(): set a folder explicitly before mapping");
+
+  return passed;
+}
+
+int
+main (int argc, char **argv)
+{
+  gtk_init (&argc, &argv);
+
+  test_action_widgets ();
+  test_reload ();
+
   return 0;
 }