completely rework scaffolding window destruction using gtkwidget destroy signals

4 files changed, 161 insertions, 117 deletions

diff --git a/gtk/scaffolding.c b/gtk/scaffolding.c
index 5a958447f..b10fb69ae 100644
--- a/gtk/scaffolding.c
+++ b/gtk/scaffolding.c
@@ -139,7 +139,6 @@ struct gtk_scaffolding {
 
 	int				throb_frame;
 	struct gui_window		*top_level;
-	int				being_destroyed;
 
 	bool				fullscreen;
 
@@ -147,9 +146,6 @@ struct gtk_scaffolding {
 	struct gtk_scaffolding 		*next, *prev;
 };
 
-/** current number of open browser windows */
-static int open_windows = 0;
-
 /** current window for model dialogue use */
 static struct gtk_scaffolding *current_model;
 
@@ -251,60 +247,57 @@ static void popup_menu_show(struct nsgtk_popup_submenu *menu, bool submenu,
 /* event handlers and support functions for them */
 
 /**
- * resource cleanup function for window closure.
+ * resource cleanup function for window destruction.
  */
-static void nsgtk_window_close(struct gtk_scaffolding *g)
+static void scaffolding_window_destroy(GtkWidget *widget, gpointer data)
 {
-	/* close all tabs first */
-	gint numbertabs = gtk_notebook_get_n_pages(g->notebook);
-	while (numbertabs-- > 1) {
-		nsgtk_tab_close_current(g->notebook);
-	}
-	LOG(("Being Destroyed = %d", g->being_destroyed));
+	struct gtk_scaffolding *gs = data;
 
-	if ((g->history_window) && (g->history_window->window)) {
-		gtk_widget_destroy(GTK_WIDGET(g->history_window->window));
-	}
+	LOG(("scaffold:%p", gs));
 
-	if (--open_windows == 0)
-		netsurf_quit = true;
+	if ((gs->history_window) && (gs->history_window->window)) {
+		gtk_widget_destroy(GTK_WIDGET(gs->history_window->window));
+	}
 
-	if (!g->being_destroyed) {
-		g->being_destroyed = 1;
-		nsgtk_window_destroy_browser(g->top_level);
+	if (gs->prev != NULL) {
+		gs->prev->next = gs->next;
+	} else {
+		scaf_list = gs->next;
+	}
+	if (gs->next != NULL) {
+		gs->next->prev = gs->prev;
 	}
-	if (g->prev != NULL)
-		g->prev->next = g->next;
-	else
-		scaf_list = g->next;
 
-	if (g->next != NULL)
-		g->next->prev = g->prev;
+	LOG(("scaffold list head: %p", scaf_list));
 
+	if (scaf_list == NULL) {
+		/* no more open windows */
+		netsurf_quit = true;
+	}
 }
 
-static gboolean nsgtk_window_delete_event(GtkWidget *widget,
+/* signal delivered on window delete event, allowing to halt close if
+ * download is in progress
+ */
+static gboolean scaffolding_window_delete_event(GtkWidget *widget,
 		GdkEvent *event, gpointer data)
 {
-	struct gtk_scaffolding *g = (struct gtk_scaffolding *)data;
-	if ((open_windows != 1) ||
-	    nsgtk_check_for_downloads(GTK_WINDOW(widget)) == false) {
-		nsgtk_window_close(g);
+	struct gtk_scaffolding *g = data;
+
+	if (nsgtk_check_for_downloads(GTK_WINDOW(widget)) == false) {
 		gtk_widget_destroy(GTK_WIDGET(g->window));
 	}
 	return TRUE;
 }
 
 /* exported interface documented in gtk_scaffold.h */
-void nsgtk_scaffolding_destroy(nsgtk_scaffolding *g)
+void nsgtk_scaffolding_destroy(nsgtk_scaffolding *gs)
 {
-	/* Our top_level has asked us to die */
-	LOG(("Being Destroyed = %d", g->being_destroyed));
-	if (g->being_destroyed) return;
-	g->being_destroyed = 1;
-	nsgtk_window_close(g);
-	/* We're now unlinked, so let's finally destroy ourselves */
-	nsgtk_window_destroy_browser(g->top_level);
+	LOG(("scaffold: %p", gs));
+
+	if (gtk_widget_in_destruction(GTK_WIDGET(gs->window)) != TRUE) {
+		gtk_widget_destroy(GTK_WIDGET(gs->window));
+	}
 }
 
 /**
@@ -522,21 +515,32 @@ static void nsgtk_window_tabs_add(GtkNotebook *notebook,
 /**
  * Update the menus when the number of tabs changes.
  */
-static void nsgtk_window_tabs_remove(GtkNotebook *notebook,
-		GtkWidget *page, guint page_num, struct gtk_scaffolding *g)
-{
+static void
+nsgtk_window_tabs_remove(GtkNotebook *notebook,
+			 GtkWidget *page,
+			 guint page_num,
+			 struct gtk_scaffolding *gs)
+{
+	/* if the scaffold is being destroyed it is not useful to
+	 * update the state, futher many of the widgets may have
+	 * already been destroyed.
+	 */
+	if (gtk_widget_in_destruction(GTK_WIDGET(gs->window)) == TRUE) {
+		return;
+	}
 
-	if (gtk_notebook_get_n_pages(notebook) == 0) {
-		nsgtk_scaffolding_destroy(g);
-	} else {
-	gboolean visible = gtk_notebook_get_show_tabs(g->notebook);
-	g_object_set(g->menu_bar->view_submenu->tabs_menuitem, "visible", visible, NULL);
-	g_object_set(g->menu_popup->view_submenu->tabs_menuitem, "visible", visible, NULL);
-	g->buttons[NEXTTAB_BUTTON]->sensitivity = visible;
-	g->buttons[PREVTAB_BUTTON]->sensitivity = visible;
-	g->buttons[CLOSETAB_BUTTON]->sensitivity = visible;
-	nsgtk_scaffolding_set_sensitivity(g);
+	if (gtk_notebook_get_n_pages(notebook) == 1) {
+		nsgtk_scaffolding_destroy(gs);
+		return;
 	}
+
+	gboolean visible = gtk_notebook_get_show_tabs(gs->notebook);
+	g_object_set(gs->menu_bar->view_submenu->tabs_menuitem, "visible", visible, NULL);
+	g_object_set(gs->menu_popup->view_submenu->tabs_menuitem, "visible", visible, NULL);
+	gs->buttons[NEXTTAB_BUTTON]->sensitivity = visible;
+	gs->buttons[PREVTAB_BUTTON]->sensitivity = visible;
+	gs->buttons[CLOSETAB_BUTTON]->sensitivity = visible;
+	nsgtk_scaffolding_set_sensitivity(gs);
 }
 
 /**
@@ -639,7 +643,8 @@ nserror nsgtk_scaffolding_new_tab(struct gui_window *gw)
 MULTIHANDLER(newtab)
 {
 	nserror error;
-	nsgtk_scaffolding_new_tab(g->top_level);
+
+	error = nsgtk_scaffolding_new_tab(g->top_level);
 	if (error != NSERROR_OK) {
 		warn_user(messages_get_errorcode(error), 0);
 	}
@@ -947,7 +952,6 @@ MULTIHANDLER(print)
 
 MULTIHANDLER(closewindow)
 {
-	nsgtk_window_close(g);
 	gtk_widget_destroy(GTK_WIDGET(g->window));
 	return TRUE;
 }
@@ -1842,12 +1846,13 @@ static bool nsgtk_new_scaffolding_popup(struct gtk_scaffolding *g, GtkAccelGroup
 
 nsgtk_scaffolding *nsgtk_new_scaffolding(struct gui_window *toplevel)
 {
-	struct gtk_scaffolding *g = malloc(sizeof(*g));
+	struct gtk_scaffolding *g;
 	char *searchname;
 	int i;
 	GtkAccelGroup *group;
 	GError* error = NULL;
 
+	g = malloc(sizeof(*g));
 	if (g == NULL) {
 		warn_user("NoMemory", 0);
 		return NULL;
@@ -1857,8 +1862,6 @@ nsgtk_scaffolding *nsgtk_new_scaffolding(struct gui_window *toplevel)
 
 	g->top_level = toplevel;
 
-	open_windows++;
-
 	/* load the window template from the glade xml file, and extract
 	 * widget references from it for later use.
 	 */
@@ -2083,8 +2086,9 @@ nsgtk_scaffolding *nsgtk_new_scaffolding(struct gui_window *toplevel)
 	g_signal_connect_after(g->notebook, "page-removed",
 			G_CALLBACK(nsgtk_window_tabs_remove), g);
 
-	/* connect signals to handlers. */
-	CONNECT(g->window, "delete-event", nsgtk_window_delete_event, g);
+	/* connect main window signals to their handlers. */
+	CONNECT(g->window, "delete-event", scaffolding_window_delete_event, g);
+	CONNECT(g->window, "destroy", scaffolding_window_destroy, g);
 
 	/* toolbar URL bar menu bar search bar signal handlers */
 	CONNECT(g->menu_bar->edit_submenu->edit, "show", nsgtk_window_edit_menu_clicked, g);
@@ -2132,8 +2136,6 @@ nsgtk_scaffolding *nsgtk_new_scaffolding(struct gui_window *toplevel)
 
 	nsgtk_scaffolding_initial_sensitivity(g);
 
-	g->being_destroyed = 0;
-
 	g->fullscreen = false;
 
 
@@ -2469,8 +2471,10 @@ void nsgtk_scaffolding_set_top_level(struct gui_window *gw)
 	}
 }
 
+/* exported interface documented in scaffolding.h */
 void nsgtk_scaffolding_set_sensitivity(struct gtk_scaffolding *g)
 {
+	int i;
 #define SENSITIVITY(q)\
 		i = q##_BUTTON;\
 		if (g->buttons[i]->main != NULL)\
@@ -2491,7 +2495,6 @@ void nsgtk_scaffolding_set_sensitivity(struct gtk_scaffolding *g)
 					g->buttons[i]->popup),\
 					g->buttons[i]->sensitivity);
 
-	int i;
 	SENSITIVITY(STOP)
 	SENSITIVITY(RELOAD)
 	SENSITIVITY(CUT)
diff --git a/gtk/scaffolding.h b/gtk/scaffolding.h
index e6426ded8..79e3fa0df 100644
--- a/gtk/scaffolding.h
+++ b/gtk/scaffolding.h
@@ -144,7 +144,27 @@ void nsgtk_scaffolding_set_top_level(struct gui_window *g);
 
 void nsgtk_scaffolding_destroy(nsgtk_scaffolding *g);
 
+/** update the sensitivity of context sensitive UI elements
+ *
+ * widgets altered in arrays:
+ *   main
+ *   right click menu
+ *   location
+ *   popup
+ * current arrays are:
+ *   stop
+ *   reload
+ *   cut
+ *   copy
+ *   paste
+ *   back
+ *   forward
+ *   nexttab
+ *   prevtab
+ *   closetab
+ */
 void nsgtk_scaffolding_set_sensitivity(struct gtk_scaffolding *g);
+
 void nsgtk_scaffolding_initial_sensitivity(struct gtk_scaffolding *g);
 void nsgtk_scaffolding_popup_menu(struct gtk_scaffolding *g, gdouble x,
     gdouble y);
diff --git a/gtk/tabs.c b/gtk/tabs.c
index b275f12e8..c5ef6fe9c 100644
--- a/gtk/tabs.c
+++ b/gtk/tabs.c
@@ -115,7 +115,7 @@ static GtkWidget *nsgtk_tab_label_setup(struct gui_window *window)
 
 /** callback when page is switched */
 
-static 	gint srcpagenum;
+static gint srcpagenum;
 
 /** The switch-page signal handler
  *
@@ -146,7 +146,8 @@ nsgtk_tab_switch_page_after(GtkNotebook *notebook,
 	addpage = g_object_get_data(G_OBJECT(notebook), "addtab");
 
 	if (selpage == addpage) {
-		if ((srcpagenum != -1) && (srcpagenum != selpagenum)) {
+		if ((srcpagenum != -1) &&
+		    (srcpagenum != (gint)selpagenum)) {
 			/* ensure the add tab is not actually selected */
 			LOG(("src %d sel %d",srcpagenum,selpagenum ));
 			srcpage = gtk_notebook_get_nth_page(notebook, srcpagenum);
@@ -176,7 +177,8 @@ static void nsgtk_tab_page_reordered(GtkNotebook *notebook,
 	pages = gtk_notebook_get_n_pages(notebook);
 	addpage = g_object_get_data(G_OBJECT(notebook), "addtab");
 
-	if ((page_num == (pages - 1)) && (child != addpage)) {
+	if (((gint)page_num == (pages - 1)) &&
+	    (child != addpage)) {
 		/* moved tab to end */
 		gtk_notebook_reorder_child(notebook, addpage, -1);
 	}
@@ -242,7 +244,7 @@ nsgtk_tab_visibility_update(GtkNotebook *notebook, GtkWidget *child, guint page)
 
 	if (addpage != NULL) {
 		pagec--; /* skip the add tab */
-		if (page == pagec) {
+		if ((gint)page == pagec) {
 			/* ensure the add new tab cannot be current */
 			gtk_notebook_set_current_page(notebook, page - 1);
 		}
@@ -289,40 +291,42 @@ void nsgtk_tab_init(struct gtk_scaffolding *gs)
 }
 
 /* exported interface documented in gtk/tabs.h */
-void nsgtk_tab_add(struct gui_window *window,
+void nsgtk_tab_add(struct gui_window *gw,
 		   GtkWidget *tab_contents,
 		   bool background)
 {
-	GtkNotebook *tabs;
+	GtkNotebook *notebook;
 	GtkWidget *tabBox;
 	gint remember;
 	gint pages;
 	gint newpage;
 
-	tabs = nsgtk_scaffolding_notebook(nsgtk_get_scaffold(window));
+	g_object_set_data(G_OBJECT(tab_contents), "gui_window", gw);
+
+	notebook = nsgtk_scaffolding_notebook(nsgtk_get_scaffold(gw));
 
-	tabBox = nsgtk_tab_label_setup(window);
+	tabBox = nsgtk_tab_label_setup(gw);
 
-	nsgtk_window_set_tab(window, tabBox);
+	nsgtk_window_set_tab(gw, tabBox);
 
-	remember = gtk_notebook_get_current_page(tabs);
+	remember = gtk_notebook_get_current_page(notebook);
 
-	pages = gtk_notebook_get_n_pages(tabs);
+	pages = gtk_notebook_get_n_pages(notebook);
 
-	newpage = gtk_notebook_insert_page(tabs, tab_contents, tabBox, pages - 1);
+	newpage = gtk_notebook_insert_page(notebook, tab_contents, tabBox, pages - 1);
 
-	gtk_notebook_set_tab_reorderable(tabs, tab_contents, true);
+	gtk_notebook_set_tab_reorderable(notebook, tab_contents, true);
 
 	gtk_widget_show_all(tab_contents);
 
 	if (background) {
-		gtk_notebook_set_current_page(tabs, remember);
+		gtk_notebook_set_current_page(notebook, remember);
 	} else {
-		gtk_notebook_set_current_page(tabs, newpage);
+		gtk_notebook_set_current_page(notebook, newpage);
 	}
 
 	gtk_widget_grab_focus(GTK_WIDGET(nsgtk_scaffolding_urlbar(
-			nsgtk_get_scaffold(window))));
+			nsgtk_get_scaffold(gw))));
 }
 
 /* exported interface documented in gtk/tabs.h */
@@ -370,7 +374,7 @@ nserror nsgtk_tab_close_current(GtkNotebook *notebook)
 	if (gw == NULL) {
 		return NSERROR_OK;
 	}
-	
+
 	nsgtk_window_destroy_browser(gw);
 
 	return NSERROR_OK;
diff --git a/gtk/window.c b/gtk/window.c
index 43908ba22..34d7a4c6f 100644
--- a/gtk/window.c
+++ b/gtk/window.c
@@ -78,6 +78,9 @@ struct gui_window {
 	/** previous event location */
 	int last_x, last_y;
 
+	/** The top level container (tabContents) */
+	GtkWidget *container;
+
 	/** display widget for this page or frame */
 	GtkLayout *layout;
 
@@ -644,6 +647,14 @@ static gboolean nsgtk_paned_size_allocate_event(GtkWidget *widget,
 	return TRUE;
 }
 
+/* destroy the browsing context as there is nothing to display it now */
+static void window_destroy(GtkWidget *widget, gpointer data)
+{
+	struct gui_window *gw = data;
+
+	browser_window_destroy(gw->bw);
+}
+
 /* Core interface docuemnted in desktop/gui.h to create a gui_window */
 struct gui_window *gui_create_browser_window(struct browser_window *bw,
 					     struct browser_window *clone,
@@ -651,6 +662,7 @@ struct gui_window *gui_create_browser_window(struct browser_window *bw,
 {
 	struct gui_window *g; /**< what we're creating to return */
 	GError* error = NULL;
+	bool tempback;
 
 	g = calloc(1, sizeof(*g));
 	if (!g) {
@@ -685,7 +697,7 @@ struct gui_window *gui_create_browser_window(struct browser_window *bw,
 
 	/* Construct our primary elements */
 
-	/* top-level document (not a frame) => create a new tab */
+	/* top-level document create a new tab */
 	GtkBuilder* xml = gtk_builder_new();
 	if (!gtk_builder_add_from_file(xml,
 				       glade_file_location->tabcontents,
@@ -695,33 +707,16 @@ struct gui_window *gui_create_browser_window(struct browser_window *bw,
 		return 0;
 	}
 
+	g->container = GTK_WIDGET(gtk_builder_get_object(xml, "tabContents"));
 	g->layout = GTK_LAYOUT(gtk_builder_get_object(xml, "layout"));
 	g->status_bar = GTK_LABEL(gtk_builder_get_object(xml, "status_bar"));
 	g->paned = GTK_PANED(gtk_builder_get_object(xml, "hpaned1"));
 
-	/* add the tab to the scaffold */
-	bool tempback = true;
-	switch (temp_open_background) {
-	case -1:
-		tempback = !(nsoption_bool(focus_new));
-		break;
-	case 0:
-		tempback = false;
-		break;
-	case 1:
-		tempback = true;
-		break;
-	}
-
-	GtkWidget *tab_contents = GTK_WIDGET(gtk_builder_get_object(xml, "tabContents"));
-	g_object_set_data(G_OBJECT(tab_contents), "gui_window", g);
-	nsgtk_tab_add(g, tab_contents, tempback);
-
-	g_object_unref(xml);
 
-	/* Attach ourselves to the list (push_top) */
-	if (window_list)
+	/* add new gui window to global list (push_top) */
+	if (window_list) {
 		window_list->prev = g;
+	}
 	g->next = window_list;
 	g->prev = NULL;
 	window_list = g;
@@ -743,8 +738,8 @@ struct gui_window *gui_create_browser_window(struct browser_window *bw,
 
 	/* set the default background colour of the drawing area to white. */
 	nsgtk_widget_override_background_color(GTK_WIDGET(g->layout),
-					       GTK_STATE_NORMAL, 0, 0xffff, 0xffff, 0xffff);
-
+					       GTK_STATE_NORMAL,
+					       0, 0xffff, 0xffff, 0xffff);
 
 	nsgtk_connect_draw_event(GTK_WIDGET(g->layout),
 				 G_CALLBACK(nsgtk_window_draw_event), g);
@@ -771,6 +766,29 @@ struct gui_window *gui_create_browser_window(struct browser_window *bw,
 	CONNECT(g->paned, "notify::position",
 		nsgtk_paned_notify__position, g);
 
+	/* gtk container destructor */
+	CONNECT(g->container, "destroy",
+		window_destroy, g);
+
+	/* add the tab container to the scaffold notebook */
+	switch (temp_open_background) {
+	case -1:
+		tempback = !(nsoption_bool(focus_new));
+		break;
+	case 0:
+		tempback = false;
+		break;
+	default:
+		tempback = true;
+		break;
+	}
+	nsgtk_tab_add(g, g->container, tempback);
+
+	/* safe to drop the reference to the xml as the container is
+	 * referenced by the notebook now.
+	 */
+	g_object_unref(xml);
+
 	return g;
 }
 
@@ -811,31 +829,30 @@ void nsgtk_window_process_reformats(void)
 	}
 }
 
-
 void nsgtk_window_destroy_browser(struct gui_window *gw)
 {
-	browser_window_destroy(gw->bw);
+	/* remove tab */
+	gtk_widget_destroy(gw->container);
 }
 
 void gui_window_destroy(struct gui_window *g)
 {
-	if (g->prev)
+	LOG(("gui_window: %p", g));
+	assert(g != NULL);
+	assert(g->bw != NULL);
+	LOG(("scaffolding: %p", g->scaffold));
+
+	if (g->prev) {
 		g->prev->next = g->next;
-	else
+	} else {
 		window_list = g->next;
+	}
 
-	if (g->next)
+	if (g->next) {
 		g->next->prev = g->prev;
+	}
 
-
-	LOG(("Destroying gui_window %p", g));
-	assert(g != NULL);
-	assert(g->bw != NULL);
-	LOG(("	   Scaffolding: %p", g->scaffold));
-	LOG(("	   Window name: %s", g->bw->name));
-
-	/* tab => remove tab */
-	gtk_widget_destroy(gtk_widget_get_parent(GTK_WIDGET(g->layout)));
+	LOG(("window list head: %p", window_list));
 }
 
 /**