Changes ui_test_utils::WindowedNotificationObserver to allow Wait to

work for any source.

BUG=none
TEST=none
R=phajdan.jr@chromium.org

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

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

3 files changed, 34 insertions, 73 deletions

diff --git a/chrome/browser/instant/instant_browsertest.cc b/chrome/browser/instant/instant_browsertest.cc
index 1ded271..ce9827c 100644
--- a/chrome/browser/instant/instant_browsertest.cc
+++ b/chrome/browser/instant/instant_browsertest.cc
@@ -29,45 +29,6 @@
 #define EXPECT_STR_EQ(ascii, utf16) \
   EXPECT_EQ(ASCIIToWide(ascii), UTF16ToWide(utf16))
 
-namespace {
-
-// Wait for DOWNLOAD_INITIATED.
-class DownloadNotificationObserver : public NotificationObserver {
- public:
-  DownloadNotificationObserver() : running_(false), fired_(false) {
-    registrar_.Add(this, NotificationType::DOWNLOAD_INITIATED,
-                   NotificationService::AllSources());
-  }
-
-  void Run() {
-    if (fired_)
-      return;
-
-    running_ = true;
-    ui_test_utils::RunMessageLoop();
-    running_ = false;
-  }
-
-  bool fired() const { return fired_; }
-
-  virtual void Observe(NotificationType type,
-                       const NotificationSource& source,
-                       const NotificationDetails& details) OVERRIDE {
-    fired_ = true;
-    if (running_)
-      MessageLoopForUI::current()->Quit();
-  }
-
- private:
-  bool running_;
-  bool fired_;
-  NotificationRegistrar registrar_;
-
-  DISALLOW_COPY_AND_ASSIGN(DownloadNotificationObserver);
-};
-
-}  // namespace
-
 class InstantTest : public InProcessBrowserTest {
  public:
   InstantTest()
@@ -803,13 +764,13 @@ IN_PROC_BROWSER_TEST_F(InstantTest, MAYBE_DownloadOnEnter) {
       NotificationType::FAIL_PROVISIONAL_LOAD_WITH_ERROR);
 
   printf("2\n");
-  DownloadNotificationObserver download_observer;
+  ui_test_utils::WindowedNotificationObserver download_observer(
+      NotificationType::DOWNLOAD_INITIATED,
+      NotificationService::AllSources());
   ASSERT_NO_FATAL_FAILURE(SendKey(ui::VKEY_RETURN));
   printf("3\n");
-  download_observer.Run();
+  download_observer.Wait();
   printf("4\n");
-  // Pressing enter should initiate a download.
-  EXPECT_TRUE(download_observer.fired());
 
   // And we should end up at about:blank.
   TabContents* contents = browser()->GetSelectedTabContents();
diff --git a/chrome/test/ui_test_utils.cc b/chrome/test/ui_test_utils.cc
index cdd7d0a..4d0a49a 100644
--- a/chrome/test/ui_test_utils.cc
+++ b/chrome/test/ui_test_utils.cc
@@ -927,13 +927,10 @@ WindowedNotificationObserver::WindowedNotificationObserver(
 WindowedNotificationObserver::~WindowedNotificationObserver() {}
 
 void WindowedNotificationObserver::Wait() {
-  if (waiting_for_ == NotificationService::AllSources()) {
-    LOG(FATAL) << "Wait called when monitoring all sources. You must use "
-               << "WaitFor in this case.";
-  }
-
-  if (seen_)
+  if (seen_ || (waiting_for_ == NotificationService::AllSources() &&
+                !sources_seen_.empty())) {
     return;
+  }
 
   running_ = true;
   ui_test_utils::RunMessageLoop();
@@ -956,7 +953,8 @@ void WindowedNotificationObserver::WaitFor(const NotificationSource& source) {
 void WindowedNotificationObserver::Observe(NotificationType type,
                                            const NotificationSource& source,
                                            const NotificationDetails& details) {
-  if (waiting_for_ == source) {
+  if (waiting_for_ == source ||
+      (running_ && waiting_for_ == NotificationService::AllSources())) {
     seen_ = true;
     if (running_)
       MessageLoopForUI::current()->Quit();
diff --git a/chrome/test/ui_test_utils.h b/chrome/test/ui_test_utils.h
index c896dc4..42e5542 100644
--- a/chrome/test/ui_test_utils.h
+++ b/chrome/test/ui_test_utils.h
@@ -393,40 +393,42 @@ class TestNotificationObserver : public NotificationObserver {
 //   signal.Wait()
 class WindowedNotificationObserver : public NotificationObserver {
  public:
-  /* Register to listen for notifications of the given type from either
-   * a specific source, or from all sources if |source| is
-   * NotificationService::AllSources(). */
+  // Register to listen for notifications of the given type from either a
+  // specific source, or from all sources if |source| is
+  // NotificationService::AllSources().
   WindowedNotificationObserver(NotificationType notification_type,
                                const NotificationSource& source);
   virtual ~WindowedNotificationObserver();
 
-  /* Wait until the specified notification occurs. You must have specified a
-   * source in the arguments to the constructor in order to use this function.
-   * Otherwise, you should use WaitFor. */
+  // Wait until the specified notification occurs.  If the notification was
+  // emitted between the construction of this object and this call then it
+  // returns immediately.
   void Wait();
 
-  /* WaitFor waits until the given notification type is received from the
-   * given object. If the notification was emitted between the construction of
-   * this object and this call then it returns immediately.
-   *
-   * Beware that this is inheriently plagued by ABA issues. Consider:
-   *   WindowedNotificationObserver is created, listening for notifications from
-   *     all sources
-   *   Object A is created with address x and fires a notification
-   *   Object A is freed
-   *   Object B is created with the same address
-   *   WaitFor is called with the address of B
-   *
-   * In this case, WaitFor will return immediately because of the
-   * notification from A (because they shared an address), despite being
-   * different objects.
-   */
+  // WaitFor waits until the given notification type is received from the
+  // given object. If the notification was emitted between the construction of
+  // this object and this call then it returns immediately.
+  //
+  // Use this variant when you supply AllSources to the constructor but want
+  // to wait for notification from a specific observer.
+  //
+  // Beware that this is inheriently plagued by ABA issues. Consider:
+  //   WindowedNotificationObserver is created, listening for notifications from
+  //     all sources
+  //   Object A is created with address x and fires a notification
+  //   Object A is freed
+  //   Object B is created with the same address
+  //   WaitFor is called with the address of B
+  //
+  // In this case, WaitFor will return immediately because of the
+  // notification from A (because they shared an address), despite being
+  // different objects.
   void WaitFor(const NotificationSource& source);
 
   // NotificationObserver:
   virtual void Observe(NotificationType type,
                        const NotificationSource& source,
-                       const NotificationDetails& details);
+                       const NotificationDetails& details) OVERRIDE;
 
  private:
   bool seen_;