PlzNavigate: Release StreamHandle.

content/browser/frame_host/render_frame_host_impl.cc

Index: content/browser/frame_host/render_frame_host_impl.cc
diff --git a/content/browser/frame_host/render_frame_host_impl.cc b/content/browser/frame_host/render_frame_host_impl.cc
index 6da622cc019fdbca2625291b78116c0a985ada03..ea3847a57742723a46789aa7cd4c92c901da379e 100644
--- a/content/browser/frame_host/render_frame_host_impl.cc
+++ b/content/browser/frame_host/render_frame_host_impl.cc
@@ -883,6 +883,8 @@ bool RenderFrameHostImpl::OnMessageReceived(const IPC::Message &msg) {
     IPC_MESSAGE_HANDLER(FrameHostMsg_RequestOverlayRoutingToken,
                         OnRequestOverlayRoutingToken)
     IPC_MESSAGE_HANDLER(FrameHostMsg_ShowCreatedWindow, OnShowCreatedWindow)
+    IPC_MESSAGE_HANDLER(FrameHostMsg_StreamHandleConsumed,
+                        OnStreamHandleConsumed)
   IPC_END_MESSAGE_MAP()
 
   // No further actions here, since we may have been deleted.
@@ -3183,8 +3185,7 @@ void RenderFrameHostImpl::CommitNavigation(
     last_navigation_previews_state_ = common_params.previews_state;
   }
 
-  // TODO(clamy): Release the stream handle once the renderer has finished
-  // reading it.
+  // Released in OnStreamHandleConsumed().

[nasko, 2017/06/12 23:05:04]

Shouldn't we also release the stream handle if the renderer process crashes? In
that case, there will be no way to consume the stream and we always navigate
again any newly recreated process.

[arthursonzogni, 2017/06/13 09:21:48]

Yes, but I do not know if it's worth knowing that the previous StreamHandle is
removed/replaced with each new navigations.
Is there any way to get notified of the RenderFrameImpl's crashes? I think I can
use RenderFrameHostImpl::CreateRenderFrame() which get called upon re-creation
of the renderer process, but it might not be ideal.

[nasko, 2017/06/13 13:51:09]

It is just clean code to release the stream handle when the renderer process
crashes. We can easily reason about resource usage.
RenderFrameHostImpl::RenderProcessGone is what you need.

   stream_handle_ = std::move(body);
 
   // When navigating to a debug url, no commit is expected from the
@@ -4052,6 +4053,12 @@ void RenderFrameHostImpl::SetLastCommittedSiteUrl(const GURL& url) {
   }
 }
 
+void RenderFrameHostImpl::OnStreamHandleConsumed(const GURL& stream_url) {
+  if (stream_handle_ && stream_handle_->GetURL() == stream_url) {

[nasko, 2017/06/12 23:05:04]

nit: No need for {}

[arthursonzogni, 2017/06/13 09:21:48]

Done.

+    stream_handle_.reset();
+  }
+}
+
 #if defined(OS_ANDROID)
 
 class RenderFrameHostImpl::JavaInterfaceProvider