Remove HasSecurityState() workaround from BroadcastChannelProvider.

The HasSecurityState() check was introduced as a temporary workaround
to eliminate crashes caused by Mojo services trying to perform
ChildProcessSecurityPolicy security checks after the RenderProcessHost
has been destroyed.  HasSecurityState() was used to detect that the
process was gone and just fail open.

Now, we have a better solution in place with
ChildProcessSecurityPolicyImpl::Handle, which extends the lifetime of
a process's security state even after the RPH has gone away.  Switch
BroadcastChannelProvider to use this approach instead.

Bug: 943887
Change-Id: Id83c27364506ff9a349a10c5f54193c2aa0f369b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2031924Reviewed-by: Marijn Kruisselbrink <mek@chromium.org>
Commit-Queue: Alex Moshchuk <alexmos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#737179}

content/browser/broadcast_channel/broadcast_channel_provider.cc

@@ -6,7 +6,6 @@
 
 #include "base/bind.h"
 #include "base/stl_util.h"
-#include "content/browser/child_process_security_policy_impl.h"
 #include "mojo/public/cpp/bindings/associated_receiver.h"
 #include "mojo/public/cpp/bindings/associated_remote.h"
 
@@ -72,9 +71,11 @@ BroadcastChannelProvider::BroadcastChannelProvider() = default;
 BroadcastChannelProvider::~BroadcastChannelProvider() = default;
 
 mojo::ReceiverId BroadcastChannelProvider::Connect(
-    RenderProcessHostId render_process_host_id,
+    SecurityPolicyHandle security_policy_handle,
     mojo::PendingReceiver<blink::mojom::BroadcastChannelProvider> receiver) {
-  return receivers_.Add(this, std::move(receiver), render_process_host_id);
+  return receivers_.Add(this, std::move(receiver),
+                        std::make_unique<SecurityPolicyHandle>(
+                            std::move(security_policy_handle)));
 }
 
 void BroadcastChannelProvider::ConnectToChannel(
@@ -83,17 +84,8 @@ void BroadcastChannelProvider::ConnectToChannel(
     mojo::PendingAssociatedRemote<blink::mojom::BroadcastChannelClient> client,
     mojo::PendingAssociatedReceiver<blink::mojom::BroadcastChannelClient>
         connection) {
-  RenderProcessHostId process_id = receivers_.current_context();
-  auto* policy = ChildProcessSecurityPolicyImpl::GetInstance();
-
-  // TODO(943887): Replace HasSecurityState() call with something that can
-  // preserve security state after process shutdown. The security state check
-  // is a temporary solution to avoid crashes when this method is run after the
-  // process associated with |process_id| has been destroyed. It temporarily
-  // restores the old behavior of always allowing access if the process is gone.
-  // See https://crbug.com/943027 for details.
-  if (!policy->CanAccessDataForOrigin(process_id, origin) &&
-      policy->HasSecurityState(process_id)) {
+  const auto& security_policy_handle = receivers_.current_context();
+  if (!security_policy_handle->CanAccessDataForOrigin(origin)) {
     mojo::ReportBadMessage("BROADCAST_CHANNEL_INVALID_ORIGIN");
     return;
   }

content/browser/broadcast_channel/broadcast_channel_provider.h

@@ -7,6 +7,7 @@
 
 #include <map>
 
+#include "content/browser/child_process_security_policy_impl.h"
 #include "content/common/content_export.h"
 #include "mojo/public/cpp/bindings/receiver_set.h"
 #include "third_party/blink/public/mojom/broadcastchannel/broadcast_channel.mojom.h"
@@ -20,9 +21,9 @@ class CONTENT_EXPORT BroadcastChannelProvider
   BroadcastChannelProvider();
   ~BroadcastChannelProvider() override;
 
-  using RenderProcessHostId = int;
+  using SecurityPolicyHandle = ChildProcessSecurityPolicyImpl::Handle;
   mojo::ReceiverId Connect(
-      RenderProcessHostId render_process_host_id,
+      SecurityPolicyHandle security_policy_handle,
       mojo::PendingReceiver<blink::mojom::BroadcastChannelProvider> receiver);
 
   void ConnectToChannel(
@@ -42,7 +43,8 @@ class CONTENT_EXPORT BroadcastChannelProvider
   void ReceivedMessageOnConnection(Connection*,
                                    const blink::CloneableMessage& message);
 
-  mojo::ReceiverSet<blink::mojom::BroadcastChannelProvider, RenderProcessHostId>
+  mojo::ReceiverSet<blink::mojom::BroadcastChannelProvider,
+                    std::unique_ptr<SecurityPolicyHandle>>
       receivers_;
   std::map<url::Origin, std::multimap<std::string, std::unique_ptr<Connection>>>
       connections_;

content/browser/isolated_origin_browsertest.cc

@@ -2883,7 +2883,9 @@ class BroadcastChannelProviderInterceptor
     // Bind the real BroadcastChannelProvider implementation.
     mojo::ReceiverId receiver_id =
         storage_partition->GetBroadcastChannelProvider()->Connect(
-            rph->GetID(), std::move(receiver));
+            ChildProcessSecurityPolicyImpl::GetInstance()->CreateHandle(
+                rph->GetID()),
+            std::move(receiver));
 
     // Now replace it with this object and keep a pointer to the real
     // implementation.

content/browser/renderer_host/render_process_host_impl.cc

@@ -2453,7 +2453,8 @@ void RenderProcessHostImpl::CreateBroadcastChannelProvider(
   }
 
   storage_partition_impl_->GetBroadcastChannelProvider()->Connect(
-      id_, std::move(receiver));
+      ChildProcessSecurityPolicyImpl::GetInstance()->CreateHandle(id_),
+      std::move(receiver));
 }
 
 void RenderProcessHostImpl::CreateCodeCacheHost(