[IndexedDB] Treat frozen clients as inactive
Before this CL, a client is inactive iff it is inside the BF cache.
To support freezing, IDBDatabase now listens for the
ContextLifecycleStateChanged notification, which invokes
DidBecomeInactive when the frame is frozen, just like when the
frame enters BFCache.
In addition, whenever a transaction ends, we check if any of the
remaining transactions are blocking other clients to determine if
the client is now eligible for freezing.
Bug: 362464956
Change-Id: I4d8092bef3189e4d42124c2cb169015570e5df96
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5958727
Commit-Queue: Charlie Reis <creis@chromium.org>
Commit-Queue: Patrick Monette <pmonette@chromium.org>
Reviewed-by: Evan Stade <estade@chromium.org>
Reviewed-by: Charlie Reis <creis@chromium.org>
Auto-Submit: Patrick Monette <pmonette@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1382575}
diff --git a/content/browser/indexed_db/instance/connection.cc b/content/browser/indexed_db/instance/connection.cc
index 1613b78..ca1f197 100644
--- a/content/browser/indexed_db/instance/connection.cc
+++ b/content/browser/indexed_db/instance/connection.cc
@@ -4,12 +4,14 @@
#include "content/browser/indexed_db/instance/connection.h"
+#include <algorithm>
#include <set>
#include <utility>
#include "base/functional/bind.h"
#include "base/functional/callback_helpers.h"
#include "base/sequence_checker.h"
+#include "base/stl_util.h"
#include "base/trace_event/base_tracing.h"
#include "components/services/storage/privileged/mojom/indexed_db_client_state_checker.mojom.h"
#include "content/browser/indexed_db/instance/callback_helpers.h"
@@ -113,24 +115,34 @@
return;
}
- if (reason ==
- storage::mojom::DisallowInactiveClientReason::kVersionChangeEvent) {
- // It's only necessary to keep the client active under this scenario.
- mojo::Remote<storage::mojom::IndexedDBClientKeepActive>
- client_keep_active_remote;
- client_state_checker_->DisallowInactiveClient(
- reason, client_keep_active_remote.BindNewPipeAndPassReceiver(),
- std::move(callback));
- client_keep_active_remotes_.Add(std::move(client_keep_active_remote));
- } else {
- client_state_checker_->DisallowInactiveClient(reason, mojo::NullReceiver(),
- std::move(callback));
- }
+ mojo::Remote<storage::mojom::IndexedDBClientKeepActive>
+ client_keep_active_remote;
+ client_state_checker_->DisallowInactiveClient(
+ reason, client_keep_active_remote.BindNewPipeAndPassReceiver(),
+ std::move(callback));
+ client_keep_active_remotes_.Add(std::move(client_keep_active_remote));
}
void Connection::RemoveTransaction(int64_t id) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
- transactions_.erase(id);
+
+ size_t removed = transactions_.erase(id);
+ if (!removed) {
+ return;
+ }
+
+ // If this client is still blocking other clients, leave the keep-actives
+ // alive.
+ for (const auto& [_, transaction] : transactions_) {
+ if (transaction->state() == Transaction::State::STARTED &&
+ transaction->IsTransactionBlockingOtherClients(
+ /*consider_priority=*/true)) {
+ return;
+ }
+ }
+
+ // Safe to make this client inactive.
+ client_keep_active_remotes_.Clear();
}
void Connection::AbortTransactionAndTearDownOnError(
@@ -874,4 +886,16 @@
other_lock_request_data->scheduling_priority;
}
+bool Connection::IsHoldingLocks(
+ const std::vector<PartitionedLockId>& lock_ids) const {
+ return std::ranges::any_of(
+ transactions_,
+ [&](const std::pair<const int64_t, std::unique_ptr<Transaction>>&
+ existing_transaction) {
+ return !base::STLSetIntersection<std::vector<PartitionedLockId>>(
+ lock_ids, existing_transaction.second->lock_ids())
+ .empty();
+ });
+}
+
} // namespace content::indexed_db
diff --git a/content/browser/indexed_db/instance/connection.h b/content/browser/indexed_db/instance/connection.h
index d521d0a..7762da4 100644
--- a/content/browser/indexed_db/instance/connection.h
+++ b/content/browser/indexed_db/instance/connection.h
@@ -119,6 +119,10 @@
static bool HasHigherPriorityThan(const PartitionedLockHolder* this_one,
const PartitionedLockHolder& other);
+ // Returns true if any of the connection's transactions is holding one of the
+ // lock IDs.
+ bool IsHoldingLocks(const std::vector<PartitionedLockId>& lock_ids) const;
+
private:
friend class TransactionTest;
FRIEND_TEST_ALL_PREFIXES(DatabaseTest, ForcedClose);
diff --git a/content/browser/indexed_db/instance/database.cc b/content/browser/indexed_db/instance/database.cc
index f65899d..85cb07f 100644
--- a/content/browser/indexed_db/instance/database.cc
+++ b/content/browser/indexed_db/instance/database.cc
@@ -6,7 +6,6 @@
#include <math.h>
-#include <algorithm>
#include <cstddef>
#include <utility>
@@ -183,16 +182,7 @@
// If any of the connection's transactions is holding one of the blocked
// lock IDs, require that client to be active.
- if (std::any_of(
- connection->transactions().begin(),
- connection->transactions().end(),
- [&](const std::pair<const int64_t, std::unique_ptr<Transaction>>&
- existing_transaction) {
- return !base::STLSetIntersection<std::vector<PartitionedLockId>>(
- blocked_lock_ids,
- existing_transaction.second->lock_ids())
- .empty();
- })) {
+ if (connection->IsHoldingLocks(blocked_lock_ids)) {
connection->DisallowInactiveClient(
storage::mojom::DisallowInactiveClientReason::
kTransactionIsAcquiringLocks,
diff --git a/content/browser/indexed_db/instance/transaction.cc b/content/browser/indexed_db/instance/transaction.cc
index 47c9851..5825f28 100644
--- a/content/browser/indexed_db/instance/transaction.cc
+++ b/content/browser/indexed_db/instance/transaction.cc
@@ -297,9 +297,8 @@
CHECK_EQ(state_, STARTED);
std::set<PartitionedLockHolder*> blocked_requests =
bucket_context_->lock_manager().GetBlockedRequests(lock_ids());
- return std::any_of(
- blocked_requests.begin(), blocked_requests.end(),
- [&](PartitionedLockHolder* blocked_lock_holder) {
+ return std::ranges::any_of(
+ blocked_requests, [&](PartitionedLockHolder* blocked_lock_holder) {
auto* lock_request_data = static_cast<LockRequestData*>(
blocked_lock_holder->GetUserData(LockRequestData::kKey));
if (!lock_request_data) {
diff --git a/content/browser/renderer_host/indexed_db_client_state_checker_factory.cc b/content/browser/renderer_host/indexed_db_client_state_checker_factory.cc
index 9768605d..b5e7e22 100644
--- a/content/browser/renderer_host/indexed_db_client_state_checker_factory.cc
+++ b/content/browser/renderer_host/indexed_db_client_state_checker_factory.cc
@@ -116,13 +116,19 @@
mojo::PendingReceiver<storage::mojom::IndexedDBClientKeepActive>
keep_active,
DisallowInactiveClientCallback callback) override {
+ CHECK(keep_active.is_valid());
bool was_active = CheckIfClientWasActive(reason);
- if (was_active && keep_active.is_valid()) {
- // This is the only reason that we need to prevent the client from
- // inactive state.
- CHECK_EQ(
- reason,
- storage::mojom::DisallowInactiveClientReason::kVersionChangeEvent);
+
+ // If the client is still active, the `keep_active` is usually dropped.
+ // That's because we always first allow a client to enter BFCache, and then
+ // evict it afterwards if we've determined it is blocking another client.
+ // The only exception is for the kVersionChangeEvent reason, which is called
+ // preemptively to ensure the client doesn't enter BFCache while the version
+ // change event is being handled.
+ // TODO(362464956): Add support for unfreezing frozen clients as well.
+ if (was_active &&
+ reason ==
+ storage::mojom::DisallowInactiveClientReason::kVersionChangeEvent) {
// If the document is active, we need to register a non sticky feature to
// prevent putting it into BFCache until the IndexedDB connection is
// successfully closed and the context is automatically destroyed.
diff --git a/third_party/blink/renderer/modules/indexeddb/idb_database.cc b/third_party/blink/renderer/modules/indexeddb/idb_database.cc
index de802c3..5dba12f1 100644
--- a/third_party/blink/renderer/modules/indexeddb/idb_database.cc
+++ b/third_party/blink/renderer/modules/indexeddb/idb_database.cc
@@ -104,7 +104,7 @@
mojo::PendingAssociatedRemote<mojom::blink::IDBDatabase> pending_database,
int connection_priority)
: ActiveScriptWrappable<IDBDatabase>({}),
- ExecutionContextLifecycleObserver(context),
+ ExecutionContextLifecycleStateObserver(context),
database_remote_(context),
connection_lifetime_(std::move(connection_lifetime)),
scheduling_priority_(connection_priority),
@@ -119,6 +119,8 @@
FrameOrWorkerScheduler::ObserverType::kWorkerScheduler,
WTF::BindRepeating(&IDBDatabase::OnSchedulerLifecycleStateChanged,
WrapWeakPersistent(this)));
+
+ UpdateStateIfNeeded();
}
void IDBDatabase::Trace(Visitor* visitor) const {
@@ -547,7 +549,21 @@
}
void IDBDatabase::ContextEnteredBackForwardCache() {
- if (database_remote_.is_bound()) {
+ if (!database_remote_.is_bound()) {
+ return;
+ }
+
+ DidBecomeInactive();
+}
+
+void IDBDatabase::ContextLifecycleStateChanged(
+ mojom::blink::FrameLifecycleState state) {
+ if (!database_remote_.is_bound()) {
+ return;
+ }
+
+ if (state == mojom::blink::FrameLifecycleState::kFrozen ||
+ state == mojom::blink::FrameLifecycleState::kFrozenAutoResumeMedia) {
DidBecomeInactive();
}
}
diff --git a/third_party/blink/renderer/modules/indexeddb/idb_database.h b/third_party/blink/renderer/modules/indexeddb/idb_database.h
index 81acfd23..38d8d98 100644
--- a/third_party/blink/renderer/modules/indexeddb/idb_database.h
+++ b/third_party/blink/renderer/modules/indexeddb/idb_database.h
@@ -36,6 +36,7 @@
#include "third_party/blink/renderer/bindings/modules/v8/v8_idb_transaction_options.h"
#include "third_party/blink/renderer/core/dom/dom_string_list.h"
#include "third_party/blink/renderer/core/execution_context/execution_context_lifecycle_observer.h"
+#include "third_party/blink/renderer/core/execution_context/execution_context_lifecycle_state_observer.h"
#include "third_party/blink/renderer/modules/event_modules.h"
#include "third_party/blink/renderer/modules/event_target_modules.h"
#include "third_party/blink/renderer/modules/indexeddb/idb_metadata.h"
@@ -58,7 +59,7 @@
class MODULES_EXPORT IDBDatabase final
: public EventTarget,
public ActiveScriptWrappable<IDBDatabase>,
- public ExecutionContextLifecycleObserver,
+ public ExecutionContextLifecycleStateObserver,
public mojom::blink::IDBDatabaseCallbacks {
DEFINE_WRAPPERTYPEINFO();
@@ -125,9 +126,11 @@
// ScriptWrappable
bool HasPendingActivity() const final;
- // ExecutionContextLifecycleObserver
+ // ExecutionContextLifecycleStateObserver
void ContextDestroyed() override;
void ContextEnteredBackForwardCache() override;
+ void ContextLifecycleStateChanged(
+ mojom::blink::FrameLifecycleState state) override;
// EventTarget
const AtomicString& InterfaceName() const override;
