Changes session restore to use a normal load rather than preferring

the cache. We need to do this else we don't honor page expiration and
end up showing stale data for some sites.

BUG=21195
TEST=make sure session restore works.

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

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

14 files changed, 107 insertions, 74 deletions

diff --git a/chrome/browser/browser.cc b/chrome/browser/browser.cc
index 60d08c8..48e46d7 100644
--- a/chrome/browser/browser.cc
+++ b/chrome/browser/browser.cc
@@ -604,10 +604,12 @@ TabContents* Browser::AddRestoredTab(
     int tab_index,
     int selected_navigation,
     bool select,
-    bool pin) {
+    bool pin,
+    bool from_last_session) {
   TabContents* new_tab = new TabContents(profile(), NULL,
       MSG_ROUTING_NONE, tabstrip_model_.GetSelectedTabContents());
-  new_tab->controller().RestoreFromState(navigations, selected_navigation);
+  new_tab->controller().RestoreFromState(navigations, selected_navigation,
+                                         from_last_session);
 
   bool really_pin =
       (pin && tab_index == tabstrip_model()->IndexOfFirstNonPinnedTab());
@@ -626,10 +628,12 @@ TabContents* Browser::AddRestoredTab(
 
 void Browser::ReplaceRestoredTab(
     const std::vector<TabNavigation>& navigations,
-    int selected_navigation) {
+    int selected_navigation,
+    bool from_last_session) {
   TabContents* replacement = new TabContents(profile(), NULL,
       MSG_ROUTING_NONE, tabstrip_model_.GetSelectedTabContents());
-  replacement->controller().RestoreFromState(navigations, selected_navigation);
+  replacement->controller().RestoreFromState(navigations, selected_navigation,
+                                             from_last_session);
 
   tabstrip_model_.ReplaceNavigationControllerAt(
       tabstrip_model_.selected_index(),
@@ -2647,26 +2651,6 @@ void Browser::SyncHistoryWithTabs(int index) {
   }
 }
 
-TabContents* Browser::BuildRestoredTab(
-      const std::vector<TabNavigation>& navigations,
-      int selected_navigation) {
-  if (!navigations.empty()) {
-    DCHECK(selected_navigation >= 0 &&
-           selected_navigation < static_cast<int>(navigations.size()));
-    // Create a NavigationController. This constructor creates the appropriate
-    // set of TabContents.
-    TabContents* new_tab = new TabContents(profile_, NULL,
-        MSG_ROUTING_NONE, tabstrip_model_.GetSelectedTabContents());
-    new_tab->controller().RestoreFromState(navigations, selected_navigation);
-    return new_tab;
-  } else {
-    // No navigations. Create a tab with about:blank.
-    return CreateTabContentsForURL(GURL(chrome::kAboutBlankURL), GURL(),
-                                   profile_, PageTransition::START_PAGE,
-                                   false, NULL);
-  }
-}
-
 ///////////////////////////////////////////////////////////////////////////////
 // Browser, OnBeforeUnload handling (private):
 
diff --git a/chrome/browser/browser.h b/chrome/browser/browser.h
index a7a3160..ae3ee18 100644
--- a/chrome/browser/browser.h
+++ b/chrome/browser/browser.h
@@ -260,12 +260,14 @@ class Browser : public TabStripModelDelegate,
   // system. If select is true, the tab is selected. |tab_index| gives the index
   // to insert the tab at. |selected_navigation| is the index of the
   // TabNavigation in |navigations| to select. If |pin| is true and |tab_index|
-  // is the last pinned tab, then the newly created tab is pinned.
+  // is the last pinned tab, then the newly created tab is pinned. If
+  // |from_last_session| is true, |navigations| are from the previous session.
   TabContents* AddRestoredTab(const std::vector<TabNavigation>& navigations,
                               int tab_index,
                               int selected_navigation,
                               bool select,
-                              bool pin);
+                              bool pin,
+                              bool from_last_session);
   // Creates a new tab with the already-created TabContents 'new_contents'.
   // The window for the added contents will be reparented correctly when this
   // method returns.  If |disposition| is NEW_POPUP, |pos| should hold the
@@ -298,7 +300,8 @@ class Browser : public TabStripModelDelegate,
   // history restored from the SessionRestore system.
   void ReplaceRestoredTab(
       const std::vector<TabNavigation>& navigations,
-      int selected_navigation);
+      int selected_navigation,
+      bool from_last_session);
 
   // Returns true if a tab can be restored.
   virtual bool CanRestoreTab();
@@ -627,12 +630,6 @@ class Browser : public TabStripModelDelegate,
   // >= index.
   void SyncHistoryWithTabs(int index);
 
-  // Called from AddRestoredTab and ReplaceRestoredTab to build a
-  // TabContents from an incoming vector of TabNavigations.
-  // Caller takes ownership of the returned TabContents.
-  TabContents* BuildRestoredTab(const std::vector<TabNavigation>& navigations,
-                                int selected_navigation);
-
   // OnBeforeUnload handling //////////////////////////////////////////////////
 
   typedef std::set<TabContents*> UnloadListenerSet;
diff --git a/chrome/browser/renderer_host/render_view_host.cc b/chrome/browser/renderer_host/render_view_host.cc
index dab321d..5c5b120 100644
--- a/chrome/browser/renderer_host/render_view_host.cc
+++ b/chrome/browser/renderer_host/render_view_host.cc
@@ -274,7 +274,7 @@ void RenderViewHost::NavigateToURL(const GURL& url) {
   params.page_id = -1;
   params.url = url;
   params.transition = PageTransition::LINK;
-  params.reload = false;
+  params.navigation_type = ViewMsg_Navigate_Params::NORMAL;
   Navigate(params);
 }
 
diff --git a/chrome/browser/sessions/session_restore.cc b/chrome/browser/sessions/session_restore.cc
index 79f3d36..6db8516 100644
--- a/chrome/browser/sessions/session_restore.cc
+++ b/chrome/browser/sessions/session_restore.cc
@@ -334,7 +334,8 @@ class SessionRestoreImpl : public NotificationObserver {
                                    static_cast<int>(i - window.tabs.begin()),
                                    selected_index,
                                    false,
-                                   tab.pinned)->controller());
+                                   tab.pinned,
+                                   true)->controller());
     }
   }
 
