[Blob URL] Update Return Value of RenderFrameHostImpl::DoesDocumentHaveStorageAccess()

Crash Bug: https://crbug.com/406845738

Bug: 406845738
Change-Id: I46366aad06ab3318dcfaf6956c5ebc817ed8d4db
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6422993
Reviewed-by: Mingyu Lei <leimy@chromium.org>
Feels: Mingyu Lei <leimy@chromium.org>
Auto-Submit: Janice Liu <janiceliu@chromium.org>
Reviewed-by: Bo Liu <boliu@chromium.org>
Reviewed-by: Andrew Williams <awillia@chromium.org>
Commit-Queue: Janice Liu <janiceliu@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1442714}

content/browser/renderer_host/render_frame_host_impl.cc

@@ -12750,10 +12750,8 @@ void RenderFrameHostImpl::ReportBlockingCrossPartitionBlobURL(
       std::move(details)));
 }
 
-void RenderFrameHostImpl::DoesDocumentHaveStorageAccess(
-    base::OnceCallback<void(bool)> callback) {
-  std::move(callback).Run(
-      StorageAccessHandle::DoesDocumentHaveStorageAccess(this));
+bool RenderFrameHostImpl::DoesDocumentHaveStorageAccess() {
+  return StorageAccessHandle::DoesDocumentHaveStorageAccess(this);
 }
 
 void RenderFrameHostImpl::BindBlobUrlStoreAssociatedReceiver(
@@ -12767,8 +12765,14 @@ void RenderFrameHostImpl::BindBlobUrlStoreAssociatedReceiver(
       base::BindRepeating(
           &RenderFrameHostImpl::ReportBlockingCrossPartitionBlobURL,
           weak_ptr_factory_.GetWeakPtr()),
-      base::BindRepeating(&RenderFrameHostImpl::DoesDocumentHaveStorageAccess,
-                          weak_ptr_factory_.GetWeakPtr()),
+      base::BindRepeating(
+          [](base::WeakPtr<RenderFrameHostImpl> frame) -> bool {
+            if (!frame) {
+              return false;
+            }
+            return frame->DoesDocumentHaveStorageAccess();
+          },
+          weak_ptr_factory_.GetWeakPtr()),
       !(GetContentClient()->browser()->IsBlobUrlPartitioningEnabled(
           GetBrowserContext())));
 }

content/browser/renderer_host/render_frame_host_impl.h

@@ -4252,7 +4252,7 @@ class CONTENT_EXPORT RenderFrameHostImpl
 
   // This runs when fetches to cross-partition, same-origin Blob URL checks for
   // storage access
-  void DoesDocumentHaveStorageAccess(base::OnceCallback<void(bool)> callback);
+  bool DoesDocumentHaveStorageAccess();
 
   // For frames and main thread worklets we use a navigation-associated
   // interface and bind `receiver` to a `BlobURLStore` instance, which

content/browser/storage_access/storage_access_handle.cc

@@ -173,23 +173,20 @@ void StorageAccessHandle::BindBlobStorage(
   static_cast<RenderFrameHostImpl&>(render_frame_host())
       .GetStoragePartition()
       ->GetBlobUrlRegistry()
-      ->AddReceiver(
-          blink::StorageKey::CreateFirstParty(
-              render_frame_host().GetStorageKey().origin()),
-          render_frame_host().GetLastCommittedOrigin(),
-          render_frame_host().GetProcess()->GetDeprecatedID(),
-          std::move(receiver),
-          /*partitioning_blob_url_closure=*/base::DoNothing(),
-          // In the case that a context is granted storage access, the
-          // StorageAccessHandle context still shouldn't bypass partitioning
-          // check. (eg. using a Blob URL created with URL.createObjectURL in
-          // the third-party context with the StorageAccessHandle's SharedWorker
-          // constructor.)
-          /*storage_access_check_callback= */
-          base::BindRepeating([](base::OnceCallback<void(bool)> callback) {
-            std::move(callback).Run(false);
-          }),
-          /*partitioning_disabled_by_policy=*/false);
+      ->AddReceiver(blink::StorageKey::CreateFirstParty(
+                        render_frame_host().GetStorageKey().origin()),
+                    render_frame_host().GetLastCommittedOrigin(),
+                    render_frame_host().GetProcess()->GetDeprecatedID(),
+                    std::move(receiver),
+                    /*partitioning_blob_url_closure=*/base::DoNothing(),
+                    // In the case that a context is granted storage access, the
+                    // StorageAccessHandle context still shouldn't bypass
+                    // partitioning check. (eg. using a Blob URL created with
+                    // URL.createObjectURL in the third-party context with the
+                    // StorageAccessHandle's SharedWorker constructor.)
+                    /*storage_access_check_callback= */
+                    base::BindRepeating([]() -> bool { return false; }),
+                    /*partitioning_disabled_by_policy=*/false);
 }
 
 void StorageAccessHandle::BindBroadcastChannel(

storage/browser/blob/blob_url_registry.cc

@@ -37,8 +37,7 @@ void BlobUrlRegistry::AddReceiver(
     base::RepeatingCallback<
         void(const GURL&, std::optional<blink::mojom::PartitioningBlobURLInfo>)>
         partitioning_blob_url_closure,
-    base::RepeatingCallback<void(base::OnceCallback<void(bool)>)>
-        storage_access_check_callback,
+    base::RepeatingCallback<bool()> storage_access_check_callback,
     bool partitioning_disabled_by_policy) {
   mojo::ReceiverId receiver_id = frame_receivers_.Add(
       std::make_unique<storage::BlobURLStoreImpl>(
@@ -65,9 +64,7 @@ void BlobUrlRegistry::AddReceiver(
       std::make_unique<storage::BlobURLStoreImpl>(
           storage_key, renderer_origin, render_process_host_id, AsWeakPtr(),
           validity_check_behavior, base::DoNothing(),
-          base::BindRepeating([](base::OnceCallback<void(bool)> callback) {
-            std::move(callback).Run(false);
-          }),
+          base::BindRepeating([]() -> bool { return false; }),
           partitioning_disabled_by_policy),
       std::move(receiver));
 }

storage/browser/blob/blob_url_registry.h

@@ -64,8 +64,7 @@ class COMPONENT_EXPORT(STORAGE_BROWSER) BlobUrlRegistry {
           void(const GURL&,
                std::optional<blink::mojom::PartitioningBlobURLInfo>)>
           partitioning_blob_url_closure,
-      base::RepeatingCallback<void(base::OnceCallback<void(bool)>)>
-          storage_access_check_callback,
+      base::RepeatingCallback<bool()> storage_access_check_callback,
       bool partitioning_disabled_by_policy = false);
 
   // Binds receivers corresponding to connections from renderer worker

storage/browser/blob/blob_url_store_impl.cc

@@ -9,7 +9,6 @@
 #include "base/functional/callback.h"
 #include "base/strings/strcat.h"
 #include "components/crash/core/common/crash_key.h"
-#include "mojo/public/cpp/bindings/callback_helpers.h"
 #include "mojo/public/cpp/bindings/receiver_set.h"
 #include "net/base/features.h"
 #include "storage/browser/blob/blob_impl.h"
@@ -76,8 +75,7 @@ BlobURLStoreImpl::BlobURLStoreImpl(
     base::RepeatingCallback<
         void(const GURL&, std::optional<blink::mojom::PartitioningBlobURLInfo>)>
         partitioning_blob_url_closure,
-    base::RepeatingCallback<void(base::OnceCallback<void(bool)>)>
-        storage_access_check_callback,
+    base::RepeatingCallback<bool()> storage_access_check_callback,
     bool partitioning_disabled_by_policy)
     : storage_key_(storage_key),
       renderer_origin_(renderer_origin),
@@ -135,10 +133,8 @@ void BlobURLStoreImpl::ResolveAsURLLoaderFactory(
     std::move(callback).Run(std::nullopt, std::nullopt);
     return;
   }
-  storage_access_check_callback_.Run(
-      base::BindOnce(&BlobURLStoreImpl::FinishResolveAsURLLoaderFactory,
-                     weak_ptr_factory_.GetWeakPtr(), url, std::move(receiver),
-                     std::move(callback)));
+  FinishResolveAsURLLoaderFactory(url, std::move(receiver), std::move(callback),
+                                  storage_access_check_callback_.Run());
 }
 
 void BlobURLStoreImpl::FinishResolveAsURLLoaderFactory(
@@ -185,12 +181,9 @@ void BlobURLStoreImpl::ResolveAsBlobURLToken(
     std::move(callback).Run(std::nullopt);
     return;
   }
-  storage_access_check_callback_.Run(
-      mojo::WrapCallbackWithDefaultInvokeIfNotRun(
-          base::BindOnce(&BlobURLStoreImpl::FinishResolveAsBlobURLToken,
-                         weak_ptr_factory_.GetWeakPtr(), url, std::move(token),
-                         is_top_level_navigation, std::move(callback)),
-          false));
+  FinishResolveAsBlobURLToken(url, std::move(token), is_top_level_navigation,
+                              std::move(callback),
+                              storage_access_check_callback_.Run());
 }
 
 void BlobURLStoreImpl::FinishResolveAsBlobURLToken(

storage/browser/blob/blob_url_store_impl.h

@@ -29,22 +29,20 @@ class COMPONENT_EXPORT(STORAGE_BROWSER) BlobURLStoreImpl
  public:
   // `partitioning_blob_url_closure` runs when the storage_key check fails
   // in `BlobURLStoreImpl::ResolveAsURLLoaderFactory`.
-  BlobURLStoreImpl(const blink::StorageKey& storage_key,
-                   const url::Origin& renderer_origin,
-                   int render_process_host_id,
-                   base::WeakPtr<BlobUrlRegistry> registry,
-                   BlobURLValidityCheckBehavior validity_check_options =
-                       BlobURLValidityCheckBehavior::DEFAULT,
-                   base::RepeatingCallback<void(
-                       const GURL&,
-                       std::optional<blink::mojom::PartitioningBlobURLInfo>)>
-                       partitioning_blob_url_closure = base::DoNothing(),
-                   base::RepeatingCallback<void(base::OnceCallback<void(bool)>)>
-                       storage_access_check_closure = base::BindRepeating(
-                           [](base::OnceCallback<void(bool)> callback) {
-                             std::move(callback).Run(false);
-                           }),
-                   bool partitioning_disabled_by_policy = false);
+  BlobURLStoreImpl(
+      const blink::StorageKey& storage_key,
+      const url::Origin& renderer_origin,
+      int render_process_host_id,
+      base::WeakPtr<BlobUrlRegistry> registry,
+      BlobURLValidityCheckBehavior validity_check_options =
+          BlobURLValidityCheckBehavior::DEFAULT,
+      base::RepeatingCallback<
+          void(const GURL&,
+               std::optional<blink::mojom::PartitioningBlobURLInfo>)>
+          partitioning_blob_url_closure = base::DoNothing(),
+      base::RepeatingCallback<bool()> storage_access_check_closure =
+          base::BindRepeating([]() -> bool { return false; }),
+      bool partitioning_disabled_by_policy = false);
 
   BlobURLStoreImpl(const BlobURLStoreImpl&) = delete;
   BlobURLStoreImpl& operator=(const BlobURLStoreImpl&) = delete;
@@ -108,8 +106,7 @@ class COMPONENT_EXPORT(STORAGE_BROWSER) BlobURLStoreImpl
       void(const GURL&, std::optional<blink::mojom::PartitioningBlobURLInfo>)>
       partitioning_blob_url_closure_;
 
-  base::RepeatingCallback<void(base::OnceCallback<void(bool)>)>
-      storage_access_check_callback_;
+  base::RepeatingCallback<bool()> storage_access_check_callback_;
 
   const bool partitioning_disabled_by_policy_;