Merge 275383 "When deleting a WebContents, keep SessionStorageNa..."

> When deleting a WebContents, keep SessionStorageNamespaces used in the tab alive until we receive an acknowledgment from the renderer that the renderer side constructs have been cleaned up. Otherwise we can receive messages from still executing content referring to sessions that were prematurely deleted.
> 
> BUG=371304
> 
> Review URL: https://codereview.chromium.org/305103003

TBR=michaeln@chromium.org

Review URL: https://codereview.chromium.org/327763003

git-svn-id: svn://svn.chromium.org/chrome/branches/1985/src@276058 0039d316-1c4b-4281-b951-d872f2087c98

8 files changed, 116 insertions, 1 deletions

diff --git a/content/browser/renderer_host/render_process_host_impl.cc b/content/browser/renderer_host/render_process_host_impl.cc
index c01dc5c..beba9f0 100644
--- a/content/browser/renderer_host/render_process_host_impl.cc
+++ b/content/browser/renderer_host/render_process_host_impl.cc
@@ -325,6 +325,27 @@ class RendererSandboxedProcessLauncherDelegate
 #endif  // OS_POSIX
 };
 
+const char kSessionStorageHolderKey[] = "kSessionStorageHolderKey";
+
+class SessionStorageHolder : public base::SupportsUserData::Data {
+ public:
+  SessionStorageHolder() {}
+  virtual ~SessionStorageHolder() {}
+
+  void Hold(const SessionStorageNamespaceMap& sessions, int view_route_id) {
+    session_storage_namespaces_awaiting_close_[view_route_id] = sessions;
+  }
+
+  void Release(int old_route_id) {
+    session_storage_namespaces_awaiting_close_.erase(old_route_id);
+  }
+
+ private:
+  std::map<int, SessionStorageNamespaceMap >
+      session_storage_namespaces_awaiting_close_;
+  DISALLOW_COPY_AND_ASSIGN(SessionStorageHolder);
+};
+
 }  // namespace
 
 RendererMainThreadFactoryFunction g_renderer_main_thread_factory = NULL;
