TabRestoreService improvements

this patch brings more code from the views into the model (TabRestoreService) so the views don't all duplicate each other, or forget to duplicate each other's behavior and then act inexplicably different. For example, only RecentlyClosedHandler was removing duplicates, and it doesn't make sense to diverge on that point. (In the end, this patch actually removes all de-duping because two tabs may diverge in their history even if their current navigation is the same.)

This also fixes the issue where filtering was done after the list had been pruned to 10 entries. The issue with doing this is that filtered entries (such as New Tabs or duplicate tabs) counted against the total of 10 entries, but wouldn't show up in the list, so you'd frequently show <10 tabs even though there are 10 valid, closed tabs we could show.

BUG=83211, 81805
TEST=
1. open 10 tabs, close them all. Start opening and closing new tabs --- the recently closed list shouldn't be affected.
2. The jumplist on Windows and the global history menu on mac/gtk should not show duplicate entries
3. if you close a window with 2 normal tabs and one ntp in it, it should show up as "3 tabs", not "2 tabs"

Review URL: http://codereview.chromium.org/8603008

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

1 files changed, 6 insertions, 15 deletions

diff --git a/chrome/browser/jumplist_win.cc b/chrome/browser/jumplist_win.cc
index 02e8d35..2413c9e 100644
--- a/chrome/browser/jumplist_win.cc
+++ b/chrome/browser/jumplist_win.cc
@@ -665,16 +665,12 @@ bool JumpList::AddTab(const TabRestoreService::Tab* tab,
                       size_t max_items) {
   // This code adds the URL and the title strings of the given tab to the
   // specified list.
-  // This code is copied from RecentlyClosedTabsHandler::TabToValue().
-  if (tab->navigations.empty() || list->size() >= max_items)
+  if (list->size() >= max_items)
     return false;
 
+  scoped_refptr<ShellLinkItem> link(new ShellLinkItem);
   const TabNavigation& current_navigation =
       tab->navigations.at(tab->current_navigation_index);
-  if (current_navigation.virtual_url() == GURL(chrome::kChromeUINewTabURL))
-    return false;
-
-  scoped_refptr<ShellLinkItem> link(new ShellLinkItem);
   std::string url = current_navigation.virtual_url().spec();
   link->SetArguments(UTF8ToWide(url));
   link->SetTitle(current_navigation.title());
@@ -683,22 +679,17 @@ bool JumpList::AddTab(const TabRestoreService::Tab* tab,
   return true;
 }
 
-bool JumpList::AddWindow(const TabRestoreService::Window* window,
+void JumpList::AddWindow(const TabRestoreService::Window* window,
                          ShellLinkItemList* list,
                          size_t max_items) {
   // This code enumerates al the tabs in the given window object and add their
   // URLs and titles to the list.
-  // This code is copied from RecentlyClosedTabsHandler::WindowToValue().
-  if (window->tabs.empty()) {
-    NOTREACHED();
-    return false;
-  }
+  DCHECK(!window->tabs.empty());
+
   for (size_t i = 0; i < window->tabs.size(); ++i) {
     if (!AddTab(&window->tabs[i], list, max_items))
-      return false;
+      return;
   }
-
-  return true;
 }
 
 bool JumpList::StartLoadingFavicon() {