Fix restore of a tab from a recently-closed browser window

chrome/browser/sessions/tab_restore_browsertest.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include <stddef.h>
 
 #include "base/command_line.h"
 #include "base/files/file_path.h"
 #include "base/macros.h"
 #include "base/run_loop.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/test/test_timeouts.h"
 #include "build/build_config.h"
 #include "chrome/app/chrome_command_ids.h"
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/sessions/tab_restore_service_factory.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_commands.h"
 #include "chrome/browser/ui/browser_list.h"
+#include "chrome/browser/ui/browser_live_tab_context.h"
 #include "chrome/browser/ui/browser_tabstrip.h"
 #include "chrome/browser/ui/find_bar/find_notification_details.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/common/chrome_paths.h"
 #include "chrome/common/url_constants.h"
 #include "chrome/test/base/in_process_browser_test.h"
 #include "chrome/test/base/ui_test_utils.h"
 #include "components/sessions/core/tab_restore_service.h"
 #include "components/sessions/core/tab_restore_service_observer.h"
 #include "content/public/browser/navigation_controller.h"
@@ skipping 373 matching lines @@
   EXPECT_EQ(url2_,
             browser->tab_strip_model()->GetActiveWebContents()->GetURL());
 
   // Restore the next-to-last-closed tab into the same window.
   ASSERT_NO_FATAL_FAILURE(RestoreTab(1, 0));
   EXPECT_EQ(2, browser->tab_strip_model()->count());
   EXPECT_EQ(url1_,
             browser->tab_strip_model()->GetActiveWebContents()->GetURL());
 }
 
+// Open a window with two tabs, close both (closing the window), then restore
+// one by ID. Guards against regression of crbug.com/622752.
+IN_PROC_BROWSER_TEST_F(TabRestoreTest, RestoreTabFromClosedWindowByID) {
+  ui_test_utils::NavigateToURLWithDisposition(
+      browser(), url1_, NEW_FOREGROUND_TAB,
+      ui_test_utils::BROWSER_TEST_WAIT_FOR_NAVIGATION);
+  ui_test_utils::NavigateToURLWithDisposition(
+      browser(), url2_, NEW_FOREGROUND_TAB,
+      ui_test_utils::BROWSER_TEST_WAIT_FOR_NAVIGATION);
+
+  // Create a new browser.
+  ui_test_utils::NavigateToURLWithDisposition(
+      browser(), GURL(chrome::kChromeUINewTabURL), NEW_WINDOW,
+      ui_test_utils::BROWSER_TEST_WAIT_FOR_BROWSER);
+  EXPECT_EQ(2u, active_browser_list_->size());
+
+  // Close the window.
+  content::WindowedNotificationObserver close_window_observer(
+      chrome::NOTIFICATION_BROWSER_CLOSED,
+      content::NotificationService::AllSources());
+  chrome::CloseWindow(browser());
+  close_window_observer.Wait();
+  EXPECT_EQ(1u, active_browser_list_->size());
+
+  // Check that the TabRestoreService has the contents of the closed window.
+  Browser* browser = GetBrowser(0);
+  sessions::TabRestoreService* service =
+      TabRestoreServiceFactory::GetForProfile(browser->profile());
+  const sessions::TabRestoreService::Entries& entries = service->entries();
+  EXPECT_EQ(1u, entries.size());
+  sessions::TabRestoreService::Entry* entry = entries.front();
+  EXPECT_EQ(entry->type, sessions::TabRestoreService::WINDOW);

[sky, 2016/07/19 17:42:40]

nit: assertions have expected, actual. You want WINDOW first.
Also, make this an ASSERT as if it's not a window then the case on the next line
is likely to cause bad things to happen.

[blundell, 2016/07/20 08:18:11]

Done.

+  sessions::TabRestoreService::Window* entry_win =
+      static_cast<sessions::TabRestoreService::Window*>(entry);
+  std::vector<sessions::TabRestoreService::Tab>& tabs = entry_win->tabs;
+  EXPECT_EQ(3u, tabs.size());
+
+  // Find the Tab to restore.
+  sessions::TabRestoreService::Tab tab_to_restore;
+  bool found_tab_to_restore = false;
+  for (const auto& tab : tabs) {
+    if (tab.navigations[tab.current_navigation_index].virtual_url() == url1_) {
+      tab_to_restore = tab;
+      found_tab_to_restore = true;
+      break;
+    }
+  }
+  EXPECT_TRUE(found_tab_to_restore);

[sky, 2016/07/19 17:42:40]

ASSERT, as if you didn't find it tab_to_restore is bad.

[blundell, 2016/07/20 08:18:11]

Done.

+
+  // Restore the tab into the current window.
+  EXPECT_EQ(1, browser->tab_strip_model()->count());
+  content::WindowedNotificationObserver tab_added_observer(
+      chrome::NOTIFICATION_TAB_PARENTED,
+      content::NotificationService::AllSources());
+  content::WindowedNotificationObserver tab_loaded_observer(
+      content::NOTIFICATION_LOAD_STOP,
+      content::NotificationService::AllSources());
+  service->RestoreEntryById(browser->live_tab_context(), tab_to_restore.id,
+                            NEW_FOREGROUND_TAB);
+  tab_added_observer.Wait();
+  tab_loaded_observer.Wait();
+
+  // Check that the tab was correctly restored.
+  EXPECT_EQ(2, browser->tab_strip_model()->count());
+  EXPECT_EQ(url1_,
+            browser->tab_strip_model()->GetActiveWebContents()->GetURL());
+}
+
 // Tests that a duplicate history entry is not created when we restore a page
 // to an existing SiteInstance.  (Bug 1230446)
 IN_PROC_BROWSER_TEST_F(TabRestoreTest, RestoreWithExistingSiteInstance) {
   ASSERT_TRUE(embedded_test_server()->Start());
 
   GURL http_url1(embedded_test_server()->GetURL("/title1.html"));
   GURL http_url2(embedded_test_server()->GetURL("/title2.html"));
   int tab_count = browser()->tab_strip_model()->count();
 
   // Add a tab
@@ skipping 208 matching lines @@
   while (browser()->tab_strip_model()->count())
     CloseTab(0);
 }
 
 // Verifies restoring a tab works on startup.
 IN_PROC_BROWSER_TEST_F(TabRestoreTest, RestoreOnStartup) {
   ASSERT_NO_FATAL_FAILURE(RestoreTab(0, 1));
   EXPECT_EQ(url1_,
             browser()->tab_strip_model()->GetWebContentsAt(1)->GetURL());
 }