 Chromium Code Reviews
 Chromium Code Reviews Issue 2727663004:
  Use TabLoader in TabRestoreService.  (Closed)
    
  
    Issue 2727663004:
  Use TabLoader in TabRestoreService.  (Closed) 
  | 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); |