From 8b3c59daea285175a4f1a5f174b388f5ac263149 Mon Sep 17 00:00:00 2001
From: Abhishek Shanthkumar <[email protected]>
Date: Thu, 30 Jul 2026 12:46:16 -0700
Subject: [PATCH] Reland "Reland "IDB: SQLite - Conditionally limit large values in getAll() response""

This is a reland of commit b2ddead35aac45fc0bfd90621607f9227f896833

The new test was too slow on MSAN, hence disabled it there.

Original change's description:
> Reland "IDB: SQLite - Conditionally limit large values in getAll() response"
>
> This is a reland of commit dd058e0bf9e5b4ce30fa2c4fd89a8e51c0153d3b
>
> It turns out that Fuchsia also has a limit on the number of handles that
> can be sent in one mojo message; this reland respects the limit on
> Fuchsia too.
>
> Original change's description:
> > IDB: SQLite - Conditionally limit large values in getAll() response
> >
> > POSIX (minus macOS) currently does not support >128 file descriptors in
> > one mojo message, triggering a CHECK - see crbug.com/439305148. Since
> > SQLite-backed IDB does not wrap large values, each value may end up
> > using shared memory, which requires file descriptors on POSIX. This CL
> > limit the number of such values sent in each response chunk of a
> > getAll() request.
> >
> > Bug: 523912081
> > Change-Id: I5797da537f57cd6de8e65fa8e33929a3f7a93ce2
> > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8083924
> > Reviewed-by: Steve Becker <[email protected]>
> > Commit-Queue: Abhishek Shanthkumar <[email protected]>
> > Cr-Commit-Position: refs/heads/main@{#1668478}
>
> Bug: 523912081
> Change-Id: I0b4fd0708d38b0fae8f751c44d7cb3e849fa8212
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8160922
> Reviewed-by: Steve Becker <[email protected]>
> Auto-Submit: Abhishek Shanthkumar <[email protected]>
> Commit-Queue: Abhishek Shanthkumar <[email protected]>
> Cr-Commit-Position: refs/heads/main@{#1669558}

Bug: 523912081
Change-Id: Ic409b9d211a2def992eaf08c78bb7cdb1d148ff7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8161063
Reviewed-by: Steve Becker <[email protected]>
Commit-Queue: Abhishek Shanthkumar <[email protected]>
Auto-Submit: Abhishek Shanthkumar <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1671338}
---

diff --git a/content/browser/indexed_db/BUILD.gn b/content/browser/indexed_db/BUILD.gn
index 1f7a146..d5fae33 100644
--- a/content/browser/indexed_db/BUILD.gn
+++ b/content/browser/indexed_db/BUILD.gn
@@ -99,6 +99,7 @@
     "//components/services/storage/public/mojom",
     "//content:export",
     "//content/public/common:common_sources",
+    "//mojo:buildflags",
     "//net",
     "//storage/browser",
     "//third_party/blink/public/common",
diff --git a/content/browser/indexed_db/indexed_db_browsertest.cc b/content/browser/indexed_db/indexed_db_browsertest.cc
index e6e5e828..a99c994 100644
--- a/content/browser/indexed_db/indexed_db_browsertest.cc
+++ b/content/browser/indexed_db/indexed_db_browsertest.cc
@@ -1663,6 +1663,18 @@
                                   blink::mojom::kIDBGetAllChunkSize)));
 }
 
+// Too slow on MSAN.
+#if defined(MEMORY_SANITIZER)
+#define MAYBE_GetAllLargeValues DISABLED_GetAllLargeValues
+#else
+#define MAYBE_GetAllLargeValues GetAllLargeValues
+#endif
+// Verifies that getAll() succeeds over an object store containing many large
+// values, testing platform limits. Regression test for crbug.com/523912081.
+IN_PROC_BROWSER_TEST_P(IndexedDBBrowserTest, MAYBE_GetAllLargeValues) {
+  SimpleTest(GetTestUrl("indexeddb", "get_all_large_values.html"));
+}
+
 // Large values are NOT wrapped when using SQLite, but are wrapped when using
 // LevelDB.
 IN_PROC_BROWSER_TEST_P(IndexedDBBrowserTest, LargeValueIsWrapped) {
diff --git a/content/browser/indexed_db/instance/database.cc b/content/browser/indexed_db/instance/database.cc
index 65da8c9..10ae6cd 100644
--- a/content/browser/indexed_db/instance/database.cc
+++ b/content/browser/indexed_db/instance/database.cc
@@ -27,6 +27,7 @@
 #include "base/trace_event/trace_event.h"
 #include "base/types/expected_macros.h"
 #include "base/unguessable_token.h"
+#include "build/build_config.h"
 #include "components/services/storage/indexed_db/locks/partitioned_lock_id.h"
 #include "components/services/storage/indexed_db/locks/partitioned_lock_manager.h"
 #include "components/services/storage/privileged/mojom/indexed_db_client_state_checker.mojom.h"
@@ -46,6 +47,8 @@
 #include "content/browser/indexed_db/instance/transaction.h"
 #include "content/browser/indexed_db/status.h"
 #include "ipc/constants.mojom.h"
+#include "mojo/buildflags.h"
+#include "mojo/public/cpp/base/big_buffer.h"
 #include "mojo/public/cpp/bindings/associated_remote.h"
 #include "mojo/public/cpp/bindings/pending_associated_receiver.h"
 #include "mojo/public/cpp/bindings/pending_associated_remote.h"
@@ -738,13 +741,45 @@
       "Value wrapping threshold is higher than BigBuffer inline size; "
       "BigBuffer may use shared memory with LevelDB backing store");
 