diff --git a/chrome/browser/sessions/tab_restore_service.cc b/chrome/browser/sessions/tab_restore_service.cc
index a5f8545..339aeb3 100644
--- a/chrome/browser/sessions/tab_restore_service.cc
+++ b/chrome/browser/sessions/tab_restore_service.cc
@@ -26,9 +26,15 @@ using base::Time;
 // ID of the next Entry.
 static SessionID::id_type next_entry_id = 1;
 
-TabRestoreService::Entry::Entry() : id(next_entry_id++), type(TAB) {}
+TabRestoreService::Entry::Entry()
+    : id(next_entry_id++),
+      type(TAB),
+      from_last_session(false) {}
 
-TabRestoreService::Entry::Entry(Type type) : id(next_entry_id++), type(type) {}
+TabRestoreService::Entry::Entry(Type type)
+    : id(next_entry_id++),
+      type(type),
+      from_last_session(false) {}
 
 // TabRestoreService ----------------------------------------------------------
 
@@ -250,7 +256,8 @@ void TabRestoreService::RestoreEntryById(Browser* browser,
     Tab* tab = static_cast<Tab*>(entry);
     if (replace_existing_tab && browser) {
       browser->ReplaceRestoredTab(tab->navigations,
-                                  tab->current_navigation_index);
+                                  tab->current_navigation_index,
+                                  tab->from_last_session);
     } else {
       // Use the tab's former browser and index, if available.
       Browser* tab_browser = NULL;
@@ -273,7 +280,7 @@ void TabRestoreService::RestoreEntryById(Browser* browser,
         tab_index = tab_browser->tab_count();
       tab_browser->AddRestoredTab(tab->navigations, tab_index,
                                   tab->current_navigation_index, true,
-                                  tab->pinned);
+                                  tab->pinned, tab->from_last_session);
     }
   } else if (entry->type == WINDOW) {
     const Window* window = static_cast<Window*>(entry);
@@ -285,7 +292,7 @@ void TabRestoreService::RestoreEntryById(Browser* browser,
                                   tab.current_navigation_index,
                                   (static_cast<int>(tab_i) ==
                                    window->selected_tab_index),
-                                  tab.pinned);
+                                  tab.pinned, tab.from_last_session);
       if (restored_tab)
         restored_tab->controller().LoadIfNecessary();
     }
