From 5ea92a92e9fa26334db821da688d3c46e295d19c Mon Sep 17 00:00:00 2001 From: "bauerb@chromium.org" Date: Tue, 11 May 2010 12:42:51 +0000 Subject: 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 --- chrome/browser/automation/automation_provider.cc | 35 +++++++++++++++---- chrome/browser/automation/automation_provider.h | 18 ++++++---- .../automation/automation_provider_observers.cc | 40 ++++++++++++++++++++-- .../automation/automation_provider_observers.h | 27 +++++++++++++-- 4 files changed, 101 insertions(+), 19 deletions(-) (limited to 'chrome/browser/automation') 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, // 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, 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(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)); +} + +DownloadShelfVisibilityObserver::~DownloadShelfVisibilityObserver() { +} + +void DownloadShelfVisibilityObserver::Observe( + NotificationType type, + const NotificationSource& source, + const NotificationDetails& details) { + if (type == NotificationType::DOWNLOAD_SHELF_VISIBILITY_CHANGED) { + Browser* browser = Source(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() {} -- cgit v1.1