PlzNavigate: Release StreamHandle.

content/browser/frame_host/render_frame_host_impl.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/browser/frame_host/render_frame_host_impl.h"
 
 #include <algorithm>
 #include <utility>
 
 #include "base/bind.h"
@@ skipping 865 matching lines @@
     IPC_MESSAGE_HANDLER(FrameHostMsg_ShowPopup, OnShowPopup)
     IPC_MESSAGE_HANDLER(FrameHostMsg_HidePopup, OnHidePopup)
 #endif
 #if defined(OS_ANDROID)
     IPC_MESSAGE_HANDLER(FrameHostMsg_NavigationHandledByEmbedder,
                         OnNavigationHandledByEmbedder)
 #endif
     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.
   return handled;
 }
 
 void RenderFrameHostImpl::OnAssociatedInterfaceRequest(
     const std::string& interface_name,
     mojo::ScopedInterfaceEndpointHandle handle) {
   if (associated_registry_->CanBindRequest(interface_name)) {
@@ skipping 2280 matching lines @@
   }
   Send(new FrameMsg_CommitNavigation(routing_id_, head, body_url, commit_data,
                                      common_params, request_params));
 
   // If a network request was made, update the Previews state.
   if (ShouldMakeNetworkRequestForURL(common_params.url) &&
       !FrameMsg_Navigate_Type::IsSameDocument(common_params.navigation_type)) {
     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
   // RenderFrameHost, nor should the throbber start. The NavigationRequest is
   // also not stored in the FrameTreeNode. Therefore do not reset it, as this
   // could cancel an existing pending navigation.
   if (!IsRendererDebugURL(common_params.url)) {
     pending_commit_ = true;
     is_loading_ = true;
   }
@@ skipping 847 matching lines @@
 
   last_committed_site_url_ = site_url;
 
   if (!last_committed_site_url_.is_empty()) {
     RenderProcessHostImpl::AddFrameWithSite(
         frame_tree_node_->navigator()->GetController()->GetBrowserContext(),
         GetProcess(), last_committed_site_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
     : public service_manager::mojom::InterfaceProvider {
  public:
   using BindCallback =
       base::Callback<void(const std::string&, mojo::ScopedMessagePipeHandle)>;
 
   JavaInterfaceProvider(
       const BindCallback& bind_callback,
@@ skipping 44 matching lines @@
 }
 
 void RenderFrameHostImpl::ForwardGetInterfaceToRenderFrame(
     const std::string& interface_name,
     mojo::ScopedMessagePipeHandle pipe) {
   GetRemoteInterfaces()->GetInterface(interface_name, std::move(pipe));
 }
 #endif
 
 }  // namespace content