-  const size_t max_values_before_sending = blink::mojom::kIDBGetAllChunkSize;
+  const size_t kMaxValuesBeforeSending = blink::mojom::kIDBGetAllChunkSize;
+
+  // Mojo limits the number of handles/file descriptors in a single message on
+  // certain platforms. Since SQLite does not wrap large values in blobs, the
+  // `mojo_base::BigBuffer` holding the value may be backed by shared memory
+  // (which uses handles/file descriptors - see
+  // `mojo::core::ipcz_driver::SharedBuffer::Serialize()`), and hence need a
+  // separate cap.
+  std::optional<uint32_t> max_shared_memory_values_in_chunk;
+  uint32_t shared_memory_values_in_chunk = 0;
+
+  if (bucket_context_->IsUsingSqlite()) {
+#if BUILDFLAG(IS_POSIX) && !BUILDFLAG(MOJO_USE_APPLE_CHANNEL)
+    // channel_posix limits the number of file descriptors per message to 128:
+    // crbug.com/439305148.
+#if BUILDFLAG(IS_ANDROID)
+    // 1 file descriptor is used per shared memory buffer.
+    max_shared_memory_values_in_chunk = 128;
+#else
+    // 2 file descriptors are used per shared memory buffer.
+    max_shared_memory_values_in_chunk = 64;
+#endif
+#elif BUILDFLAG(IS_FUCHSIA)
+    // channel_fuchsia.cc limits the number of handles per message to 64
+    // (ZX_CHANNEL_MAX_MSG_HANDLES): crbug.com/508116627.
+    // 1 platform handle is used per shared memory buffer.
+    max_shared_memory_values_in_chunk = 64;
+#endif
+  }
 
   for (uint32_t i = 0; i < max_count; ++i) {
     // Periodically stream records if we have too many.
-    if (found_records.size() >= max_values_before_sending) {
+    if (found_records.size() >= kMaxValuesBeforeSending ||
+        (max_shared_memory_values_in_chunk.has_value() &&
+         shared_memory_values_in_chunk >=
+             max_shared_memory_values_in_chunk.value())) {
       result_sink->SendResults(std::move(found_records), /*done=*/false);
       found_records.clear();
+      shared_memory_values_in_chunk = 0;
     }
 
     // Cursor creation performs the first seek, returning a nullptr cursor when
@@ -795,6 +830,12 @@
       NOTREACHED();
     }
 
+    if (return_record->return_value &&
+        return_record->return_value->value->bits.storage_type() ==
+            mojo_base::BigBuffer::StorageType::kSharedMemory) {
+      ++shared_memory_values_in_chunk;
+    }
+
     found_records.emplace_back(std::move(return_record));
   }
 
diff --git a/content/test/content_test_bundle_data.filelist b/content/test/content_test_bundle_data.filelist
index b9b0c99..b12410f 100644
--- a/content/test/content_test_bundle_data.filelist
+++ b/content/test/content_test_bundle_data.filelist
@@ -7927,6 +7927,8 @@
 data/indexeddb/force_close_event.js
 data/indexeddb/get_all_chunking.html
 data/indexeddb/get_all_chunking.js
+data/indexeddb/get_all_large_values.html
+data/indexeddb/get_all_large_values.js
 data/indexeddb/index_test.html
 data/indexeddb/index_test.js
 data/indexeddb/key_path_test.html
diff --git a/content/test/data/indexeddb/get_all_large_values.html b/content/test/data/indexeddb/get_all_large_values.html
new file mode 100644
index 0000000..f1ee18a
--- /dev/null
+++ b/content/test/data/indexeddb/get_all_large_values.html
@@ -0,0 +1,16 @@
+<!DOCTYPE html>
+<html>
+<!--
+Copyright 2026 The Chromium Authors
+Use of this source code is governed by a BSD-style license that can be
+found in the LICENSE file.
+-->
+<head>
+<title>Test getAll() over a store containing many large values</title>
+<script type="text/javascript" src="common.js"></script>
+<script type="text/javascript" src="get_all_large_values.js"></script>
+</head>

