Reland r46681: Use IPC to wait for download in DownloadTest.

Add AutomationMsg_WaitForDownloadShelfVisibilityChange and use it in UITestBase for the download tests.

This should decrease flakiness of download tests.

TEST=DownloadTest.*, especially DownloadTest.[Dont]CloseNewTab*
BUG=43066

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

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

4 files changed, 101 insertions, 19 deletions

diff --git a/chrome/browser/automation/automation_provider.cc b/chrome/browser/automation/automation_provider.cc
index 93ad3d5..bd341a6 100644
--- a/chrome/browser/automation/automation_provider.cc
+++ b/chrome/browser/automation/automation_provider.cc
@@ -213,11 +213,6 @@ NotificationObserver* AutomationProvider::AddNavigationStatusListener(
   return observer;
 }
 
-void AutomationProvider::RemoveNavigationStatusListener(
-    NotificationObserver* obs) {
-  notification_observer_list_.RemoveObserver(obs);
-}
-
 NotificationObserver* AutomationProvider::AddTabStripObserver(
     Browser* parent,
     IPC::Message* reply_message) {
@@ -228,7 +223,7 @@ NotificationObserver* AutomationProvider::AddTabStripObserver(
   return observer;
 }
 
-void AutomationProvider::RemoveTabStripObserver(NotificationObserver* obs) {
+void AutomationProvider::RemoveObserver(NotificationObserver* obs) {
   notification_observer_list_.RemoveObserver(obs);
 }
 
@@ -496,6 +491,9 @@ void AutomationProvider::OnMessageReceived(const IPC::Message& message) {
     IPC_MESSAGE_HANDLER(AutomationMsg_SetEnableExtensionAutomation,
                         SetEnableExtensionAutomation)
 #endif
+    IPC_MESSAGE_HANDLER_DELAY_REPLY(
+        AutomationMsg_WaitForDownloadShelfVisibilityChange,
+        WaitForDownloadShelfVisibilityChange)
     IPC_MESSAGE_HANDLER(AutomationMsg_SetShelfVisibility, SetShelfVisibility)
     IPC_MESSAGE_HANDLER(AutomationMsg_BlockedPopupCount, GetBlockedPopupCount)
     IPC_MESSAGE_HANDLER(AutomationMsg_SelectAll, SelectAll)
@@ -599,7 +597,7 @@ void AutomationProvider::AppendTab(int handle, const GURL& url,
   if (append_tab_response < 0) {
     // The append tab failed. Remove the TabStripObserver
     if (observer) {
-      RemoveTabStripObserver(observer);
+      RemoveObserver(observer);
       delete observer;
     }
 
@@ -1591,6 +1589,29 @@ void AutomationProvider::RemoveBookmark(int handle,
   *success = false;
 }
 
+void AutomationProvider::WaitForDownloadShelfVisibilityChange(
+    int browser_handle,
+    bool visibility,
+    IPC::Message* reply_message) {
+  bool success = false;
+  if (browser_tracker_->ContainsHandle(browser_handle)) {
+    Browser* browser = browser_tracker_->GetResource(browser_handle);
+    if (browser->window()->IsDownloadShelfVisible() == visibility) {
+      success = true;
+    } else {
+      notification_observer_list_.AddObserver(
+          new DownloadShelfVisibilityObserver(this,
+                                              browser,
+                                              visibility,
+                                              reply_message));
+      return;
+    }
+  }
+  AutomationMsg_WaitForDownloadShelfVisibilityChange::WriteReplyParams(
+      reply_message, success);
+  Send(reply_message);
+}
+
 // Sample json input: { "command": "GetHistoryInfo",
 //                      "search_text": "some text" }
 // Refer chrome/test/pyautolib/history_info.py for sample json output.
diff --git a/chrome/browser/automation/automation_provider.h b/chrome/browser/automation/automation_provider.h
index 5662835..bf53b36 100644
--- a/chrome/browser/automation/automation_provider.h
+++ b/chrome/browser/automation/automation_provider.h
@@ -79,22 +79,21 @@ class AutomationProvider : public base::RefCounted<AutomationProvider>,
   // complete, the completed_response object is sent; if the server requires
   // authentication, we instead send the auth_needed_response object.  A pointer
   // to the added navigation observer is returned. This object should NOT be
-  // deleted and should be released by calling the corresponding
-  // RemoveNavigationStatusListener method.
+  // deleted and should be released by calling the RemoveObserver method.
   NotificationObserver* AddNavigationStatusListener(
       NavigationController* tab, IPC::Message* reply_message,
       int number_of_navigations, bool include_current_navigation);
 
-  void RemoveNavigationStatusListener(NotificationObserver* obs);
-
   // Add an observer for the TabStrip. Currently only Tab append is observed. A
   // navigation listener is created on successful notification of tab append. A
   // pointer to the added navigation observer is returned. This object should
-  // NOT be deleted and should be released by calling the corresponding
-  // RemoveTabStripObserver method.
+  // NOT be deleted and should be released by calling the RemoveObserver method.
   NotificationObserver* AddTabStripObserver(Browser* parent,
                                             IPC::Message* reply_message);
-  void RemoveTabStripObserver(NotificationObserver* obs);
+
+  // Remove an observer. The |NotificationObserver| still needs to be deleted
+  // afterwards.
+  void RemoveObserver(NotificationObserver* obs);
 
   // Get the index of a particular NavigationController object
   // in the given parent window.  This method uses
@@ -334,6 +333,11 @@ class AutomationProvider : public base::RefCounted<AutomationProvider>,
                       int64 id,
                       bool* success);
 
+  // Wait for the download shelf to appear or disappear.
+  void WaitForDownloadShelfVisibilityChange(int browser_handle,
+                                            bool visibility,
+                                            IPC::Message* reply_message);
+
   // Get info about downloads. This includes only ones that have been
   // registered by the history system.
   // Uses the JSON interface for input/output.
diff --git a/chrome/browser/automation/automation_provider_observers.cc b/chrome/browser/automation/automation_provider_observers.cc
index 3ca82e6..c6343c0 100644
--- a/chrome/browser/automation/automation_provider_observers.cc
+++ b/chrome/browser/automation/automation_provider_observers.cc
@@ -10,6 +10,7 @@
 #include "chrome/app/chrome_dll_resource.h"
 #include "chrome/browser/automation/automation_provider.h"
 #include "chrome/browser/bookmarks/bookmark_model.h"
+#include "chrome/browser/browser_window.h"
 #include "chrome/browser/dom_operation_notification_details.h"
 #include "chrome/browser/extensions/extension_host.h"
 #include "chrome/browser/extensions/extension_process_manager.h"
@@ -159,7 +160,7 @@ NavigationNotificationObserver::~NavigationNotificationObserver() {
     reply_message_ = NULL;
   }
 
-  automation_->RemoveNavigationStatusListener(this);
+  automation_->RemoveObserver(this);
 }
 
 void NavigationNotificationObserver::Observe(
@@ -234,7 +235,7 @@ void TabStripNotificationObserver::Observe(NotificationType type,
     ObserveTab(Source<NavigationController>(source).ptr());
 
     // If verified, no need to observe anymore
-    automation_->RemoveTabStripObserver(this);
+    automation_->RemoveObserver(this);
     delete this;
   } else {
     NOTREACHED();
@@ -821,7 +822,7 @@ DocumentPrintedNotificationObserver::~DocumentPrintedNotificationObserver() {
   DCHECK(reply_message_ != NULL);
   AutomationMsg_PrintNow::WriteReplyParams(reply_message_, success_);
   automation_->Send(reply_message_);
-  automation_->RemoveNavigationStatusListener(this);
+  automation_->RemoveObserver(this);
 }
 
 void DocumentPrintedNotificationObserver::Observe(
@@ -907,6 +908,39 @@ void LoginManagerObserver::Observe(NotificationType type,
 }
 #endif
 
+DownloadShelfVisibilityObserver::DownloadShelfVisibilityObserver(
+    AutomationProvider* automation,
+    Browser* browser,
+    bool visibility,
+    IPC::Message* reply_message)
+      : automation_(automation),
+        visibility_(visibility),
+        reply_message_(reply_message) {
+  registrar_.Add(this, NotificationType::DOWNLOAD_SHELF_VISIBILITY_CHANGED,
+                 Source<Browser>(browser));
+}
+
+DownloadShelfVisibilityObserver::~DownloadShelfVisibilityObserver() {
+}
+
+void DownloadShelfVisibilityObserver::Observe(
+    NotificationType type,
+    const NotificationSource& source,
+    const NotificationDetails& details) {
+  if (type == NotificationType::DOWNLOAD_SHELF_VISIBILITY_CHANGED) {
+    Browser* browser = Source<Browser>(source).ptr();
+    if (browser->window()->IsDownloadShelfVisible() == visibility_) {
+      AutomationMsg_WaitForDownloadShelfVisibilityChange::WriteReplyParams(
+          reply_message_, true);
+      automation_->Send(reply_message_);
+      automation_->RemoveObserver(this);
+      delete this;
+    }
+  } else {
+    NOTREACHED();
+  }
+}
+
 AutomationProviderBookmarkModelObserver::AutomationProviderBookmarkModelObserver(
     AutomationProvider* provider,
     IPC::Message* reply_message,
diff --git a/chrome/browser/automation/automation_provider_observers.h b/chrome/browser/automation/automation_provider_observers.h
index efb01e0..e6e434e 100644
--- a/chrome/browser/automation/automation_provider_observers.h
+++ b/chrome/browser/automation/automation_provider_observers.h
@@ -488,6 +488,29 @@ class LoginManagerObserver : public NotificationObserver {
 };
 #endif
 
+// Waits for the download shelf to appear or disappear
+// (depending on |visibility|).
+class DownloadShelfVisibilityObserver : public NotificationObserver {
+ public:
+  DownloadShelfVisibilityObserver(AutomationProvider* automation,
+                                  Browser* browser,
+                                  bool visiblity,
+                                  IPC::Message* reply_message);
+  ~DownloadShelfVisibilityObserver();
+
+  // NotificationObserver interface.
+  virtual void Observe(NotificationType type, const NotificationSource& source,
+                       const NotificationDetails& details);
+
+ private:
+  NotificationRegistrar registrar_;
+  AutomationProvider* automation_;
+  bool visibility_;
+  IPC::Message* reply_message_;
+
+  DISALLOW_COPY_AND_ASSIGN(DownloadShelfVisibilityObserver);
+};
+
 // Waits for the bookmark model to load.
 class AutomationProviderBookmarkModelObserver : BookmarkModelObserver {
  public:
@@ -536,8 +559,8 @@ class AutomationProviderBookmarkModelObserver : BookmarkModelObserver {
 
 // When asked for pending downloads, the DownloadManager places
 // results in a DownloadManager::Observer.
-class AutomationProviderDownloadManagerObserver :
-    public DownloadManager::Observer {
+class AutomationProviderDownloadManagerObserver
+    : public DownloadManager::Observer {
  public:
   AutomationProviderDownloadManagerObserver() : DownloadManager::Observer()  {}
   virtual ~AutomationProviderDownloadManagerObserver() {}