Fix Cross-Process Redirect Navigation

(Second attempt, after committing in http://crrev.com/167800.)

BUG=104493
TEST=Go back when clicking an SSL search result to a hosted app.


Review URL: https://chromiumcodereview.appspot.com/11359098

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

17 files changed, 106 insertions, 45 deletions

diff --git a/content/browser/renderer_host/render_view_host_delegate.h b/content/browser/renderer_host/render_view_host_delegate.h
index 784ea53a..c424f1a 100644
--- a/content/browser/renderer_host/render_view_host_delegate.h
+++ b/content/browser/renderer_host/render_view_host_delegate.h
@@ -239,7 +239,8 @@ class CONTENT_EXPORT RenderViewHostDelegate {
                               const GURL& url,
                               const Referrer& referrer,
                               WindowOpenDisposition disposition,
-                              int64 source_frame_id) {}
+                              int64 source_frame_id,
+                              bool is_redirect) {}
 
   // The page wants to transfer the request to a new renderer.
   virtual void RequestTransferURL(
@@ -247,7 +248,8 @@ class CONTENT_EXPORT RenderViewHostDelegate {
       const Referrer& referrer,
       WindowOpenDisposition disposition,
       int64 source_frame_id,
-      const GlobalRequestID& old_request_id) {}
+      const GlobalRequestID& old_request_id,
+      bool is_redirect) {}
 
   // The page wants to close the active view in this tab.
   virtual void RouteCloseEvent(RenderViewHost* rvh) {}
diff --git a/content/browser/renderer_host/render_view_host_impl.cc b/content/browser/renderer_host/render_view_host_impl.cc
index 35d0c63..569001c 100644
--- a/content/browser/renderer_host/render_view_host_impl.cc
+++ b/content/browser/renderer_host/render_view_host_impl.cc
@@ -1364,16 +1364,15 @@ void RenderViewHostImpl::OnMsgToggleFullscreen(bool enter_fullscreen) {
   WasResized();
 }
 
-void RenderViewHostImpl::OnMsgOpenURL(const GURL& url,
-                                      const Referrer& referrer,
-                                      WindowOpenDisposition disposition,
-                                      int64 source_frame_id) {
-  GURL validated_url(url);
+void RenderViewHostImpl::OnMsgOpenURL(
+    const ViewHostMsg_OpenURL_Params& params) {
+  GURL validated_url(params.url);
   FilterURL(ChildProcessSecurityPolicyImpl::GetInstance(),
             GetProcess(), false, &validated_url);
 
   delegate_->RequestOpenURL(
-      this, validated_url, referrer, disposition, source_frame_id);
+      this, validated_url, params.referrer, params.disposition, params.frame_id,
+      params.is_cross_site_redirect);
 }
 
 void RenderViewHostImpl::OnMsgDidContentsPreferredSizeChange(
diff --git a/content/browser/renderer_host/render_view_host_impl.h b/content/browser/renderer_host/render_view_host_impl.h
index ae8cc82..26c2777 100644
--- a/content/browser/renderer_host/render_view_host_impl.h
+++ b/content/browser/renderer_host/render_view_host_impl.h
@@ -34,6 +34,7 @@ struct AccessibilityHostMsg_NotificationParams;
 struct MediaPlayerAction;
 struct ViewHostMsg_CreateWindow_Params;
 struct ViewHostMsg_DidFailProvisionalLoadWithError_Params;
+struct ViewHostMsg_OpenURL_Params;
 struct ViewHostMsg_ShowPopup_Params;
 struct ViewMsg_Navigate_Params;
 struct ViewMsg_PostMessage_Params;
@@ -510,10 +511,7 @@ class CONTENT_EXPORT RenderViewHostImpl
   void OnMsgDocumentOnLoadCompletedInMainFrame(int32 page_id);
   void OnMsgContextMenu(const ContextMenuParams& params);
   void OnMsgToggleFullscreen(bool enter_fullscreen);
-  void OnMsgOpenURL(const GURL& url,
-                    const Referrer& referrer,
-                    WindowOpenDisposition disposition,
-                    int64 source_frame_id);
+  void OnMsgOpenURL(const ViewHostMsg_OpenURL_Params& params);
   void OnMsgDidContentsPreferredSizeChange(const gfx::Size& new_size);
   void OnMsgDidChangeScrollbarsForMainFrame(bool has_horizontal_scrollbar,
                                             bool has_vertical_scrollbar);
diff --git a/content/browser/renderer_host/transfer_navigation_resource_throttle.cc b/content/browser/renderer_host/transfer_navigation_resource_throttle.cc
index ce06e8a..b623dad 100644
--- a/content/browser/renderer_host/transfer_navigation_resource_throttle.cc
+++ b/content/browser/renderer_host/transfer_navigation_resource_throttle.cc
@@ -35,7 +35,8 @@ void RequestTransferURLOnUIThread(int render_process_id,
     return;
 
   delegate->RequestTransferURL(
-      new_url, referrer, window_open_disposition, frame_id, global_request_id);
+      new_url, referrer, window_open_disposition,
+      frame_id, global_request_id, false);
 }
 
 }  // namespace