diff --git a/content/browser/indexed_db/indexed_db_browsertest.cc b/content/browser/indexed_db/indexed_db_browsertest.cc
index e6e5e828..a99c994 100644
--- a/content/browser/indexed_db/indexed_db_browsertest.cc
+++ b/content/browser/indexed_db/indexed_db_browsertest.cc
@@ -1663,6 +1663,18 @@
                                   blink::mojom::kIDBGetAllChunkSize)));
 }
 
+// Too slow on MSAN.
+#if defined(MEMORY_SANITIZER)
+#define MAYBE_GetAllLargeValues DISABLED_GetAllLargeValues
+#else
+#define MAYBE_GetAllLargeValues GetAllLargeValues
+#endif
+// Verifies that getAll() succeeds over an object store containing many large
+// values, testing platform limits. Regression test for crbug.com/523912081.
+IN_PROC_BROWSER_TEST_P(IndexedDBBrowserTest, MAYBE_GetAllLargeValues) {
+  SimpleTest(GetTestUrl("indexeddb", "get_all_large_values.html"));
+}
+
 // Large values are NOT wrapped when using SQLite, but are wrapped when using
 // LevelDB.
 IN_PROC_BROWSER_TEST_P(IndexedDBBrowserTest, LargeValueIsWrapped) {
diff --git a/content/test/content_test_bundle_data.filelist b/content/test/content_test_bundle_data.filelist
index b9b0c99..b12410f 100644
--- a/content/test/content_test_bundle_data.filelist
+++ b/content/test/content_test_bundle_data.filelist
@@ -7927,6 +7927,8 @@
 data/indexeddb/force_close_event.js
 data/indexeddb/get_all_chunking.html
 data/indexeddb/get_all_chunking.js
+data/indexeddb/get_all_large_values.html
+data/indexeddb/get_all_large_values.js
 data/indexeddb/index_test.html
 data/indexeddb/index_test.js
 data/indexeddb/key_path_test.html
diff --git a/content/test/data/indexeddb/get_all_large_values.html b/content/test/data/indexeddb/get_all_large_values.html
new file mode 100644
index 0000000..f1ee18a
--- /dev/null
+++ b/content/test/data/indexeddb/get_all_large_values.html
@@ -0,0 +1,16 @@
+<!DOCTYPE html>
+<html>
+<!--
+Copyright 2026 The Chromium Authors
+Use of this source code is governed by a BSD-style license that can be
+found in the LICENSE file.
+-->
+<head>
+<title>Test getAll() over a store containing many large values</title>
+<script type="text/javascript" src="common.js"></script>
+<script type="text/javascript" src="get_all_large_values.js"></script>
+</head>
+<body onLoad="test()">
+<div id="status">Starting...</div>
+</body>
+</html>
diff --git a/content/test/data/indexeddb/get_all_large_values.js b/content/test/data/indexeddb/get_all_large_values.js
new file mode 100644
index 0000000..7f682a5
--- /dev/null
+++ b/content/test/data/indexeddb/get_all_large_values.js
@@ -0,0 +1,43 @@
+// Copyright 2026 The Chromium Authors
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+'use strict';
+
+const DB_NAME = 'large-values-db';
+const STORE_NAME = 'store';
+
+// Each record exceeds the threshold for inlining bytes in BigBuffer.
+const kRecordSize = 64 * 1024;
+const kRecordCount = 128 + 1;
+const kRecords = [];
+
+function createDb(db) {
+  const store = db.createObjectStore(STORE_NAME);
+  for (let i = 0; i < kRecordCount; i++) {
+    const record = new Uint8Array(kRecordSize);
+    crypto.getRandomValues(record);
+    kRecords.push(record);
+    store.put(record, i);
+  }
+}
+
+async function test() {
+  const db = await promiseDeleteThenOpenDb(DB_NAME, createDb);
+  const request =
+      db.transaction(STORE_NAME, 'readonly').objectStore(STORE_NAME).getAll();
+  request.onerror = unexpectedErrorCallback;
+  request.onsuccess = () => {
+    if (request.result.length !== kRecordCount) {
+      fail(
+          'getAll() returned wrong number of records: ' +
+          request.result.length + ' vs ' + kRecordCount);
+      return;
+    }
+    if (request.result.toString() !== kRecords.toString()) {
+      fail('getAll() returned wrong records');
+      return;
+    }
+    done();
+  };
+}