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

15 files changed, 146 insertions, 9 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;
diff --git a/content/common/view_messages.h b/content/common/view_messages.h
index 8a2d3dd..3770af5 100644
--- a/content/common/view_messages.h
+++ b/content/common/view_messages.h
@@ -574,6 +574,7 @@ IPC_MESSAGE_ROUTED1(ViewMsg_UpdateWebPreferences,
 IPC_MESSAGE_CONTROL0(ViewMsg_TimezoneChange)
 
 // Tells the render view to close.
+// Expects a Close_ACK message when finished.
 IPC_MESSAGE_ROUTED0(ViewMsg_Close)
 
 IPC_STRUCT_BEGIN(ViewMsg_Resize_Params)
@@ -1116,6 +1117,11 @@ IPC_MESSAGE_ROUTED5(ViewHostMsg_Find_Reply,
                     int /* active_match_ordinal */,
                     bool /* final_update */)
 
+// Indicates that the render view has been closed in respose to a
+// Close message.
+IPC_MESSAGE_CONTROL1(ViewHostMsg_Close_ACK,
+                     int /* old_route_id */);
+
 // Indicates that the current page has been closed, after a ClosePage
 // message.
 IPC_MESSAGE_ROUTED0(ViewHostMsg_ClosePage_ACK)
diff --git a/content/public/browser/navigation_controller.h b/content/public/browser/navigation_controller.h
index c883e1e..08ea0b9 100644
--- a/content/public/browser/navigation_controller.h
+++ b/content/public/browser/navigation_controller.h
@@ -13,6 +13,7 @@
 #include "base/strings/string16.h"
 #include "content/common/content_export.h"
 #include "content/public/browser/global_request_id.h"
+#include "content/public/browser/session_storage_namespace.h"
 #include "content/public/common/page_transition_types.h"
 #include "content/public/common/referrer.h"
 #include "url/gurl.h"
@@ -27,14 +28,8 @@ namespace content {
 
 class BrowserContext;
 class NavigationEntry;
-class SessionStorageNamespace;
 class WebContents;
 
-// Used to store the mapping of a StoragePartition id to
-// SessionStorageNamespace.
-typedef std::map<std::string, scoped_refptr<SessionStorageNamespace> >
-    SessionStorageNamespaceMap;
-
 // A NavigationController maintains the back-forward list for a WebContents and
 // manages all navigation within that list.
 //
@@ -356,7 +351,7 @@ class NavigationController {
   // which cannot be used on iOS.
 #if !defined(OS_IOS)
   // Returns all the SessionStorageNamespace objects that this
-  // NavigationController knows about.
+  // NavigationController knows about, the map key is a StoragePartition id.
   virtual const SessionStorageNamespaceMap&
       GetSessionStorageNamespaceMap() const = 0;
 
diff --git a/content/public/browser/session_storage_namespace.h b/content/public/browser/session_storage_namespace.h
index ec49419..374b7b0 100644
--- a/content/public/browser/session_storage_namespace.h
+++ b/content/public/browser/session_storage_namespace.h
@@ -5,6 +5,7 @@
 #ifndef CONTENT_PUBLIC_BROWSER_SESSION_STORAGE_NAMESPACE_H_
 #define CONTENT_PUBLIC_BROWSER_SESSION_STORAGE_NAMESPACE_H_
 
+#include <map>
 #include <string>
 
 #include "base/basictypes.h"
@@ -79,6 +80,10 @@ class SessionStorageNamespace
   virtual ~SessionStorageNamespace() {}
 };
 
+// Used to store mappings of StoragePartition id to SessionStorageNamespace.
+typedef std::map<std::string, scoped_refptr<SessionStorageNamespace> >
+    SessionStorageNamespaceMap;
+
 }  // namespace content
 
 #endif  // CONTENT_PUBLIC_BROWSER_SESSION_STORAGE_NAMESPACE_H_
diff --git a/content/renderer/render_view_impl.cc b/content/renderer/render_view_impl.cc
index 042f7cb..49b3938 100644
--- a/content/renderer/render_view_impl.cc
+++ b/content/renderer/render_view_impl.cc
@@ -3312,12 +3312,19 @@ void RenderViewImpl::OnPluginImeCompositionCompleted(const base::string16& text,
 }
 #endif  // OS_MACOSX
 
+void RenderViewImpl::OnClose() {
+  if (closing_)
+    RenderThread::Get()->Send(new ViewHostMsg_Close_ACK(routing_id_));
+  RenderWidget::OnClose();
+}
+
 void RenderViewImpl::Close() {
   // We need to grab a pointer to the doomed WebView before we destroy it.
   WebView* doomed = webview();
   RenderWidget::Close();
   g_view_map.Get().erase(doomed);
   g_routing_id_view_map.Get().erase(routing_id_);
+  RenderThread::Get()->Send(new ViewHostMsg_Close_ACK(routing_id_));
 }
 
 void RenderViewImpl::DidHandleKeyEvent() {
diff --git a/content/renderer/render_view_impl.h b/content/renderer/render_view_impl.h
index 8ef97ae..796cb6a 100644
--- a/content/renderer/render_view_impl.h
+++ b/content/renderer/render_view_impl.h
@@ -528,6 +528,7 @@ class CONTENT_EXPORT RenderViewImpl
 
  protected:
   // RenderWidget overrides:
+  virtual void OnClose() OVERRIDE;
   virtual void Close() OVERRIDE;
   virtual void OnResize(const ViewMsg_Resize_Params& params) OVERRIDE;
   virtual void DidInitiatePaint() OVERRIDE;
diff --git a/content/renderer/render_widget.h b/content/renderer/render_widget.h
index 97a3b29..33213bd 100644
--- a/content/renderer/render_widget.h
+++ b/content/renderer/render_widget.h
@@ -340,7 +340,7 @@ class CONTENT_EXPORT RenderWidget
   void OnCursorVisibilityChange(bool is_visible);
   void OnMouseCaptureLost();
   virtual void OnSetFocus(bool enable);
-  void OnClose();
+  virtual void OnClose();
   void OnCreatingNewAck();
   virtual void OnResize(const ViewMsg_Resize_Params& params);
   void OnChangeResizeRect(const gfx::Rect& resizer_rect);
diff --git a/content/test/data/access-session-storage.html b/content/test/data/access-session-storage.html
new file mode 100644
index 0000000..38b9fe8
--- /dev/null
+++ b/content/test/data/access-session-storage.html
@@ -0,0 +1,8 @@
+<html>
+<head><title>Access Session Storage</title></head>
+<body>
+<script>
+var x = window.sessionStorage.length;
+</script>
+</body>
+</html>