diff --git a/content/browser/web_contents/navigation_controller_impl.cc b/content/browser/web_contents/navigation_controller_impl.cc
index 0e5952b..4202b73 100644
--- a/content/browser/web_contents/navigation_controller_impl.cc
+++ b/content/browser/web_contents/navigation_controller_impl.cc
@@ -329,7 +329,7 @@ void NavigationControllerImpl::ReloadInternal(bool check_for_repost,
       // a reload in the renderer.
       reload_type = NavigationController::NO_RELOAD;
 
-      nav_entry->set_is_cross_site_reload(true);
+      nav_entry->set_should_replace_entry(true);
       pending_entry_ = nav_entry;
     } else {
       pending_entry_index_ = current_index;
@@ -658,6 +658,8 @@ void NavigationControllerImpl::LoadURLWithParams(const LoadURLParams& params) {
           params.is_renderer_initiated,
           params.extra_headers,
           browser_context_));
+  if (params.is_cross_site_redirect)
+    entry->set_should_replace_entry(true);
   entry->SetIsOverridingUserAgent(override);
   entry->set_transferred_global_request_id(
       params.transferred_global_request_id);
@@ -708,8 +710,10 @@ bool NavigationControllerImpl::RendererDidNavigate(
   // If we are doing a cross-site reload, we need to replace the existing
   // navigation entry, not add another entry to the history. This has the side
   // effect of removing forward browsing history, if such existed.
+  // Or if we are doing a cross-site redirect navigation,
+  // we will do a similar thing.
   details->did_replace_entry =
-      pending_entry_ && pending_entry_->is_cross_site_reload();
+      pending_entry_ && pending_entry_->should_replace_entry();
 
   // is_in_page must be computed before the entry gets committed.
   details->is_in_page = IsURLInPageNavigation(
diff --git a/content/browser/web_contents/navigation_entry_impl.cc b/content/browser/web_contents/navigation_entry_impl.cc
index d5476ca..53d70aa 100644
--- a/content/browser/web_contents/navigation_entry_impl.cc
+++ b/content/browser/web_contents/navigation_entry_impl.cc
@@ -45,7 +45,7 @@ NavigationEntryImpl::NavigationEntryImpl()
       restore_type_(RESTORE_NONE),
       is_overriding_user_agent_(false),
       is_renderer_initiated_(false),
-      is_cross_site_reload_(false),
+      should_replace_entry_(false),
       can_load_local_resources_(false) {
 }
 
@@ -70,7 +70,7 @@ NavigationEntryImpl::NavigationEntryImpl(SiteInstanceImpl* instance,
       restore_type_(RESTORE_NONE),
       is_overriding_user_agent_(false),
       is_renderer_initiated_(is_renderer_initiated),
-      is_cross_site_reload_(false),
+      should_replace_entry_(false),
       can_load_local_resources_(false) {
 }
 
diff --git a/content/browser/web_contents/navigation_entry_impl.h b/content/browser/web_contents/navigation_entry_impl.h
index 795dafa..55477e2 100644
--- a/content/browser/web_contents/navigation_entry_impl.h
+++ b/content/browser/web_contents/navigation_entry_impl.h
@@ -157,13 +157,14 @@ class CONTENT_EXPORT NavigationEntryImpl
     return transferred_global_request_id_;
   }
 
-  // Whether this (pending) navigation is reload across site instances.
+  // Whether this (pending) navigation needs to replace current entry.
   // Resets to false after commit.
-  void set_is_cross_site_reload(bool is_cross_site_reload) {
-    is_cross_site_reload_ = is_cross_site_reload;
+  bool should_replace_entry() const {
+    return should_replace_entry_;
   }
-  bool is_cross_site_reload() const {
-    return is_cross_site_reload_;
+
+  void set_should_replace_entry(bool should_replace_entry) {
+    should_replace_entry_ = should_replace_entry;
   }
 
  private:
@@ -232,7 +233,11 @@ class CONTENT_EXPORT NavigationEntryImpl
   // In such case, we must treat it as an existing navigation in the new site
   // instance, instead of a new navigation. This value should not be persisted
   // and is not needed after the entry commits.
-  bool is_cross_site_reload_;
+  //
+  // We also use this flag for cross-process redirect navigations, so that the
+  // browser will replace the current navigation entry (which is the page
+  // doing the redirect).
+  bool should_replace_entry_;
 
   // Set when this entry should be able to access local file:// resources. This
   // value is not needed after the entry commits and is not persisted.
diff --git a/content/browser/web_contents/web_contents_impl.cc b/content/browser/web_contents/web_contents_impl.cc
index 0ef81c6..ba052a0 100644
--- a/content/browser/web_contents/web_contents_impl.cc
+++ b/content/browser/web_contents/web_contents_impl.cc
@@ -2870,7 +2870,8 @@ void WebContentsImpl::RequestOpenURL(RenderViewHost* rvh,
                                      const GURL& url,
                                      const Referrer& referrer,
                                      WindowOpenDisposition disposition,
-                                     int64 source_frame_id) {
+                                     int64 source_frame_id,
+                                     bool is_cross_site_redirect) {
   // If this came from a swapped out RenderViewHost, we only allow the request
   // if we are still in the same BrowsingInstance.
   if (static_cast<RenderViewHostImpl*>(rvh)->is_swapped_out() &&
@@ -2881,7 +2882,7 @@ void WebContentsImpl::RequestOpenURL(RenderViewHost* rvh,
   // Delegate to RequestTransferURL because this is just the generic
   // case where |old_request_id| is empty.
   RequestTransferURL(url, referrer, disposition, source_frame_id,
-                     GlobalRequestID());
+                     GlobalRequestID(), is_cross_site_redirect);
 }
 
 void WebContentsImpl::RequestTransferURL(
@@ -2889,7 +2890,8 @@ void WebContentsImpl::RequestTransferURL(
     const Referrer& referrer,
     WindowOpenDisposition disposition,
     int64 source_frame_id,
-    const GlobalRequestID& old_request_id) {
+    const GlobalRequestID& old_request_id,
+    bool is_cross_site_redirect) {
   WebContents* new_contents = NULL;
   PageTransition transition_type = PAGE_TRANSITION_LINK;
   if (render_manager_.web_ui()) {
@@ -2911,6 +2913,7 @@ void WebContentsImpl::RequestTransferURL(
     OpenURLParams params(url, referrer, source_frame_id, disposition,
         PAGE_TRANSITION_LINK, true /* is_renderer_initiated */);
     params.transferred_global_request_id = old_request_id;
+    params.is_cross_site_redirect = is_cross_site_redirect;
     new_contents = OpenURL(params);
   }
   if (new_contents) {
diff --git a/content/browser/web_contents/web_contents_impl.h b/content/browser/web_contents/web_contents_impl.h
index 4bfaf17..11bff14 100644
--- a/content/browser/web_contents/web_contents_impl.h
+++ b/content/browser/web_contents/web_contents_impl.h
@@ -344,13 +344,15 @@ class CONTENT_EXPORT WebContentsImpl
                               const GURL& url,
                               const Referrer& referrer,
                               WindowOpenDisposition disposition,
-                              int64 source_frame_id) OVERRIDE;
+                              int64 source_frame_id,
+                              bool is_cross_site_redirect) OVERRIDE;
   virtual void RequestTransferURL(
       const GURL& url,
       const Referrer& referrer,
       WindowOpenDisposition disposition,
       int64 source_frame_id,
-      const GlobalRequestID& transferred_global_request_id) OVERRIDE;
+      const GlobalRequestID& transferred_global_request_id,
+      bool is_cross_site_redirect) OVERRIDE;
   virtual void RouteCloseEvent(RenderViewHost* rvh) OVERRIDE;
   virtual void RouteMessageEvent(
       RenderViewHost* rvh,
diff --git a/content/common/view_messages.h b/content/common/view_messages.h
index 0474a55..9da7930 100644
--- a/content/common/view_messages.h
+++ b/content/common/view_messages.h
@@ -746,6 +746,15 @@ IPC_STRUCT_BEGIN(ViewMsg_Navigate_Params)
   IPC_STRUCT_MEMBER(bool, can_load_local_resources)
 IPC_STRUCT_END()
 
+// Parameters for an OpenURL request.
+IPC_STRUCT_BEGIN(ViewHostMsg_OpenURL_Params)
+  IPC_STRUCT_MEMBER(GURL, url)
+  IPC_STRUCT_MEMBER(content::Referrer, referrer)
+  IPC_STRUCT_MEMBER(WindowOpenDisposition, disposition)
+  IPC_STRUCT_MEMBER(int64, frame_id)
+  IPC_STRUCT_MEMBER(bool, is_cross_site_redirect)
+IPC_STRUCT_END()
+
 IPC_STRUCT_BEGIN(ViewMsg_New_Params)
   // Renderer-wide preferences.
   IPC_STRUCT_MEMBER(content::RendererPreferences, renderer_preferences)
@@ -1897,11 +1906,7 @@ IPC_SYNC_MESSAGE_ROUTED4_2(ViewHostMsg_RunJavaScriptMessage,
                            string16     /* out - user_input field */)
 
 // Requests that the given URL be opened in the specified manner.
-IPC_MESSAGE_ROUTED4(ViewHostMsg_OpenURL,
-                    GURL /* url */,
-                    content::Referrer /* referrer */,
-                    WindowOpenDisposition /* disposition */,
-                    int64 /* frame id */)
+IPC_MESSAGE_ROUTED1(ViewHostMsg_OpenURL, ViewHostMsg_OpenURL_Params)
 
 // Notifies that the preferred size of the content changed.
 IPC_MESSAGE_ROUTED1(ViewHostMsg_DidContentsPreferredSizeChange,
diff --git a/content/public/browser/navigation_controller.cc b/content/public/browser/navigation_controller.cc
index 4c9ba58..5fc56dc 100644
--- a/content/public/browser/navigation_controller.cc
+++ b/content/public/browser/navigation_controller.cc
@@ -15,7 +15,8 @@ NavigationController::LoadURLParams::LoadURLParams(const GURL& url)
       is_renderer_initiated(false),
       override_user_agent(UA_OVERRIDE_INHERIT),
       browser_initiated_post_data(NULL),
-      can_load_local_resources(false) {
+      can_load_local_resources(false),
+      is_cross_site_redirect(false) {
 }
 
 NavigationController::LoadURLParams::~LoadURLParams() {
@@ -33,7 +34,8 @@ NavigationController::LoadURLParams::LoadURLParams(
       transferred_global_request_id(other.transferred_global_request_id),
       base_url_for_data_url(other.base_url_for_data_url),
       virtual_url_for_data_url(other.virtual_url_for_data_url),
-      browser_initiated_post_data(other.browser_initiated_post_data) {
+      browser_initiated_post_data(other.browser_initiated_post_data),
+      is_cross_site_redirect(false) {
 }
 
 NavigationController::LoadURLParams&
@@ -50,6 +52,7 @@ NavigationController::LoadURLParams::operator=(
   base_url_for_data_url = other.base_url_for_data_url;
   virtual_url_for_data_url = other.virtual_url_for_data_url;
   browser_initiated_post_data = other.browser_initiated_post_data;
+  is_cross_site_redirect = other.is_cross_site_redirect;
 
   return *this;
 }
diff --git a/content/public/browser/navigation_controller.h b/content/public/browser/navigation_controller.h
index 8a64d48..d37960b 100644
--- a/content/public/browser/navigation_controller.h
+++ b/content/public/browser/navigation_controller.h
@@ -151,6 +151,10 @@ class NavigationController {
     // True if this URL should be able to access local resources.
     bool can_load_local_resources;
 
+    // Indicates whether this navigation involves a cross-process redirect,
+    // in which case it should replace the current navigation entry.
+    bool is_cross_site_redirect;
+
     explicit LoadURLParams(const GURL& url);
     ~LoadURLParams();
 
diff --git a/content/public/browser/page_navigator.cc b/content/public/browser/page_navigator.cc
index 029afbc..608b2d0 100644
--- a/content/public/browser/page_navigator.cc
+++ b/content/public/browser/page_navigator.cc
@@ -17,7 +17,8 @@ OpenURLParams::OpenURLParams(
       source_frame_id(-1),
       disposition(disposition),
       transition(transition),
-      is_renderer_initiated(is_renderer_initiated) {
+      is_renderer_initiated(is_renderer_initiated),
+      is_cross_site_redirect(false) {
 }
 
 OpenURLParams::OpenURLParams(
@@ -32,14 +33,16 @@ OpenURLParams::OpenURLParams(
       source_frame_id(source_frame_id),
       disposition(disposition),
       transition(transition),
-      is_renderer_initiated(is_renderer_initiated) {
+      is_renderer_initiated(is_renderer_initiated),
+      is_cross_site_redirect(false) {
 }
 
 OpenURLParams::OpenURLParams()
     : source_frame_id(-1),
       disposition(UNKNOWN),
       transition(PageTransitionFromInt(0)),
-      is_renderer_initiated(false) {
+      is_renderer_initiated(false),
+      is_cross_site_redirect(false) {
 }
 
 OpenURLParams::~OpenURLParams() {
diff --git a/content/public/browser/page_navigator.h b/content/public/browser/page_navigator.h
index 90eaef6..bc716df 100644
--- a/content/public/browser/page_navigator.h
+++ b/content/public/browser/page_navigator.h
@@ -64,6 +64,10 @@ struct CONTENT_EXPORT OpenURLParams {
   // transferred to a new renderer.
   GlobalRequestID transferred_global_request_id;
 
+  // Indicates whether this navigation involves a cross-process redirect,
+  // in which case it should replace the current navigation entry.
+  bool is_cross_site_redirect;
+
  private:
   OpenURLParams();
 };
diff --git a/content/public/renderer/navigation_state.cc b/content/public/renderer/navigation_state.cc
index 9208efa..37fcfb7 100644
--- a/content/public/renderer/navigation_state.cc
+++ b/content/public/renderer/navigation_state.cc
@@ -18,7 +18,8 @@ NavigationState::NavigationState(content::PageTransition transition_type,
       was_within_same_page_(false),
       transferred_request_child_id_(-1),
       transferred_request_request_id_(-1),
-      allow_download_(true) {
+      allow_download_(true),
+      is_redirect_in_progress_(false) {
 }
 
 NavigationState::~NavigationState() {}
diff --git a/content/public/renderer/navigation_state.h b/content/public/renderer/navigation_state.h
index f13b9b7..58a6534 100644
--- a/content/public/renderer/navigation_state.h
+++ b/content/public/renderer/navigation_state.h
@@ -78,6 +78,12 @@ class NavigationState {
   bool allow_download() const {
     return allow_download_;
   }
+  void set_is_redirect_in_progress(bool value) {
+    is_redirect_in_progress_ = value;
+  }
+  bool is_redirect_in_progress() const {
+    return is_redirect_in_progress_;
+  }
 
  private:
   NavigationState(content::PageTransition transition_type,
@@ -96,6 +102,13 @@ class NavigationState {
   int transferred_request_request_id_;
   bool allow_download_;
 
+  // If we are handling a top-level client-side redirect, we need to set this
+  // to true when calling willPerformClientRedirect.
+  // If DecidePolicyForNavigation decides this redirect is a cross-process
+  // navigation, it will pass this flag to the browser process through OpenURL,
+  // which will eventually replace the current navigation entry.
+  bool is_redirect_in_progress_;
+
   DISALLOW_COPY_AND_ASSIGN(NavigationState);
 };
 
diff --git a/content/renderer/render_view_impl.cc b/content/renderer/render_view_impl.cc
index 14de9e9..8bccb4f 100644
--- a/content/renderer/render_view_impl.cc
+++ b/content/renderer/render_view_impl.cc
@@ -1660,12 +1660,17 @@ void RenderViewImpl::OpenURL(WebFrame* frame,
                              const GURL& url,
                              const Referrer& referrer,
                              WebNavigationPolicy policy) {
-  Send(new ViewHostMsg_OpenURL(
-      routing_id_,
-      url,
-      referrer,
-      NavigationPolicyToDisposition(policy),
-      frame->identifier()));
+  ViewHostMsg_OpenURL_Params params;
+  params.url = url;
+  params.referrer = referrer;
+  params.disposition = NavigationPolicyToDisposition(policy);
+  params.frame_id = frame->identifier();
+  DocumentState* document_state =
+      DocumentState::FromDataSource(frame->dataSource());
+  params.is_cross_site_redirect =
+      document_state->navigation_state()->is_redirect_in_progress();
+
+  Send(new ViewHostMsg_OpenURL(routing_id_, params));
 }
 
 // WebViewDelegate ------------------------------------------------------------
@@ -2959,6 +2964,10 @@ void RenderViewImpl::willPerformClientRedirect(
     double fire_time) {
   // Replace any occurrences of swappedout:// with about:blank.
   const WebURL& blank_url = GURL(chrome::kAboutBlankURL);
+  DocumentState* document_state =
+      DocumentState::FromDataSource(frame->dataSource());
+  document_state->navigation_state()->set_is_redirect_in_progress(true);
+
   FOR_EACH_OBSERVER(
       RenderViewObserver, observers_,
       WillPerformClientRedirect(frame,
@@ -2967,6 +2976,10 @@ void RenderViewImpl::willPerformClientRedirect(
 }
 
 void RenderViewImpl::didCancelClientRedirect(WebFrame* frame) {
+  DocumentState* document_state =
+      DocumentState::FromDataSource(frame->dataSource());
+  document_state->navigation_state()->set_is_redirect_in_progress(false);
+
   FOR_EACH_OBSERVER(
       RenderViewObserver, observers_, DidCancelClientRedirect(frame));
 }
@@ -3401,6 +3414,7 @@ void RenderViewImpl::didCommitProvisionalLoad(WebFrame* frame,
   // If this committed load was initiated by a client redirect, we're
   // at the last stop now, so clear it.
   completed_client_redirect_src_ = Referrer();
+  navigation_state->set_is_redirect_in_progress(false);
 
   // Check whether we have new encoding name.
   UpdateEncoding(frame, frame->view()->pageEncoding().utf8());