[GTTF] Miscellanous UI tests cleanups:

- use built-in timeouts instead of "inventing" them in each test case
- avoid unneeded checks and operations
- use automation calls more effectively
- use FLAKY mark instead of DISABLED to maintain test coverage
- split some tests to make the above possible

TEST=UI test based
BUG=39785

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

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

16 files changed, 122 insertions, 146 deletions

diff --git a/chrome/browser/metrics/metrics_service_uitest.cc b/chrome/browser/metrics/metrics_service_uitest.cc
index 7465e17..5dfe9d1 100644
--- a/chrome/browser/metrics/metrics_service_uitest.cc
+++ b/chrome/browser/metrics/metrics_service_uitest.cc
@@ -102,7 +102,7 @@ TEST_F(MetricsServiceTest, CrashRenderers) {
   }
 
   // Give the browser a chance to notice the crashed tab.
-  PlatformThread::Sleep(1000);
+  PlatformThread::Sleep(sleep_timeout_ms());
 
   QuitBrowser();
 
diff --git a/chrome/browser/printing/printing_layout_uitest.cc b/chrome/browser/printing/printing_layout_uitest.cc
index 5fb1b3f..d205f7b 100644
--- a/chrome/browser/printing/printing_layout_uitest.cc
+++ b/chrome/browser/printing/printing_layout_uitest.cc
@@ -5,6 +5,7 @@
 #include "base/command_line.h"
 #include "base/file_util.h"
 #include "base/simple_thread.h"
+#include "base/test/test_file_util.h"
 #include "base/win_util.h"
 #include "chrome/common/chrome_paths.h"
 #include "chrome/test/automation/browser_proxy.h"
