Get rid of WaitForWindowCountToChange.

The function had confusing semantics, was marked to be removed,
and could induce flakiness in tests which used it.

WaitForWindowCountToBecome is much better replacement for it.

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

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

4 files changed, 7 insertions, 49 deletions

diff --git a/chrome/browser/sessions/session_restore_uitest.cc b/chrome/browser/sessions/session_restore_uitest.cc
index cd00a93..431b66e 100644
--- a/chrome/browser/sessions/session_restore_uitest.cc
+++ b/chrome/browser/sessions/session_restore_uitest.cc
@@ -50,11 +50,8 @@ class SessionRestoreUITest : public UITest {
     ASSERT_TRUE(browser_proxy.get());
     ASSERT_TRUE(browser_proxy->ApplyAccelerator(IDC_CLOSE_WINDOW));
     browser_proxy.reset();
-    int window_count;
-    ASSERT_TRUE(automation()->WaitForWindowCountToChange(initial_count,
-                                                         &window_count,
+    ASSERT_TRUE(automation()->WaitForWindowCountToBecome(initial_count - 1,
                                                          action_timeout_ms()));
-    ASSERT_EQ(initial_count - 1, window_count);
   }
 
   void AssertOneWindowWithOneTab() {
@@ -273,10 +270,7 @@ TEST_F(SessionRestoreUITest, DISABLED_DontRestoreWhileIncognito) {
 
   // Create an off the record window and wait for it to appear.
   ASSERT_TRUE(browser_proxy->ApplyAccelerator(IDC_NEW_INCOGNITO_WINDOW));
-  int window_count;
-  ASSERT_TRUE(automation()->WaitForWindowCountToChange(1, &window_count,
-                                                       action_timeout_ms()) &&
-              window_count == 2);
+  ASSERT_TRUE(automation()->WaitForWindowCountToBecome(2, action_timeout_ms()));
 
   // Close the first window.
   CloseWindow(0, 2);
@@ -291,9 +285,7 @@ TEST_F(SessionRestoreUITest, DISABLED_DontRestoreWhileIncognito) {
   LaunchBrowser(launch_arguments_, false);
 
   // A new window should appear;
-  ASSERT_TRUE(automation()->WaitForWindowCountToChange(1, &window_count,
-                                                       action_timeout_ms()) &&
-              window_count == 2);
+  ASSERT_TRUE(automation()->WaitForWindowCountToBecome(2, action_timeout_ms()));
 
   // And it shouldn't have url1 in it.
   browser_proxy.reset(automation()->GetBrowserWindow(1));
@@ -320,10 +312,7 @@ TEST_F(SessionRestoreUITest, DISABLED_TwoWindowsCloseOneRestoreOnlyOne) {
 
   // Open a second window.
   ASSERT_TRUE(automation()->OpenNewBrowserWindow(SW_SHOWNORMAL));
-  int window_count;
-  ASSERT_TRUE(automation()->WaitForWindowCountToChange(1, &window_count,
-                                                       action_timeout_ms()) &&
-              window_count == 2);
+  ASSERT_TRUE(automation()->WaitForWindowCountToBecome(2, action_timeout_ms()));
 
   // Close it.
   CloseWindow(1, 2);
@@ -353,10 +342,7 @@ TEST_F(SessionRestoreUITest,
   app_launch_arguments.AppendSwitchWithValue(switches::kApp,
                                              UTF8ToWide(url2.spec()));
   LaunchBrowser(app_launch_arguments, false);
-  int window_count;
-  ASSERT_TRUE(automation()->WaitForWindowCountToChange(1, &window_count,
-                                                       action_timeout_ms()));
-  ASSERT_EQ(2, window_count);
+  ASSERT_TRUE(automation()->WaitForWindowCountToBecome(2, action_timeout_ms()));
 
   // Close the first window.
   CloseWindow(0, 2);
@@ -365,9 +351,7 @@ TEST_F(SessionRestoreUITest,
   CommandLine restore_launch_arguments = launch_arguments_;
   restore_launch_arguments.AppendSwitch(switches::kRestoreLastSession);
   LaunchBrowser(restore_launch_arguments, false);
-  ASSERT_TRUE(automation()->WaitForWindowCountToChange(1, &window_count,
-                                                       action_timeout_ms()));
-  ASSERT_EQ(2, window_count);
+  ASSERT_TRUE(automation()->WaitForWindowCountToBecome(2, action_timeout_ms()));
   GURL url;
   AssertWindowHasOneTab(1, &url);
   ASSERT_EQ(url1, url);
diff --git a/chrome/test/automated_ui_tests/automated_ui_tests.cc b/chrome/test/automated_ui_tests/automated_ui_tests.cc
index 9fcf67f..65e03c2 100644
--- a/chrome/test/automated_ui_tests/automated_ui_tests.cc
+++ b/chrome/test/automated_ui_tests/automated_ui_tests.cc
@@ -465,11 +465,9 @@ bool AutomatedUITest::CloseActiveTab() {
       return false;
     }
   } else if (tab_count == 1 && browser_windows_count > 1) {
-    int new_window_count;
     return_value = browser->RunCommand(IDC_CLOSE_TAB);
     // Wait for the window to close before we continue.
-    if (!automation()->WaitForWindowCountToChange(browser_windows_count,
-                                                  &new_window_count,
+    if (!automation()->WaitForWindowCountToBecome(browser_windows_count - 1,
                                                   action_max_timeout_ms())) {
       AddWarningAttribute("window_count_failed_to_change");
       return false;
diff --git a/chrome/test/automation/automation_proxy.cc b/chrome/test/automation/automation_proxy.cc
index 35d44f8..3a8f406 100644
--- a/chrome/test/automation/automation_proxy.cc
+++ b/chrome/test/automation/automation_proxy.cc
@@ -251,20 +251,6 @@ bool AutomationProxy::GetBrowserWindowCount(int* num_windows) {
   return succeeded;
 }
 
-bool AutomationProxy::WaitForWindowCountToChange(int count, int* new_count,
-                                                 int wait_timeout) {
-  const TimeTicks start = TimeTicks::Now();
-  const TimeDelta timeout = TimeDelta::FromMilliseconds(wait_timeout);
-  while (TimeTicks::Now() - start < timeout) {
-    bool succeeded = GetBrowserWindowCount(new_count);
-    if (!succeeded) return false;
-    if (count != *new_count) return true;
-    PlatformThread::Sleep(automation::kSleepTime);
-  }
-  // Window count never changed.
-  return false;
-}
-
 bool AutomationProxy::WaitForWindowCountToBecome(int count,
                                                  int wait_timeout) {
   const TimeTicks start = TimeTicks::Now();
diff --git a/chrome/test/automation/automation_proxy.h b/chrome/test/automation/automation_proxy.h
index e80259f..138edd0 100644
--- a/chrome/test/automation/automation_proxy.h
+++ b/chrome/test/automation/automation_proxy.h
@@ -89,16 +89,6 @@ class AutomationProxy : public IPC::Channel::Listener,
   // true on success. False likely indicates an IPC error.
   bool GetBrowserWindowCount(int* num_windows);
 
-  // Block the thread until the window count changes.
-  // First parameter is the original window count.
-  // The second parameter is updated with the number of window tabs.
-  // The third parameter specifies the timeout length for the wait loop.
-  // Returns false if the window count does not change before time out.
-  // TODO(evanm): this function has a confusing name and semantics; it should
-  // be deprecated for WaitForWindowCountToBecome.
-  bool WaitForWindowCountToChange(int count, int* new_counter,
-                                  int wait_timeout);
-
   // Block the thread until the window count becomes the provided value.
   // Returns true on success.
   bool WaitForWindowCountToBecome(int target_count, int wait_timeout);