diff options
| -rw-r--r-- | chrome/browser/ssl/ssl_browser_tests.cc | 21 | ||||
| -rw-r--r-- | chrome/test/ui_test_utils.h | 101 |
2 files changed, 107 insertions, 15 deletions
diff --git a/chrome/browser/ssl/ssl_browser_tests.cc b/chrome/browser/ssl/ssl_browser_tests.cc index 711a22e..bdaeb61 100644 --- a/chrome/browser/ssl/ssl_browser_tests.cc +++ b/chrome/browser/ssl/ssl_browser_tests.cc @@ -220,24 +220,11 @@ IN_PROC_BROWSER_TEST_F(SSLUITest, TestHTTPSExpiredCertAndDontProceed) { CheckUnauthenticatedState(tab); } -#if defined(OS_LINUX) && defined(TOOLKIT_VIEWS) -// TODO(jcampan|oshima): On linux/views, the WaitForLoadStop call -// below sometimes waits forever because LOAD_STOP notification can -// happen before WaitLorLoadStop is called. Marking this test as Flaky. -// See http://crbug.com/28098. -#define MAYBE_TestHTTPSErrorWithNoNavEntry FLAKY_TestHTTPSErrorWithNoNavEntry -#elif defined(OS_MACOSX) -// Also flaky on Mac. http://crbug.com/29992 -#define MAYBE_TestHTTPSErrorWithNoNavEntry FLAKY_TestHTTPSErrorWithNoNavEntry -#else -#define MAYBE_TestHTTPSErrorWithNoNavEntry TestHTTPSErrorWithNoNavEntry -#endif - // Open a page with a HTTPS error in a tab with no prior navigation (through a // link with a blank target). This is to test that the lack of navigation entry // does not cause any problems (it was causing a crasher, see // http://crbug.com/19941). -IN_PROC_BROWSER_TEST_F(SSLUITest, MAYBE_TestHTTPSErrorWithNoNavEntry) { +IN_PROC_BROWSER_TEST_F(SSLUITest, TestHTTPSErrorWithNoNavEntry) { scoped_refptr<HTTPTestServer> http_server = PlainServer(); ASSERT_TRUE(http_server.get() != NULL); scoped_refptr<HTTPSTestServer> bad_https_server = BadCertServer(); @@ -249,6 +236,10 @@ IN_PROC_BROWSER_TEST_F(SSLUITest, MAYBE_TestHTTPSErrorWithNoNavEntry) { L"files/ssl/page_with_blank_target.html")); bool success = false; + + ui_test_utils::WindowedNotificationObserver<NavigationController> + load_stop_signal(NotificationType::LOAD_STOP, NULL); + // Simulate clicking the link (and therefore navigating to that new page). // This will causes a new tab to be created. EXPECT_TRUE(ui_test_utils::ExecuteJavaScriptAndExtractBool( @@ -265,7 +256,7 @@ IN_PROC_BROWSER_TEST_F(SSLUITest, MAYBE_TestHTTPSErrorWithNoNavEntry) { // Since the navigation was initiated by the renderer (when we clicked on the // link) and since the main page network request failed, we won't get a // navigation entry committed. So we'll just wait for the load to stop. - ui_test_utils::WaitForLoadStop( + load_stop_signal.WaitFor( &(browser()->GetSelectedTabContents()->controller())); // We should have an interstitial page showing. diff --git a/chrome/test/ui_test_utils.h b/chrome/test/ui_test_utils.h index 2e5d52a..74482b8 100644 --- a/chrome/test/ui_test_utils.h +++ b/chrome/test/ui_test_utils.h @@ -6,13 +6,17 @@ #define CHROME_TEST_UI_TEST_UTILS_H_ #include <string> +#include <set> #include "base/basictypes.h" +#include "base/message_loop.h" #include "base/scoped_temp_dir.h" #include "base/string16.h" #include "chrome/browser/view_ids.h" #include "chrome/common/notification_observer.h" +#include "chrome/common/notification_registrar.h" #include "chrome/common/notification_type.h" +#include "chrome/common/notification_service.h" class AppModalDialog; class Browser; @@ -218,6 +222,103 @@ class TestWebSocketServer { DISALLOW_COPY_AND_ASSIGN(TestWebSocketServer); }; +// A WindowedNotificationObserver allows code to watch for a notification +// over a window of time. Typically testing code will need to do something +// like this: +// PerformAction() +// WaitForCompletionNotification() +// This leads to flakiness as there's a window between PerformAction returning +// and the observers getting registered, where a notification will be missed. +// +// Rather, one can do this: +// WindowedNotificationObserver<type> signal(...) +// PerformAction() +// wait_for_signal.Wait() +template <class T> +class WindowedNotificationObserver : public NotificationObserver { + public: + /* Register to listen for notifications of the given type from either + * a specific source, of from all sources if |source| is NULL */ + WindowedNotificationObserver(NotificationType notification_type, + T* source) + : seen_(false), + running_(false), + waiting_for_(source) { + if (source) { + registrar_.Add(this, notification_type, waiting_for_); + } else { + registrar_.Add(this, notification_type, + NotificationService::AllSources()); + } + } + + /* Wait sleeps 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. */ + void Wait() { + if (!waiting_for_.ptr()) { + LOG(FATAL) << "Wait called when monitoring all sources. You must use " + << "WaitFor in this case."; + } + + if (seen_) + return; + + running_ = true; + ui_test_utils::RunMessageLoop(); + } + + /* 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 with NULL source + * 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(T* source) { + if (waiting_for_.ptr()) { + LOG(FATAL) << "WaitFor called when already waiting on a specific " + << "source. Use Wait in this case."; + } + + waiting_for_ = Source<T>(source); + if (sources_seen_.count(waiting_for_.map_key()) > 0) + return; + + running_ = true; + ui_test_utils::RunMessageLoop(); + } + + virtual void Observe(NotificationType type, + const NotificationSource& source, + const NotificationDetails& details) { + if (waiting_for_ == source) { + seen_ = true; + if (running_) + MessageLoopForUI::current()->Quit(); + } else { + sources_seen_.insert(source.map_key()); + } + } + + private: + bool seen_; + bool running_; + std::set<uintptr_t> sources_seen_; + Source<T> waiting_for_; + NotificationRegistrar registrar_; + + DISALLOW_COPY_AND_ASSIGN(WindowedNotificationObserver); +}; + } // namespace ui_test_utils #endif // CHROME_TEST_UI_TEST_UTILS_H_ |