GTTF: Make WaitForTabCountToBecome automation call not Sleep.

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

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

15 files changed, 127 insertions, 37 deletions

diff --git a/chrome/browser/automation/automation_provider_observers.cc b/chrome/browser/automation/automation_provider_observers.cc
index 0107708..a30e801 100644
--- a/chrome/browser/automation/automation_provider_observers.cc
+++ b/chrome/browser/automation/automation_provider_observers.cc
@@ -340,6 +340,49 @@ 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 e55fec8..c9e3afd 100644
--- a/chrome/browser/automation/automation_provider_observers.h
+++ b/chrome/browser/automation/automation_provider_observers.h
@@ -19,6 +19,7 @@
 #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"
@@ -191,6 +192,37 @@ 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 62e98a9..1a0afca 100644
--- a/chrome/browser/automation/testing_automation_provider.cc
+++ b/chrome/browser/automation/testing_automation_provider.cc
@@ -403,6 +403,8 @@ 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()
@@ -3689,6 +3691,23 @@ 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 10fe387..565bd8c 100644
--- a/chrome/browser/automation/testing_automation_provider.h
+++ b/chrome/browser/automation/testing_automation_provider.h
@@ -596,6 +596,10 @@ 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 fcd73fb..4e48d5a 100644
--- a/chrome/browser/tab_restore_uitest.cc
+++ b/chrome/browser/tab_restore_uitest.cc
@@ -568,14 +568,12 @@ 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,
-                                                     action_max_timeout_ms()));
+  ASSERT_TRUE(browser_proxy->WaitForTabCountToBecome(initial_tab_count + 1));
   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,
-                                                     action_max_timeout_ms()));
+  ASSERT_TRUE(browser_proxy->WaitForTabCountToBecome(initial_tab_count + 2));
   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 7eb6f4f..63886cf 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, action_timeout_ms()));
+  ASSERT_TRUE(browser->WaitForTabCountToBecome(2));
 
   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, action_timeout_ms()));
+  ASSERT_TRUE(browser->WaitForTabCountToBecome(1));
   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 8bc37e3..a9238d6 100644
--- a/chrome/browser/views/tabs/tab_dragging_test.cc
+++ b/chrome/browser/views/tabs/tab_dragging_test.cc
@@ -83,8 +83,7 @@ TEST_F(TabDraggingTest, MAYBE_Tab1Tab2) {
   ASSERT_TRUE(tab3.get());
 
   // Make sure 3 tabs are open.
-  ASSERT_TRUE(browser->WaitForTabCountToBecome(initial_tab_count + 2,
-                                               10000));
+  ASSERT_TRUE(browser->WaitForTabCountToBecome(initial_tab_count + 2));
 
   // Get bounds for the tabs.
   gfx::Rect bounds1;
@@ -182,8 +181,7 @@ TEST_F(TabDraggingTest, MAYBE_Tab1Tab3) {
   ASSERT_TRUE(tab3.get());
 
   // Make sure 3 tabs are open.
-  ASSERT_TRUE(browser->WaitForTabCountToBecome(initial_tab_count + 2,
-                                               10000));
+  ASSERT_TRUE(browser->WaitForTabCountToBecome(initial_tab_count + 2));
 
   // Get bounds for the tabs.
   gfx::Rect bounds1;
@@ -290,8 +288,7 @@ TEST_F(TabDraggingTest, MAYBE_Tab1Tab3Escape) {
   ASSERT_TRUE(tab3.get());
 
   // Make sure 3 tabs are open.
-  ASSERT_TRUE(browser->WaitForTabCountToBecome(initial_tab_count + 2,
-                                               10000));
+  ASSERT_TRUE(browser->WaitForTabCountToBecome(initial_tab_count + 2));
 
   // Get bounds for the tabs.
   gfx::Rect bounds1;
@@ -401,8 +398,7 @@ TEST_F(TabDraggingTest, MAYBE_Tab2OutOfTabStrip) {
   ASSERT_TRUE(tab3.get());
 
   // Make sure 3 tabs are opened.
-  ASSERT_TRUE(browser->WaitForTabCountToBecome(initial_tab_count + 2,
-                                               10000));
+  ASSERT_TRUE(browser->WaitForTabCountToBecome(initial_tab_count + 2));
 
   // 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 328a5a49..072b8cc 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, action_max_timeout_ms()));
+    ASSERT_TRUE(browser->WaitForTabCountToBecome(1));
     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 b3952b2..c4c1eec 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,8 +10,7 @@
 TEST_F(AutomatedUITestBase, DragOut) {
   NewTab();
   NewTab();
-  ASSERT_TRUE(active_browser()->
-      WaitForTabCountToBecome(3, action_max_timeout_ms()));
+  ASSERT_TRUE(active_browser()->WaitForTabCountToBecome(3));
   PlatformThread::Sleep(sleep_timeout_ms());
   ASSERT_TRUE(DragTabOut());
   int window_count;
@@ -22,8 +21,7 @@ TEST_F(AutomatedUITestBase, DragOut) {
 TEST_F(AutomatedUITestBase, DragLeftRight) {
   NewTab();
   NewTab();
-  ASSERT_TRUE(active_browser()->
-      WaitForTabCountToBecome(3, action_max_timeout_ms()));
+  ASSERT_TRUE(active_browser()->WaitForTabCountToBecome(3));
   // 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 0ef68ad..196fe0f 100644
--- a/chrome/test/automation/automation_messages_internal.h
+++ b/chrome/test/automation/automation_messages_internal.h
@@ -1428,4 +1428,10 @@ 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 df3929a..b5a89eb 100644
--- a/chrome/test/automation/browser_proxy.cc
+++ b/chrome/test/automation/browser_proxy.cc
@@ -192,19 +192,14 @@ bool BrowserProxy::SimulateDrag(const gfx::Point& start,
   return result;
 }
 
-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;
+bool BrowserProxy::WaitForTabCountToBecome(int count) {
+  bool success = false;
+  if (!sender_->Send(new AutomationMsg_WaitForTabCountToBecome(
+                         0, handle_, count, &success))) {
+    return false;
   }
-  // If we get here, the tab count doesn't match.
-  return false;
+
+  return success;
 }
 
 bool BrowserProxy::WaitForTabToBecomeActive(int tab,
diff --git a/chrome/test/automation/browser_proxy.h b/chrome/test/automation/browser_proxy.h
index 7dd2671..3fb06d4 100644
--- a/chrome/test/automation/browser_proxy.h
+++ b/chrome/test/automation/browser_proxy.h
@@ -115,9 +115,8 @@ 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, int wait_timeout) WARN_UNUSED_RESULT;
+  bool WaitForTabCountToBecome(int count) 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 d002a3e..a8ee85d 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, action_timeout_ms()));
+  ASSERT_TRUE(browser->WaitForTabCountToBecome(2));
 
   // 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, action_max_timeout_ms()));
+  ASSERT_TRUE(browser->WaitForTabCountToBecome(1));
 #endif
 }
 
diff --git a/chrome/test/tab_switching/tab_switching_test.cc b/chrome/test/tab_switching/tab_switching_test.cc
index ba72e7e..8544167 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, 10000));
+          initial_tab_count + new_tab_count));
 
       // 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 758ca156f..f3215d4 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, action_max_timeout_ms()));
+  ASSERT_TRUE(browser->WaitForTabCountToBecome(2));
   // Close the tab, removing the one and only before unload handler.
   scoped_refptr<TabProxy> tab(browser->GetTab(1));
   ASSERT_TRUE(tab->Close(true));