Don't update URL bar or SSL icon for pending history navs until they commit.

BUG=89564
TEST=Go back or forward to a slow URL.  No omnibox change until it commits.


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

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

chrome/browser/ui/toolbar/toolbar_model.cc

@@ -43,7 +43,7 @@ string16 ToolbarModel::GetText() const {
     Profile* profile =
         Profile::FromBrowserContext(navigation_controller->browser_context());
     languages = profile->GetPrefs()->GetString(prefs::kAcceptLanguages);
-    NavigationEntry* entry = navigation_controller->GetActiveEntry();
+    NavigationEntry* entry = navigation_controller->GetVisibleEntry();
     if (!navigation_controller->tab_contents()->ShouldDisplayURL()) {
       // Explicitly hide the URL for this tab.
       url = GURL();
@@ -69,7 +69,7 @@ ToolbarModel::SecurityLevel ToolbarModel::GetSecurityLevel() const {
   if (!navigation_controller)  // We might not have a controller on init.
     return NONE;
 
-  NavigationEntry* entry = navigation_controller->GetActiveEntry();
+  NavigationEntry* entry = navigation_controller->GetVisibleEntry();
   if (!entry)
     return NONE;
 
@@ -119,7 +119,7 @@ string16 ToolbarModel::GetEVCertName() const {
   // Note: Navigation controller and active entry are guaranteed non-NULL or
   // the security level would be NONE.
   CertStore::GetInstance()->RetrieveCert(
-      GetNavigationController()->GetActiveEntry()->ssl().cert_id(), &cert);
+      GetNavigationController()->GetVisibleEntry()->ssl().cert_id(), &cert);
   return GetEVCertName(*cert);
 }
 

content/browser/tab_contents/navigation_controller.cc

@@ -285,6 +285,15 @@ NavigationEntry* NavigationController::GetActiveEntry() const {
   return GetLastCommittedEntry();
 }
 
+NavigationEntry* NavigationController::GetVisibleEntry() const {
+  if (transient_entry_index_ != -1)
+    return entries_[transient_entry_index_].get();
+  // Only return pending_entry for new navigations.
+  if (pending_entry_ && pending_entry_->page_id() == -1)
+    return pending_entry_;
+  return GetLastCommittedEntry();
+}
+
 int NavigationController::GetCurrentEntryIndex() const {
   if (transient_entry_index_ != -1)
     return transient_entry_index_;

content/browser/tab_contents/navigation_controller.h

@@ -78,9 +78,18 @@ class NavigationController {
   // NOTE: This can be NULL!!
   //
   // If you are trying to get the current state of the NavigationController,
-  // this is the method you will typically want to call.
+  // this is the method you will typically want to call.  If you want to display
+  // the active entry to the user (e.g., in the location bar), use
+  // GetVisibleEntry instead.
   NavigationEntry* GetActiveEntry() const;
 
+  // Returns the same entry as GetActiveEntry, except that it ignores pending
+  // history navigation entries.  This should be used when displaying info to
+  // the user, so that the location bar and other indicators do not update for
+  // a back/forward navigation until the pending entry commits.  This approach
+  // guards against URL spoofs on slow history navigations.
+  NavigationEntry* GetVisibleEntry() const;
+
   // Returns the index from which we would go back/forward or reload.  This is
   // the last_committed_entry_index_ if pending_entry_index_ is -1.  Otherwise,
   // it is the pending_entry_index_.

content/browser/tab_contents/navigation_controller_unittest.cc

@@ -1724,7 +1724,10 @@ TEST_F(NavigationControllerTest, TransientEntry) {
   controller().GoToIndex(1);
   // The navigation should have been initiated, transient entry should be gone.
   EXPECT_EQ(url1, controller().GetActiveEntry()->url());
+  // Visible entry does not update for history navigations until commit.
+  EXPECT_EQ(url3, controller().GetVisibleEntry()->url());
   rvh()->SendNavigate(1, url1);
+  EXPECT_EQ(url1, controller().GetVisibleEntry()->url());
 
   // Add a transient and go to an entry after the current one.
   transient_entry = new NavigationEntry;
@@ -1734,9 +1737,11 @@ TEST_F(NavigationControllerTest, TransientEntry) {
   controller().GoToIndex(3);
   // The navigation should have been initiated, transient entry should be gone.
   // Because of the transient entry that is removed, going to index 3 makes us
-  // land on url2.
+  // land on url2 (which is visible after the commit).
   EXPECT_EQ(url2, controller().GetActiveEntry()->url());
+  EXPECT_EQ(url1, controller().GetVisibleEntry()->url());
   rvh()->SendNavigate(2, url2);
+  EXPECT_EQ(url2, controller().GetVisibleEntry()->url());
 
   // Add a transient and go forward.
   transient_entry = new NavigationEntry;
@@ -1747,7 +1752,9 @@ TEST_F(NavigationControllerTest, TransientEntry) {
   controller().GoForward();
   // We should have navigated, transient entry should be gone.
   EXPECT_EQ(url3, controller().GetActiveEntry()->url());
+  EXPECT_EQ(url2, controller().GetVisibleEntry()->url());
   rvh()->SendNavigate(3, url3);
+  EXPECT_EQ(url3, controller().GetVisibleEntry()->url());
 
   // Ensure the URLS are correct.
   EXPECT_EQ(controller().entry_count(), 5);