@@ -869,8 +876,10 @@ void TabRestoreService::LoadStateChanged() {
   }
 
   // And add them.
-  for (size_t i = 0; i < staging_entries_.size(); ++i)
+  for (size_t i = 0; i < staging_entries_.size(); ++i) {
+    staging_entries_[i]->from_last_session = true;
     AddEntry(staging_entries_[i], false, false);
+  }
 
   // AddEntry takes ownership of the entry, need to clear out entries so that
   // it doesn't delete them.
diff --git a/chrome/browser/sessions/tab_restore_service.h b/chrome/browser/sessions/tab_restore_service.h
index a3e611c..50fc0a7 100644
--- a/chrome/browser/sessions/tab_restore_service.h
+++ b/chrome/browser/sessions/tab_restore_service.h
@@ -70,6 +70,11 @@ class TabRestoreService : public BaseSessionService {
 
     // The time when the window or tab was closed.
     base::Time timestamp;
+
+    // Is this entry from the last session? This is set to true for entries that
+    // were closed during the last session, and false for entries that were
+    // closed during this session.
+    bool from_last_session;
   };
 
   // Represents a previously open tab.
diff --git a/chrome/browser/tab_contents/navigation_controller.cc b/chrome/browser/tab_contents/navigation_controller.cc
index 1532bca..9f1fedf 100644
--- a/chrome/browser/tab_contents/navigation_controller.cc
+++ b/chrome/browser/tab_contents/navigation_controller.cc
@@ -67,12 +67,15 @@ void SetContentStateIfEmpty(NavigationEntry* entry) {
 // Configure all the NavigationEntries in entries for restore. This resets
 // the transition type to reload and makes sure the content state isn't empty.
 void ConfigureEntriesForRestore(
-    std::vector<linked_ptr<NavigationEntry> >* entries) {
+    std::vector<linked_ptr<NavigationEntry> >* entries,
+    bool from_last_session) {
   for (size_t i = 0; i < entries->size(); ++i) {
     // Use a transition type of reload so that we don't incorrectly increase
     // the typed count.
     (*entries)[i]->set_transition_type(PageTransition::RELOAD);
-    (*entries)[i]->set_restored(true);
+    (*entries)[i]->set_restore_type(from_last_session ?
+        NavigationEntry::RESTORE_LAST_SESSION :
+        NavigationEntry::RESTORE_CURRENT_SESSION);
     // NOTE(darin): This code is only needed for backwards compat.
     SetContentStateIfEmpty((*entries)[i].get());
   }
@@ -145,7 +148,8 @@ NavigationController::~NavigationController() {
 
 void NavigationController::RestoreFromState(
     const std::vector<TabNavigation>& navigations,
-    int selected_navigation) {
+    int selected_navigation,
+    bool from_last_session) {
   // Verify that this controller is unused and that the input is valid.
   DCHECK(entry_count() == 0 && !pending_entry());
   DCHECK(selected_navigation >= 0 &&
@@ -156,7 +160,7 @@ void NavigationController::RestoreFromState(
   CreateNavigationEntriesFromTabNavigations(navigations, &entries_);
 
   // And finish the restore.
-  FinishRestore(selected_navigation);
+  FinishRestore(selected_navigation, from_last_session);
 }
 
 void NavigationController::Reload(bool check_for_repost) {
@@ -422,8 +426,10 @@ bool NavigationController::RendererDidNavigate(
   // TODO(brettw) this seems slightly bogus as we don't really know if the
   // pending entry is what this navigation is for. There is a similar TODO
   // w.r.t. the pending entry in RendererDidNavigateToNewPage.
-  if (pending_entry_index_ >= 0)
+  if (pending_entry_index_ >= 0) {
     pending_entry_->set_site_instance(tab_contents_->GetSiteInstance());
+    pending_entry_->set_restore_type(NavigationEntry::RESTORE_NONE);
+  }
 
   // Do navigation-type specific actions. These will make and commit an entry.
   details->type = ClassifyNavigation(params);
@@ -828,7 +834,7 @@ void NavigationController::CopyStateFrom(const NavigationController& source) {
         new NavigationEntry(*source.entries_[i])));
   }
 
-  FinishRestore(source.last_committed_entry_index_);
+  FinishRestore(source.last_committed_entry_index_, false);
 }
 
 void NavigationController::DiscardNonCommittedEntries() {
@@ -958,9 +964,10 @@ void NavigationController::NotifyEntryChanged(const NavigationEntry* entry,
                                          Details<EntryChangedDetails>(&det));
 }
 
-void NavigationController::FinishRestore(int selected_index) {
+void NavigationController::FinishRestore(int selected_index,
+                                         bool from_last_session) {
   DCHECK(selected_index >= 0 && selected_index < entry_count());
-  ConfigureEntriesForRestore(&entries_);
+  ConfigureEntriesForRestore(&entries_, from_last_session);
 
   set_max_restored_page_id(entry_count());
 
diff --git a/chrome/browser/tab_contents/navigation_controller.h b/chrome/browser/tab_contents/navigation_controller.h
index 0ac4c3b..a227765 100644
--- a/chrome/browser/tab_contents/navigation_controller.h
+++ b/chrome/browser/tab_contents/navigation_controller.h
@@ -150,10 +150,12 @@ class NavigationController {
 
   // Initializes this NavigationController with the given saved navigations,
   // using selected_navigation as the currently loaded entry. Before this call
-  // the controller should be unused (there should be no current entry). This is
-  // used for session restore.
+  // the controller should be unused (there should be no current entry). If
+  // from_last_session is true, navigations are from the previous session,
+  // otherwise they are from the current session (undo tab close).
+  // This is used for session restore.
   void RestoreFromState(const std::vector<TabNavigation>& navigations,
-                        int selected_navigation);
+                        int selected_navigation, bool from_last_session);
 
   // Active entry --------------------------------------------------------------
 
@@ -441,8 +443,8 @@ class NavigationController {
 
   // Invoked after session/tab restore or cloning a tab. Resets the transition
   // type of the entries, updates the max page id and creates the active
-  // contents.
-  void FinishRestore(int selected_index);
+  // contents. See RestoreFromState for a description of from_last_session.
+  void FinishRestore(int selected_index, bool from_last_session);
 
   // Inserts a new entry or replaces the current entry with a new one, removing
   // all entries after it. The new entry will become the active one.
diff --git a/chrome/browser/tab_contents/navigation_controller_unittest.cc b/chrome/browser/tab_contents/navigation_controller_unittest.cc
index fc31c75..142ad8b 100644
--- a/chrome/browser/tab_contents/navigation_controller_unittest.cc
+++ b/chrome/browser/tab_contents/navigation_controller_unittest.cc
@@ -1146,7 +1146,7 @@ TEST_F(NavigationControllerTest, RestoreNavigate) {
                                       PageTransition::LINK));
   TabContents our_contents(profile(), NULL, MSG_ROUTING_NONE, NULL);
   NavigationController& our_controller = our_contents.controller();
-  our_controller.RestoreFromState(navigations, 0);
+  our_controller.RestoreFromState(navigations, 0, true);
   our_controller.GoToIndex(0);
 
   // We should now have one entry, and it should be "pending".
@@ -1154,6 +1154,8 @@ TEST_F(NavigationControllerTest, RestoreNavigate) {
   EXPECT_EQ(our_controller.GetEntryAtIndex(0),
             our_controller.pending_entry());
   EXPECT_EQ(0, our_controller.GetEntryAtIndex(0)->page_id());
+  EXPECT_EQ(NavigationEntry::RESTORE_LAST_SESSION,
+            our_controller.GetEntryAtIndex(0)->restore_type());
 
   // Say we navigated to that entry.
   ViewHostMsg_FrameNavigate_Params params = {0};
@@ -1174,6 +1176,8 @@ TEST_F(NavigationControllerTest, RestoreNavigate) {
   EXPECT_FALSE(our_controller.pending_entry());
   EXPECT_EQ(url,
             our_controller.GetLastCommittedEntry()->site_instance()->site());
+  EXPECT_EQ(NavigationEntry::RESTORE_NONE,
+            our_controller.GetEntryAtIndex(0)->restore_type());
 }
 
 // Make sure that the page type and stuff is correct after an interstitial.
diff --git a/chrome/browser/tab_contents/navigation_entry.cc b/chrome/browser/tab_contents/navigation_entry.cc
index f2de70ec..826e648 100644
--- a/chrome/browser/tab_contents/navigation_entry.cc
+++ b/chrome/browser/tab_contents/navigation_entry.cc
@@ -43,7 +43,7 @@ NavigationEntry::NavigationEntry()
       page_id_(-1),
       transition_type_(PageTransition::LINK),
       has_post_data_(false),
-      restored_(false) {
+      restore_type_(RESTORE_NONE) {
 }
 
 NavigationEntry::NavigationEntry(SiteInstance* instance,
@@ -61,7 +61,7 @@ NavigationEntry::NavigationEntry(SiteInstance* instance,
       page_id_(page_id),
       transition_type_(transition_type),
       has_post_data_(false),
-      restored_(false) {
+      restore_type_(RESTORE_NONE) {
 }
 
 NavigationEntry::~NavigationEntry() {
diff --git a/chrome/browser/tab_contents/navigation_entry.h b/chrome/browser/tab_contents/navigation_entry.h
index 1d58ce6..555edfc 100644
--- a/chrome/browser/tab_contents/navigation_entry.h
+++ b/chrome/browser/tab_contents/navigation_entry.h
@@ -360,14 +360,26 @@ class NavigationEntry {
     return has_post_data_;
   }
 
-  // Was this entry created from session/tab restore? If so this is true and
-  // gets set to false once we navigate to it.
-  // (See NavigationController::DidNavigateToEntry).
-  void set_restored(bool restored) {
-    restored_ = restored;
+  // Enumerations of the possible restore types.
+  enum RestoreType {
+    // The entry has been restored is from the last session.
+    RESTORE_LAST_SESSION,
+
+    // The entry has been restored from the current session. This is used when
+    // the user issues 'reopen closed tab'.
+    RESTORE_CURRENT_SESSION,
+
+    // The entry was not restored.
+    RESTORE_NONE
+  };
+
+  // The RestoreType for this entry. This is set if the entry was retored. This
+  // is set to RESTORE_NONE once the entry is loaded.
+  void set_restore_type(RestoreType type) {
+    restore_type_ = type;
   }
-  bool restored() const {
-    return restored_;
+  RestoreType restore_type() const {
+    return restore_type_;
   }
 
  private:
@@ -392,7 +404,7 @@ class NavigationEntry {
   PageTransition::Type transition_type_;
   GURL user_typed_url_;
   bool has_post_data_;
-  bool restored_;
+  RestoreType restore_type_;
 
   // This is a cached version of the result of GetTitleForDisplay. It prevents
   // us from having to do URL formatting on the URL evey time the title is
diff --git a/chrome/browser/tab_contents/navigation_entry_unittest.cc b/chrome/browser/tab_contents/navigation_entry_unittest.cc
index 0353de2..4461cd5 100644
--- a/chrome/browser/tab_contents/navigation_entry_unittest.cc
+++ b/chrome/browser/tab_contents/navigation_entry_unittest.cc
@@ -174,8 +174,8 @@ TEST_F(NavigationEntryTest, NavigationEntryAccessors) {
   EXPECT_TRUE(entry2_.get()->has_post_data());
 
   // Restored
-  EXPECT_FALSE(entry1_.get()->restored());
-  EXPECT_FALSE(entry2_.get()->restored());
-  entry2_.get()->set_restored(true);
-  EXPECT_TRUE(entry2_.get()->restored());
+  EXPECT_EQ(NavigationEntry::RESTORE_NONE, entry1_->restore_type());
+  EXPECT_EQ(NavigationEntry::RESTORE_NONE, entry2_->restore_type());
+  entry2_->set_restore_type(NavigationEntry::RESTORE_LAST_SESSION);
+  EXPECT_EQ(NavigationEntry::RESTORE_LAST_SESSION, entry2_->restore_type());
 }
diff --git a/chrome/browser/tab_contents/render_view_host_manager.cc b/chrome/browser/tab_contents/render_view_host_manager.cc
index 3b58f2f..03e1f3b 100644
--- a/chrome/browser/tab_contents/render_view_host_manager.cc
+++ b/chrome/browser/tab_contents/render_view_host_manager.cc
@@ -341,7 +341,7 @@ SiteInstance* RenderViewHostManager::GetSiteInstanceForEntry(
     if (curr_instance->HasRelatedSiteInstance(dest_url)) {
       return curr_instance->GetRelatedSiteInstance(dest_url);
     } else {
-      if (entry.restored())
+      if (entry.restore_type() != NavigationEntry::RESTORE_NONE)
         curr_instance->SetSite(dest_url);
       return curr_instance;
     }
diff --git a/chrome/browser/tab_contents/tab_contents.cc b/chrome/browser/tab_contents/tab_contents.cc
index 8d98d80..e72df68 100644
--- a/chrome/browser/tab_contents/tab_contents.cc
+++ b/chrome/browser/tab_contents/tab_contents.cc
@@ -180,14 +180,26 @@ BOOL CALLBACK InvalidateWindow(HWND hwnd, LPARAM lparam) {
 }
 #endif
 
-void MakeNavigateParams(const NavigationEntry& entry, bool reload,
-                        ViewMsg_Navigate_Params* params) {
+ViewMsg_Navigate_Params::NavigationType GetNavigationType(
+    Profile* profile, const NavigationEntry& entry, bool reload) {
+  if (reload)
+    return ViewMsg_Navigate_Params::RELOAD;
+
+  if (entry.restore_type() == NavigationEntry::RESTORE_LAST_SESSION &&
+      profile->DidLastSessionExitCleanly())
+    return ViewMsg_Navigate_Params::RESTORE;
+
+  return ViewMsg_Navigate_Params::NORMAL;
+}
+
+void MakeNavigateParams(Profile* profile, const NavigationEntry& entry,
+                        bool reload, ViewMsg_Navigate_Params* params) {
   params->page_id = entry.page_id();
   params->url = entry.url();
   params->referrer = entry.referrer();
   params->transition = entry.transition_type();
   params->state = entry.content_state();
-  params->reload = reload;
+  params->navigation_type = GetNavigationType(profile, entry, reload);
   params->request_time = base::Time::Now();
 }
 
@@ -693,7 +705,7 @@ bool TabContents::NavigateToPendingEntry(bool reload) {
 
   // Navigate in the desired RenderViewHost.
   ViewMsg_Navigate_Params navigate_params;
-  MakeNavigateParams(entry, reload, &navigate_params);
+  MakeNavigateParams(profile(), entry, reload, &navigate_params);
   dest_render_view_host->Navigate(navigate_params);
 
   if (entry.page_id() == -1) {