Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(3313)

Unified Diff: chrome/browser/ui/browser_tabrestore_browsertest.cc

Issue 2727663004: Use TabLoader in TabRestoreService. (Closed)
Patch Set: Created 3 years, 10 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: chrome/browser/ui/browser_tabrestore_browsertest.cc
diff --git a/chrome/browser/ui/browser_tabrestore_browsertest.cc b/chrome/browser/ui/browser_tabrestore_browsertest.cc
index 5fd6ac3b360794502826296ed6e0d22dce54bf7c..0fdfad57e4bf9f8476ed94609b7f825d8c7b774d 100644
--- a/chrome/browser/ui/browser_tabrestore_browsertest.cc
+++ b/chrome/browser/ui/browser_tabrestore_browsertest.cc
@@ -13,6 +13,7 @@
#include "chrome/test/base/in_process_browser_test.h"
#include "chrome/test/base/interactive_test_utils.h"
#include "components/sessions/core/tab_restore_service.h"
+#include "content/public/browser/notification_service.h"
#include "content/public/browser/web_contents.h"
#include "content/public/common/url_constants.h"
#include "content/public/test/browser_test_utils.h"
@@ -90,6 +91,21 @@ IN_PROC_BROWSER_TEST_F(BrowserTabRestoreTest, RecentTabsMenuTabDisposition) {
EXPECT_EQ(2u, active_browser_list->size());
browser = active_browser_list->get(1);
EXPECT_EQ(3, browser->tab_strip_model()->count());
+ // For the two test tabs we've just received "READY" DOM message.
+ // But there won't be such message from the "about:blank" tab.
+ // And it is possible that TabLoader hasn't loaded it yet.
+ // Thus we should wait for "load stop" event before we will perform
+ // CheckVisbility on "about:blank".
+ {
+ const auto* about_blank = browser->tab_strip_model()->GetWebContentsAt(0);
sky 2017/03/02 19:12:05 about_blank_contents? optional: mild preference fo
Michael K. (Yandex Team) 2017/03/03 08:26:37 Done.
+ EXPECT_EQ("about:blank", about_blank->GetURL().spec());
+ content::WindowedNotificationObserver load_stop_observer(
sky 2017/03/02 19:12:05 Move this into if?
Michael K. (Yandex Team) 2017/03/03 08:26:37 Done.
+ content::NOTIFICATION_LOAD_STOP,
+ content::Source<content::NavigationController>(
+ &about_blank->GetController()));
+ if (about_blank->IsLoading() || about_blank->GetController().NeedsReload())
+ load_stop_observer.Wait();
+ }
// The middle tab only should have visible disposition.
CheckVisbility(browser->tab_strip_model(), 1);
@@ -132,6 +148,18 @@ IN_PROC_BROWSER_TEST_F(BrowserTabRestoreTest, DelegateRestoreTabDisposition) {
EXPECT_EQ(2u, active_browser_list->size());
browser = active_browser_list->get(1);
EXPECT_EQ(3, browser->tab_strip_model()->count());
+ // The same as in RecentTabsMenuTabDisposition test case.
+ // See there for the explanation.
+ {
+ const auto* about_blank = browser->tab_strip_model()->GetWebContentsAt(0);
sky 2017/03/02 19:12:05 Similar comments as above.
Michael K. (Yandex Team) 2017/03/03 08:26:37 Done.
+ EXPECT_EQ("about:blank", about_blank->GetURL().spec());
+ content::WindowedNotificationObserver load_stop_observer(
+ content::NOTIFICATION_LOAD_STOP,
+ content::Source<content::NavigationController>(
+ &about_blank->GetController()));
+ if (about_blank->IsLoading() || about_blank->GetController().NeedsReload())
+ load_stop_observer.Wait();
+ }
// The middle tab only should have visible disposition.
CheckVisbility(browser->tab_strip_model(), 1);

Powered by Google App Engine
This is Rietveld 408576698