Try fix a disabled AutomatedUITestBase test.

Also remove release_active_browser() as it sets
active_browser_ being NULL and potentially
dangerous.

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

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

4 files changed, 30 insertions, 16 deletions

diff --git a/chrome/test/automated_ui_tests/automated_ui_test_base.cc b/chrome/test/automated_ui_tests/automated_ui_test_base.cc
index 9b3c631..3cded46 100644
--- a/chrome/test/automated_ui_tests/automated_ui_test_base.cc
+++ b/chrome/test/automated_ui_tests/automated_ui_test_base.cc
@@ -29,7 +29,8 @@ bool AutomatedUITestBase::DuplicateTab() {
   return RunCommand(IDC_DUPLICATE_TAB);
 }
 
-bool AutomatedUITestBase::OpenAndActivateNewBrowserWindow() {
+bool AutomatedUITestBase::OpenAndActivateNewBrowserWindow(
+    BrowserProxy** previous_browser) {
   if (!automation()->OpenNewBrowserWindow(SW_SHOWNORMAL)) {
     LogWarningMessage("failed_to_open_new_browser_window");
     return false;
@@ -50,7 +51,10 @@ bool AutomatedUITestBase::OpenAndActivateNewBrowserWindow() {
     LogWarningMessage("failed_to_activate_tab");
     return false;
   }
-  set_active_browser(browser.release());
+
+  active_browser_.swap(browser);
+  if (previous_browser)
+    *previous_browser = browser.release();
   return true;
 }
 
diff --git a/chrome/test/automated_ui_tests/automated_ui_test_base.h b/chrome/test/automated_ui_tests/automated_ui_test_base.h
index 3783def..163f388 100644
--- a/chrome/test/automated_ui_tests/automated_ui_test_base.h
+++ b/chrome/test/automated_ui_tests/automated_ui_test_base.h
@@ -33,7 +33,10 @@ class AutomatedUITestBase : public UITest {
   // Opens a new browser window by calling automation()->OpenNewBrowserWindow.
   // Then activates the tab opened in the new window.
   // Returns true if window is successfully created.
-  bool OpenAndActivateNewBrowserWindow();
+  // If optional parameter previous_browser is passed in, it is set to be the
+  // previous browser window when new window is successfully created, and the
+  // caller owns previous_browser.
+  bool OpenAndActivateNewBrowserWindow(BrowserProxy** previous_browser);
 
   // Runs the specified browser command in the current active browser.
   // See browser_commands.cc for the list of commands.
@@ -54,7 +57,6 @@ class AutomatedUITestBase : public UITest {
   void set_active_browser(BrowserProxy* browser) {
     active_browser_.reset(browser);
   }
-  BrowserProxy* release_active_browser() { return active_browser_.release(); }
   BrowserProxy* active_browser() const { return active_browser_.get(); }
 
  private:
diff --git a/chrome/test/automated_ui_tests/automated_ui_test_test.cc b/chrome/test/automated_ui_tests/automated_ui_test_test.cc
index c917212..8fe4043 100644
--- a/chrome/test/automated_ui_tests/automated_ui_test_test.cc
+++ b/chrome/test/automated_ui_tests/automated_ui_test_test.cc
@@ -30,41 +30,49 @@ TEST_F(AutomatedUITestBase, DuplicateTab) {
   ASSERT_EQ(3, tab_count);
 }
 
-// TODO(huanr) mmoss disabled this test because it's crashing periodically on
-// the buildbots and not giving useful debugging info.
-TEST_F(AutomatedUITestBase, DISABLED_OpenNewBrowserWindow) {
+TEST_F(AutomatedUITestBase, OpenCloseBrowserWindow) {
   int num_browser_windows;
   int tab_count;
   automation()->GetBrowserWindowCount(&num_browser_windows);
   ASSERT_EQ(1, num_browser_windows);
-  scoped_ptr<BrowserProxy>browser_1(release_active_browser());
-  browser_1->GetTabCount(&tab_count);
+  active_browser()->GetTabCount(&tab_count);
   ASSERT_EQ(1, tab_count);
 
-  ASSERT_TRUE(OpenAndActivateNewBrowserWindow());
+  BrowserProxy* previous_browser;
+  ASSERT_TRUE(OpenAndActivateNewBrowserWindow(&previous_browser));
+  scoped_ptr<BrowserProxy>browser_1(previous_browser);
   automation()->GetBrowserWindowCount(&num_browser_windows);
   ASSERT_EQ(2, num_browser_windows);
   active_browser()->GetTabCount(&tab_count);
   ASSERT_EQ(1, tab_count);
   NewTab();
-  scoped_ptr<BrowserProxy>browser_2(release_active_browser());
   browser_1->GetTabCount(&tab_count);
   ASSERT_EQ(1, tab_count);
-  browser_2->GetTabCount(&tab_count);
+  active_browser()->GetTabCount(&tab_count);
   ASSERT_EQ(2, tab_count);
 
-  ASSERT_TRUE(OpenAndActivateNewBrowserWindow());
+  ASSERT_TRUE(OpenAndActivateNewBrowserWindow(&previous_browser));
+  scoped_ptr<BrowserProxy>browser_2(previous_browser);
   automation()->GetBrowserWindowCount(&num_browser_windows);
   ASSERT_EQ(3, num_browser_windows);
   active_browser()->GetTabCount(&tab_count);
   ASSERT_EQ(1, tab_count);
   NewTab();
   NewTab();
-  scoped_ptr<BrowserProxy>browser_3(release_active_browser());
   browser_1->GetTabCount(&tab_count);
   ASSERT_EQ(1, tab_count);
   browser_2->GetTabCount(&tab_count);
   ASSERT_EQ(2, tab_count);
-  browser_3->GetTabCount(&tab_count);
+  active_browser()->GetTabCount(&tab_count);
   ASSERT_EQ(3, tab_count);
+
+  bool application_closed;
+  CloseBrowser(browser_1.get(), &application_closed);
+  ASSERT_FALSE(application_closed);
+  automation()->GetBrowserWindowCount(&num_browser_windows);
+  ASSERT_EQ(2, num_browser_windows);
+  CloseBrowser(browser_2.get(), &application_closed);
+  ASSERT_FALSE(application_closed);
+  automation()->GetBrowserWindowCount(&num_browser_windows);
+  ASSERT_EQ(1, num_browser_windows);
 }
diff --git a/chrome/test/automated_ui_tests/automated_ui_tests.cc b/chrome/test/automated_ui_tests/automated_ui_tests.cc
index e634a71..f566364 100644
--- a/chrome/test/automated_ui_tests/automated_ui_tests.cc
+++ b/chrome/test/automated_ui_tests/automated_ui_tests.cc
@@ -315,7 +315,7 @@ bool AutomatedUITest::DoAction(const std::string & action) {
   } else if (LowerCaseEqualsASCII(action, "newtab")) {
     did_complete_action = NewTab();
   } else if (LowerCaseEqualsASCII(action, "openwindow")) {
-    did_complete_action = OpenAndActivateNewBrowserWindow();
+    did_complete_action = OpenAndActivateNewBrowserWindow(NULL);
   } else if (LowerCaseEqualsASCII(action, "options")) {
     did_complete_action = Options();
   } else if (LowerCaseEqualsASCII(action, "pagedown")) {