Revert 46681 - 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/1933005

TBR=bauerb@chromium.org
Review URL: http://codereview.chromium.org/2037003

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

4 files changed, 19 insertions, 97 deletions

diff --git a/chrome/browser/automation/automation_provider.cc b/chrome/browser/automation/automation_provider.cc
index e907d45..76b9b9b 100644
--- a/chrome/browser/automation/automation_provider.cc
+++ b/chrome/browser/automation/automation_provider.cc
@@ -213,6 +213,11 @@ NotificationObserver* AutomationProvider::AddNavigationStatusListener(
   return observer;
 }
 
+void AutomationProvider::RemoveNavigationStatusListener(
+    NotificationObserver* obs) {
+  notification_observer_list_.RemoveObserver(obs);
+}
+
 NotificationObserver* AutomationProvider::AddTabStripObserver(
     Browser* parent,
     IPC::Message* reply_message) {
@@ -223,7 +228,7 @@ NotificationObserver* AutomationProvider::AddTabStripObserver(
   return observer;
 }
 
-void AutomationProvider::RemoveObserver(NotificationObserver* obs) {
+void AutomationProvider::RemoveTabStripObserver(NotificationObserver* obs) {
   notification_observer_list_.RemoveObserver(obs);
 }
 
@@ -491,9 +496,6 @@ 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)
@@ -597,7 +599,7 @@ void AutomationProvider::AppendTab(int handle, const GURL& url,
   if (append_tab_response < 0) {
     // The append tab failed. Remove the TabStripObserver
     if (observer) {
-      RemoveObserver(observer);
+      RemoveTabStripObserver(observer);
       delete observer;
     }
 
@@ -1589,29 +1591,6 @@ 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 9195244..83b52e6 100644
--- a/chrome/browser/automation/automation_provider.h
+++ b/chrome/browser/automation/automation_provider.h
@@ -79,21 +79,22 @@ 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 RemoveObserver method.
+  // deleted and should be released by calling the corresponding
+  // RemoveNavigationStatusListener 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 RemoveObserver method.
+  // NOT be deleted and should be released by calling the corresponding
+  // RemoveTabStripObserver method.
   NotificationObserver* AddTabStripObserver(Browser* parent,
                                             IPC::Message* reply_message);
-
-  // Remove an observer. The |NotificationObserver| still needs to be deleted
-  // afterwards.
-  void RemoveObserver(NotificationObserver* obs);
+  void RemoveTabStripObserver(NotificationObserver* obs);
 
   // Get the index of a particular NavigationController object
   // in the given parent window.  This method uses
@@ -333,11 +334,6 @@ 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 bb706d3..eb8e247 100644
--- a/chrome/browser/automation/automation_provider_observers.cc
+++ b/chrome/browser/automation/automation_provider_observers.cc
@@ -10,7 +10,6 @@
 #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"
@@ -160,7 +159,7 @@ NavigationNotificationObserver::~NavigationNotificationObserver() {
     reply_message_ = NULL;
   }
 
-  automation_->RemoveObserver(this);
+  automation_->RemoveNavigationStatusListener(this);
 }
 
 void NavigationNotificationObserver::Observe(
@@ -235,7 +234,7 @@ void TabStripNotificationObserver::Observe(NotificationType type,
     ObserveTab(Source<NavigationController>(source).ptr());
 
     // If verified, no need to observe anymore
-    automation_->RemoveObserver(this);
+    automation_->RemoveTabStripObserver(this);
     delete this;
   } else {
     NOTREACHED();
@@ -822,7 +821,7 @@ DocumentPrintedNotificationObserver::~DocumentPrintedNotificationObserver() {
   DCHECK(reply_message_ != NULL);
   AutomationMsg_PrintNow::WriteReplyParams(reply_message_, success_);
   automation_->Send(reply_message_);
-  automation_->RemoveObserver(this);
+  automation_->RemoveNavigationStatusListener(this);
 }
 
 void DocumentPrintedNotificationObserver::Observe(
@@ -908,36 +907,6 @@ 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));
-}
-
-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 1821d33..efb01e0 100644
--- a/chrome/browser/automation/automation_provider_observers.h
+++ b/chrome/browser/automation/automation_provider_observers.h
@@ -488,28 +488,6 @@ 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);
-
-  // 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:
@@ -558,8 +536,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() {}