Skip to content

Commit

Permalink
IndexedDB: move AsyncTaskContext off of IDBFactoryClient
Browse files Browse the repository at this point in the history
IDBRequest is the more natural place to track the async task, and in
fact, already does have an AsyncTaskContext. Use that one instead to
track events on its subclass, IDBOpenDBRequest.

Bug: 1475187
Change-Id: I1362b31a82cf080ac10a684af55e6fba875cd937
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4809007
Reviewed-by: Nathan Memmott <memmott@chromium.org>
Commit-Queue: Evan Stade <estade@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1188592}
  • Loading branch information
Evan Stade authored and Chromium LUCI CQ committed Aug 25, 2023
1 parent 19551b6 commit 33f1311
Show file tree
Hide file tree
Showing 5 changed files with 18 additions and 16 deletions.
18 changes: 4 additions & 14 deletions third_party/blink/renderer/modules/indexeddb/idb_factory_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@
#include "third_party/blink/public/mojom/indexeddb/indexeddb.mojom-blink.h"
#include "third_party/blink/public/platform/web_blob_info.h"
#include "third_party/blink/renderer/core/dom/dom_exception.h"
#include "third_party/blink/renderer/core/probe/core_probes.h"
#include "third_party/blink/renderer/modules/indexed_db_names.h"
#include "third_party/blink/renderer/modules/indexeddb/idb_metadata.h"
#include "third_party/blink/renderer/modules/indexeddb/idb_open_db_request.h"
Expand All @@ -52,8 +51,6 @@ IDBFactoryClient::IDBFactoryClient(IDBOpenDBRequest* request)
: request_(request) {
task_runner_ =
request_->GetExecutionContext()->GetTaskRunner(TaskType::kDatabaseAccess);
async_task_context_.Schedule(request_->GetExecutionContext(),
indexed_db_names::kIndexedDB);
}