@@ -1323,6 +1344,7 @@ bool RenderProcessHostImpl::OnMessageReceived(const IPC::Message& msg) {
       IPC_MESSAGE_HANDLER(ViewHostMsg_UserMetricsRecordAction,
                           OnUserMetricsRecordAction)
       IPC_MESSAGE_HANDLER(ViewHostMsg_SavedPageAsMHTML, OnSavedPageAsMHTML)
+      IPC_MESSAGE_HANDLER(ViewHostMsg_Close_ACK, OnCloseACK)
       // Adding single handlers for your service here is fine, but once your
       // service needs more than one handler, please extract them into a new
       // message filter and add that filter to CreateMessageFilters().
@@ -1454,6 +1476,7 @@ void RenderProcessHostImpl::Cleanup() {
     message_port_message_filter_ = NULL;
     geolocation_dispatcher_host_ = NULL;
     screen_orientation_dispatcher_host_ = NULL;
+    RemoveUserData(kSessionStorageHolderKey);
 
     // Remove ourself from the list of renderer processes so that we can't be
     // reused in between now and when the Delete task runs.
@@ -1842,6 +1865,7 @@ void RenderProcessHostImpl::ProcessDied(bool already_dead) {
   message_port_message_filter_ = NULL;
   geolocation_dispatcher_host_ = NULL;
   screen_orientation_dispatcher_host_ = NULL;
+  RemoveUserData(kSessionStorageHolderKey);
 
   IDMap<IPC::Listener>::iterator iter(&listeners_);
   while (!iter.IsAtEnd()) {
@@ -1910,6 +1934,24 @@ RenderProcessHostImpl::screen_orientation_dispatcher_host() const {
   return make_scoped_refptr(screen_orientation_dispatcher_host_);
 }
 
+void RenderProcessHostImpl::ReleaseOnCloseACK(
+    RenderProcessHost* host,
+    const SessionStorageNamespaceMap& sessions,
+    int view_route_id) {
+  DCHECK(host);
+  if (sessions.empty())
+    return;
+  SessionStorageHolder* holder = static_cast<SessionStorageHolder*>
+      (host->GetUserData(kSessionStorageHolderKey));
+  if (!holder) {
+    holder = new SessionStorageHolder();
+    host->SetUserData(
+        kSessionStorageHolderKey,
+        holder);
+  }
+  holder->Hold(sessions, view_route_id);
+}
+
 void RenderProcessHostImpl::OnShutdownRequest() {
   // Don't shut down if there are active RenderViews, or if there are pending
   // RenderViews being swapped back in.
@@ -2013,6 +2055,14 @@ void RenderProcessHostImpl::OnUserMetricsRecordAction(
   RecordComputedAction(action);
 }
 
+void RenderProcessHostImpl::OnCloseACK(int old_route_id) {
+  SessionStorageHolder* holder = static_cast<SessionStorageHolder*>
+      (GetUserData(kSessionStorageHolderKey));
+  if (!holder)
+    return;
+  holder->Release(old_route_id);
+}
+
 void RenderProcessHostImpl::OnSavedPageAsMHTML(int job_id, int64 data_size) {
   MHTMLGenerationManager::GetInstance()->MHTMLGenerated(job_id, data_size);
 }
diff --git a/content/browser/renderer_host/render_process_host_impl.h b/content/browser/renderer_host/render_process_host_impl.h
index 358109f..03903ae 100644
--- a/content/browser/renderer_host/render_process_host_impl.h
+++ b/content/browser/renderer_host/render_process_host_impl.h
@@ -14,6 +14,7 @@
 #include "base/process/process.h"
 #include "base/timer/timer.h"
 #include "content/browser/child_process_launcher.h"
+#include "content/browser/dom_storage/session_storage_namespace_impl.h"
 #include "content/browser/geolocation/geolocation_dispatcher_host.h"
 #include "content/browser/power_monitor_message_broadcaster.h"
 #include "content/common/content_export.h"
@@ -176,6 +177,14 @@ class CONTENT_EXPORT RenderProcessHostImpl
   scoped_refptr<ScreenOrientationDispatcherHost>
       screen_orientation_dispatcher_host() const;
 
+  // Used to extend the lifetime of the sessions until the render view
+  // in the renderer is fully closed. This is static because its also called
+  // with mock hosts as input in test cases.
+  static void ReleaseOnCloseACK(
+      RenderProcessHost* host,
+      const SessionStorageNamespaceMap& sessions,
+      int view_route_id);
+
   // Register/unregister the host identified by the host id in the global host
   // list.
   static void RegisterHost(int host_id, RenderProcessHost* host);
@@ -286,6 +295,7 @@ class CONTENT_EXPORT RenderProcessHostImpl
   void SuddenTerminationChanged(bool enabled);
   void OnUserMetricsRecordAction(const std::string& action);
   void OnSavedPageAsMHTML(int job_id, int64 mhtml_file_size);
+  void OnCloseACK(int old_route_id);
 
   // CompositorSurfaceBuffersSwapped handler when there's no RWH.
   void OnCompositorSurfaceBuffersSwappedNoHost(
diff --git a/content/browser/renderer_host/render_view_host_browsertest.cc b/content/browser/renderer_host/render_view_host_browsertest.cc
index 31ecee9..6015c04 100644
--- a/content/browser/renderer_host/render_view_host_browsertest.cc
+++ b/content/browser/renderer_host/render_view_host_browsertest.cc
@@ -129,4 +129,36 @@ IN_PROC_BROWSER_TEST_F(RenderViewHostTest, IsFocusedElementEditable) {
   EXPECT_TRUE(rvh->IsFocusedElementEditable());
 }
 
+IN_PROC_BROWSER_TEST_F(RenderViewHostTest, ReleaseSessionOnCloseACK) {
+  ASSERT_TRUE(embedded_test_server()->InitializeAndWaitUntilReady());
+  GURL test_url = embedded_test_server()->GetURL(
+      "/access-session-storage.html");
+  NavigateToURL(shell(), test_url);
+
+  // Make a new Shell, a seperate tab with it's own session namespace and
+  // have it start loading a url but still be in progress.
+  ShellAddedObserver new_shell_observer;
+  EXPECT_TRUE(ExecuteScript(shell()->web_contents(), "window.open();"));
+  Shell* new_shell = new_shell_observer.GetShell();
+  new_shell->LoadURL(test_url);
+  RenderViewHost* rvh = new_shell->web_contents()->GetRenderViewHost();
+  SiteInstance* site_instance = rvh->GetSiteInstance();
+  scoped_refptr<SessionStorageNamespace> session_namespace =
+      rvh->GetDelegate()->GetSessionStorageNamespace(site_instance);
+  EXPECT_FALSE(session_namespace->HasOneRef());
+
+  // Close it, or rather start the close operation. The session namespace
+  // should remain until RPH gets an ACK from the renderer about having
+  // closed the view.
+  new_shell->Close();
+  EXPECT_FALSE(session_namespace->HasOneRef());
+
+  // Do something that causes ipc queues to flush and tasks in
+  // flight to complete such that we should have received the ACK.
+  NavigateToURL(shell(), test_url);
+
+  // Verify we have the only remaining reference to the namespace.
+  EXPECT_TRUE(session_namespace->HasOneRef());
+}
+
 }  // namespace content
diff --git a/content/browser/renderer_host/render_view_host_delegate.cc b/content/browser/renderer_host/render_view_host_delegate.cc
index 84bbba8..88e590b 100644
--- a/content/browser/renderer_host/render_view_host_delegate.cc
+++ b/content/browser/renderer_host/render_view_host_delegate.cc
@@ -35,6 +35,11 @@ SessionStorageNamespace* RenderViewHostDelegate::GetSessionStorageNamespace(
   return NULL;
 }
 
+SessionStorageNamespaceMap
+RenderViewHostDelegate::GetSessionStorageNamespaceMap() {
+  return SessionStorageNamespaceMap();
+}
+
 FrameTree* RenderViewHostDelegate::GetFrameTree() {
   return NULL;
 }
diff --git a/content/browser/renderer_host/render_view_host_delegate.h b/content/browser/renderer_host/render_view_host_delegate.h
index bfa45e8..21e244d 100644
--- a/content/browser/renderer_host/render_view_host_delegate.h
+++ b/content/browser/renderer_host/render_view_host_delegate.h
@@ -12,6 +12,7 @@
 #include "base/i18n/rtl.h"
 #include "base/process/kill.h"
 #include "base/strings/string16.h"
+#include "content/browser/dom_storage/session_storage_namespace_impl.h"
 #include "content/common/content_export.h"
 #include "content/public/common/media_stream_request.h"
 #include "content/public/common/page_transition_types.h"
@@ -294,6 +295,10 @@ class CONTENT_EXPORT RenderViewHostDelegate {
   virtual SessionStorageNamespace* GetSessionStorageNamespace(
       SiteInstance* instance);
 
+  // Returns a copy of the map of all session storage namespaces related
+  // to this view.
+  virtual SessionStorageNamespaceMap GetSessionStorageNamespaceMap();
+
   // Returns true if the RenderViewHost will never be visible.
   virtual bool IsNeverVisible();
 
diff --git a/content/browser/renderer_host/render_view_host_impl.cc b/content/browser/renderer_host/render_view_host_impl.cc
index faea5bd..abd160d4 100644
--- a/content/browser/renderer_host/render_view_host_impl.cc
+++ b/content/browser/renderer_host/render_view_host_impl.cc
@@ -1071,6 +1071,15 @@ void RenderViewHostImpl::Shutdown() {
     run_modal_opener_id_ = MSG_ROUTING_NONE;
   }
 
+  // We can't release the SessionStorageNamespace until our peer
+  // in the renderer has wound down.
+  if (GetProcess()->HasConnection()) {
+    RenderProcessHostImpl::ReleaseOnCloseACK(
+        GetProcess(),
+        delegate_->GetSessionStorageNamespaceMap(),
+        GetRoutingID());
+  }
+
   RenderWidgetHostImpl::Shutdown();
 }
 
diff --git a/content/browser/web_contents/web_contents_impl.cc b/content/browser/web_contents/web_contents_impl.cc
index 0f3ea7a..52a13a3 100644
--- a/content/browser/web_contents/web_contents_impl.cc
+++ b/content/browser/web_contents/web_contents_impl.cc
@@ -1669,6 +1669,10 @@ SessionStorageNamespace* WebContentsImpl::GetSessionStorageNamespace(
   return controller_.GetSessionStorageNamespace(instance);
 }
 
+SessionStorageNamespaceMap WebContentsImpl::GetSessionStorageNamespaceMap() {
+  return controller_.GetSessionStorageNamespaceMap();
+}
+
 FrameTree* WebContentsImpl::GetFrameTree() {
   return &frame_tree_;
 }
diff --git a/content/browser/web_contents/web_contents_impl.h b/content/browser/web_contents/web_contents_impl.h
index 92c8fea..b3e6a17 100644
--- a/content/browser/web_contents/web_contents_impl.h
+++ b/content/browser/web_contents/web_contents_impl.h
@@ -57,7 +57,6 @@ class RenderViewHostDelegateView;
 class RenderViewHostImpl;
 class RenderWidgetHostImpl;
 class SavePackage;
-class SessionStorageNamespaceImpl;
 class SiteInstance;
 class TestWebContents;
 class WebContentsDelegate;
@@ -450,6 +449,7 @@ class CONTENT_EXPORT WebContentsImpl
       const MediaResponseCallback& callback) OVERRIDE;
   virtual SessionStorageNamespace* GetSessionStorageNamespace(
       SiteInstance* instance) OVERRIDE;
+  virtual SessionStorageNamespaceMap GetSessionStorageNamespaceMap() OVERRIDE;
   virtual FrameTree* GetFrameTree() OVERRIDE;
   virtual void AccessibilityEventReceived(
       const std::vector<AXEventNotificationDetails>& details) OVERRIDE;