Handle destruction of test shell properly.

When test shell's destructor runs, it tries to delete its child objects.
But when the GTK widgets are destroyed, they also try to delete the associated
objects.  We don't want both to run.

Also, there were a couple of incorrect uses of the GTK API (e.g. destroy-event
instead of destroy) that were obfuscating the problem.

This fixes a crash on shutdown, so I'm restoring the LoadURL() call in
TestShell's destructor.

Review URL: http://codereview.chromium.org/13214


git-svn-id: svn://svn.chromium.org/chrome/trunk/src@6475 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 33 insertions, 29 deletions

diff --git a/webkit/tools/test_shell/test_shell.cc b/webkit/tools/test_shell/test_shell.cc
index d132a7f..7679265 100644
--- a/webkit/tools/test_shell/test_shell.cc
+++ b/webkit/tools/test_shell/test_shell.cc
@@ -124,28 +124,32 @@ TestShell::TestShell()
 }
 
 TestShell::~TestShell() {
-    // Call GC twice to clean up garbage.
-    CallJSGC();
-    CallJSGC();
-
-    PlatformCleanUp();
-
-    StatsTable *table = StatsTable::current();
-    if (dump_stats_table_on_exit_) {
-      // Dump the stats table.
-      printf("<stats>\n");
-      if (table != NULL) {
-          int counter_max = table->GetMaxCounters();
-          for (int index=0; index < counter_max; index++) {
-              std::string name(table->GetRowName(index));
-              if (name.length() > 0) {
-                  int value = table->GetRowValue(index);
-                  printf("%s:\t%d\n", name.c_str(), value);
-              }
-          }
+  // Navigate to an empty page to fire all the destruction logic for the
+  // current page.
+  LoadURL(L"about:blank");
+
+  // Call GC twice to clean up garbage.
+  CallJSGC();
+  CallJSGC();
+
+  PlatformCleanUp();
+
+  StatsTable *table = StatsTable::current();
+  if (dump_stats_table_on_exit_) {
+    // Dump the stats table.
+    printf("<stats>\n");
+    if (table != NULL) {
+      int counter_max = table->GetMaxCounters();
+      for (int index=0; index < counter_max; index++) {
+        std::string name(table->GetRowName(index));
+        if (name.length() > 0) {
+          int value = table->GetRowValue(index);
+          printf("%s:\t%d\n", name.c_str(), value);
+        }
       }
-      printf("</stats>\n");
     }
+    printf("</stats>\n");
+  }
 }
 
 void TestShell::ShutdownTestShell() {
diff --git a/webkit/tools/test_shell/test_shell_gtk.cc b/webkit/tools/test_shell/test_shell_gtk.cc
index fa09346..35b4e69 100644
--- a/webkit/tools/test_shell/test_shell_gtk.cc
+++ b/webkit/tools/test_shell/test_shell_gtk.cc
@@ -135,13 +135,17 @@ bool TestShell::CreateNewWindow(const std::wstring& startingURL,
 }
 
 void TestShell::PlatformCleanUp() {
+  // The GTK widgets will be destroyed, which will free the associated
+  // objects.  So we don't need the scoped_ptr to free the webViewHost.
+  m_webViewHost.release();
 }
 
 // GTK callbacks ------------------------------------------------------
 namespace {
 
 // Callback for when the main window is destroyed.
-void MainWindowDestroyed(GtkWindow* window, TestShell* shell) {
+gboolean MainWindowDestroyed(GtkWindow* window, TestShell* shell) {
+
   TestShell::RemoveWindowFromList(GTK_WIDGET(window));
 
   if (TestShell::windowList()->empty() || shell->is_modal()) {
@@ -150,6 +154,8 @@ void MainWindowDestroyed(GtkWindow* window, TestShell* shell) {
   }
 
   delete shell;
+
+  return FALSE;  // Don't stop this message.
 }
 
 // Callback for when you click the back button.
diff --git a/webkit/tools/test_shell/webwidget_host_gtk.cc b/webkit/tools/test_shell/webwidget_host_gtk.cc
index bafc570..389fd61 100644
--- a/webkit/tools/test_shell/webwidget_host_gtk.cc
+++ b/webkit/tools/test_shell/webwidget_host_gtk.cc
@@ -49,8 +49,7 @@ gboolean ExposeEvent(GtkWidget* widget, GdkEventExpose* expose,
   return FALSE;
 }
 
-gboolean DestroyEvent(GtkWidget* widget, GdkEvent* event,
-                      WebWidgetHost* host) {
+gboolean WindowDestroyed(GtkWidget* widget, WebWidgetHost* host) {
   host->WindowDestroyed();
   return FALSE;
 }
@@ -133,7 +132,7 @@ gfx::WindowHandle WebWidgetHost::CreateWindow(gfx::WindowHandle box,
   GTK_WIDGET_SET_FLAGS(widget, GTK_CAN_FOCUS);
   g_signal_connect(widget, "configure-event", G_CALLBACK(ConfigureEvent), host);
   g_signal_connect(widget, "expose-event", G_CALLBACK(ExposeEvent), host);
-  g_signal_connect(widget, "destroy-event", G_CALLBACK(DestroyEvent), host);
+  g_signal_connect(widget, "destroy", G_CALLBACK(::WindowDestroyed), host);
   g_signal_connect(widget, "key-press-event", G_CALLBACK(KeyPressReleaseEvent),
                    host);
   g_signal_connect(widget, "key-release-event",
@@ -285,11 +284,6 @@ void WebWidgetHost::PaintRect(const gfx::Rect& rect) {
   set_painting(false);
 }
 
-// -----------------------------------------------------------------------------
-// This is called when the GTK window is destroyed. In the Windows code this
-// deletes this object. Since it's only test_shell it probably doesn't matter
-// that much.
-// -----------------------------------------------------------------------------
 void WebWidgetHost::WindowDestroyed() {
   delete this;
 }