Implement restoring closed tab menu item. Reworked cross-platform code to handle the case of restoring when there are no browsers open by making window restore re-use a given browser if it has no tabs. Removed unit test that assumes it can pass a NULL Browser. Wrote a new UI test to cover the same area, but it's disabled until another bug is fixed, and I didn't want to hold up landing this feature. Added key shortcut for "open window in incognito mode".

BUG=13758
TEST=restoring closed tabs with visible windows and with no windows remaining open.
Review URL: http://codereview.chromium.org/125257

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@18806 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 52 insertions, 67 deletions

diff --git a/chrome/browser/sessions/tab_restore_service.cc b/chrome/browser/sessions/tab_restore_service.cc
index 2915deb..4bbd660 100644
--- a/chrome/browser/sessions/tab_restore_service.cc
+++ b/chrome/browser/sessions/tab_restore_service.cc
@@ -218,58 +218,61 @@ void TabRestoreService::RestoreEntryById(Browser* browser,
   Entry* entry = *i;
   entries_.erase(i);
   i = entries_.end();
-  if (browser) {  // Browser is null during testing.
-    if (entry->type == TAB) {
-      Tab* tab = static_cast<Tab*>(entry);
-      if (replace_existing_tab) {
-        browser->ReplaceRestoredTab(tab->navigations,
-                                    tab->current_navigation_index);
+
+  // |browser| will be NULL in cases where one isn't already available (eg,
+  // when invoked on Mac OS X with no windows open). In this case, create a
+  // new browser into which we restore the tabs.
+  if (entry->type == TAB) {
+    Tab* tab = static_cast<Tab*>(entry);
+    if (replace_existing_tab && browser) {
+      browser->ReplaceRestoredTab(tab->navigations,
+                                  tab->current_navigation_index);
+    } else {
+      // Use the tab's former browser and index, if available.
+      Browser* tab_browser = NULL;
+      int tab_index = -1;
+      if (tab->has_browser())
+        tab_browser = BrowserList::FindBrowserWithID(tab->browser_id);
+
+      if (tab_browser) {
+        tab_index = tab->tabstrip_index;
       } else {
-        // Use the tab's former browser and index, if available.
-        Browser* tab_browser = NULL;
-        int tab_index = -1;
-        if (tab->has_browser())
-          tab_browser = BrowserList::FindBrowserWithID(tab->browser_id);
-
-        if (tab_browser) {
-          tab_index = tab->tabstrip_index;
-        } else {
-          tab_browser = Browser::Create(profile());
-          if (tab->has_browser()) {
-            UpdateTabBrowserIDs(tab->browser_id,
-                                tab_browser->session_id().id());
-          }
-          tab_browser->window()->Show();
+        tab_browser = Browser::Create(profile());
+        if (tab->has_browser()) {
+          UpdateTabBrowserIDs(tab->browser_id,
+                              tab_browser->session_id().id());
         }
-
-        if (tab_index < 0 || tab_index > tab_browser->tab_count())
-          tab_index = tab_browser->tab_count();
-        tab_browser->AddRestoredTab(tab->navigations, tab_index,
-                                    tab->current_navigation_index, true);
-      }
-    } else if (entry->type == WINDOW) {
-      const Window* window = static_cast<Window*>(entry);
-      Browser* browser = Browser::Create(profile());
-      for (size_t tab_i = 0; tab_i < window->tabs.size(); ++tab_i) {
-        const Tab& tab = window->tabs[tab_i];
-        TabContents* restored_tab =
-            browser->AddRestoredTab(tab.navigations, browser->tab_count(),
-                                    tab.current_navigation_index,
-                                    (static_cast<int>(tab_i) ==
-                                     window->selected_tab_index));
-        if (restored_tab)
-          restored_tab->controller().LoadIfNecessary();
+        tab_browser->window()->Show();
       }
-      // All the window's tabs had the same former browser_id.
-      if (window->tabs[0].has_browser()) {
-        UpdateTabBrowserIDs(window->tabs[0].browser_id,
-                            browser->session_id().id());
-      }
-      browser->window()->Show();
-    } else {
-      NOTREACHED();
+
+      if (tab_index < 0 || tab_index > tab_browser->tab_count())
+        tab_index = tab_browser->tab_count();
+      tab_browser->AddRestoredTab(tab->navigations, tab_index,
+                                  tab->current_navigation_index, true);
+    }
+  } else if (entry->type == WINDOW) {
+    const Window* window = static_cast<Window*>(entry);
+    browser = Browser::Create(profile());
+    for (size_t tab_i = 0; tab_i < window->tabs.size(); ++tab_i) {
+      const Tab& tab = window->tabs[tab_i];
+      TabContents* restored_tab =
+          browser->AddRestoredTab(tab.navigations, browser->tab_count(),
+                                  tab.current_navigation_index,
+                                  (static_cast<int>(tab_i) ==
+                                   window->selected_tab_index));
+      if (restored_tab)
+        restored_tab->controller().LoadIfNecessary();
     }
+    // All the window's tabs had the same former browser_id.
+    if (window->tabs[0].has_browser()) {
+      UpdateTabBrowserIDs(window->tabs[0].browser_id,
+                          browser->session_id().id());
+    }
+    browser->window()->Show();
+  } else {
+    NOTREACHED();
   }
+
   delete entry;
   restoring_ = false;
   NotifyTabsChanged();
diff --git a/chrome/browser/sessions/tab_restore_service.h b/chrome/browser/sessions/tab_restore_service.h
index 6b1da34..a74f335 100644
--- a/chrome/browser/sessions/tab_restore_service.h
+++ b/chrome/browser/sessions/tab_restore_service.h
@@ -134,7 +134,8 @@ class TabRestoreService : public BaseSessionService {
 
   // Restores an entry by id. If there is no entry with an id matching |id|,
   // this does nothing. If |replace_existing_tab| is true and id identifies a
-  // tab, the newly created tab replaces the selected tab in |browser|.
+  // tab, the newly created tab replaces the selected tab in |browser|. If
+  // |browser| is NULL, this creates a new window for the entry.
   void RestoreEntryById(Browser* browser,
                         SessionID::id_type id,
                         bool replace_existing_tab);
diff --git a/chrome/browser/sessions/tab_restore_service_unittest.cc b/chrome/browser/sessions/tab_restore_service_unittest.cc
index 7a24a91..c1ad7b1 100644
--- a/chrome/browser/sessions/tab_restore_service_unittest.cc
+++ b/chrome/browser/sessions/tab_restore_service_unittest.cc
@@ -157,25 +157,6 @@ TEST_F(TabRestoreServiceTest, Restore) {
   EXPECT_EQ(2, tab->current_navigation_index);
 }
 
-// Make sure a tab that is restored doesn't come back.
-TEST_F(TabRestoreServiceTest, DontLoadRestoredTab) {
-  AddThreeNavigations();
-
-  // Have the service record the tab.
-  service_->CreateHistoricalTab(&controller());
-  ASSERT_EQ(1U, service_->entries().size());
-
-  // Restore the tab.
-  service_->RestoreEntryById(NULL, service_->entries().front()->id, true);
-  ASSERT_TRUE(service_->entries().empty());
-
-  // Recreate the service and have it load the tabs.
-  RecreateService();
-
-  // There should be no entries.
-  ASSERT_EQ(0U, service_->entries().size());
-}
-
 // Make sure we persist entries to disk that have post data.
 TEST_F(TabRestoreServiceTest, DontPersistPostData) {
   AddThreeNavigations();