IDBFactoryClient::~IDBFactoryClient() {
Expand All @@ -67,7 +64,6 @@ void IDBFactoryClient::Detach() {

void IDBFactoryClient::DetachFromRequest() {
if (request_) {
async_task_context_.Cancel();
request_->FactoryClientDestroyed(this);
}
}
Expand All @@ -91,8 +87,6 @@ void IDBFactoryClient::Error(mojom::blink::IDBException code,
return;
}

probe::AsyncTask async_task(request_->GetExecutionContext(),
&async_task_context_, "error");
IDBOpenDBRequest* request = request_.Get();
Detach();
request->OnDBFactoryError(MakeGarbageCollected<DOMException>(
Expand All @@ -108,14 +102,13 @@ void IDBFactoryClient::OpenSuccess(
task_runner_);
}
if (request_) {
probe::AsyncTask async_task(request_->GetExecutionContext(),
&async_task_context_, "success");
#if DCHECK_IS_ON()
DCHECK(!request_->TransactionHasQueuedResults());
#endif // DCHECK_IS_ON()
IDBOpenDBRequest* request = request_.Get();
Detach();
request->OnOpenDBSuccess(std::move(db), IDBDatabaseMetadata(metadata));
// `this` may be deleted because event dispatch can run a nested loop.
} else if (db) {
db->Close();
}
Expand All @@ -126,24 +119,22 @@ void IDBFactoryClient::DeleteSuccess(int64_t old_version) {
return;
}

probe::AsyncTask async_task(request_->GetExecutionContext(),
&async_task_context_, "success");
IDBOpenDBRequest* request = request_.Get();
Detach();
request->OnDeleteDBSuccess(old_version);
// `this` may be deleted because event dispatch can run a nested loop.
}

void IDBFactoryClient::Blocked(int64_t old_version) {
if (!request_) {
return;
}

probe::AsyncTask async_task(request_->GetExecutionContext(),
&async_task_context_, "blocked");
#if DCHECK_IS_ON()
DCHECK(!request_->TransactionHasQueuedResults());
#endif // DCHECK_IS_ON()
request_->OnBlocked(old_version);
// `this` may be deleted because event dispatch can run a nested loop.
// Not resetting |request_|. In this instance we will have to forward at
// least one other call in the set UpgradeNeeded() / OpenSuccess() /
// Error().
Expand All @@ -161,14 +152,13 @@ void IDBFactoryClient::UpgradeNeeded(
task_runner_);
}
if (request_) {
probe::AsyncTask async_task(request_->GetExecutionContext(),
&async_task_context_, "upgradeNeeded");
#if DCHECK_IS_ON()
DCHECK(!request_->TransactionHasQueuedResults());
#endif // DCHECK_IS_ON()
request_->OnUpgradeNeeded(old_version, std::move(db),
IDBDatabaseMetadata(metadata), data_loss,
data_loss_message);
// `this` may be deleted because event dispatch can run a nested loop.
// Not resetting |request_|. In this instance we will have to forward at
// least one other call in the set UpgradeNeeded() / OpenSuccess() /
// Error().
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@
#include "base/task/single_thread_task_runner.h"
#include "mojo/public/cpp/bindings/pending_associated_remote.h"
#include "third_party/blink/public/mojom/indexeddb/indexeddb.mojom-blink.h"
#include "third_party/blink/renderer/core/probe/async_task_context.h"
#include "third_party/blink/renderer/modules/modules_export.h"
#include "third_party/blink/renderer/platform/heap/persistent.h"
#include "third_party/blink/renderer/platform/wtf/allocator/allocator.h"
Expand Down Expand Up @@ -70,7 +69,6 @@ class MODULES_EXPORT IDBFactoryClient final
void DetachFromRequest();

Persistent<IDBOpenDBRequest> request_;
probe::AsyncTaskContext async_task_context_;
scoped_refptr<base::SingleThreadTaskRunner> task_runner_;
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
#include "third_party/abseil-cpp/absl/types/optional.h"
#include "third_party/blink/renderer/core/dom/dom_exception.h"
#include "third_party/blink/renderer/core/execution_context/execution_context.h"
#include "third_party/blink/renderer/core/probe/core_probes.h"
#include "third_party/blink/renderer/modules/indexeddb/idb_database.h"
#include "third_party/blink/renderer/modules/indexeddb/idb_version_change_event.h"
#include "third_party/blink/renderer/platform/heap/garbage_collected.h"
Expand Down Expand Up @@ -92,6 +93,8 @@ const AtomicString& IDBOpenDBRequest::InterfaceName() const {

void IDBOpenDBRequest::OnBlocked(int64_t old_version) {
TRACE_EVENT0("IndexedDB", "IDBOpenDBRequest::onBlocked()");
probe::AsyncTask async_task(GetExecutionContext(), async_task_context(),
"blocked");
if (!CanStillSendResult()) {
return;
}
Expand All @@ -109,6 +112,8 @@ void IDBOpenDBRequest::OnUpgradeNeeded(int64_t old_version,
mojom::blink::IDBDataLoss data_loss,
String data_loss_message) {
TRACE_EVENT0("IndexedDB", "IDBOpenDBRequest::onUpgradeNeeded()");
probe::AsyncTask async_task(GetExecutionContext(), async_task_context(),
"upgradeNeeded");
if (!CanStillSendResult()) {
metrics_.RecordAndReset();
return;
Expand Down Expand Up @@ -144,6 +149,9 @@ void IDBOpenDBRequest::OnUpgradeNeeded(int64_t old_version,
void IDBOpenDBRequest::OnOpenDBSuccess(std::unique_ptr<WebIDBDatabase> backend,
const IDBDatabaseMetadata& metadata) {
TRACE_EVENT0("IndexedDB", "IDBOpenDBRequest::onSuccess(database)");
probe::AsyncTask async_task(GetExecutionContext(), async_task_context(),
"success");

if (!CanStillSendResult()) {
metrics_.RecordAndReset();
return;
Expand All @@ -170,6 +178,8 @@ void IDBOpenDBRequest::OnOpenDBSuccess(std::unique_ptr<WebIDBDatabase> backend,

void IDBOpenDBRequest::OnDeleteDBSuccess(int64_t old_version) {
TRACE_EVENT0("IndexedDB", "IDBOpenDBRequest::onDeleteDBSuccess(int64_t)");
probe::AsyncTask async_task(GetExecutionContext(), async_task_context(),
"success");
if (!CanStillSendResult()) {
metrics_.RecordAndReset();
return;
Expand Down
2 changes: 2 additions & 0 deletions third_party/blink/renderer/modules/indexeddb/idb_request.cc
Original file line number Diff line number Diff line change
Expand Up @@ -595,6 +595,8 @@ void IDBRequest::OnGotKeyGeneratorCurrentNumber(

void IDBRequest::SendError(DOMException* error, bool force) {
TRACE_EVENT0("IndexedDB", "IDBRequest::SendError()");
probe::AsyncTask async_task(GetExecutionContext(), async_task_context(),
"error");
if (!GetExecutionContext() || (request_aborted_ && !force)) {
metrics_.RecordAndReset();
return;
Expand Down
2 changes: 2 additions & 0 deletions third_party/blink/renderer/modules/indexeddb/idb_request.h
Original file line number Diff line number Diff line change
Expand Up @@ -368,6 +368,8 @@ class MODULES_EXPORT IDBRequest : public EventTarget,
// registered against it.
v8::Isolate* isolate_;

probe::AsyncTaskContext* async_task_context() { return &async_task_context_; }

AsyncTraceState metrics_;

private:
Expand Down

0 comments on commit 33f1311

Please sign in to comment.