Fix a race condition where rapid back/forward clicks could close a tab

This can be triggered when you're on the new tab page, going to *two* other
sites, then rapidly hitting back and forward randomly. If a cross-site
transition was canceled before the original page responds with an "OK to close
me" message, it will mistakenly categorize the close as not just for the
RenderView (correspondong to one side of the cross-site transition) but for the
entire tab.

This change adds an explicit parameter on the messages indicating whether it's
for interstials or for the tab so we don't have to rely on the request still
being active.

This also adds the "requesting process + route" in addition to the
"new process + request" so we can be more clear about sending the messages to
the correct place. The previous patch conbimed these in a confusing way.

BUG=16246
TEST=none
Review URL: http://codereview.chromium.org/159255

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

17 files changed, 206 insertions, 111 deletions

diff --git a/chrome/browser/browser.cc b/chrome/browser/browser.cc
index b373b4f..64d478a 100644
--- a/chrome/browser/browser.cc
+++ b/chrome/browser/browser.cc
@@ -2487,7 +2487,7 @@ void Browser::ProcessPendingTabs() {
     // Null check render_view_host here as this gets called on a PostTask and
     // the tab's render_view_host may have been nulled out.
     if (tab->render_view_host()) {
-      tab->render_view_host()->FirePageUnload();
+      tab->render_view_host()->ClosePage(false, -1, -1);
     } else {
       ClearUnloadState(tab);
     }
diff --git a/chrome/browser/renderer_host/browser_render_process_host.cc b/chrome/browser/renderer_host/browser_render_process_host.cc
index 9c6930a..26e7f92 100644
--- a/chrome/browser/renderer_host/browser_render_process_host.cc
+++ b/chrome/browser/renderer_host/browser_render_process_host.cc
@@ -500,10 +500,8 @@ void BrowserRenderProcessHost::CancelResourceRequests(int render_widget_id) {
 }
 
 void BrowserRenderProcessHost::CrossSiteClosePageACK(
-    int new_render_process_host_id,
-    int new_request_id) {
-  widget_helper_->CrossSiteClosePageACK(new_render_process_host_id,
-                                        new_request_id);
+    const ViewMsg_ClosePage_Params& params) {
+  widget_helper_->CrossSiteClosePageACK(params);
 }
 
 bool BrowserRenderProcessHost::WaitForPaintMsg(int render_widget_id,
diff --git a/chrome/browser/renderer_host/browser_render_process_host.h b/chrome/browser/renderer_host/browser_render_process_host.h
index f02a1d0..63fb849 100644
--- a/chrome/browser/renderer_host/browser_render_process_host.h
+++ b/chrome/browser/renderer_host/browser_render_process_host.h
@@ -57,8 +57,7 @@ class BrowserRenderProcessHost : public RenderProcessHost,
   virtual bool Init();
   virtual int GetNextRoutingID();
   virtual void CancelResourceRequests(int render_widget_id);
-  virtual void CrossSiteClosePageACK(int new_render_process_host_id,
-                                     int new_request_id);
+  virtual void CrossSiteClosePageACK(const ViewMsg_ClosePage_Params& params);
   virtual bool WaitForPaintMsg(int render_widget_id,
                                const base::TimeDelta& max_delay,
                                IPC::Message* msg);
diff --git a/chrome/browser/renderer_host/mock_render_process_host.cc b/chrome/browser/renderer_host/mock_render_process_host.cc
index 4531232..a32ebed 100644
--- a/chrome/browser/renderer_host/mock_render_process_host.cc
+++ b/chrome/browser/renderer_host/mock_render_process_host.cc
@@ -30,8 +30,7 @@ void MockRenderProcessHost::CancelResourceRequests(int render_widget_id) {
 }
 
 void MockRenderProcessHost::CrossSiteClosePageACK(
-    int new_render_process_host_id,
-    int new_request_id) {
+    const ViewMsg_ClosePage_Params& params) {
 }
 
 bool MockRenderProcessHost::WaitForPaintMsg(int render_widget_id,
diff --git a/chrome/browser/renderer_host/mock_render_process_host.h b/chrome/browser/renderer_host/mock_render_process_host.h
index 1699a95..d8b04e6 100644
--- a/chrome/browser/renderer_host/mock_render_process_host.h
+++ b/chrome/browser/renderer_host/mock_render_process_host.h
@@ -33,8 +33,7 @@ class MockRenderProcessHost : public RenderProcessHost {
   virtual bool Init();
   virtual int GetNextRoutingID();
   virtual void CancelResourceRequests(int render_widget_id);
-  virtual void CrossSiteClosePageACK(int new_render_process_host_id,
-                                     int new_request_id);
+  virtual void CrossSiteClosePageACK(const ViewMsg_ClosePage_Params& params);
   virtual bool WaitForPaintMsg(int render_widget_id,
                                const base::TimeDelta& max_delay,
                                IPC::Message* msg);
diff --git a/chrome/browser/renderer_host/render_process_host.h b/chrome/browser/renderer_host/render_process_host.h
index 4ce3741..c091e85 100644
--- a/chrome/browser/renderer_host/render_process_host.h
+++ b/chrome/browser/renderer_host/render_process_host.h
@@ -16,6 +16,7 @@
 #include "ipc/ipc_sync_channel.h"
 
 class Profile;
+struct ViewMsg_ClosePage_Params;
 
 // Virtual interface that represents the browser side of the browser <->
 // renderer communication channel. There will generally be one
@@ -118,8 +119,8 @@ class RenderProcessHost : public IPC::Channel::Sender,
   // ResourceDispatcherHost.  Necessary for a cross-site request, in the case
   // that the original RenderViewHost is not live and thus cannot run an
   // onunload handler.
-  virtual void CrossSiteClosePageACK(int new_render_process_host_id,
-                                     int new_request_id) = 0;
+  virtual void CrossSiteClosePageACK(
+      const ViewMsg_ClosePage_Params& params) = 0;
 
   // Called on the UI thread to wait for the next PaintRect message for the
   // specified render widget.  Returns true if successful, and the msg out-
diff --git a/chrome/browser/renderer_host/render_view_host.cc b/chrome/browser/renderer_host/render_view_host.cc
index ff99693..070bd55 100644
--- a/chrome/browser/renderer_host/render_view_host.cc
+++ b/chrome/browser/renderer_host/render_view_host.cc
@@ -320,44 +320,37 @@ void RenderViewHost::FirePageBeforeUnload() {
   }
 }
 
-void RenderViewHost::FirePageUnload() {
-  ClosePage(process()->pid(), routing_id());
-}
-
-// static
-void RenderViewHost::ClosePageIgnoringUnloadEvents(int render_process_host_id,
-                                                   int request_id) {
-  RenderViewHost* rvh = RenderViewHost::FromID(render_process_host_id,
-                                               request_id);
-  if (!rvh)
-    return;
-
-  rvh->StopHangMonitorTimeout();
-  rvh->is_waiting_for_unload_ack_ = false;
-
-  rvh->set_sudden_termination_allowed(true);
-  rvh->delegate()->Close(rvh);
-}
-
-void RenderViewHost::ClosePage(int new_render_process_host_id,
+void RenderViewHost::ClosePage(bool for_cross_site_transition,
+                               int new_render_process_host_id,
                                int new_request_id) {
   // Start the hang monitor in case the renderer hangs in the unload handler.
   is_waiting_for_unload_ack_ = true;
   StartHangMonitorTimeout(TimeDelta::FromMilliseconds(kUnloadTimeoutMS));
 
+  ViewMsg_ClosePage_Params params;
+  params.closing_process_id = process()->pid();
+  params.closing_route_id = routing_id();
+  params.for_cross_site_transition = for_cross_site_transition;
+  params.new_render_process_host_id = new_render_process_host_id;
+  params.new_request_id = new_request_id;
   if (IsRenderViewLive()) {
-    Send(new ViewMsg_ClosePage(routing_id(),
-                               new_render_process_host_id,
-                               new_request_id));
+    Send(new ViewMsg_ClosePage(routing_id(), params));
   } else {
     // This RenderViewHost doesn't have a live renderer, so just skip closing
     // the page.  We must notify the ResourceDispatcherHost on the IO thread,
     // which we will do through the RenderProcessHost's widget helper.
-    process()->CrossSiteClosePageACK(new_render_process_host_id,
-                                     new_request_id);
+    process()->CrossSiteClosePageACK(params);
   }
 }
 
+void RenderViewHost::ClosePageIgnoringUnloadEvents() {
+  StopHangMonitorTimeout();
+  is_waiting_for_unload_ack_ = false;
+
+  sudden_termination_allowed_ = true;
+  delegate_->Close(this);
+}
+
 void RenderViewHost::SetHasPendingCrossSiteRequest(bool has_pending_request,
                                                    int request_id) {
   Singleton<CrossSiteRequestManager>()->SetHasPendingCrossSiteRequest(
diff --git a/chrome/browser/renderer_host/render_view_host.h b/chrome/browser/renderer_host/render_view_host.h
index 26612b0..6d196f4e 100644
--- a/chrome/browser/renderer_host/render_view_host.h
+++ b/chrome/browser/renderer_host/render_view_host.h
@@ -149,27 +149,24 @@ class RenderViewHost : public RenderWidgetHost,
   void SetNavigationsSuspended(bool suspend);
 
   // Causes the renderer to invoke the onbeforeunload event handler.  The
-  // result will be returned via ViewMsg_ShouldClose.
+  // result will be returned via ViewMsg_ShouldClose. See also ClosePage which
+  // will fire the PageUnload event.
   void FirePageBeforeUnload();
 
-  // Close the page after the page has responded that it can be closed via
-  // ViewMsg_ShouldClose. This is where the page itself is closed. The
-  // unload handler is triggered here, which can block with a dialog, but cannot
-  // cancel the close of the page.
-  void FirePageUnload();
+  // Causes the renderer to close the current page, including running its
+  // onunload event handler.  A ClosePage_ACK message will be sent to the
+  // ResourceDispatcherHost when it is finished.
+  //
+  // Please see ViewMsg_ClosePage in resource_messages_internal.h for a
+  // description of the parameters.
+  void ClosePage(bool for_cross_site_transition,
+                 int new_render_process_host_id,
+                 int new_request_id);
 
   // Close the page ignoring whether it has unload events registers.
   // This is called after the beforeunload and unload events have fired
   // and the user has agreed to continue with closing the page.
-  static void ClosePageIgnoringUnloadEvents(int render_process_host_id,
-                                            int request_id);
-
-  // Causes the renderer to close the current page, including running its
-  // onunload event handler.  A ClosePage_ACK message will be sent to the
-  // ResourceDispatcherHost when it is finished. |new_render_process_host_id|
-  // and |new_request_id| will help the ResourceDispatcherHost identify which
-  // response is associated with this event.
-  void ClosePage(int new_render_process_host_id, int new_request_id);
+  void ClosePageIgnoringUnloadEvents();
 
   // Sets whether this RenderViewHost has an outstanding cross-site request,
   // for which another renderer will need to run an onunload event handler.
diff --git a/chrome/browser/renderer_host/render_widget_helper.cc b/chrome/browser/renderer_host/render_widget_helper.cc
index 9d898b8..91ea97b 100644
--- a/chrome/browser/renderer_host/render_widget_helper.cc
+++ b/chrome/browser/renderer_host/render_widget_helper.cc
@@ -84,14 +84,13 @@ void RenderWidgetHelper::CancelResourceRequests(int render_widget_id) {
   }
 }
 
-void RenderWidgetHelper::CrossSiteClosePageACK(int new_render_process_host_id,
-                                               int new_request_id) {
+void RenderWidgetHelper::CrossSiteClosePageACK(
+    const ViewMsg_ClosePage_Params& params) {
   if (g_browser_process->io_thread()) {
     g_browser_process->io_thread()->message_loop()->PostTask(FROM_HERE,
         NewRunnableMethod(this,
                           &RenderWidgetHelper::OnCrossSiteClosePageACK,
-                          new_render_process_host_id,
-                          new_request_id));
+                          params));
   }
 }
 
@@ -196,10 +195,8 @@ void RenderWidgetHelper::OnCancelResourceRequests(
 }
 
 void RenderWidgetHelper::OnCrossSiteClosePageACK(
-    int new_render_process_host_id,
-    int new_request_id) {
-  resource_dispatcher_host_->OnClosePageACK(
-      new_render_process_host_id, new_request_id);
+    ViewMsg_ClosePage_Params params) {
+  resource_dispatcher_host_->OnClosePageACK(params);
 }
 
 void RenderWidgetHelper::CreateNewWindow(int opener_id,
diff --git a/chrome/browser/renderer_host/render_widget_helper.h b/chrome/browser/renderer_host/render_widget_helper.h
index 0ff672b..87c97be 100644
--- a/chrome/browser/renderer_host/render_widget_helper.h
+++ b/chrome/browser/renderer_host/render_widget_helper.h
@@ -26,6 +26,7 @@ class TimeDelta;
 
 class MessageLoop;
 class ResourceDispatcherHost;
+struct ViewMsg_ClosePage_Params;
 
 // Instantiated per RenderProcessHost to provide various optimizations on
 // behalf of a RenderWidgetHost.  This class bridges between the IO thread
@@ -104,8 +105,7 @@ class RenderWidgetHelper :
   // corresponding functions in RenderProcessHost. See those declarations
   // for documentation.
   void CancelResourceRequests(int render_widget_id);
-  void CrossSiteClosePageACK(int new_render_process_host_id,
-                             int new_request_id);
+  void CrossSiteClosePageACK(const ViewMsg_ClosePage_Params& params);
   bool WaitForPaintMsg(int render_widget_id,
                        const base::TimeDelta& max_delay,
                        IPC::Message* msg);
@@ -174,8 +174,7 @@ class RenderWidgetHelper :
   void OnCancelResourceRequests(int render_widget_id);
 
   // Called on the IO thread to resume a cross-site response.
-  void OnCrossSiteClosePageACK(int new_render_process_host_id,
-                               int new_request_id);
+  void OnCrossSiteClosePageACK(ViewMsg_ClosePage_Params params);
 
 #if defined(OS_MACOSX)
   // Called on destruction to release all allocated transport DIBs
diff --git a/chrome/browser/renderer_host/resource_dispatcher_host.cc b/chrome/browser/renderer_host/resource_dispatcher_host.cc
index 5e46a8a..a7b034f 100644
--- a/chrome/browser/renderer_host/resource_dispatcher_host.cc
+++ b/chrome/browser/renderer_host/resource_dispatcher_host.cc
@@ -109,10 +109,37 @@ const int kMaxPendingDataMessages = 20;
 // This bound is 25MB, which allows for around 6000 outstanding requests.
 const int kMaxOutstandingRequestsCostPerProcess = 26214400;
 
-// A NotificationTask proxies a resource dispatcher notification from the IO
-// thread to the RenderViewHostDelegate on the UI thread. It should be
-// constructed on the IO thread and run in the UI thread.
-class NotificationTask : public Task {
+// Calls ClosePageIgnoringUnloadEvents on the UI thread for the given
+// RenderView.
+//
+// If there are more functions we need to call on RVH, we should generalize this
+// like the "Delegate" notification task below.
+class RVHCloseNotificationTask : public Task {
+ public:
+  RVHCloseNotificationTask(int render_process_host_id,
+                           int render_view_host_id)
+      : render_process_host_id_(render_process_host_id),
+        render_view_host_id_(render_view_host_id) {
+  }
+
+  virtual void Run() {
+    RenderViewHost* rvh = RenderViewHost::FromID(render_process_host_id_,
+                                                 render_view_host_id_);
+    if (rvh)
+      rvh->ClosePageIgnoringUnloadEvents();
+  }
+
+ private:
+  int render_process_host_id_;
+  int render_view_host_id_;
+
+  DISALLOW_COPY_AND_ASSIGN(RVHCloseNotificationTask);
+};
+
+// A RVHDelegateNotificationTask proxies a resource dispatcher notification
+// from the IO thread to the RenderViewHostDelegate on the UI thread. It should
+// be constructed on the IO thread and run in the UI thread.
+class RVHDelegateNotificationTask : public Task {
  public:
   typedef void (RenderViewHostDelegate::Resource::* ResourceFunction)
       (ResourceRequestDetails*);
@@ -122,11 +149,13 @@ class NotificationTask : public Task {
   //
   // This object will take ownership of the details pointer, which must be
   // allocated on the heap.
-  NotificationTask(
+  RVHDelegateNotificationTask(
       URLRequest* request,
       ResourceFunction function,
       ResourceRequestDetails* details)
-      : function_(function),
+      : render_process_host_id_(-1),
+        render_view_host_id_(-1),
+        function_(function),
         details_(details) {
     if (!ResourceDispatcherHost::RenderViewForRequest(request,
                                                       &render_process_host_id_,
@@ -156,7 +185,7 @@ class NotificationTask : public Task {
   // The details for the notification.
   scoped_ptr<ResourceRequestDetails> details_;
 
-  DISALLOW_COPY_AND_ASSIGN(NotificationTask);
+  DISALLOW_COPY_AND_ASSIGN(RVHDelegateNotificationTask);
 };
 
 // Consults the RendererSecurity policy to determine whether the
@@ -525,23 +554,28 @@ void ResourceDispatcherHost::OnCancelRequest(int request_id) {
   CancelRequest(receiver_->GetProcessId(), request_id, true, true);
 }
 
-void ResourceDispatcherHost::OnClosePageACK(int new_render_process_host_id,
-                                           int new_request_id) {
-  GlobalRequestID global_id(new_render_process_host_id, new_request_id);
-  PendingRequestList::iterator i = pending_requests_.find(global_id);
-  if (i == pending_requests_.end()) {
-    // If there are no matching pending requests, then this is not a
-    // cross-site navigation and we are just closing the tab/browser.
-    ui_loop_->PostTask(FROM_HERE, NewRunnableFunction(
-        &RenderViewHost::ClosePageIgnoringUnloadEvents,
-        new_render_process_host_id,
-        new_request_id));
-    return;
-  }
-
-  ExtraRequestInfo* info = ExtraInfoForRequest(i->second);
-  if (info->cross_site_handler) {
-    info->cross_site_handler->ResumeResponse();
+void ResourceDispatcherHost::OnClosePageACK(
+    const ViewMsg_ClosePage_Params& params) {
+  if (params.for_cross_site_transition) {
+    // Closes for cross-site transitions are handled such that the cross-site
+    // transition continues.
+    GlobalRequestID global_id(params.new_render_process_host_id,
+                              params.new_request_id);
+    PendingRequestList::iterator i = pending_requests_.find(global_id);
+    if (i != pending_requests_.end()) {
+      // The response we were meant to resume could have already been canceled.
+      ExtraRequestInfo* info = ExtraInfoForRequest(i->second);
+      if (info->cross_site_handler)
+        info->cross_site_handler->ResumeResponse();
+    }
+  } else {
+    // This is a tab close, so just forward the message to close it.
+    DCHECK(params.new_render_process_host_id == -1);
+    DCHECK(params.new_request_id == -1);
+    ui_loop_->PostTask(
+        FROM_HERE,
+        new RVHCloseNotificationTask(params.closing_process_id,
+                                     params.closing_route_id));
   }
 }
 
@@ -1403,7 +1437,7 @@ void ResourceDispatcherHost::NotifyResponseStarted(URLRequest* request,
   FOR_EACH_OBSERVER(Observer, observer_list_, OnRequestStarted(this, request));
 
   // Notify the observers on the UI thread.
-  ui_loop_->PostTask(FROM_HERE, new NotificationTask(request,
+  ui_loop_->PostTask(FROM_HERE, new RVHDelegateNotificationTask(request,
       &RenderViewHostDelegate::Resource::DidStartReceivingResourceResponse,
       new ResourceRequestDetails(request,
          GetCertID(request, process_id))));
@@ -1428,7 +1462,7 @@ void ResourceDispatcherHost::NotifyReceivedRedirect(URLRequest* request,
 
   // Notify the observers on the UI thread.
   ui_loop_->PostTask(FROM_HERE,
-      new NotificationTask(request,
+      new RVHDelegateNotificationTask(request,
           &RenderViewHostDelegate::Resource::DidRedirectResource,
           new ResourceRedirectDetails(request, cert_id, new_url)));
 }
diff --git a/chrome/browser/renderer_host/resource_dispatcher_host.h b/chrome/browser/renderer_host/resource_dispatcher_host.h
index 70ca89f..b75d408 100644
--- a/chrome/browser/renderer_host/resource_dispatcher_host.h
+++ b/chrome/browser/renderer_host/resource_dispatcher_host.h
@@ -40,6 +40,7 @@ class SSLClientAuthHandler;
 class URLRequestContext;
 class WebKitThread;
 struct ViewHostMsg_Resource_Request;
+struct ViewMsg_ClosePage_Params;
 
 class ResourceDispatcherHost : public URLRequest::Delegate {
  public:
@@ -298,7 +299,7 @@ class ResourceDispatcherHost : public URLRequest::Delegate {
   MessageLoop* ui_loop() const { return ui_loop_; }
 
   // Called when the onunload handler for a cross-site request has finished.
-  void OnClosePageACK(int process_id, int request_id);
+  void OnClosePageACK(const ViewMsg_ClosePage_Params& params);
 
   // Force cancels any pending requests for the given process.
   void CancelRequestsForProcess(int process_id);
diff --git a/chrome/browser/tab_contents/render_view_host_manager.cc b/chrome/browser/tab_contents/render_view_host_manager.cc
index e5b3645..4146f862 100644
--- a/chrome/browser/tab_contents/render_view_host_manager.cc
+++ b/chrome/browser/tab_contents/render_view_host_manager.cc
@@ -145,8 +145,14 @@ bool RenderViewHostManager::ShouldCloseTabOnUnresponsiveRenderer() {
     // be swapped in as part of the usual DidNavigate logic.  (If the unload
     // handler later finishes, this call will be ignored because the state in
     // CrossSiteResourceHandler will already be cleaned up.)
-    current_host()->process()->CrossSiteClosePageACK(
-        pending_render_view_host_->process()->pid(), pending_request_id);
+    ViewMsg_ClosePage_Params params;
+    params.closing_process_id = render_view_host_->process()->pid();
+    params.closing_route_id = render_view_host_->routing_id();
+    params.for_cross_site_transition = true;
+    params.new_render_process_host_id =
+        pending_render_view_host_->process()->pid();
+    params.new_request_id = pending_request_id;
+    current_host()->process()->CrossSiteClosePageACK(params);
   }
   return false;
 }
@@ -215,7 +221,7 @@ void RenderViewHostManager::ShouldClosePage(bool proceed) {
 
     if (proceed_to_fire_unload) {
       // This is not a cross-site navigation, the tab is being closed.
-      render_view_host_->FirePageUnload();
+      render_view_host_->ClosePage(true, -1, -1);
     }
     return;
   }
@@ -246,7 +252,8 @@ void RenderViewHostManager::OnCrossSiteResponse(int new_render_process_host_id,
   // will send a ClosePage_ACK to the ResourceDispatcherHost with the given
   // IDs (of the pending RVH's request), allowing the pending RVH's response to
   // resume.
-  render_view_host_->ClosePage(new_render_process_host_id, new_request_id);
+  render_view_host_->ClosePage(true,
+                               new_render_process_host_id, new_request_id);
 
   // ResourceDispatcherHost has told us to run the onunload handler, which
   // means it is not a download or unsafe page, and we are going to perform the
diff --git a/chrome/common/render_messages.h b/chrome/common/render_messages.h
index 6835b49..2ca15fd 100644
--- a/chrome/common/render_messages.h
+++ b/chrome/common/render_messages.h
@@ -223,6 +223,42 @@ struct ViewMsg_UploadFile_Params {
   std::wstring other_values;
 };
 
+// Information on closing a tab. This is used both for ViewMsg_ClosePage, and
+// the corresponding ViewHostMsg_ClosePage_ACK.
+struct ViewMsg_ClosePage_Params {
+  // The identifier of the RenderProcessHost for the currently closing view.
+  //
+  // These first two parameters are technically redundant since they are
+  // needed only when processing the ACK message, and the processor
+  // theoretically knows both the process and route ID. However, this is
+  // difficult to figure out with our current implementation, so this
+  // information is duplicate here.
+  int closing_process_id;
+
+  // The route identifier for the currently closing RenderView.
+  int closing_route_id;
+
+  // True when this close is for the first (closing) tab of a cross-site
+  // transition where we switch processes. False indicates the close is for the
+  // entire tab.
+  //
+  // When true, the new_* variables below must be filled in. Otherwise they must
+  // both be -1.
+  bool for_cross_site_transition;
+
+  // The identifier of the RenderProcessHost for the new view attempting to
+  // replace the closing one above. This must be valid when
+  // for_cross_site_transition is set, and must be -1 otherwise.
+  int new_render_process_host_id;
+
+  // The identifier of the *request* the new view made that is causing the
+  // cross-site transition. This is *not* a route_id, but the request that we
+  // will resume once the ACK from the closing view has been received. This
+  // must be valid when for_cross_site_transition is set, and must be -1
+  // otherwise.
+  int new_request_id;
+};
+
 // Parameters for a resource request.
 struct ViewHostMsg_Resource_Request {
   // The request method: GET, POST, etc.
@@ -1243,6 +1279,41 @@ struct ParamTraits<NavigationGesture> {
   }
 };
 
+// Traits for ViewMsg_Close_Params.
+template <>
+struct ParamTraits<ViewMsg_ClosePage_Params> {
+  typedef ViewMsg_ClosePage_Params param_type;
+  static void Write(Message* m, const param_type& p) {
+    WriteParam(m, p.closing_process_id);
+    WriteParam(m, p.closing_route_id);
+    WriteParam(m, p.for_cross_site_transition);
+    WriteParam(m, p.new_render_process_host_id);
+    WriteParam(m, p.new_request_id);
+  }
+
+  static bool Read(const Message* m, void** iter, param_type* r) {
+    return ReadParam(m, iter, &r->closing_process_id) &&
+           ReadParam(m, iter, &r->closing_route_id) &&
+           ReadParam(m, iter, &r->for_cross_site_transition) &&
+           ReadParam(m, iter, &r->new_render_process_host_id) &&
+           ReadParam(m, iter, &r->new_request_id);
+  }
+
+  static void Log(const param_type& p, std::wstring* l) {
+    l->append(L"(");
+    LogParam(p.closing_process_id, l);
+    l->append(L", ");
+    LogParam(p.closing_route_id, l);
+    l->append(L", ");
+    LogParam(p.for_cross_site_transition, l);
+    l->append(L", ");
+    LogParam(p.new_render_process_host_id, l);
+    l->append(L", ");
+    LogParam(p.new_request_id, l);
+    l->append(L")");
+  }
+};
+
 // Traits for ViewHostMsg_Resource_Request
 template <>
 struct ParamTraits<ViewHostMsg_Resource_Request> {
diff --git a/chrome/common/render_messages_internal.h b/chrome/common/render_messages_internal.h
index f240a34..36afb03 100644
--- a/chrome/common/render_messages_internal.h
+++ b/chrome/common/render_messages_internal.h
@@ -466,10 +466,12 @@ IPC_BEGIN_MESSAGES(View)
   IPC_MESSAGE_ROUTED0(ViewMsg_ShouldClose)
 
   // Instructs the renderer to close the current page, including running the
-  // onunload event handler.  Expects a ClosePage_ACK message when finished.
-  IPC_MESSAGE_ROUTED2(ViewMsg_ClosePage,
-                      int /* new_render_process_host_id */,
-                      int /* new_request_id */)
+  // onunload event handler. See the struct in render_messages.h for more.
+  //
+  // Expects a ClosePage_ACK message when finished, where the parameters are
+  // echoed back.
+  IPC_MESSAGE_ROUTED1(ViewMsg_ClosePage,
+                      ViewMsg_ClosePage_Params)
 
   // Asks the renderer to send back stats on the WebCore cache broken down by
   // resource types.
@@ -1222,10 +1224,9 @@ IPC_BEGIN_MESSAGES(ViewHost)
                       bool /* proceed */)
 
   // Indicates that the current page has been closed, after a ClosePage
-  // message.
-  IPC_MESSAGE_ROUTED2(ViewHostMsg_ClosePage_ACK,
-                      int /* new_render_process_host_id */,
-                      int /* new_request_id */)
+  // message. The parameters are just echoed from the ClosePage request.
+  IPC_MESSAGE_ROUTED1(ViewHostMsg_ClosePage_ACK,
+                      ViewMsg_ClosePage_Params)
 
   IPC_MESSAGE_ROUTED4(ViewHostMsg_DidDownloadFavIcon,
                       int /* Identifier of the request */,
diff --git a/chrome/renderer/render_view.cc b/chrome/renderer/render_view.cc
index a2a37e1..f41e007 100644
--- a/chrome/renderer/render_view.cc
+++ b/chrome/renderer/render_view.cc
@@ -2736,8 +2736,7 @@ void RenderView::OnMsgShouldClose() {
   Send(new ViewHostMsg_ShouldClose_ACK(routing_id_, should_close));
 }
 
-void RenderView::OnClosePage(int new_render_process_host_id,
-                             int new_request_id) {
+void RenderView::OnClosePage(const ViewMsg_ClosePage_Params& params) {
   // TODO(creis): We'd rather use webview()->Close() here, but that currently
   // sets the WebView's delegate_ to NULL, preventing any JavaScript dialogs
   // in the onunload handler from appearing.  For now, we're bypassing that and
@@ -2751,14 +2750,13 @@ void RenderView::OnClosePage(int new_render_process_host_id,
     // TODO(davemoore) this code should be removed once WillCloseFrame() gets
     // called when a page is destroyed. DumpLoadHistograms() is safe to call
     // multiple times for the same frame, but it will simplify things.
-    if (url.SchemeIs("http") || url.SchemeIs("https"))
+    if (url.SchemeIs(chrome::kHttpScheme) || url.SchemeIs(chrome::kHttpsScheme))
       DumpLoadHistograms();
     main_frame->ClosePage();
   }
 
-  Send(new ViewHostMsg_ClosePage_ACK(routing_id_,
-                                     new_render_process_host_id,
-                                     new_request_id));
+  // Just echo back the params in the ACK.
+  Send(new ViewHostMsg_ClosePage_ACK(routing_id_, params));
 }
 
 void RenderView::OnThemeChanged() {
diff --git a/chrome/renderer/render_view.h b/chrome/renderer/render_view.h
index d7288e6..7aa408029b 100644
--- a/chrome/renderer/render_view.h
+++ b/chrome/renderer/render_view.h
@@ -56,6 +56,7 @@ class WebPluginDelegateProxy;
 class WebDevToolsAgentDelegate;
 struct ContextMenuMediaParams;
 struct ThumbnailScore;
+struct ViewMsg_ClosePage_Params;
 struct ViewMsg_Navigate_Params;
 struct ViewMsg_UploadFile_Params;
 struct WebDropData;
@@ -552,7 +553,7 @@ class RenderView : public RenderWidget,
 
   // Runs the onunload handler and closes the page, replying with ClosePage_ACK
   // (with the given RPH and request IDs, to help track the request).
-  void OnClosePage(int new_render_process_host_id, int new_request_id);
+  void OnClosePage(const ViewMsg_ClosePage_Params& params);
 
   // Notification about ui theme changes.
   void OnThemeChanged();