Direct Sockets: Detect when render frame is deleted

DirectSocketsServiceImpl is now a WebContentsObserver.

OpenTcpSocket() and OpenUdpSocket() now fail safely if they are called
after the RenderFrameHost is destroyed.


Bug: 1122917
Change-Id: I4a880c9aee73271cd5895818e0da3d85e32a8e5b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2383531
Auto-Submit: Eric Willigers <ericwilligers@chromium.org>
Commit-Queue: Glen Robertson <glenrob@chromium.org>
Reviewed-by: Glen Robertson <glenrob@chromium.org>
Cr-Commit-Position: refs/heads/master@{#803005}

content/browser/direct_sockets/direct_sockets_service_impl.cc

@@ -11,6 +11,7 @@
 #include "base/optional.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/storage_partition.h"
+#include "content/public/browser/web_contents.h"
 #include "content/public/common/content_features.h"
 #include "mojo/public/cpp/bindings/self_owned_receiver.h"
 
@@ -28,7 +29,8 @@ GetPermissionCallbackForTesting() {
 }  // namespace
 
 DirectSocketsServiceImpl::DirectSocketsServiceImpl(RenderFrameHost& frame_host)
-    : frame_host_(&frame_host) {}
+    : WebContentsObserver(WebContents::FromRenderFrameHost(&frame_host)),
+      frame_host_(&frame_host) {}
 
 // static
 void DirectSocketsServiceImpl::CreateForFrame(
@@ -86,10 +88,23 @@ void DirectSocketsServiceImpl::SetPermissionCallbackForTesting(
   GetPermissionCallbackForTesting() = std::move(callback);
 }
 
+void DirectSocketsServiceImpl::RenderFrameDeleted(
+    RenderFrameHost* render_frame_host) {
+  if (render_frame_host == frame_host_)
+    frame_host_ = nullptr;
+}
+
+void DirectSocketsServiceImpl::WebContentsDestroyed() {
+  frame_host_ = nullptr;
+}
+
 net::Error DirectSocketsServiceImpl::EnsurePermission(
     const blink::mojom::DirectSocketOptions& options) {
   DCHECK(base::FeatureList::IsEnabled(features::kDirectSockets));
 
+  if (!frame_host_)
+    return net::ERR_CONTEXT_SHUT_DOWN;
+
   if (GetPermissionCallbackForTesting())
     return GetPermissionCallbackForTesting().Run(options);
 

content/browser/direct_sockets/direct_sockets_service_impl.h

@@ -10,6 +10,7 @@
 #include "base/callback.h"
 #include "content/common/content_export.h"
 #include "content/public/browser/render_frame_host.h"
+#include "content/public/browser/web_contents_observer.h"
 #include "mojo/public/cpp/bindings/pending_receiver.h"
 #include "net/base/net_errors.h"
 #include "third_party/blink/public/mojom/direct_sockets/direct_sockets.mojom.h"
@@ -24,7 +25,8 @@ namespace content {
 
 // Implementation of the DirectSocketsService Mojo service.
 class CONTENT_EXPORT DirectSocketsServiceImpl
-    : public blink::mojom::DirectSocketsService {
+    : public blink::mojom::DirectSocketsService,
+      public WebContentsObserver {
  public:
   using PermissionCallback = base::RepeatingCallback<net::Error(
       const blink::mojom::DirectSocketOptions&)>;
@@ -45,14 +47,20 @@ class CONTENT_EXPORT DirectSocketsServiceImpl
   void OpenUdpSocket(blink::mojom::DirectSocketOptionsPtr options,
                      OpenUdpSocketCallback callback) override;
 
+  // WebContentsObserver override:
+  void RenderFrameDeleted(RenderFrameHost* render_frame_host) override;
+  void WebContentsDestroyed() override;
+
   static void SetPermissionCallbackForTesting(PermissionCallback callback);
 
  private:
+  friend class DirectSocketsUnitTest;
+
   net::Error EnsurePermission(const blink::mojom::DirectSocketOptions& options);
 
   network::mojom::NetworkContext* GetNetworkContext();
 
-  RenderFrameHost* const frame_host_;
+  RenderFrameHost* frame_host_;
 };
 
 }  // namespace content

content/browser/direct_sockets/direct_sockets_unittest.cc

@@ -0,0 +1,57 @@
+// Copyright 2020 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 <memory>
+
+#include "base/test/scoped_feature_list.h"
+#include "content/browser/direct_sockets/direct_sockets_service_impl.h"
+#include "content/public/common/content_features.h"
+#include "content/public/test/test_renderer_host.h"
+#include "net/base/net_errors.h"
+#include "third_party/blink/public/mojom/direct_sockets/direct_sockets.mojom.h"
+
+namespace content {
+
+class DirectSocketsUnitTest : public RenderViewHostTestHarness {
+ public:
+  DirectSocketsUnitTest() {
+    feature_list_.InitAndEnableFeature(features::kDirectSockets);
+  }
+  ~DirectSocketsUnitTest() override = default;
+
+  void SetUp() override {
+    RenderViewHostTestHarness::SetUp();
+    direct_sockets_service_ =
+        std::make_unique<DirectSocketsServiceImpl>(*main_rfh());
+  }
+
+  DirectSocketsServiceImpl& direct_sockets_service() {
+    return *direct_sockets_service_;
+  }
+
+  net::Error EnsurePermission(
+      const blink::mojom::DirectSocketOptions& options) {
+    return direct_sockets_service().EnsurePermission(options);
+  }
+
+ private:
+  base::test::ScopedFeatureList feature_list_;
+  std::unique_ptr<DirectSocketsServiceImpl> direct_sockets_service_;
+};
+
+TEST_F(DirectSocketsUnitTest, RenderFrameDeleted) {
+  direct_sockets_service().RenderFrameDeleted(main_rfh());
+
+  blink::mojom::DirectSocketOptions options;
+  EXPECT_EQ(EnsurePermission(options), net::ERR_CONTEXT_SHUT_DOWN);
+}
+
+TEST_F(DirectSocketsUnitTest, WebContentsDestroyed) {
+  direct_sockets_service().WebContentsDestroyed();
+
+  blink::mojom::DirectSocketOptions options;
+  EXPECT_EQ(EnsurePermission(options), net::ERR_CONTEXT_SHUT_DOWN);
+}
+
+}  // namespace content

content/test/BUILD.gn

@@ -2372,6 +2372,7 @@ test("content_unittests") {
     # Non-Android.
     sources += [
       "../browser/devtools/protocol/webauthn_handler_unittest.cc",
+      "../browser/direct_sockets/direct_sockets_unittest.cc",
       "../browser/host_zoom_map_impl_unittest.cc",
       "../browser/serial/serial_unittest.cc",
       "../browser/speech/chunked_byte_buffer_unittest.cc",