@@ -49,16 +50,13 @@ class PrintingLayoutTest : public PrintingTest<UITest> {
   void PrintNowTab() {
     scoped_refptr<TabProxy> tab_proxy(GetActiveTab());
     ASSERT_TRUE(tab_proxy.get());
-    if (!tab_proxy.get())
-      return;
-
     ASSERT_TRUE(tab_proxy->PrintNow());
   }
 
   // Finds the dump for the last print job and compares it to the data named
   // |verification_name|. Compares the saved printed job pixels with the test
   // data pixels and returns the percentage of different pixels; 0 for success,
-  // ]0, 100] for failure.
+  // [0, 100] for failure.
   double CompareWithResult(const std::wstring& verification_name) {
     FilePath test_result(ScanFiles(verification_name));
     if (test_result.value().empty()) {
@@ -118,21 +116,8 @@ class PrintingLayoutTest : public PrintingTest<UITest> {
 
   // Makes sure the directory exists and is empty.
   void CleanupDumpDirectory() {
-    // Tries to delete the dumping directory for around 10 seconds.
-    for (int i = 0; i < 100 && file_util::PathExists(emf_path()); ++i) {
-      // It's fine fail sometimes because of opened left over .PRN file.
-      // Explanation:
-      // When calling PrintNowTab(), it makes sure the page is rendered and
-      // sent to the spooler. It does *not* wait for the spooler to flush the
-      // job. It is completely unnecessary to wait for that. So the printer
-      // may write the file too late. Since the printer holds an exclusive
-      // access to the file, it can't be deleted until the printer is done.
-      if (file_util::Delete(emf_path(), true)) {
-        break;
-      }
-      PlatformThread::Sleep(100);
-    }
-    file_util::CreateDirectory(emf_path());
+    EXPECT_TRUE(file_util::DieFileDie(emf_path(), true));
+    EXPECT_TRUE(file_util::CreateDirectory(emf_path()));
   }
 
   // Returns if Clear Type is currently enabled.
diff --git a/chrome/browser/process_singleton_linux_uitest.cc b/chrome/browser/process_singleton_linux_uitest.cc
index 8bef102..d0ba750 100644
--- a/chrome/browser/process_singleton_linux_uitest.cc
+++ b/chrome/browser/process_singleton_linux_uitest.cc
@@ -32,7 +32,8 @@ typedef UITest ProcessSingletonLinuxTest;
 // A helper method to call ProcessSingleton::NotifyOtherProcess().
 // |url| will be added to CommandLine for current process, so that it can be
 // sent to browser process by ProcessSingleton::NotifyOtherProcess().
-ProcessSingleton::NotifyResult NotifyOtherProcess(const std::string& url) {
+ProcessSingleton::NotifyResult NotifyOtherProcess(const std::string& url,
+                                                  int timeout_ms) {
   FilePath user_data_dir;
   PathService::Get(chrome::DIR_USER_DATA, &user_data_dir);
 
@@ -49,10 +50,8 @@ ProcessSingleton::NotifyResult NotifyOtherProcess(const std::string& url) {
 
   ProcessSingleton process_singleton(user_data_dir);
 
-  // Use a short timeout to keep tests fast.
-  const int kTimeoutSeconds = 3;
-  return process_singleton.NotifyOtherProcessWithTimeout(new_cmd_line,
-                                                         kTimeoutSeconds);
+  return process_singleton.NotifyOtherProcessWithTimeout(
+      new_cmd_line, timeout_ms / 1000);
 }
 
 }  // namespace
@@ -95,7 +94,8 @@ TEST_F(ProcessSingletonLinuxTest, NotifyOtherProcessSuccess) {
   std::string url("about:blank");
   int original_tab_count = GetTabCount();
 
-  EXPECT_EQ(ProcessSingleton::PROCESS_NOTIFIED, NotifyOtherProcess(url));
+  EXPECT_EQ(ProcessSingleton::PROCESS_NOTIFIED,
+            NotifyOtherProcess(url, action_timeout_ms()));
   EXPECT_EQ(original_tab_count + 1, GetTabCount());
   EXPECT_EQ(url, GetActiveTabURL().spec());
 }
@@ -115,10 +115,11 @@ TEST_F(ProcessSingletonLinuxTest, NotifyOtherProcessFailure) {
   HANDLE_EINTR(waitpid(pid, 0, WUNTRACED));
 
   std::string url("about:blank");
-  EXPECT_EQ(ProcessSingleton::PROCESS_NONE, NotifyOtherProcess(url));
+  EXPECT_EQ(ProcessSingleton::PROCESS_NONE,
+            NotifyOtherProcess(url, action_timeout_ms()));
 
   // Wait for a while to make sure the browser process is actually killed.
-  EXPECT_FALSE(CrashAwareSleep(1000));
+  EXPECT_FALSE(CrashAwareSleep(sleep_timeout_ms()));
 }
 
 // Test that we can still notify a process on the same host even after the
@@ -131,7 +132,8 @@ TEST_F(ProcessSingletonLinuxTest, NotifyOtherProcessHostChanged) {
   int original_tab_count = GetTabCount();
 
   std::string url("about:blank");
-  EXPECT_EQ(ProcessSingleton::PROCESS_NOTIFIED, NotifyOtherProcess(url));
+  EXPECT_EQ(ProcessSingleton::PROCESS_NOTIFIED,
+            NotifyOtherProcess(url, action_timeout_ms()));
   EXPECT_EQ(original_tab_count + 1, GetTabCount());
   EXPECT_EQ(url, GetActiveTabURL().spec());
 }
@@ -146,12 +148,13 @@ TEST_F(ProcessSingletonLinuxTest, NotifyOtherProcessDifferingHost) {
   // Kill the browser process, so that it does not respond on the socket.
   kill(pid, SIGKILL);
   // Wait for a while to make sure the browser process is actually killed.
-  EXPECT_FALSE(CrashAwareSleep(1000));
+  EXPECT_FALSE(CrashAwareSleep(sleep_timeout_ms()));
 
   FilePath lock_path = user_data_dir().Append(chrome::kSingletonLockFilename);
   EXPECT_EQ(0, unlink(lock_path.value().c_str()));
   EXPECT_EQ(0, symlink("FAKEFOOHOST-1234", lock_path.value().c_str()));
 
   std::string url("about:blank");
-  EXPECT_EQ(ProcessSingleton::PROFILE_IN_USE, NotifyOtherProcess(url));
+  EXPECT_EQ(ProcessSingleton::PROFILE_IN_USE,
+            NotifyOtherProcess(url, action_timeout_ms()));
 }
diff --git a/chrome/browser/process_singleton_win_uitest.cc b/chrome/browser/process_singleton_win_uitest.cc
index 7611a83..929e271b2 100644
--- a/chrome/browser/process_singleton_win_uitest.cc
+++ b/chrome/browser/process_singleton_win_uitest.cc
@@ -33,11 +33,12 @@ namespace {
 // NewRunnableMethod class to run the StartChrome methods in many threads.
 class ChromeStarter : public base::RefCountedThreadSafe<ChromeStarter> {
  public:
-  ChromeStarter()
+  explicit ChromeStarter(int timeout_ms)
       : ready_event_(false /* manual */, false /* signaled */),
         done_event_(false /* manual */, false /* signaled */),
         process_handle_(NULL),
-        process_terminated_(false) {
+        process_terminated_(false),
+        timeout_ms_(timeout_ms) {
   }
 
   // We must reset some data members since we reuse the same ChromeStarter
@@ -75,9 +76,8 @@ class ChromeStarter : public base::RefCountedThreadSafe<ChromeStarter> {
     // We can wait on the handle here, we should get stuck on one and only
     // one process. The test below will take care of killing that process
     // to unstuck us once it confirms there is only one.
-    static const int64 kWaitForProcessDeath = 5000;
     process_terminated_ = base::WaitForSingleProcess(process_handle_,
-                                                     kWaitForProcessDeath);
+                                                     timeout_ms_);
     // Let the test know we are done.
     done_event_.Signal();
   }
@@ -90,10 +90,14 @@ class ChromeStarter : public base::RefCountedThreadSafe<ChromeStarter> {
 
  private:
   friend class base::RefCountedThreadSafe<ChromeStarter>;
+
   ~ChromeStarter() {
     if (process_handle_ != NULL)
       base::CloseProcessHandle(process_handle_);
   }
+
+  int timeout_ms_;
+
   DISALLOW_COPY_AND_ASSIGN(ChromeStarter);
 };
 
@@ -111,7 +115,7 @@ class ProcessSingletonWinTest : public UITest {
     for (size_t i = 0; i < kNbThreads; ++i) {
       chrome_starter_threads_[i].reset(new base::Thread("ChromeStarter"));
       ASSERT_TRUE(chrome_starter_threads_[i]->Start());
-      chrome_starters_[i] = new ChromeStarter;
+      chrome_starters_[i] = new ChromeStarter(action_max_timeout_ms());
     }
   }
 
diff --git a/chrome/browser/repost_form_warning_uitest.cc b/chrome/browser/repost_form_warning_uitest.cc
index 755ae85..b17e588 100644
--- a/chrome/browser/repost_form_warning_uitest.cc
+++ b/chrome/browser/repost_form_warning_uitest.cc
@@ -13,26 +13,23 @@
 #include "chrome/test/ui/ui_test.h"
 #include "net/url_request/url_request_unittest.h"
 
-using std::wstring;
-
 namespace {
 
 const wchar_t kDocRoot[] = L"chrome/test/data";
 
 }  // namespace
 
-class RepostFormWarningTest : public UITest {
-};
-
+typedef UITest RepostFormWarningTest;
 
 TEST_F(RepostFormWarningTest, TestDoubleReload) {
   scoped_refptr<HTTPTestServer> server =
       HTTPTestServer::CreateServer(kDocRoot, NULL);
   ASSERT_TRUE(NULL != server.get());
   scoped_refptr<BrowserProxy> browser(automation()->GetBrowserWindow(0));
-  EXPECT_TRUE(browser.get());
+  ASSERT_TRUE(browser.get());
 
   scoped_refptr<TabProxy> tab(browser->GetTab(0));
+  ASSERT_TRUE(tab.get());
 
   // Load a form.
   ASSERT_TRUE(tab->NavigateToURL(server->TestServerPageW(L"files/form.html")));
@@ -56,6 +53,7 @@ TEST_F(RepostFormWarningTest, TestLoginAfterRepost) {
   ASSERT_TRUE(browser.get());
 
   scoped_refptr<TabProxy> tab(browser->GetTab(0));
+  ASSERT_TRUE(tab.get());
 
   // Load a form.
   ASSERT_TRUE(tab->NavigateToURL(server->TestServerPageW(L"files/form.html")));
diff --git a/chrome/browser/session_history_uitest.cc b/chrome/browser/session_history_uitest.cc
index c293a9d..830f9be 100644
--- a/chrome/browser/session_history_uitest.cc
+++ b/chrome/browser/session_history_uitest.cc
@@ -14,8 +14,6 @@
 #include "net/base/net_util.h"
 #include "net/url_request/url_request_unittest.h"
 
-using std::wstring;
-
 namespace {
 
 const wchar_t kDocRoot[] = L"chrome/test/data";
@@ -32,9 +30,7 @@ class SessionHistoryTest : public UITest {
     window_ = automation()->GetBrowserWindow(0);
     ASSERT_TRUE(window_.get());
 
-    int active_tab_index = -1;
-    ASSERT_TRUE(window_->GetActiveTabIndex(&active_tab_index));
-    tab_ = window_->GetTab(active_tab_index);
+    tab_ = window_->GetActiveTab();
     ASSERT_TRUE(tab_.get());
   }
 
@@ -67,8 +63,8 @@ class SessionHistoryTest : public UITest {
     ASSERT_TRUE(tab_->NavigateToURL(url));
   }
 
-  wstring GetTabTitle() {
-    wstring title;
+  std::wstring GetTabTitle() {
+    std::wstring title;
     EXPECT_TRUE(tab_->GetTabTitle(&title));
     return title;
   }
diff --git a/chrome/browser/sessions/session_restore_uitest.cc b/chrome/browser/sessions/session_restore_uitest.cc
index aefa6aa..fe4ffeb 100644
--- a/chrome/browser/sessions/session_restore_uitest.cc
+++ b/chrome/browser/sessions/session_restore_uitest.cc
@@ -73,7 +73,7 @@ class SessionRestoreUITest : public UITest {
 
     scoped_refptr<TabProxy> tab_proxy(browser_proxy->GetActiveTab());
     ASSERT_TRUE(tab_proxy.get());
-    ASSERT_TRUE(tab_proxy->WaitForTabToBeRestored(action_timeout_ms()));
+    ASSERT_TRUE(tab_proxy->WaitForTabToBeRestored(action_max_timeout_ms()));
 
     ASSERT_TRUE(tab_proxy->GetCurrentURL(url));
   }
@@ -100,7 +100,7 @@ TEST_F(SessionRestoreUITest, Basic) {
   scoped_refptr<BrowserProxy> browser_proxy(automation()->GetBrowserWindow(0));
   ASSERT_TRUE(browser_proxy.get());
   scoped_refptr<TabProxy> tab_proxy(browser_proxy->GetTab(0));
-  ASSERT_TRUE(tab_proxy->WaitForTabToBeRestored(action_timeout_ms()));
+  ASSERT_TRUE(tab_proxy->WaitForTabToBeRestored(action_max_timeout_ms()));
 
   ASSERT_EQ(url2_, GetActiveTabURL());
   ASSERT_EQ(AUTOMATION_MSG_NAVIGATION_SUCCESS, tab_proxy->GoBack());
@@ -126,7 +126,7 @@ TEST_F(SessionRestoreUITest, RestoresForwardAndBackwardNavs) {
   scoped_refptr<BrowserProxy> browser_proxy(automation()->GetBrowserWindow(0));
   ASSERT_TRUE(browser_proxy.get());
   scoped_refptr<TabProxy> tab_proxy(browser_proxy->GetTab(0));
-  ASSERT_TRUE(tab_proxy->WaitForTabToBeRestored(action_timeout_ms()));
+  ASSERT_TRUE(tab_proxy->WaitForTabToBeRestored(action_max_timeout_ms()));
 
   ASSERT_TRUE(GetActiveTabURL() == url2_);
   ASSERT_TRUE(tab_proxy->GoForward());
@@ -220,16 +220,16 @@ TEST_F(SessionRestoreUITest, TwoTabsSecondSelected) {
 
   scoped_refptr<TabProxy> tab_proxy(browser_proxy->GetActiveTab());
   ASSERT_TRUE(tab_proxy.get());
-  ASSERT_TRUE(tab_proxy->WaitForTabToBeRestored(action_timeout_ms()));
+  ASSERT_TRUE(tab_proxy->WaitForTabToBeRestored(action_max_timeout_ms()));
 
-  ASSERT_TRUE(GetActiveTabURL() == url2_);
+  ASSERT_EQ(url2_, GetActiveTabURL());
 
   ASSERT_TRUE(browser_proxy->ActivateTab(0));
   tab_proxy = browser_proxy->GetActiveTab();
   ASSERT_TRUE(tab_proxy.get());
-  ASSERT_TRUE(tab_proxy->WaitForTabToBeRestored(action_timeout_ms()));
+  ASSERT_TRUE(tab_proxy->WaitForTabToBeRestored(action_max_timeout_ms()));
 
-  ASSERT_TRUE(GetActiveTabURL() == url1_);
+  ASSERT_EQ(url1_, GetActiveTabURL());
 }
 
 // Creates two tabs, closes one, quits and makes sure only one tab is restored.
@@ -258,7 +258,7 @@ TEST_F(SessionRestoreUITest, ClosedTabStaysClosed) {
 
   AssertOneWindowWithOneTab();
 
-  ASSERT_TRUE(GetActiveTabURL() == url1_);
+  ASSERT_EQ(url1_, GetActiveTabURL());
 }
 
 // Creates a tabbed browser and popup and makes sure we restore both.
@@ -324,7 +324,6 @@ TEST_F(SessionRestoreUITest, NormalAndPopup) {
   }
 }
 
-#if defined(OS_WIN)
 // Creates a browser, goes incognito, closes browser, launches and make sure
 // we don't restore.
 //
@@ -366,7 +365,7 @@ TEST_F(SessionRestoreUITest, FLAKY_DontRestoreWhileIncognito) {
   ASSERT_TRUE(browser_proxy.get());
   scoped_refptr<TabProxy> tab_proxy(browser_proxy->GetTab(0));
   ASSERT_TRUE(tab_proxy.get());
-  ASSERT_TRUE(tab_proxy->WaitForTabToBeRestored(action_timeout_ms()));
+  ASSERT_TRUE(tab_proxy->WaitForTabToBeRestored(action_max_timeout_ms()));
   GURL url;
   ASSERT_TRUE(tab_proxy->GetCurrentURL(&url));
   ASSERT_TRUE(url != url1_);
@@ -396,7 +395,7 @@ TEST_F(SessionRestoreUITest, TwoWindowsCloseOneRestoreOnlyOne) {
 
   AssertOneWindowWithOneTab();
 
-  ASSERT_TRUE(GetActiveTabURL() == url1_);
+  ASSERT_EQ(url1_, GetActiveTabURL());
 }
 
 // Launches an app window, closes tabbed browser, launches and makes sure
@@ -430,7 +429,7 @@ TEST_F(SessionRestoreUITest,
 
   AssertOneWindowWithOneTab();
 
-  ASSERT_TRUE(GetActiveTabURL() == url1_);
+  ASSERT_EQ(url1_, GetActiveTabURL());
 }
 
 // Make sure after a restore the number of processes matches that of the number
@@ -473,13 +472,12 @@ TEST_F(SessionRestoreUITest, ShareProcessesOnRestore) {
 
   scoped_refptr<TabProxy> tab_proxy(browser_proxy->GetTab(tab_count - 2));
   ASSERT_TRUE(tab_proxy.get() != NULL);
-  ASSERT_TRUE(tab_proxy->WaitForTabToBeRestored(action_timeout_ms()));
+  ASSERT_TRUE(tab_proxy->WaitForTabToBeRestored(action_max_timeout_ms()));
   tab_proxy = browser_proxy->GetTab(tab_count - 1);
   ASSERT_TRUE(tab_proxy.get() != NULL);
-  ASSERT_TRUE(tab_proxy->WaitForTabToBeRestored(action_timeout_ms()));
+  ASSERT_TRUE(tab_proxy->WaitForTabToBeRestored(action_max_timeout_ms()));
 
   ASSERT_EQ(expected_process_count, GetBrowserProcessCount());
 }
-#endif
 
 }  // namespace
diff --git a/chrome/browser/tab_restore_uitest.cc b/chrome/browser/tab_restore_uitest.cc
index 56ff39f..ebc6f09 100644
--- a/chrome/browser/tab_restore_uitest.cc
+++ b/chrome/browser/tab_restore_uitest.cc
@@ -98,14 +98,10 @@ class TabRestoreUITest : public UITest {
   // the final number of tabs.
   int AddSomeTabs(BrowserProxy* browser, int how_many) {
     int starting_tab_count = -1;
-    // Use EXPECT instead of ASSERT throughout to avoid trying to return void.
     EXPECT_TRUE(browser->GetTabCount(&starting_tab_count));
 
     for (int i = 0; i < how_many; ++i) {
       EXPECT_TRUE(browser->AppendTab(url1_));
-      int current_tab_count;
-      EXPECT_TRUE(browser->GetTabCount(&current_tab_count));
-      EXPECT_EQ(starting_tab_count + i + 1, current_tab_count);
     }
     int tab_count;
     EXPECT_TRUE(browser->GetTabCount(&tab_count));
diff --git a/chrome/browser/unload_uitest.cc b/chrome/browser/unload_uitest.cc
index 5ee30b7..62072f9 100644
--- a/chrome/browser/unload_uitest.cc
+++ b/chrome/browser/unload_uitest.cc
@@ -105,7 +105,7 @@ class UnloadTest : public UITest {
 
   void WaitForBrowserClosed() {
     const int kCheckDelayMs = 100;
-    int max_wait_time = 5000;
+    int max_wait_time = action_max_timeout_ms();
     while (max_wait_time > 0) {
       max_wait_time -= kCheckDelayMs;
       PlatformThread::Sleep(kCheckDelayMs);
@@ -116,7 +116,7 @@ class UnloadTest : public UITest {
 
   void CheckTitle(const std::wstring& expected_title) {
     const int kCheckDelayMs = 100;
-    int max_wait_time = 5000;
+    int max_wait_time = action_max_timeout_ms();
     while (max_wait_time > 0) {
       max_wait_time -= kCheckDelayMs;
       PlatformThread::Sleep(kCheckDelayMs);
diff --git a/chrome/test/automation/automation_proxy_uitest.cc b/chrome/test/automation/automation_proxy_uitest.cc
index b037a25..d784bfc 100644
--- a/chrome/test/automation/automation_proxy_uitest.cc
+++ b/chrome/test/automation/automation_proxy_uitest.cc
@@ -233,19 +233,16 @@ TEST_F(AutomationProxyTest, ActivateTab) {
 
   ASSERT_TRUE(window->AppendTab(GURL("about:blank")));
 
-  int at_index = 1;
-  ASSERT_TRUE(window->ActivateTab(at_index));
+  ASSERT_TRUE(window->ActivateTab(1));
   int active_tab_index = -1;
   ASSERT_TRUE(window->GetActiveTabIndex(&active_tab_index));
-  ASSERT_EQ(at_index, active_tab_index);
+  ASSERT_EQ(1, active_tab_index);
 
-  at_index = 0;
-  ASSERT_TRUE(window->ActivateTab(at_index));
+  ASSERT_TRUE(window->ActivateTab(0));
   ASSERT_TRUE(window->GetActiveTabIndex(&active_tab_index));
-  ASSERT_EQ(at_index, active_tab_index);
+  ASSERT_EQ(0, active_tab_index);
 }
 
-
 TEST_F(AutomationProxyTest, GetTab) {
   scoped_refptr<BrowserProxy> window(automation()->GetBrowserWindow(0));
   ASSERT_TRUE(window.get());
@@ -364,13 +361,11 @@ TEST_F(AutomationProxyTest2, GetActiveTabIndex) {
 
   int active_tab_index = -1;
   ASSERT_TRUE(window->GetActiveTabIndex(&active_tab_index));
-  int tab_count;
-  ASSERT_TRUE(window->GetTabCount(&tab_count));
   ASSERT_EQ(0, active_tab_index);
-  int at_index = 1;
-  ASSERT_TRUE(window->ActivateTab(at_index));
+
+  ASSERT_TRUE(window->ActivateTab(1));
   ASSERT_TRUE(window->GetActiveTabIndex(&active_tab_index));
-  ASSERT_EQ(at_index, active_tab_index);
+  ASSERT_EQ(1, active_tab_index);
 }
 
 TEST_F(AutomationProxyTest2, GetTabTitle) {
@@ -442,13 +437,11 @@ TEST_F(AutomationProxyTest, AcceleratorNewTab) {
   scoped_refptr<BrowserProxy> window(automation()->GetBrowserWindow(0));
 
   int tab_count = -1;
-  ASSERT_TRUE(window->GetTabCount(&tab_count));
-  EXPECT_EQ(1, tab_count);
-
   ASSERT_TRUE(window->RunCommand(IDC_NEW_TAB));
   ASSERT_TRUE(window->GetTabCount(&tab_count));
   EXPECT_EQ(2, tab_count);
-  scoped_refptr<TabProxy> tab(window->GetTab(tab_count - 1));
+
+  scoped_refptr<TabProxy> tab(window->GetTab(1));
   ASSERT_TRUE(tab.get());
 }
 
@@ -566,24 +559,8 @@ TEST_F(AutomationProxyTest3, FrameDocumentCanBeAccessed) {
   std::wstring jscript3 = CreateJSStringForDOMQuery(L"mydiv");
   ASSERT_TRUE(tab->ExecuteAndExtractString(xpath3, jscript3, &actual));
   ASSERT_EQ(L"DIV", actual);
-
-  // TODO(evanm): fix or remove this.
-  // This part of the test appears to verify that executing JS fails
-  // non-HTML pages, but the new tab is now HTML so this test isn't
-  // correct.
-#if 0
-  // Open a new Destinations tab to execute script inside.
-  window->RunCommand(IDC_NEWTAB);
-  tab = window->GetTab(1);
-  ASSERT_TRUE(tab.get());
-  ASSERT_TRUE(window->ActivateTab(1));
-
-  ASSERT_FALSE(tab->ExecuteAndExtractString(xpath1, jscript1, &actual));
-#endif
 }
 
-// TODO(port): Need to port constrained_window_proxy.* first.
-#if defined(OS_WIN)
 TEST_F(AutomationProxyTest, BlockedPopupTest) {
   scoped_refptr<BrowserProxy> window(automation()->GetBrowserWindow(0));
   ASSERT_TRUE(window.get());
@@ -597,11 +574,10 @@ TEST_F(AutomationProxyTest, BlockedPopupTest) {
 
   ASSERT_TRUE(tab->NavigateToURL(net::FilePathToFileURL(filename)));
 
-  ASSERT_TRUE(tab->WaitForBlockedPopupCountToChangeTo(2, 5000));
+  ASSERT_TRUE(tab->WaitForBlockedPopupCountToChangeTo(2,
+                                                      action_max_timeout_ms()));
 }
 
-#endif  // defined(OS_WIN)
-
 // TODO(port): Remove HWND if possible
 #if defined(OS_WIN)
 
@@ -1357,7 +1333,7 @@ TEST_F(AutomationProxyVisibleTest, AutocompleteMatchesTest) {
   EXPECT_TRUE(browser->ApplyAccelerator(IDC_FOCUS_LOCATION));
   EXPECT_TRUE(edit->is_valid());
   EXPECT_TRUE(edit->SetText(L"Roflcopter"));
-  EXPECT_TRUE(edit->WaitForQuery(30000));
+  EXPECT_TRUE(edit->WaitForQuery(action_max_timeout_ms()));
   bool query_in_progress;
   EXPECT_TRUE(edit->IsQueryInProgress(&query_in_progress));
   EXPECT_FALSE(query_in_progress);
@@ -1366,9 +1342,14 @@ TEST_F(AutomationProxyVisibleTest, AutocompleteMatchesTest) {
   EXPECT_FALSE(matches.empty());
 }
 
-// This test is flaky, see http://crbug.com/5314. Disabled because it hangs
-// on Mac (http://crbug.com/25039).
-TEST_F(AutomationProxyTest, DISABLED_AppModalDialogTest) {
+#if defined(OS_MACOSX)
+// Hangs on Mac, http://crbug.com/25039.
+#define AppModalDialogTest DISABLED_AppModalDialogTest
+#else
+// Flaky, http://crbug.com/5314.
+#define AppModalDialogTest FLAKY_AppModalDialogTest
+#endif
+TEST_F(AutomationProxyTest, AppModalDialogTest) {
   scoped_refptr<BrowserProxy> browser(automation()->GetBrowserWindow(0));
   ASSERT_TRUE(browser.get());
   scoped_refptr<TabProxy> tab(browser->GetTab(0));
@@ -1480,7 +1461,7 @@ TEST_F(AutomationProxyTest5, TestLifetimeOfDomAutomationController) {
             tab->NavigateToURL(net::FilePathToFileURL(filename)));
 
   // Allow some time for the popup to show up and close.
-  PlatformThread::Sleep(2000);
+  PlatformThread::Sleep(sleep_timeout_ms());
 
   std::wstring expected(L"string");
   std::wstring jscript = CreateJSString(L"\"" + expected + L"\"");
diff --git a/chrome/test/ui/fast_shutdown_uitest.cc b/chrome/test/ui/fast_shutdown_uitest.cc
index 8b2b554..6f4e174 100644
--- a/chrome/test/ui/fast_shutdown_uitest.cc
+++ b/chrome/test/ui/fast_shutdown_uitest.cc
@@ -26,8 +26,7 @@ TEST_F(FastShutdown, DISABLED_SlowTermination) {
   ASSERT_TRUE(window.get());
 
   // This page has an unload handler.
-  GURL url = GetTestUrl(L"fast_shutdown", L"on_unloader.html");
-  NavigateToURLBlockUntilNavigationsComplete(url, 1);
+  NavigateToURL(GetTestUrl(L"fast_shutdown", L"on_unloader.html"));
   gfx::Rect bounds;
   ASSERT_TRUE(window->GetViewBounds(VIEW_ID_TAB_CONTAINER, &bounds, true));
   // This click will launch a popup which has a before unload handler.
diff --git a/chrome/test/ui/history_uitest.cc b/chrome/test/ui/history_uitest.cc
index 522c221..22c1aee 100644
--- a/chrome/test/ui/history_uitest.cc
+++ b/chrome/test/ui/history_uitest.cc
@@ -33,34 +33,36 @@ class HistoryTester : public UITest {
 // These tests are flaky because automatic and user-initiated transitions are
 // distinguished based on the interval between page load and redirect.
 
-TEST_F(HistoryTester, DISABLED_VerifyHistoryLength) {
+// Flaky, http://crbug.com/39785.
+TEST_F(HistoryTester, FLAKY_VerifyHistoryLength1) {
   // Test the history length for the following page transitions.
-  //
-  // Test case 1:
   //   -open-> Page 1.
-  // Test case 2:
-  //   -open-> Page 2 -redirect-> Page 3.
-  // Test case 3:
-  //   -open-> Page 4 -navigate_backward-> Page 3 -navigate_backward->Page 1
-  //   -navigate_forward-> Page 3 -navigate_forward-> Page 4
-  //
-  // Note that Page 2 is not visited on navigating backward/forward.
 
-  // Test case 1
   std::wstring test_case_1 = L"history_length_test_page_1.html";
   GURL url_1 = GetTestUrl(L"History", test_case_1);
   NavigateToURL(url_1);
   WaitForFinish("History_Length_Test_1", "1", url_1, kTestCompleteCookie,
-                  kTestCompleteSuccess, action_max_timeout_ms());
+                kTestCompleteSuccess, action_max_timeout_ms());
+}
+
+// Disabled, http://crbug.com/39785.
+TEST_F(HistoryTester, DISABLED_VerifyHistoryLength2) {
+  // Test the history length for the following page transitions.
+  //   -open-> Page 2 -redirect-> Page 3.
 
-  // Test case 2
   std::wstring test_case_2 = L"history_length_test_page_2.html";
   GURL url_2 = GetTestUrl(L"History", test_case_2);
   NavigateToURL(url_2);
   WaitForFinish("History_Length_Test_2", "1", url_2, kTestCompleteCookie,
                 kTestCompleteSuccess, action_max_timeout_ms());
+}
+
+// Disabled, http://crbug.com/39785.
+TEST_F(HistoryTester, DISABLED_VerifyHistoryLength3) {
+  // Test the history length for the following page transitions.
+  //   -open-> Page 4 -navigate_backward-> Page 3 -navigate_backward->Page 1
+  //   -navigate_forward-> Page 3 -navigate_forward-> Page 4
 
-  // Test case 3
   std::wstring test_case_3 = L"history_length_test_page_4.html";
   GURL url_3 = GetTestUrl(L"History", test_case_3);
   NavigateToURL(url_3);
@@ -69,7 +71,8 @@ TEST_F(HistoryTester, DISABLED_VerifyHistoryLength) {
 }
 
 #if defined(OS_WIN) || defined(OS_LINUX)
-TEST_F(HistoryTester, DISABLED_ConsiderRedirectAfterGestureAsUserInitiated) {
+// Flaky, http://crbug.com/39785.
+TEST_F(HistoryTester, FLAKY_ConsiderRedirectAfterGestureAsUserInitiated) {
   // Test the history length for the following page transition.
   //
   // -open-> Page 11 -slow_redirect-> Page 12.
@@ -86,7 +89,9 @@ TEST_F(HistoryTester, DISABLED_ConsiderRedirectAfterGestureAsUserInitiated) {
 
   // Simulate click. This only works for Windows.
   scoped_refptr<BrowserProxy> browser = automation()->GetBrowserWindow(0);
+  ASSERT_TRUE(browser.get());
   scoped_refptr<WindowProxy> window = browser->GetWindow();
+  ASSERT_TRUE(window.get());
   gfx::Rect tab_view_bounds;
   ASSERT_TRUE(window->GetViewBounds(VIEW_ID_TAB_CONTAINER, &tab_view_bounds,
                                     true));
@@ -100,7 +105,8 @@ TEST_F(HistoryTester, DISABLED_ConsiderRedirectAfterGestureAsUserInitiated) {
 }
 #endif  // defined(OS_WIN) || defined(OS_LINUX)
 
-TEST_F(HistoryTester, DISABLED_ConsiderSlowRedirectAsUserInitiated) {
+// Flaky, http://crbug.com/39785.
+TEST_F(HistoryTester, FLAKY_ConsiderSlowRedirectAsUserInitiated) {
   // Test the history length for the following page transition.
   //
   // -open-> Page 21 -redirect-> Page 22.
diff --git a/chrome/test/ui/mouseleave_uitest.cc b/chrome/test/ui/mouseleave_uitest.cc
index 91a269f..be647b2 100644
--- a/chrome/test/ui/mouseleave_uitest.cc
+++ b/chrome/test/ui/mouseleave_uitest.cc
@@ -29,7 +29,7 @@
 #elif defined(OS_WIN)
 // Test succeeds locally, flaky on trybot
 // http://code.google.com/p/chromium/issues/detail?id=26349
-#define MAYBE_TestOnMouseOut DISABLED_TestOnMouseOut
+#define MAYBE_TestOnMouseOut FLAKY_TestOnMouseOut
 #endif
 
 namespace {
@@ -47,10 +47,12 @@ class MouseLeaveTest : public UITest {
 TEST_F(MouseLeaveTest, MAYBE_TestOnMouseOut) {
   GURL test_url = GetTestUrl(L"", L"mouseleave.html");
 
-  scoped_refptr<TabProxy> tab(GetActiveTab());
-  ASSERT_TRUE(tab.get());
   scoped_refptr<BrowserProxy> browser = automation()->GetBrowserWindow(0);
+  ASSERT_TRUE(browser.get());
   scoped_refptr<WindowProxy> window = browser->GetWindow();
+  ASSERT_TRUE(window.get());
+  scoped_refptr<TabProxy> tab(GetActiveTab());
+  ASSERT_TRUE(tab.get());
 
   gfx::Rect tab_view_bounds;
   ASSERT_TRUE(window->GetViewBounds(VIEW_ID_TAB_CONTAINER, &tab_view_bounds,
@@ -73,8 +75,7 @@ TEST_F(MouseLeaveTest, MAYBE_TestOnMouseOut) {
   // Wait for the onload() handler to complete so we can do the
   // next part of the test.
   ASSERT_TRUE(WaitUntilCookieValue(
-      tab.get(), test_url, "__state", timeout_ms,
-      "initial"));
+      tab.get(), test_url, "__state", timeout_ms, "initial"));
 
   // Move the cursor to the top-center of the content, which will trigger
   // a javascript onMouseOver event.
@@ -82,8 +83,7 @@ TEST_F(MouseLeaveTest, MAYBE_TestOnMouseOut) {
 
   // Wait on the correct intermediate value of the cookie.
   ASSERT_TRUE(WaitUntilCookieValue(
-      tab.get(), test_url, "__state", timeout_ms,
-      "initial,entered"));
+      tab.get(), test_url, "__state", timeout_ms, "initial,entered"));
 
   // Move the cursor above the content again, which should trigger
   // a javascript onMouseOut event.
@@ -91,8 +91,7 @@ TEST_F(MouseLeaveTest, MAYBE_TestOnMouseOut) {
 
   // Wait on the correct final value of the cookie.
   ASSERT_TRUE(WaitUntilCookieValue(
-      tab.get(), test_url, "__state", timeout_ms,
-      "initial,entered,left"));
+      tab.get(), test_url, "__state", timeout_ms, "initial,entered,left"));
 }
 
 }  // namespace
diff --git a/chrome/test/ui/omnibox_uitest.cc b/chrome/test/ui/omnibox_uitest.cc
index ebf5d7b..6ccb107 100644
--- a/chrome/test/ui/omnibox_uitest.cc
+++ b/chrome/test/ui/omnibox_uitest.cc
@@ -76,7 +76,9 @@ bool OmniboxTest::IsMatch(const std::wstring& input_text,
 void OmniboxTest::RunQueryChain(const std::wstring& input_text) {
   // Get a handle on the omnibox and give it focus.
   scoped_refptr<BrowserProxy> browser(automation()->GetBrowserWindow(0));
+  ASSERT_TRUE(browser.get());
   scoped_refptr<WindowProxy> window(browser->GetWindow());
+  ASSERT_TRUE(window.get());
   scoped_refptr<AutocompleteEditProxy> autocomplete_edit(
       browser->GetAutocompleteEdit());
   ASSERT_TRUE(browser->ApplyAccelerator(IDC_FOCUS_LOCATION));
@@ -90,7 +92,7 @@ void OmniboxTest::RunQueryChain(const std::wstring& input_text) {
     // matches to be returned to us.
     ASSERT_TRUE(autocomplete_edit->SetText(input_text.substr(0, i)));
     PerfTimer timer;
-    if (autocomplete_edit->WaitForQuery(30000)) {
+    if (autocomplete_edit->WaitForQuery(action_max_timeout_ms())) {
       ASSERT_TRUE(autocomplete_edit->GetAutocompleteMatches(&matches));
       int64 time_elapsed = timer.Elapsed().InMilliseconds();
 
diff --git a/chrome/test/ui/v8_benchmark_uitest.cc b/chrome/test/ui/v8_benchmark_uitest.cc
index 792328e..b020af1 100644
--- a/chrome/test/ui/v8_benchmark_uitest.cc
+++ b/chrome/test/ui/v8_benchmark_uitest.cc
@@ -40,6 +40,7 @@ class V8BenchmarkTest : public UITest {
     GURL test_url(net::FilePathToFileURL(test_path));
 
     scoped_refptr<TabProxy> tab(GetActiveTab());
+    ASSERT_TRUE(tab.get());
     ASSERT_EQ(AUTOMATION_MSG_NAVIGATION_SUCCESS, tab->NavigateToURL(test_url));
 
     // Wait for the test to finish.
diff --git a/chrome/worker/worker_uitest.cc b/chrome/worker/worker_uitest.cc
index b7cb91e..a356e96 100644
--- a/chrome/worker/worker_uitest.cc
+++ b/chrome/worker/worker_uitest.cc
@@ -33,14 +33,19 @@ class WorkerTest : public UILayoutTest {
 
   void RunIncognitoTest(const std::wstring& test_case) {
     scoped_refptr<BrowserProxy> browser(automation()->GetBrowserWindow(0));
+    ASSERT_TRUE(browser.get());
+
     // Open an Incognito window.
-    int window_count;
     ASSERT_TRUE(browser->RunCommand(IDC_NEW_INCOGNITO_WINDOW));
     scoped_refptr<BrowserProxy> incognito(automation()->GetBrowserWindow(1));
-    scoped_refptr<TabProxy> tab(incognito->GetTab(0));
+    ASSERT_TRUE(incognito.get());
+    int window_count;
     ASSERT_TRUE(automation()->GetBrowserWindowCount(&window_count));
     ASSERT_EQ(2, window_count);
 
+    scoped_refptr<TabProxy> tab(incognito->GetTab(0));
+    ASSERT_TRUE(tab.get());
+
     GURL url = GetTestUrl(L"workers", test_case);
     ASSERT_TRUE(tab->NavigateToURL(url));
 
@@ -155,9 +160,11 @@ const wchar_t kDocRoot[] = L"chrome/test/data/workers";
 TEST_F(WorkerTest, DISABLED_WorkerHttpAuth) {
   scoped_refptr<HTTPTestServer> server =
       HTTPTestServer::CreateServer(kDocRoot, NULL);
-
   ASSERT_TRUE(NULL != server.get());
+
   scoped_refptr<TabProxy> tab(GetActiveTab());
+  ASSERT_TRUE(tab.get());
+
   GURL url = server->TestServerPage("files/worker_auth.html");
   EXPECT_TRUE(NavigateAndWaitForAuth(tab, url));
 }
@@ -170,8 +177,10 @@ TEST_F(WorkerTest, DISABLED_SharedWorkerHttpAuth) {
   scoped_refptr<HTTPTestServer> server =
       HTTPTestServer::CreateServer(kDocRoot, NULL);
   ASSERT_TRUE(NULL != server.get());
+
   scoped_refptr<TabProxy> tab(GetActiveTab());
-  EXPECT_EQ(1, GetTabCount());
+  ASSERT_TRUE(tab.get());
+
   GURL url = server->TestServerPage("files/shared_worker_auth.html");
   EXPECT_TRUE(NavigateAndWaitForAuth(tab, url));
   // TODO(atwilson): Add support to automation framework to test for auth
@@ -489,10 +498,7 @@ TEST_F(WorkerTest, DISABLED_LimitPerPage) {
   GURL url = GetTestUrl(L"workers", L"many_workers.html");
   url = GURL(url.spec() + StringPrintf("?count=%d", max_workers_per_tab + 1));
 
-  scoped_refptr<TabProxy> tab(GetActiveTab());
-  ASSERT_TRUE(tab.get());
-  ASSERT_TRUE(tab->NavigateToURL(url));
-
+  NavigateToURL(url);
   ASSERT_TRUE(WaitForProcessCountToBe(1, max_workers_per_tab));
 }
 
@@ -570,6 +576,7 @@ TEST_F(WorkerTest, MultipleTabsQueuedSharedWorker) {
   // Create same set of workers in new tab (leaves one worker queued from this
   // tab).
   scoped_refptr<BrowserProxy> window(automation()->GetBrowserWindow(0));
+  ASSERT_TRUE(window.get());
   ASSERT_TRUE(window->AppendTab(url));
   ASSERT_TRUE(WaitForProcessCountToBe(2, max_workers_per_tab));
 
@@ -600,6 +607,7 @@ TEST_F(WorkerTest, DISABLED_QueuedSharedWorkerStartedFromOtherTab) {
   // the same worker that was queued in the first window, to ensure it gets
   // connected to the first window too.
   scoped_refptr<BrowserProxy> window(automation()->GetBrowserWindow(0));
+  ASSERT_TRUE(window.get());
   GURL url2 = GetTestUrl(L"workers", L"single_shared_worker.html");
   url2 = GURL(url2.spec() + StringPrintf("?id=%d", max_workers_per_tab));
   ASSERT_TRUE(window->AppendTab(url2));