Revert 58563 - GTTF: Make WaitForTabCountToBecome automation call not Sleep.

(unfortunately, Mac & Windows started failing tab perf tests on this change)

Sleeping is an unreliable method to wait for things.
Instead, we set up an observer.

BUG=none
TEST=none

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

TBR=phajdan.jr@chromium.org
Review URL: http://codereview.chromium.org/3344009

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

15 files changed, 37 insertions, 127 deletions

diff --git a/chrome/browser/automation/automation_provider_observers.cc b/chrome/browser/automation/automation_provider_observers.cc
index a30e801..0107708 100644
--- a/chrome/browser/automation/automation_provider_observers.cc
+++ b/chrome/browser/automation/automation_provider_observers.cc
@@ -340,49 +340,6 @@ void TabClosedNotificationObserver::set_for_browser_command(
   for_browser_command_ = for_browser_command;
 }
 
-TabCountChangeObserver::TabCountChangeObserver(AutomationProvider* automation,
-                                               Browser* browser,
-                                               IPC::Message* reply_message,
-                                               int target_tab_count)
-    : automation_(automation),
-      reply_message_(reply_message),
-      tab_strip_model_(browser->tabstrip_model()),
-      target_tab_count_(target_tab_count) {
-  tab_strip_model_->AddObserver(this);
-  CheckTabCount();
-}
-
-TabCountChangeObserver::~TabCountChangeObserver() {
-  tab_strip_model_->RemoveObserver(this);
-}
-
-void TabCountChangeObserver::TabInsertedAt(TabContents* contents,
-                                           int index,
-                                           bool foreground) {
-  CheckTabCount();
-}
-
-void TabCountChangeObserver::TabClosingAt(TabContents* contents, int index) {
-  CheckTabCount();
-}
-
-void TabCountChangeObserver::TabStripModelDeleted() {
-  AutomationMsg_WaitForTabCountToBecome::WriteReplyParams(reply_message_,
-                                                          false);
-  automation_->Send(reply_message_);
-  delete this;
-}
-
-void TabCountChangeObserver::CheckTabCount() {
-  if (tab_strip_model_->count() != target_tab_count_)
-    return;
-
-  AutomationMsg_WaitForTabCountToBecome::WriteReplyParams(reply_message_,
-                                                          true);
-  automation_->Send(reply_message_);
-  delete this;
-}
-
 bool DidExtensionHostsStopLoading(ExtensionProcessManager* manager) {
   for (ExtensionProcessManager::const_iterator iter = manager->begin();
        iter != manager->end(); ++iter) {
diff --git a/chrome/browser/automation/automation_provider_observers.h b/chrome/browser/automation/automation_provider_observers.h
index c9e3afd..e55fec8 100644
--- a/chrome/browser/automation/automation_provider_observers.h
+++ b/chrome/browser/automation/automation_provider_observers.h
@@ -19,7 +19,6 @@
 #include "chrome/browser/importer/importer_data_types.h"
 #include "chrome/browser/password_manager/password_store.h"
 #include "chrome/browser/tab_contents/tab_contents.h"
-#include "chrome/browser/tabs/tab_strip_model.h"
 #include "chrome/common/notification_observer.h"
 #include "chrome/common/notification_registrar.h"
 #include "chrome/common/notification_type.h"
@@ -192,37 +191,6 @@ class TabClosedNotificationObserver : public TabStripNotificationObserver {
   DISALLOW_COPY_AND_ASSIGN(TabClosedNotificationObserver);
 };
 
-// Notifies when the tab count reaches the target number.
-class TabCountChangeObserver : public TabStripModelObserver {
- public:
-  TabCountChangeObserver(AutomationProvider* automation,
-                         Browser* browser,
-                         IPC::Message* reply_message,
-                         int target_tab_count);
-  // Implementation of TabStripModelObserver.
-  virtual void TabInsertedAt(TabContents* contents,
-                             int index,
-                             bool foreground);
-  virtual void TabClosingAt(TabContents* contents, int index);
-  virtual void TabStripModelDeleted();
-
- private:
-  ~TabCountChangeObserver();
-
-  // Checks if the current tab count matches our target, and if so,
-  // sends the reply message and deletes self.
-  void CheckTabCount();
-
-  AutomationProvider* automation_;
-  IPC::Message* reply_message_;
-
-  TabStripModel* tab_strip_model_;
-
-  const int target_tab_count_;
-
-  DISALLOW_COPY_AND_ASSIGN(TabCountChangeObserver);
-};
-
 // Observes when an extension has finished installing or possible install
 // errors. This does not guarantee that the extension is ready for use.
 class ExtensionInstallNotificationObserver : public NotificationObserver {
diff --git a/chrome/browser/automation/testing_automation_provider.cc b/chrome/browser/automation/testing_automation_provider.cc
index 1a0afca..62e98a9 100644
--- a/chrome/browser/automation/testing_automation_provider.cc
+++ b/chrome/browser/automation/testing_automation_provider.cc
@@ -403,8 +403,6 @@ void TestingAutomationProvider::OnMessageReceived(
     IPC_MESSAGE_HANDLER(AutomationMsg_BlockedPopupCount, GetBlockedPopupCount)
     IPC_MESSAGE_HANDLER_DELAY_REPLY(AutomationMsg_SendJSONRequest,
                                     SendJSONRequest)
-    IPC_MESSAGE_HANDLER_DELAY_REPLY(AutomationMsg_WaitForTabCountToBecome,
-                                    WaitForTabCountToBecome)
 
     IPC_MESSAGE_UNHANDLED(AutomationProvider::OnMessageReceived(message));
   IPC_END_MESSAGE_MAP()
@@ -3691,23 +3689,6 @@ std::map<AutoFillFieldType, std::wstring>
   return credit_card_type_to_string;
 }
 
-void TestingAutomationProvider::WaitForTabCountToBecome(
-    int browser_handle,
-    int target_tab_count,
-    IPC::Message* reply_message) {
-  if (!browser_tracker_->ContainsHandle(browser_handle)) {
-    AutomationMsg_WaitForTabCountToBecome::WriteReplyParams(reply_message,
-                                                            false);
-    Send(reply_message);
-    return;
-  }
-
-  Browser* browser = browser_tracker_->GetResource(browser_handle);
-
-  // The observer will delete itself.
-  new TabCountChangeObserver(this, browser, reply_message, target_tab_count);
-}
-
 // TODO(brettw) change this to accept GURLs when history supports it
 void TestingAutomationProvider::OnRedirectQueryComplete(
     HistoryService::Handle request_handle,
diff --git a/chrome/browser/automation/testing_automation_provider.h b/chrome/browser/automation/testing_automation_provider.h
index 565bd8c..10fe387 100644
--- a/chrome/browser/automation/testing_automation_provider.h
+++ b/chrome/browser/automation/testing_automation_provider.h
@@ -596,10 +596,6 @@ class TestingAutomationProvider : public AutomationProvider,
   static std::map<AutoFillFieldType, std::wstring>
       GetCreditCardFieldToStringMap();
 
-  void WaitForTabCountToBecome(int browser_handle,
-                               int target_tab_count,
-                               IPC::Message* reply_message);
-
   // Callback for history redirect queries.
   virtual void OnRedirectQueryComplete(
       HistoryService::Handle request_handle,
diff --git a/chrome/browser/tab_restore_uitest.cc b/chrome/browser/tab_restore_uitest.cc
index 4e48d5a..fcd73fb 100644
--- a/chrome/browser/tab_restore_uitest.cc
+++ b/chrome/browser/tab_restore_uitest.cc
@@ -568,12 +568,14 @@ TEST_F(TabRestoreUITest, MAYBE_RestoreWindow) {
   int initial_tab_count;
   ASSERT_TRUE(browser_proxy->GetTabCount(&initial_tab_count));
   ASSERT_TRUE(browser_proxy->AppendTab(url1_));
-  ASSERT_TRUE(browser_proxy->WaitForTabCountToBecome(initial_tab_count + 1));
+  ASSERT_TRUE(browser_proxy->WaitForTabCountToBecome(initial_tab_count + 1,
+                                                     action_max_timeout_ms()));
   scoped_refptr<TabProxy> new_tab(browser_proxy->GetTab(initial_tab_count));
   ASSERT_TRUE(new_tab.get());
   ASSERT_EQ(AUTOMATION_MSG_NAVIGATION_SUCCESS, new_tab->NavigateToURL(url1_));
   ASSERT_TRUE(browser_proxy->AppendTab(url2_));
-  ASSERT_TRUE(browser_proxy->WaitForTabCountToBecome(initial_tab_count + 2));
+  ASSERT_TRUE(browser_proxy->WaitForTabCountToBecome(initial_tab_count + 2,
+                                                     action_max_timeout_ms()));
   new_tab = browser_proxy->GetTab(initial_tab_count + 1);
   ASSERT_TRUE(new_tab.get());
   ASSERT_EQ(AUTOMATION_MSG_NAVIGATION_SUCCESS, new_tab->NavigateToURL(url2_));
diff --git a/chrome/browser/unload_uitest.cc b/chrome/browser/unload_uitest.cc
index 63886cf..7eb6f4f 100644
--- a/chrome/browser/unload_uitest.cc
+++ b/chrome/browser/unload_uitest.cc
@@ -433,7 +433,7 @@ TEST_F(UnloadTest, MAYBE_BrowserCloseTabWhenOtherTabHasListener) {
   // popup will be constrained, which isn't what we want to test.
   ASSERT_TRUE(window->SimulateOSClick(tab_view_bounds.CenterPoint(),
                                       views::Event::EF_LEFT_BUTTON_DOWN));
-  ASSERT_TRUE(browser->WaitForTabCountToBecome(2));
+  ASSERT_TRUE(browser->WaitForTabCountToBecome(2, action_timeout_ms()));
 
   scoped_refptr<TabProxy> popup_tab(browser->GetActiveTab());
   ASSERT_TRUE(popup_tab.get());
@@ -442,7 +442,7 @@ TEST_F(UnloadTest, MAYBE_BrowserCloseTabWhenOtherTabHasListener) {
   EXPECT_EQ(std::wstring(L"popup"), popup_title);
   EXPECT_TRUE(popup_tab->Close(true));
 
-  ASSERT_TRUE(browser->WaitForTabCountToBecome(1));
+  ASSERT_TRUE(browser->WaitForTabCountToBecome(1, action_timeout_ms()));
   scoped_refptr<TabProxy> main_tab(browser->GetActiveTab());
   ASSERT_TRUE(main_tab.get());
   std::wstring main_title;
diff --git a/chrome/browser/views/tabs/tab_dragging_test.cc b/chrome/browser/views/tabs/tab_dragging_test.cc
index a9238d6..8bc37e3 100644
--- a/chrome/browser/views/tabs/tab_dragging_test.cc
+++ b/chrome/browser/views/tabs/tab_dragging_test.cc
@@ -83,7 +83,8 @@ TEST_F(TabDraggingTest, MAYBE_Tab1Tab2) {
   ASSERT_TRUE(tab3.get());
 
   // Make sure 3 tabs are open.
-  ASSERT_TRUE(browser->WaitForTabCountToBecome(initial_tab_count + 2));
+  ASSERT_TRUE(browser->WaitForTabCountToBecome(initial_tab_count + 2,
+                                               10000));
 
   // Get bounds for the tabs.
   gfx::Rect bounds1;
@@ -181,7 +182,8 @@ TEST_F(TabDraggingTest, MAYBE_Tab1Tab3) {
   ASSERT_TRUE(tab3.get());
 
   // Make sure 3 tabs are open.
-  ASSERT_TRUE(browser->WaitForTabCountToBecome(initial_tab_count + 2));
+  ASSERT_TRUE(browser->WaitForTabCountToBecome(initial_tab_count + 2,
+                                               10000));
 
   // Get bounds for the tabs.
   gfx::Rect bounds1;
@@ -288,7 +290,8 @@ TEST_F(TabDraggingTest, MAYBE_Tab1Tab3Escape) {
   ASSERT_TRUE(tab3.get());
 
   // Make sure 3 tabs are open.
-  ASSERT_TRUE(browser->WaitForTabCountToBecome(initial_tab_count + 2));
+  ASSERT_TRUE(browser->WaitForTabCountToBecome(initial_tab_count + 2,
+                                               10000));
 
   // Get bounds for the tabs.
   gfx::Rect bounds1;
@@ -398,7 +401,8 @@ TEST_F(TabDraggingTest, MAYBE_Tab2OutOfTabStrip) {
   ASSERT_TRUE(tab3.get());
 
   // Make sure 3 tabs are opened.
-  ASSERT_TRUE(browser->WaitForTabCountToBecome(initial_tab_count + 2));
+  ASSERT_TRUE(browser->WaitForTabCountToBecome(initial_tab_count + 2,
+                                               10000));
 
   // Make sure all the tab URL specs are different.
   ASSERT_TRUE(tab1_url != tab2_url);
diff --git a/chrome/common/logging_chrome_uitest.cc b/chrome/common/logging_chrome_uitest.cc
index 072b8cc..328a5a49 100644
--- a/chrome/common/logging_chrome_uitest.cc
+++ b/chrome/common/logging_chrome_uitest.cc
@@ -183,7 +183,7 @@ TEST_F(RendererCrashTest, Crash) {
   } else {
     scoped_refptr<BrowserProxy> browser(automation()->GetBrowserWindow(0));
     ASSERT_TRUE(browser.get());
-    ASSERT_TRUE(browser->WaitForTabCountToBecome(1));
+    ASSERT_TRUE(browser->WaitForTabCountToBecome(1, action_max_timeout_ms()));
     expected_crashes_ = EXPECTED_CRASH_CRASHES;
   }
 }
diff --git a/chrome/test/automated_ui_tests/automated_ui_test_interactive_test.cc b/chrome/test/automated_ui_tests/automated_ui_test_interactive_test.cc
index c4c1eec..b3952b2 100644
--- a/chrome/test/automated_ui_tests/automated_ui_test_interactive_test.cc
+++ b/chrome/test/automated_ui_tests/automated_ui_test_interactive_test.cc
@@ -10,7 +10,8 @@
 TEST_F(AutomatedUITestBase, DragOut) {
   NewTab();
   NewTab();
-  ASSERT_TRUE(active_browser()->WaitForTabCountToBecome(3));
+  ASSERT_TRUE(active_browser()->
+      WaitForTabCountToBecome(3, action_max_timeout_ms()));
   PlatformThread::Sleep(sleep_timeout_ms());
   ASSERT_TRUE(DragTabOut());
   int window_count;
@@ -21,7 +22,8 @@ TEST_F(AutomatedUITestBase, DragOut) {
 TEST_F(AutomatedUITestBase, DragLeftRight) {
   NewTab();
   NewTab();
-  ASSERT_TRUE(active_browser()->WaitForTabCountToBecome(3));
+  ASSERT_TRUE(active_browser()->
+      WaitForTabCountToBecome(3, action_max_timeout_ms()));
   // TODO(phajdan.jr): We need a WaitForTabstripAnimationsToEnd() function.
   // Every sleep in this file should be replaced with it.
   PlatformThread::Sleep(sleep_timeout_ms());
diff --git a/chrome/test/automation/automation_messages_internal.h b/chrome/test/automation/automation_messages_internal.h
index 196fe0f..0ef68ad 100644
--- a/chrome/test/automation/automation_messages_internal.h
+++ b/chrome/test/automation/automation_messages_internal.h
@@ -1428,10 +1428,4 @@ IPC_BEGIN_MESSAGES(Automation)
   //   None expected
   IPC_MESSAGE_ROUTED2(AutomationMsg_SetZoomLevel, int, int)
 
-  // Waits for tab count to reach target value.
-  IPC_SYNC_MESSAGE_ROUTED2_1(AutomationMsg_WaitForTabCountToBecome,
-                             int /* browser handle */,
-                             int /* target tab count */,
-                             bool /* success */)
-
 IPC_END_MESSAGES(Automation)
diff --git a/chrome/test/automation/browser_proxy.cc b/chrome/test/automation/browser_proxy.cc
index b5a89eb..df3929a 100644
--- a/chrome/test/automation/browser_proxy.cc
+++ b/chrome/test/automation/browser_proxy.cc
@@ -192,14 +192,19 @@ bool BrowserProxy::SimulateDrag(const gfx::Point& start,
   return result;
 }
 
-bool BrowserProxy::WaitForTabCountToBecome(int count) {
-  bool success = false;
-  if (!sender_->Send(new AutomationMsg_WaitForTabCountToBecome(
-                         0, handle_, count, &success))) {
-    return false;
+bool BrowserProxy::WaitForTabCountToBecome(int count, int wait_timeout) {
+  const TimeTicks start = TimeTicks::Now();
+  const TimeDelta timeout = TimeDelta::FromMilliseconds(wait_timeout);
+  while (TimeTicks::Now() - start < timeout) {
+    PlatformThread::Sleep(automation::kSleepTime);
+    int new_count;
+    if (!GetTabCount(&new_count))
+      return false;
+    if (count == new_count)
+      return true;
   }
-
-  return success;
+  // If we get here, the tab count doesn't match.
+  return false;
 }
 
 bool BrowserProxy::WaitForTabToBecomeActive(int tab,
diff --git a/chrome/test/automation/browser_proxy.h b/chrome/test/automation/browser_proxy.h
index 3fb06d4..7dd2671 100644
--- a/chrome/test/automation/browser_proxy.h
+++ b/chrome/test/automation/browser_proxy.h
@@ -115,8 +115,9 @@ class BrowserProxy : public AutomationResourceProxy {
                             bool press_escape_en_route) WARN_UNUSED_RESULT;
 
   // Block the thread until the tab count is |count|.
+  // |wait_timeout| is the timeout, in milliseconds, for waiting.
   // Returns true on success.
-  bool WaitForTabCountToBecome(int count) WARN_UNUSED_RESULT;
+  bool WaitForTabCountToBecome(int count, int wait_timeout) WARN_UNUSED_RESULT;
 
   // Block the thread until the specified tab is the active tab.
   // |wait_timeout| is the timeout, in milliseconds, for waiting.
diff --git a/chrome/test/interactive_ui/keyboard_access_uitest.cc b/chrome/test/interactive_ui/keyboard_access_uitest.cc
index a8ee85d..d002a3e 100644
--- a/chrome/test/interactive_ui/keyboard_access_uitest.cc
+++ b/chrome/test/interactive_ui/keyboard_access_uitest.cc
@@ -87,7 +87,7 @@ void KeyboardAccessTest::TestMenuKeyboardAccess(bool alternate_key_sequence) {
   ASSERT_TRUE(window->SimulateOSKeyPress(app::VKEY_RETURN, 0));
 
   // Wait for the new tab to appear.
-  ASSERT_TRUE(browser->WaitForTabCountToBecome(2));
+  ASSERT_TRUE(browser->WaitForTabCountToBecome(2, action_timeout_ms()));
 
   // Make sure that the new tab index is 1.
   ASSERT_TRUE(browser->GetActiveTabIndex(&tab_index));
@@ -132,7 +132,7 @@ TEST_F(KeyboardAccessTest, FAILS_ReserveKeyboardAccelerators) {
   ASSERT_TRUE(browser->ActivateTab(1));
   ASSERT_TRUE(window->SimulateOSKeyPress(
       app::VKEY_F4, views::Event::EF_CONTROL_DOWN));
-  ASSERT_TRUE(browser->WaitForTabCountToBecome(1));
+  ASSERT_TRUE(browser->WaitForTabCountToBecome(1, action_max_timeout_ms()));
 #endif
 }
 
diff --git a/chrome/test/tab_switching/tab_switching_test.cc b/chrome/test/tab_switching/tab_switching_test.cc
index 8544167..ba72e7e 100644
--- a/chrome/test/tab_switching/tab_switching_test.cc
+++ b/chrome/test/tab_switching/tab_switching_test.cc
@@ -87,7 +87,7 @@ class TabSwitchingUITest : public UIPerfTest {
       ASSERT_TRUE(browser_proxy_->GetTabCount(&initial_tab_count));
       int new_tab_count = OpenTabs();
       ASSERT_TRUE(browser_proxy_->WaitForTabCountToBecome(
-          initial_tab_count + new_tab_count));
+          initial_tab_count + new_tab_count, 10000));
 
       // Switch linearly between tabs.
       ASSERT_TRUE(browser_proxy_->ActivateTab(0));
diff --git a/chrome/test/ui/fast_shutdown_uitest.cc b/chrome/test/ui/fast_shutdown_uitest.cc
index f3215d4..758ca156f 100644
--- a/chrome/test/ui/fast_shutdown_uitest.cc
+++ b/chrome/test/ui/fast_shutdown_uitest.cc
@@ -47,7 +47,7 @@ TEST_F(FastShutdown, MAYBE_SlowTermination) {
   // This click will launch a popup which has a before unload handler.
   ASSERT_TRUE(window->SimulateOSClick(bounds.CenterPoint(),
                                       views::Event::EF_LEFT_BUTTON_DOWN));
-  ASSERT_TRUE(browser->WaitForTabCountToBecome(2));
+  ASSERT_TRUE(browser->WaitForTabCountToBecome(2, action_max_timeout_ms()));
   // Close the tab, removing the one and only before unload handler.
   scoped_refptr<TabProxy> tab(browser->GetTab(1));
   ASSERT_TRUE(tab->Close(true));