[GTTF] Clean up browser tests:

- use more ui_test_utils functions to simplify the code
- expose more consistent and powerful utilities in ui_test_utils
- minor style improvements
- move some tests from DISABLED to FLAKY so we don't lose coverage
- remove redundant timeouts
- check more return values

TEST=browser_tests
BUG=none

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

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

1 files changed, 13 insertions, 39 deletions

diff --git a/chrome/browser/download/save_page_browsertest.cc b/chrome/browser/download/save_page_browsertest.cc
index 142ed9a..799c6ac 100644
--- a/chrome/browser/download/save_page_browsertest.cc
+++ b/chrome/browser/download/save_page_browsertest.cc
@@ -16,6 +16,8 @@
 #include "chrome/test/ui_test_utils.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
+namespace {
+
 static const FilePath::CharType* kTestDir = FILE_PATH_LITERAL("save_page");
 
 static const char* kAppendedExtension =
@@ -25,37 +27,6 @@ static const char* kAppendedExtension =
     ".html";
 #endif
 
-namespace {
-
-class SavePageFinishedObserver : public NotificationObserver {
- public:
-  SavePageFinishedObserver() {
-    registrar_.Add(this, NotificationType::SAVE_PACKAGE_SUCCESSFULLY_FINISHED,
-                   NotificationService::AllSources());
-    ui_test_utils::RunMessageLoop();
-  }
-
-  GURL page_url() const { return page_url_; }
-
-  virtual void Observe(NotificationType type,
-                       const NotificationSource& source,
-                       const NotificationDetails& details) {
-    if (type == NotificationType::SAVE_PACKAGE_SUCCESSFULLY_FINISHED) {
-      page_url_ = *Details<GURL>(details).ptr();
-      MessageLoopForUI::current()->Quit();
-    } else {
-      NOTREACHED();
-    }
-  }
-
- private:
-  NotificationRegistrar registrar_;
-
-  GURL page_url_;
-
-  DISALLOW_COPY_AND_ASSIGN(SavePageFinishedObserver);
-};
-
 class SavePageBrowserTest : public InProcessBrowserTest {
  protected:
   void SetUp() {
@@ -64,6 +35,14 @@ class SavePageBrowserTest : public InProcessBrowserTest {
     InProcessBrowserTest::SetUp();
   }
 
+  GURL WaitForSavePackageToFinish() {
+    ui_test_utils::TestNotificationObserver observer;
+    ui_test_utils::RegisterAndWait(&observer,
+        NotificationType::SAVE_PACKAGE_SUCCESSFULLY_FINISHED,
+        NotificationService::AllSources());
+    return *Details<GURL>(observer.details()).ptr();
+  }
+
   // Path to directory containing test data.
   FilePath test_dir_;
 
@@ -85,9 +64,7 @@ IN_PROC_BROWSER_TEST_F(SavePageBrowserTest, SaveHTMLOnly) {
   ASSERT_TRUE(current_tab->SavePage(full_file_name, dir,
                                     SavePackage::SAVE_AS_ONLY_HTML));
 
-  SavePageFinishedObserver observer;
-
-  EXPECT_EQ(url, observer.page_url());
+  EXPECT_EQ(url, WaitForSavePackageToFinish());
 
   if (browser()->SupportsWindowFeature(Browser::FEATURE_DOWNLOADSHELF))
     EXPECT_TRUE(browser()->window()->IsDownloadShelfVisible());
@@ -113,9 +90,7 @@ IN_PROC_BROWSER_TEST_F(SavePageBrowserTest, SaveCompleteHTML) {
   ASSERT_TRUE(current_tab->SavePage(full_file_name, dir,
                                     SavePackage::SAVE_AS_COMPLETE_HTML));
 
-  SavePageFinishedObserver observer;
-
-  EXPECT_EQ(url, observer.page_url());
+  EXPECT_EQ(url, WaitForSavePackageToFinish());
 
   if (browser()->SupportsWindowFeature(Browser::FEATURE_DOWNLOADSHELF))
     EXPECT_TRUE(browser()->window()->IsDownloadShelfVisible());
@@ -156,9 +131,8 @@ IN_PROC_BROWSER_TEST_F(SavePageBrowserTest, FileNameFromPageTitle) {
 
   ASSERT_TRUE(current_tab->SavePage(full_file_name, dir,
                                     SavePackage::SAVE_AS_COMPLETE_HTML));
-  SavePageFinishedObserver observer;
 
-  EXPECT_EQ(url, observer.page_url());
+  EXPECT_EQ(url, WaitForSavePackageToFinish());
 
   if (browser()->SupportsWindowFeature(Browser::FEATURE_DOWNLOADSHELF))
     EXPECT_TRUE(browser()->window()->IsDownloadShelfVisible());