Revert "[IndexedDB] Modify sync "AllowIndexedDB" checks to be async"

This reverts commit 51a8a86.

Reason for revert: Suspected to cause crashes that are considered
a beta blocker for M108. 

Original change's description:
> [IndexedDB] Modify sync "AllowIndexedDB" checks to be async
>
> This change modifies the current use of 'AllowStorageAccessSync' in
> IDBFactory to use the async version 'AllowStorageAccess' instead.
>
> LOW_COVERAGE_REASON= No change in existing behavior to IndexedDB
>
> Bug: 1328954
> Change-Id: I8f09ffb7c0e372803505fa359edf67c7d1713091
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3786956
> Reviewed-by: Joshua Bell <jsbell@chromium.org>
> Reviewed-by: Christian Dullweber <dullweber@chromium.org>
> Commit-Queue: Christine Smith <christinesm@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#1055401}

(cherry picked from commit ad1ee07)

Bug: 1328954
Change-Id: Ib3553d6996b65f2e06399487392f2b6404f05503
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3935051
Auto-Submit: Christian Dullweber <dullweber@chromium.org>
Commit-Queue: Joshua Bell <jsbell@chromium.org>
Reviewed-by: Ayu Ishii <ayui@chromium.org>
Reviewed-by: Joshua Bell <jsbell@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1055919}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3938363
Owners-Override: Prudhvikumar Bommana <pbommana@google.com>
Commit-Queue: Prudhvikumar Bommana <pbommana@google.com>
Cr-Commit-Position: refs/branch-heads/5343@{#4}
Cr-Branched-From: d1a9027-refs/heads/main@{#1055528}

chrome/browser/content_settings/content_settings_browsertest.cc

@@ -659,7 +659,11 @@ IN_PROC_BROWSER_TEST_P(CookieSettingsTest, BlockCookiesAlsoBlocksIndexedDB) {
       "    });"
       "  }"
       "  try {"
-      "    await wrap(indexedDB.%s%s);"
+      "    let promiselike = indexedDB.%s%s;"
+      "    if (typeof promiselike.then !== 'undefined') {"
+      "      await promiselike;"
+      "    }"
+      "    await wrap(promiselike);"
       "  } catch(e) {"
       "    return `${name} - ${e.toString()}`;"
       "  }"
@@ -684,45 +688,19 @@ IN_PROC_BROWSER_TEST_P(CookieSettingsTest, BlockCookiesAlsoBlocksIndexedDB) {
         base::StringPrintf(kBaseExpected, op.cmd),
         EvalJs(tab, base::StringPrintf(kBaseScript, op.cmd, op.cmd, op.args)));
   }
-}
-
-IN_PROC_BROWSER_TEST_P(CookieSettingsTest,
-                       BlockCookiesAlsoBlocksIndexedDBPromiseBased) {
-  ASSERT_TRUE(ui_test_utils::NavigateToURL(browser(), GetPageURL()));
-  content_settings::CookieSettings* settings =
-      CookieSettingsFactory::GetForProfile(browser()->profile()).get();
-  settings->SetCookieSetting(GetPageURL(), CONTENT_SETTING_BLOCK);
-
-  content::WebContents* tab =
-      browser()->tab_strip_model()->GetActiveWebContents();
-
-  const char kPromiseBaseScript[] =
-      "(async function() {"
-      "  const name = `%s`;"
-      "  try {"
-      "    await indexedDB.%s%s;"
-      "  } catch(e) {"
-      "    return `${name} - ${e.toString()}`;"
-      "  }"
-      "  return `${name} - success`;"
-      "}())";
-
-  struct TestOp {
-    const char* cmd;
-    const char* args;
-  };
 
   const TestOp kPromiseTestOps[] = {
       {.cmd = "databases", .args = "()"},
   };
 
-  const char kBaseExpected[] =
-      "%s - UnknownError: The user denied permission to access the database.";
+  const char kPromiseBaseExpected[] =
+      "%s - UnknownError: Failed to execute '%s' on 'IDBFactory': The user "
+      "denied permission to access the database.";
 
   for (auto& op : kPromiseTestOps) {
-    EXPECT_EQ(base::StringPrintf(kBaseExpected, op.cmd),
-              EvalJs(tab, base::StringPrintf(kPromiseBaseScript, op.cmd, op.cmd,
-                                             op.args)));
+    EXPECT_EQ(
+        base::StringPrintf(kPromiseBaseExpected, op.cmd, op.cmd),
+        EvalJs(tab, base::StringPrintf(kBaseScript, op.cmd, op.cmd, op.args)));
   }
 }
 

third_party/blink/renderer/modules/indexeddb/idb_factory.cc

@@ -41,7 +41,6 @@
 #include "third_party/blink/public/platform/web_content_settings_client.h"
 #include "third_party/blink/public/platform/web_security_origin.h"
 #include "third_party/blink/renderer/bindings/core/v8/script_promise_resolver.h"
-#include "third_party/blink/renderer/bindings/core/v8/v8_throw_dom_exception.h"
 #include "third_party/blink/renderer/bindings/modules/v8/v8_binding_for_modules.h"
 #include "third_party/blink/renderer/bindings/modules/v8/v8_idb_database_info.h"
 #include "third_party/blink/renderer/core/dom/dom_exception.h"
@@ -262,9 +261,6 @@ ScriptPromise IDBFactory::GetDatabaseInfo(ScriptState* script_state,
 
   auto* resolver = MakeGarbageCollected<ScriptPromiseResolver>(script_state);
 
-  ExecutionContext* context = ExecutionContext::From(script_state);
-  DCHECK(context->IsContextThread());
-
   if (!IsContextValid(ExecutionContext::From(script_state))) {
     resolver->Reject();
     return resolver->Promise();
@@ -279,73 +275,49 @@ ScriptPromise IDBFactory::GetDatabaseInfo(ScriptState* script_state,
     return resolver->Promise();
   }
 
+  if (!AllowIndexedDB(script_state)) {
+    exception_state.ThrowDOMException(DOMExceptionCode::kUnknownError,
+                                      kPermissionDeniedErrorMessage);
+    resolver->Reject();
+    return resolver->Promise();
+  }
+
   auto callbacks = std::make_unique<WebIDBGetDBNamesCallbacksImpl>(resolver);
   callbacks->SetState(nullptr, WebIDBCallbacksImpl::kNoTransaction);
-
-  AllowIndexedDB(
-      context, WTF::BindOnce(&IDBFactory::GetDatabaseInfoImpl,
-                             WrapWeakPersistent(this), WrapPersistent(context),
-                             WrapPersistent(resolver), std::move(callbacks)));
+  GetFactory(ExecutionContext::From(script_state))
+      ->GetDatabaseInfo(GetCallbacksProxy(std::move(callbacks)));
   return resolver->Promise();
 }
 
-void IDBFactory::GetDatabaseInfoImpl(
-    ExecutionContext* context,
-    ScriptPromiseResolver* resolver,
-    std::unique_ptr<WebIDBCallbacks> callbacks) {
-  ScriptState* script_state = resolver->GetScriptState();
-  if (!allowed_.value()) {
-    ScriptState::Scope scope(script_state);
-    resolver->Reject(V8ThrowDOMException::CreateOrDie(
-        script_state->GetIsolate(), DOMExceptionCode::kUnknownError,
-        kPermissionDeniedErrorMessage));
-    return;
-  }
-  auto& factory = GetFactory(context);
-  factory->GetDatabaseInfo(GetCallbacksProxy(std::move(callbacks)));
-}
-
 void IDBFactory::GetDatabaseInfo(
     ScriptState* script_state,
     std::unique_ptr<mojom::blink::IDBCallbacks> callbacks) {
-  ExecutionContext* context = ExecutionContext::From(script_state);
-  DCHECK(context->IsContextThread());
-
   // TODO(jsbell): Used only by inspector; remove unneeded checks/exceptions?
-  if (!IsContextValid(context)) {
+  if (!IsContextValid(ExecutionContext::From(script_state))) {
     return;
   }
 
-  if (!context->GetSecurityOrigin()->CanAccessDatabase()) {
+  if (!ExecutionContext::From(script_state)
+           ->GetSecurityOrigin()
+           ->CanAccessDatabase()) {
     callbacks->Error(mojom::blink::IDBException::kAbortError,
                      "Access to the IndexedDB API is denied in this context.");
     return;
   }
 
+  if (!AllowIndexedDB(script_state)) {
+    callbacks->Error(mojom::blink::IDBException::kUnknownError,
+                     kPermissionDeniedErrorMessage);
+    return;
+  }
+
   mojo::PendingAssociatedRemote<mojom::blink::IDBCallbacks> pending_callbacks;
   mojo::MakeSelfOwnedAssociatedReceiver(
       std::move(callbacks),
       pending_callbacks.InitWithNewEndpointAndPassReceiver());
 
-  AllowIndexedDB(
-      context,
-      WTF::BindOnce(&IDBFactory::GetDatabaseInfoImplHelper,
-                    WrapWeakPersistent(this), WrapPersistent(context),
-                    std::move(callbacks), std::move(pending_callbacks)));
-  return;
-}
-
-void IDBFactory::GetDatabaseInfoImplHelper(
-    ExecutionContext* context,
-    std::unique_ptr<mojom::blink::IDBCallbacks> callbacks,
-    mojo::PendingAssociatedRemote<mojom::blink::IDBCallbacks>
-        pending_callbacks) {
-  if (!allowed_.value()) {
-    callbacks->Error(mojom::blink::IDBException::kUnknownError,
-                     kPermissionDeniedErrorMessage);
-    return;
-  }
-  GetFactory(context)->GetDatabaseInfo(std::move(pending_callbacks));
+  GetFactory(ExecutionContext::From(script_state))
+      ->GetDatabaseInfo(std::move(pending_callbacks));
 }
 
 IDBOpenDBRequest* IDBFactory::open(ScriptState* script_state,
@@ -366,28 +338,29 @@ IDBOpenDBRequest* IDBFactory::OpenInternal(ScriptState* script_state,
   TRACE_EVENT1("IndexedDB", "IDBFactory::open", "name", name.Utf8());
   IDBRequest::AsyncTraceState metrics("IDBFactory::open");
   DCHECK(version >= 1 || version == IDBDatabaseMetadata::kNoVersion);
-
-  ExecutionContext* context = ExecutionContext::From(script_state);
-  DCHECK(context->IsContextThread());
-
-  if (!IsContextValid(context))
+  if (!IsContextValid(ExecutionContext::From(script_state)))
     return nullptr;
-  if (!context->GetSecurityOrigin()->CanAccessDatabase()) {
+  if (!ExecutionContext::From(script_state)
+           ->GetSecurityOrigin()
+           ->CanAccessDatabase()) {
     exception_state.ThrowSecurityError(
         "access to the Indexed Database API is denied in this context.");
     return nullptr;
   }
 
-  if (context->GetSecurityOrigin()->IsLocal()) {
-    UseCounter::Count(context, WebFeature::kFileAccessedDatabase);
+  if (ExecutionContext::From(script_state)->GetSecurityOrigin()->IsLocal()) {
+    UseCounter::Count(ExecutionContext::From(script_state),
+                      WebFeature::kFileAccessedDatabase);
   }
 
   int64_t transaction_id = IDBDatabase::NextTransactionId();
 
-  auto& factory = GetFactory(context);
+  auto& factory = GetFactory(ExecutionContext::From(script_state));
 
   auto transaction_backend = std::make_unique<WebIDBTransaction>(
-      context->GetTaskRunner(TaskType::kDatabaseAccess), transaction_id);
+      ExecutionContext::From(script_state)
+          ->GetTaskRunner(TaskType::kDatabaseAccess),
+      transaction_id);
   mojo::PendingAssociatedReceiver<mojom::blink::IDBTransaction>
       transaction_receiver = transaction_backend->CreateReceiver();
   mojo::PendingAssociatedRemote<mojom::blink::IDBDatabaseCallbacks>
@@ -397,37 +370,19 @@ IDBOpenDBRequest* IDBFactory::OpenInternal(ScriptState* script_state,
       std::move(transaction_backend), transaction_id, version,
       std::move(metrics), GetObservedFeature());
 
-  AllowIndexedDB(
-      context, WTF::BindOnce(
-                   &IDBFactory::OpenInternalImpl, WrapWeakPersistent(this),
-                   WrapPersistent(context), WrapPersistent(request),
-                   std::move(callbacks_remote), std::move(transaction_receiver),
-                   std::ref(factory), name, version, transaction_id));
-  return request;
-}
-
-void IDBFactory::OpenInternalImpl(
-    ExecutionContext* context,
-    IDBOpenDBRequest* request,
-    mojo::PendingAssociatedRemote<mojom::blink::IDBDatabaseCallbacks>
-        callbacks_remote,
-    mojo::PendingAssociatedReceiver<mojom::blink::IDBTransaction>
-        transaction_receiver,
-    mojo::Remote<mojom::blink::IDBFactory>& factory,
-    const String& name,
-    int64_t version,
-    int64_t transaction_id) {
-  if (!allowed_.value()) {
+  if (!AllowIndexedDB(script_state)) {
     request->HandleResponse(MakeGarbageCollected<DOMException>(
         DOMExceptionCode::kUnknownError, kPermissionDeniedErrorMessage));
-    return;
+    return request;
   }
 
   auto callbacks = request->CreateWebCallbacks();
   callbacks->SetState(nullptr, WebIDBCallbacksImpl::kNoTransaction);
+
   factory->Open(GetCallbacksProxy(std::move(callbacks)),
                 std::move(callbacks_remote), name, version,
                 std::move(transaction_receiver), transaction_id);
+  return request;
 }
 
 IDBOpenDBRequest* IDBFactory::open(ScriptState* script_state,
@@ -460,21 +415,22 @@ IDBOpenDBRequest* IDBFactory::DeleteDatabaseInternal(
     bool force_close) {
   TRACE_EVENT1("IndexedDB", "IDBFactory::deleteDatabase", "name", name.Utf8());
   IDBRequest::AsyncTraceState metrics("IDBFactory::deleteDatabase");
-  ExecutionContext* context = ExecutionContext::From(script_state);
-
-  DCHECK(context->IsContextThread());
-  if (!IsContextValid(context))
+  if (!IsContextValid(ExecutionContext::From(script_state)))
     return nullptr;
-  if (!context->GetSecurityOrigin()->CanAccessDatabase()) {
+  if (!ExecutionContext::From(script_state)
+           ->GetSecurityOrigin()
+           ->CanAccessDatabase()) {
     exception_state.ThrowSecurityError(
         "access to the Indexed Database API is denied in this context.");
     return nullptr;
   }
-  if (context->GetSecurityOrigin()->IsLocal()) {
-    UseCounter::Count(context, WebFeature::kFileAccessedDatabase);
+
+  if (ExecutionContext::From(script_state)->GetSecurityOrigin()->IsLocal()) {
+    UseCounter::Count(ExecutionContext::From(script_state),
+                      WebFeature::kFileAccessedDatabase);
   }
 
-  auto& factory = GetFactory(context);
+  auto& factory = GetFactory(ExecutionContext::From(script_state));
 
   auto* request = MakeGarbageCollected<IDBOpenDBRequest>(
       script_state,
@@ -483,29 +439,17 @@ IDBOpenDBRequest* IDBFactory::DeleteDatabaseInternal(
       IDBDatabaseMetadata::kDefaultVersion, std::move(metrics),
       GetObservedFeature());
 
-  AllowIndexedDB(
-      context, WTF::BindOnce(&IDBFactory::DeleteDatabaseInternalImpl,
-                             WrapWeakPersistent(this), WrapPersistent(context),
-                             WrapPersistent(request), std::ref(factory), name,
-                             force_close));
-  return request;
-}
-
-void IDBFactory::DeleteDatabaseInternalImpl(
-    ExecutionContext* context,
-    IDBOpenDBRequest* request,
-    mojo::Remote<mojom::blink::IDBFactory>& factory,
-    const String& name,
-    bool force_close) {
-  if (!allowed_.value()) {
+  if (!AllowIndexedDB(script_state)) {
     request->HandleResponse(MakeGarbageCollected<DOMException>(
         DOMExceptionCode::kUnknownError, kPermissionDeniedErrorMessage));
-    return;
+    return request;
   }
+
   auto callbacks = request->CreateWebCallbacks();
   callbacks->SetState(nullptr, WebIDBCallbacksImpl::kNoTransaction);
   factory->DeleteDatabase(GetCallbacksProxy(std::move(callbacks)), name,
                           force_close);
+  return request;
 }
 
 int16_t IDBFactory::cmp(ScriptState* script_state,
@@ -539,51 +483,30 @@ int16_t IDBFactory::cmp(ScriptState* script_state,
   return static_cast<int16_t>(first->Compare(second.get()));
 }
 
-void IDBFactory::AllowIndexedDB(ExecutionContext* context,
-                                base::OnceCallback<void()> callback) {
-  DCHECK(context->IsContextThread());
-  SECURITY_DCHECK(context->IsWindow() || context->IsWorkerGlobalScope());
-  auto wrapped_callback =
-      WTF::BindOnce(&IDBFactory::DidAllowIndexedDB, WrapWeakPersistent(this),
-                    std::move(callback));
-
-  if (allowed_.has_value()) {
-    std::move(wrapped_callback).Run(allowed_.value());
-    return;
-  }
-
-  WebContentSettingsClient* settings_client = nullptr;
-
-  if (auto* window = DynamicTo<LocalDOMWindow>(context)) {
+bool IDBFactory::AllowIndexedDB(ScriptState* script_state) {
+  ExecutionContext* execution_context = ExecutionContext::From(script_state);
+  DCHECK(execution_context->IsContextThread());
+  SECURITY_DCHECK(execution_context->IsWindow() ||
+                  execution_context->IsWorkerGlobalScope());
+  if (auto* window = DynamicTo<LocalDOMWindow>(execution_context)) {
     LocalFrame* frame = window->GetFrame();
-    if (!frame) {
-      std::move(wrapped_callback).Run(false);
-      return;
+    if (!frame)
+      return false;
+    if (auto* settings_client = frame->GetContentSettingsClient()) {
+      // This triggers a sync IPC.
+      return settings_client->AllowStorageAccessSync(
+          WebContentSettingsClient::StorageType::kIndexedDB);
     }
-    settings_client = window->GetFrame()->GetContentSettingsClient();
-  } else {
-    settings_client = To<WorkerGlobalScope>(context)->ContentSettingsClient();
-  }
-
-  if (!settings_client) {
-    std::move(wrapped_callback).Run(true);
-    return;
-  }
-  settings_client->AllowStorageAccess(
-      WebContentSettingsClient::StorageType::kIndexedDB,
-      std::move(wrapped_callback));
-}
-
-void IDBFactory::DidAllowIndexedDB(base::OnceCallback<void()> callback,
-                                   bool allow_access) {
-  if (allowed_.has_value()) {
-    DCHECK_EQ(allowed_.value(), allow_access);
-  } else {
-    allowed_ = allow_access;
+    return true;
   }
 
-  std::move(callback).Run();
-  return;
+  WebContentSettingsClient* content_settings_client =
+      To<WorkerGlobalScope>(execution_context)->ContentSettingsClient();
+  if (!content_settings_client)
+    return true;
+  // This triggers a sync IPC.
+  return content_settings_client->AllowStorageAccessSync(
+      WebContentSettingsClient::StorageType::kIndexedDB);
 }
 
 mojo::PendingAssociatedRemote<mojom::blink::IDBCallbacks>