Don't show URL for pending new navigations initiated by the renderer.

BUG=99016
TEST=Click a link to a slow view-source: URL.

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

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

12 files changed, 147 insertions, 28 deletions

diff --git a/content/browser/site_instance_unittest.cc b/content/browser/site_instance_unittest.cc
index 339151d..5c355dc 100644
--- a/content/browser/site_instance_unittest.cc
+++ b/content/browser/site_instance_unittest.cc
@@ -188,7 +188,8 @@ TEST_F(SiteInstanceTest, SiteInstanceDestructor) {
 
   NavigationEntry* e1 = new NavigationEntry(instance, 0, url, GURL(),
                                             string16(),
-                                            content::PAGE_TRANSITION_LINK);
+                                            content::PAGE_TRANSITION_LINK,
+                                            false);
 
   // Redundantly setting e1's SiteInstance shouldn't affect the ref count.
   e1->set_site_instance(instance);
@@ -197,7 +198,8 @@ TEST_F(SiteInstanceTest, SiteInstanceDestructor) {
   // Add a second reference
   NavigationEntry* e2 = new NavigationEntry(instance, 0, url,
                                             GURL(), string16(),
-                                            content::PAGE_TRANSITION_LINK);
+                                            content::PAGE_TRANSITION_LINK,
+                                            false);
 
   // Now delete both entries and be sure the SiteInstance goes away.
   delete e1;
@@ -252,7 +254,8 @@ TEST_F(SiteInstanceTest, CloneNavigationEntry) {
 
   NavigationEntry* e1 = new NavigationEntry(instance1, 0, url, GURL(),
                                             string16(),
-                                            content::PAGE_TRANSITION_LINK);
+                                            content::PAGE_TRANSITION_LINK,
+                                            false);
   // Clone the entry
   NavigationEntry* e2 = new NavigationEntry(*e1);
 
diff --git a/content/browser/tab_contents/navigation_controller.cc b/content/browser/tab_contents/navigation_controller.cc
index ddaf300..11c90d3 100644
--- a/content/browser/tab_contents/navigation_controller.cc
+++ b/content/browser/tab_contents/navigation_controller.cc
@@ -222,7 +222,7 @@ bool NavigationController::IsInitialNavigation() {
 // static
 NavigationEntry* NavigationController::CreateNavigationEntry(
     const GURL& url, const GURL& referrer, content::PageTransition transition,
-    const std::string& extra_headers,
+    bool is_renderer_initiated, const std::string& extra_headers,
     content::BrowserContext* browser_context) {
   // Allow the browser URL handler to rewrite the URL. This will, for example,
   // remove "view-source:" from the beginning of the URL to get the URL that
@@ -240,7 +240,8 @@ NavigationEntry* NavigationController::CreateNavigationEntry(
       loaded_url,
       referrer,
       string16(),
-      transition);
+      transition,
+      is_renderer_initiated);
   entry->set_virtual_url(url);
   entry->set_user_typed_url(url);
   entry->set_update_virtual_url_with_url(reverse_on_redirect);
@@ -291,8 +292,15 @@ NavigationEntry* NavigationController::GetActiveEntry() const {
 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)
+  // Only return the pending_entry for new (non-history), browser-initiated
+  // navigations, in order to prevent URL spoof attacks.
+  // Ideally we would also show the pending entry's URL for new renderer-
+  // initiated navigations with no last committed entry (e.g., a link opening
+  // in a new tab), but an attacker can insert content into the about:blank
+  // page while the pending URL loads in that case.
+  if (pending_entry_ &&
+      pending_entry_->page_id() == -1 &&
+      !pending_entry_->is_renderer_initiated())
     return pending_entry_;
   return GetLastCommittedEntry();
 }
