looking at test shell tests on mac:

- unforking TestShell::CreateNewWindow, all code must live in a platform's
  Initialize method instead (some tests rely on knowing what CreateNewWindow
  does). (mac had issues w/ this)
- Mac: make the code path for tests that forces down windows properly clean up
  the bits in the same way a trip through the event loop would.
- add a few more logs/checks in the mac code to catch issues.
Review URL: http://codereview.chromium.org/18166

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

4 files changed, 36 insertions, 41 deletions

diff --git a/webkit/tools/test_shell/test_shell.cc b/webkit/tools/test_shell/test_shell.cc
index 41d2140..3247022 100755
--- a/webkit/tools/test_shell/test_shell.cc
+++ b/webkit/tools/test_shell/test_shell.cc
@@ -168,6 +168,18 @@ TestShell::~TestShell() {
   }
 }
 
+bool TestShell::CreateNewWindow(const std::wstring& startingURL,
+                                TestShell** result) {
+  TestShell* shell = new TestShell();
+  bool rv = shell->Initialize(startingURL);
+  if (rv) {
+    if (result)
+      *result = shell;
+    TestShell::windowList()->push_back(shell->m_mainWnd);
+  }
+  return rv;
+}
+
 void TestShell::ShutdownTestShell() {
 #if defined(OS_WIN) || defined(OS_MACOSX)
     PlatformShutdown();
diff --git a/webkit/tools/test_shell/test_shell_gtk.cc b/webkit/tools/test_shell/test_shell_gtk.cc
index 72f2f82..972256e 100644
--- a/webkit/tools/test_shell/test_shell_gtk.cc
+++ b/webkit/tools/test_shell/test_shell_gtk.cc
@@ -139,18 +139,6 @@ void TestShell::InitializeTestShell(bool layout_test_mode) {
     LOG(FATAL) << "Failed to set the default font configuration";
 }
 
-// static
-bool TestShell::CreateNewWindow(const std::wstring& startingURL,
-                                TestShell** result) {
-  TestShell *shell = new TestShell();
-  if (!shell->Initialize(startingURL))
-    return false;
-  if (result)
-    *result = shell;
-  TestShell::windowList()->push_back(shell->m_mainWnd);
-  return true;
-}
-
 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.
diff --git a/webkit/tools/test_shell/test_shell_mac.mm b/webkit/tools/test_shell/test_shell_mac.mm
index 47fd8a3..554f3c2 100644
--- a/webkit/tools/test_shell/test_shell_mac.mm
+++ b/webkit/tools/test_shell/test_shell_mac.mm
@@ -66,8 +66,7 @@ base::LazyInstance <std::map<gfx::NativeWindow, TestShell *> >
 
 // Receives notification that the window is closing so that it can start the
 // tear-down process. Is responsible for deleting itself when done.
-@interface WindowCloseDelegate : NSObject {
-}
+@interface WindowCloseDelegate : NSObject
 @end
 
 @implementation WindowCloseDelegate
@@ -111,8 +110,11 @@ void TestShell::PlatformCleanUp() {
 // static
 void TestShell::DestroyAssociatedShell(gfx::NativeWindow handle) {
   TestShell* shell = window_map_.Get()[handle];
-  if (shell)
+  if (shell) {
     window_map_.Get().erase(handle);
+  } else {
+    LOG(ERROR) << "Failed to find shell for window during destroy";
+  }
   delete shell;
 }
 
@@ -431,6 +433,9 @@ bool TestShell::Initialize(const std::wstring& startingURL) {
                                 defer:NO];
   [m_mainWnd setTitle:@"TestShell"];
   
+  // Add to our map
+  window_map_.Get()[m_mainWnd] = this;
+  
   // Create a window delegate to watch for when it's asked to go away. It will
   // clean itself up so we don't need to hold a reference.
   [m_mainWnd setDelegate:[[WindowCloseDelegate alloc] init]];
@@ -615,24 +620,23 @@ void TestShell::InteractiveSetFocus(WebWidgetHost* host, bool enable) {
 #endif
 }
 
-// static*
-bool TestShell::CreateNewWindow(const std::wstring& startingURL,
-                                TestShell** result) {
-  TestShell* shell = new TestShell();
-  bool rv = shell->Initialize(startingURL);
-  if (rv) {
-    if (result)
-      *result = shell;
-    TestShell::windowList()->push_back(shell->m_mainWnd);
-    window_map_.Get()[shell->m_mainWnd] = shell;
-  }
-  return rv;
-}
-
 // static
 void TestShell::DestroyWindow(gfx::NativeWindow windowHandle) {
+  // This code is like -cleanup: on our window delegate.  This call needs to be
+  // able to force down a window for tests, so it closes down the window making
+  // sure it cleans up the window delegate and the test shells list of windows
+  // and map of windows to shells.
+
   TestShell::RemoveWindowFromList(windowHandle);
+  TestShell::DestroyAssociatedShell(windowHandle);
+
+  id windowDelegate = [windowHandle delegate];
+  DCHECK(windowDelegate);
+  [windowHandle setDelegate:nil];
+  [windowDelegate release];
+
   [windowHandle close];
+  [windowHandle autorelease];
 }
 
 WebWidget* TestShell::CreatePopupWidget(WebView* webview) {
@@ -670,6 +674,8 @@ void TestShell::ResizeSubViews() {
     TestShell* shell = window_map_.Get()[window];
     if (shell)
       webkit_glue::DumpBackForwardList(shell->webView(), NULL, result);
+    else
+      LOG(ERROR) << "Failed to find shell for window during dump";
   }
 }
 
@@ -683,6 +689,7 @@ void TestShell::ResizeSubViews() {
 
   NSWindow* window = *(TestShell::windowList()->begin());
   TestShell* shell = window_map_.Get()[window];
+  DCHECK(shell);
   shell->ResetTestController();
 
   // ResetTestController may have closed the window we were holding on to. 
diff --git a/webkit/tools/test_shell/test_shell_win.cc b/webkit/tools/test_shell/test_shell_win.cc
index 1dec4db..dd6682a 100644
--- a/webkit/tools/test_shell/test_shell_win.cc
+++ b/webkit/tools/test_shell/test_shell_win.cc
@@ -150,18 +150,6 @@ void TestShell::InitializeTestShell(bool layout_test_mode) {
   }
 }
 
-bool TestShell::CreateNewWindow(const std::wstring& startingURL,
-                                TestShell** result) {
-  TestShell* shell = new TestShell();
-  bool rv = shell->Initialize(startingURL);
-  if (rv) {
-    if (result)
-      *result = shell;
-    TestShell::windowList()->push_back(shell->m_mainWnd);
-  }
-  return rv;
-}
-
 void TestShell::DestroyWindow(gfx::NativeWindow windowHandle) {
   // Do we want to tear down some of the machinery behind the scenes too?
   RemoveWindowFromList(windowHandle);