@@ -496,6 +504,23 @@ void NavigationController::LoadURL(
   needs_reload_ = false;
 
   NavigationEntry* entry = CreateNavigationEntry(url, referrer, transition,
+                                                 false,
+                                                 extra_headers,
+                                                 browser_context_);
+
+  LoadEntry(entry);
+}
+
+void NavigationController::LoadURLFromRenderer(
+    const GURL& url,
+    const GURL& referrer,
+    content::PageTransition transition,
+    const std::string& extra_headers) {
+  // The user initiated a load, we don't need to reload anymore.
+  needs_reload_ = false;
+
+  NavigationEntry* entry = CreateNavigationEntry(url, referrer, transition,
+                                                 true,
                                                  extra_headers,
                                                  browser_context_);
 
@@ -571,6 +596,10 @@ bool NavigationController::RendererDidNavigate(
   NavigationEntry* active_entry = GetActiveEntry();
   active_entry->set_content_state(params.content_state);
 
+  // Once committed, we do not need to track if the entry was initiated by
+  // the renderer.
+  active_entry->set_is_renderer_initiated(false);
+
   // The active entry's SiteInstance should match our SiteInstance.
   DCHECK(active_entry->site_instance() == tab_contents_->GetSiteInstance());
 
diff --git a/content/browser/tab_contents/navigation_controller.h b/content/browser/tab_contents/navigation_controller.h
index cadf2d5..7127057 100644
--- a/content/browser/tab_contents/navigation_controller.h
+++ b/content/browser/tab_contents/navigation_controller.h
@@ -180,6 +180,13 @@ class CONTENT_EXPORT NavigationController {
                content::PageTransition type,
                const std::string& extra_headers);
 
+  // Same as LoadURL, but for renderer-initiated navigations.  This state is
+  // important for tracking whether to display pending URLs.
+  void LoadURLFromRenderer(const GURL& url,
+                           const GURL& referrer,
+                           content::PageTransition type,
+                           const std::string& extra_headers);
+
   // Loads the current page if this NavigationController was restored from
   // history and the current page has not loaded yet.
   void LoadIfNecessary();
@@ -331,6 +338,7 @@ class CONTENT_EXPORT NavigationController {
       const GURL& url,
       const GURL& referrer,
       content::PageTransition transition,
+      bool is_renderer_initiated,
       const std::string& extra_headers,
       content::BrowserContext* browser_context);
 
diff --git a/content/browser/tab_contents/navigation_controller_unittest.cc b/content/browser/tab_contents/navigation_controller_unittest.cc
index 55da502..c8cebd0 100644
--- a/content/browser/tab_contents/navigation_controller_unittest.cc
+++ b/content/browser/tab_contents/navigation_controller_unittest.cc
@@ -1469,7 +1469,8 @@ TEST_F(NavigationControllerTest, RestoreNavigate) {
   GURL url("http://foo");
   std::vector<NavigationEntry*> entries;
   NavigationEntry* entry = NavigationController::CreateNavigationEntry(
-      url, GURL(), content::PAGE_TRANSITION_RELOAD, std::string(), profile());
+      url, GURL(), content::PAGE_TRANSITION_RELOAD, false, std::string(),
+      profile());
   entry->set_page_id(0);
   entry->set_title(ASCIIToUTF16("Title"));
   entry->set_content_state("state");
@@ -1527,7 +1528,8 @@ TEST_F(NavigationControllerTest, RestoreNavigateAfterFailure) {
   GURL url("http://foo");
   std::vector<NavigationEntry*> entries;
   NavigationEntry* entry = NavigationController::CreateNavigationEntry(
-      url, GURL(), content::PAGE_TRANSITION_RELOAD, std::string(), profile());
+      url, GURL(), content::PAGE_TRANSITION_RELOAD, false, std::string(),
+      profile());
   entry->set_page_id(0);
   entry->set_title(ASCIIToUTF16("Title"));
   entry->set_content_state("state");
@@ -1811,6 +1813,42 @@ TEST_F(NavigationControllerTest, TransientEntry) {
   EXPECT_EQ(controller().GetEntryAtIndex(4)->url(), url4);
 }
 
+// Tests that the URLs for renderer-initiated navigations are not displayed to
+// the user until the navigation commits, to prevent URL spoof attacks.
+// See http://crbug.com/99016.
+TEST_F(NavigationControllerTest, DontShowRendererURLUntilCommit) {
+  TestNotificationTracker notifications;
+  RegisterForAllNavNotifications(&notifications, &controller());
+
+  const GURL url0("http://foo/0");
+  const GURL url1("http://foo/1");
+
+  // For typed navigations (browser-initiated), both active and visible entries
+  // should update before commit.
+  controller().LoadURL(url0, GURL(), content::PAGE_TRANSITION_TYPED,
+                       std::string());
+  EXPECT_EQ(url0, controller().GetActiveEntry()->url());
+  EXPECT_EQ(url0, controller().GetVisibleEntry()->url());
+  rvh()->SendNavigate(0, url0);
+
+  // For link clicks (renderer-initiated navigations), the active entry should
+  // update before commit but the visible should not.
+  controller().LoadURLFromRenderer(url1, GURL(), content::PAGE_TRANSITION_LINK,
+                                   std::string());
+  EXPECT_EQ(url1, controller().GetActiveEntry()->url());
+  EXPECT_EQ(url0, controller().GetVisibleEntry()->url());
+  EXPECT_TRUE(controller().pending_entry()->is_renderer_initiated());
+
+  // After commit, both should be updated, and we should no longer treat the
+  // entry as renderer-initiated.
+  rvh()->SendNavigate(1, url1);
+  EXPECT_EQ(url1, controller().GetActiveEntry()->url());
+  EXPECT_EQ(url1, controller().GetVisibleEntry()->url());
+  EXPECT_FALSE(controller().GetLastCommittedEntry()->is_renderer_initiated());
+
+  notifications.Reset();
+}
+
 // Tests that IsInPageNavigation returns appropriate results.  Prevents
 // regression for bug 1126349.
 TEST_F(NavigationControllerTest, IsInPageNavigation) {
diff --git a/content/browser/tab_contents/navigation_entry.cc b/content/browser/tab_contents/navigation_entry.cc
index a74e144..22f421c 100644
--- a/content/browser/tab_contents/navigation_entry.cc
+++ b/content/browser/tab_contents/navigation_entry.cc
@@ -42,7 +42,8 @@ NavigationEntry::NavigationEntry()
       page_id_(-1),
       transition_type_(content::PAGE_TRANSITION_LINK),
       has_post_data_(false),
-      restore_type_(RESTORE_NONE) {
+      restore_type_(RESTORE_NONE),
+      is_renderer_initiated_(false) {
 }
 
 NavigationEntry::NavigationEntry(SiteInstance* instance,
@@ -50,7 +51,8 @@ NavigationEntry::NavigationEntry(SiteInstance* instance,
                                  const GURL& url,
                                  const GURL& referrer,
                                  const string16& title,
-                                 content::PageTransition transition_type)
+                                 content::PageTransition transition_type,
+                                 bool is_renderer_initiated)
     : unique_id_(GetUniqueID()),
       site_instance_(instance),
       page_type_(NORMAL_PAGE),
@@ -61,7 +63,8 @@ NavigationEntry::NavigationEntry(SiteInstance* instance,
       page_id_(page_id),
       transition_type_(transition_type),
       has_post_data_(false),
-      restore_type_(RESTORE_NONE) {
+      restore_type_(RESTORE_NONE),
+      is_renderer_initiated_(is_renderer_initiated) {
 }
 
 NavigationEntry::~NavigationEntry() {
diff --git a/content/browser/tab_contents/navigation_entry.h b/content/browser/tab_contents/navigation_entry.h
index d07c28f..c0b4df6 100644
--- a/content/browser/tab_contents/navigation_entry.h
+++ b/content/browser/tab_contents/navigation_entry.h
@@ -186,7 +186,8 @@ class CONTENT_EXPORT NavigationEntry {
                   const GURL& url,
                   const GURL& referrer,
                   const string16& title,
-                  content::PageTransition transition_type);
+                  content::PageTransition transition_type,
+                  bool is_renderer_initiated);
   ~NavigationEntry();
 
   // Page-related stuff --------------------------------------------------------
@@ -351,6 +352,15 @@ class CONTENT_EXPORT NavigationEntry {
     return transition_type_;
   }
 
+  // Whether this (pending) navigation is renderer-initiated.  Resets to false
+  // for all types of navigations after commit.
+  void set_is_renderer_initiated(bool is_renderer_initiated) {
+    is_renderer_initiated_ = is_renderer_initiated;
+  }
+  bool is_renderer_initiated() const {
+    return is_renderer_initiated_;
+  }
+
   // The user typed URL was the URL that the user initiated the navigation
   // with, regardless of any redirects. This is used to generate keywords, for
   // example, based on "what the user thinks the site is called" rather than
@@ -430,6 +440,11 @@ class CONTENT_EXPORT NavigationEntry {
   // This member is not persisted with sesssion restore.
   std::string extra_headers_;
 
+  // Whether the entry, while loading, was created for a renderer-initiated
+  // navigation.  This dictates whether the URL should be displayed before the
+  // navigation commits.  It is cleared on commit and not persisted.
+  bool is_renderer_initiated_;
+
   // This is a cached version of the result of GetTitleForDisplay. It prevents
   // us from having to do URL formatting on the URL every time the title is
   // displayed. When the URL, virtual URL, or title is set, this should be
diff --git a/content/browser/tab_contents/navigation_entry_unittest.cc b/content/browser/tab_contents/navigation_entry_unittest.cc
index dc80bbd..2171a70 100644
--- a/content/browser/tab_contents/navigation_entry_unittest.cc
+++ b/content/browser/tab_contents/navigation_entry_unittest.cc
@@ -22,7 +22,8 @@ class NavigationEntryTest : public testing::Test {
                                       GURL("test:url"),
                                       GURL("from"),
                                       ASCIIToUTF16("title"),
-                                      content::PAGE_TRANSITION_TYPED));
+                                      content::PAGE_TRANSITION_TYPED,
+                                      false));
   }
 
   virtual void TearDown() {
@@ -175,6 +176,12 @@ TEST_F(NavigationEntryTest, NavigationEntryAccessors) {
   entry2_.get()->set_transition_type(content::PAGE_TRANSITION_RELOAD);
   EXPECT_EQ(content::PAGE_TRANSITION_RELOAD, entry2_.get()->transition_type());
 
+  // Is renderer initiated
+  EXPECT_FALSE(entry1_.get()->is_renderer_initiated());
+  EXPECT_FALSE(entry2_.get()->is_renderer_initiated());
+  entry2_.get()->set_is_renderer_initiated(true);
+  EXPECT_TRUE(entry2_.get()->is_renderer_initiated());
+
   // Post Data
   EXPECT_FALSE(entry1_.get()->has_post_data());
   EXPECT_FALSE(entry2_.get()->has_post_data());
diff --git a/content/browser/tab_contents/page_navigator.cc b/content/browser/tab_contents/page_navigator.cc
index 5e94f74..1898fb7 100644
--- a/content/browser/tab_contents/page_navigator.cc
+++ b/content/browser/tab_contents/page_navigator.cc
@@ -13,16 +13,19 @@ OpenURLParams::OpenURLParams(
     const GURL& url,
     const GURL& referrer,
     WindowOpenDisposition disposition,
-    content::PageTransition transition)
+    content::PageTransition transition,
+    bool is_renderer_initiated)
     : url(url),
       referrer(referrer),
       disposition(disposition),
-      transition(transition) {
+      transition(transition),
+      is_renderer_initiated(is_renderer_initiated) {
 }
 
 OpenURLParams::OpenURLParams()
     : disposition(UNKNOWN),
-      transition(content::PageTransitionFromInt(0)) {
+      transition(content::PageTransitionFromInt(0)),
+      is_renderer_initiated(false) {
 }
 
 OpenURLParams::~OpenURLParams() {
diff --git a/content/browser/tab_contents/page_navigator.h b/content/browser/tab_contents/page_navigator.h
index 5a7877c..9369a7a 100644
--- a/content/browser/tab_contents/page_navigator.h
+++ b/content/browser/tab_contents/page_navigator.h
@@ -23,7 +23,8 @@ struct CONTENT_EXPORT OpenURLParams {
   OpenURLParams(const GURL& url,
                 const GURL& referrer,
                 WindowOpenDisposition disposition,
-                content::PageTransition transition);
+                content::PageTransition transition,
+                bool is_renderer_initiated);
   ~OpenURLParams();
 class TabContents;
 
@@ -37,6 +38,9 @@ class TabContents;
   // The transition type of navigation.
   content::PageTransition transition;
 
+  // Whether this navigation is initiated by the renderer process.
+  bool is_renderer_initiated;
+
   // The override encoding of the URL contents to be opened.
   std::string override_encoding;
 
diff --git a/content/browser/tab_contents/render_view_host_manager_unittest.cc b/content/browser/tab_contents/render_view_host_manager_unittest.cc
index 7146b6bc..20fd953 100644
--- a/content/browser/tab_contents/render_view_host_manager_unittest.cc
+++ b/content/browser/tab_contents/render_view_host_manager_unittest.cc
@@ -206,7 +206,8 @@ TEST_F(RenderViewHostManagerTest, Navigate) {
   const GURL kUrl1("http://www.google.com/");
   NavigationEntry entry1(NULL /* instance */, -1 /* page_id */, kUrl1,
                          GURL() /* referrer */, string16() /* title */,
-                         content::PAGE_TRANSITION_TYPED);
+                         content::PAGE_TRANSITION_TYPED,
+                         false /* is_renderer_init */);
   host = manager.Navigate(entry1);
 
   // The RenderViewHost created in Init will be reused.
@@ -225,7 +226,8 @@ TEST_F(RenderViewHostManagerTest, Navigate) {
   const GURL kUrl2("http://www.google.com/foo");
   NavigationEntry entry2(NULL /* instance */, -1 /* page_id */, kUrl2,
                          kUrl1 /* referrer */, string16() /* title */,
-                         content::PAGE_TRANSITION_LINK);
+                         content::PAGE_TRANSITION_LINK,
+                         true /* is_renderer_init */);
   host = manager.Navigate(entry2);
 
   // The RenderViewHost created in Init will be reused.
@@ -242,7 +244,8 @@ TEST_F(RenderViewHostManagerTest, Navigate) {
   const GURL kUrl3("http://webkit.org/");
   NavigationEntry entry3(NULL /* instance */, -1 /* page_id */, kUrl3,
                          kUrl2 /* referrer */, string16() /* title */,
-                         content::PAGE_TRANSITION_LINK);
+                         content::PAGE_TRANSITION_LINK,
+                         false /* is_renderer_init */);
   host = manager.Navigate(entry3);
 
   // A new RenderViewHost should be created.
@@ -277,7 +280,8 @@ TEST_F(RenderViewHostManagerTest, WebUI) {
   const GURL kUrl(chrome::kTestNewTabURL);
   NavigationEntry entry(NULL /* instance */, -1 /* page_id */, kUrl,
                         GURL() /* referrer */, string16() /* title */,
-                        content::PAGE_TRANSITION_TYPED);
+                        content::PAGE_TRANSITION_TYPED,
+                        false /* is_renderer_init */);
   RenderViewHost* host = manager.Navigate(entry);
 
   EXPECT_TRUE(host);
@@ -315,7 +319,8 @@ TEST_F(RenderViewHostManagerTest, NonWebUIChromeURLs) {
   const GURL kNtpUrl(chrome::kTestNewTabURL);
   NavigationEntry ntp_entry(NULL /* instance */, -1 /* page_id */, kNtpUrl,
                             GURL() /* referrer */, string16() /* title */,
-                            content::PAGE_TRANSITION_TYPED);
+                            content::PAGE_TRANSITION_TYPED,
+                            false /* is_renderer_init */);
 
   // about: URLs are not Web UI pages.
   GURL about_url(chrome::kTestMemoryURL);
@@ -325,7 +330,8 @@ TEST_F(RenderViewHostManagerTest, NonWebUIChromeURLs) {
       &about_url, profile(), &reverse_on_redirect);
   NavigationEntry about_entry(NULL /* instance */, -1 /* page_id */, about_url,
                               GURL() /* referrer */, string16() /* title */,
-                              content::PAGE_TRANSITION_TYPED);
+                              content::PAGE_TRANSITION_TYPED,
+                              false /* is_renderer_init */);
 
   EXPECT_TRUE(ShouldSwapProcesses(&manager, &ntp_entry, &about_entry));
 }
diff --git a/content/browser/tab_contents/tab_contents.cc b/content/browser/tab_contents/tab_contents.cc
index 8557a2b..90d5eab 100644
--- a/content/browser/tab_contents/tab_contents.cc
+++ b/content/browser/tab_contents/tab_contents.cc
@@ -536,7 +536,8 @@ TabContents* TabContents::OpenURL(const GURL& url,
                                   const GURL& referrer,
                                   WindowOpenDisposition disposition,
                                   content::PageTransition transition) {
-  return OpenURL(OpenURLParams(url, referrer, disposition, transition));
+  return OpenURL(OpenURLParams(url, referrer, disposition, transition,
+                               false));
 }
 
 TabContents* TabContents::OpenURL(const OpenURLParams& params) {
@@ -1685,8 +1686,9 @@ void TabContents::RequestOpenURL(const GURL& url,
             render_manager_.web_ui()->link_transition_type());
     transition_type = render_manager_.web_ui()->link_transition_type();
   } else {
-    new_contents = OpenURL(
-        url, referrer, disposition, content::PAGE_TRANSITION_LINK);
+    new_contents = OpenURL(OpenURLParams(
+        url, referrer, disposition, content::PAGE_TRANSITION_LINK,
+        true /* is_renderer_initiated */));
   }
   if (new_contents) {
     // Notify observers.
diff --git a/content/browser/tab_contents/tab_contents_delegate.cc b/content/browser/tab_contents/tab_contents_delegate.cc
index cfae17b..293e7bb 100644
--- a/content/browser/tab_contents/tab_contents_delegate.cc
+++ b/content/browser/tab_contents/tab_contents_delegate.cc
@@ -23,7 +23,8 @@ TabContents* TabContentsDelegate::OpenURLFromTab(
     WindowOpenDisposition disposition,
     content::PageTransition transition) {
   return OpenURLFromTab(source,
-                        OpenURLParams(url, referrer, disposition, transition));
+                        OpenURLParams(url, referrer, disposition, transition,
+                                      false));
 }
 
 TabContents* TabContentsDelegate::OpenURLFromTab(TabContents* source,