Skip to content

Commit

Permalink
chore: cherry-pick 0dc563cbbca5 from chromium (#25239)
Browse files Browse the repository at this point in the history
  • Loading branch information
nornagon committed Sep 2, 2020
1 parent 2cf35c0 commit 4ad543c
Show file tree
Hide file tree
Showing 3 changed files with 112 additions and 0 deletions.
2 changes: 2 additions & 0 deletions patches/chromium/.patches
Expand Up @@ -121,5 +121,7 @@ backport_1081722.patch
backport_1073409.patch
backport_1074340.patch
cherry-pick-70579363ce7b.patch
indexeddb_fix_crash_in_webidbgetdbnamescallbacksimpl.patch
indexeddb_reset_async_tasks_in_webidbgetdbnamescallbacksimpl.patch
cherry-pick-138b748dd0a4.patch
cherry-pick-bee371eeaf66.patch
@@ -0,0 +1,48 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Adrienne Walker <enne@chromium.org>
Date: Tue, 4 Aug 2020 20:10:23 +0000
Subject: indexeddb: fix crash in WebIDBGetDBNamesCallbacksImpl

Resolve() can end up freeing WebIDBGetDBNamesCallbacksImpl by throwing a
mojo error that deletes the self-owned associated receiver that owns it.
So, don't call any other functions after it.

As the promise resolver can only resolve/reject once, it is safe to
not clear it.

(cherry picked from commit da90fc39f5ca0f8dc1c665fbabad8ec229826f89)

Bug: 1106682
Change-Id: Iea943f3c5c1e57adb6ad399baff49522f54d264b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2311620
Commit-Queue: Daniel Murphy <dmurph@chromium.org>
Reviewed-by: Daniel Murphy <dmurph@chromium.org>
Auto-Submit: enne <enne@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#790857}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2337096
Reviewed-by: enne <enne@chromium.org>
Commit-Queue: enne <enne@chromium.org>
Cr-Commit-Position: refs/branch-heads/4147@{#1023}
Cr-Branched-From: 16307825352720ae04d898f37efa5449ad68b606-refs/heads/master@{#768962}

diff --git a/third_party/blink/renderer/modules/indexeddb/idb_factory.cc b/third_party/blink/renderer/modules/indexeddb/idb_factory.cc
index c7b06b4c851b973e4933d6b7635ca7fd32936551..645e5cbf682c2a26f6a3e0742afb4e77c4388770 100644
--- a/third_party/blink/renderer/modules/indexeddb/idb_factory.cc
+++ b/third_party/blink/renderer/modules/indexeddb/idb_factory.cc
@@ -105,7 +105,6 @@ class WebIDBGetDBNamesCallbacksImpl : public WebIDBCallbacks {
promise_resolver_->Reject(MakeGarbageCollected<DOMException>(
DOMExceptionCode::kUnknownError,
"The databases() promise was rejected."));
- promise_resolver_.Clear();
}

void SuccessNamesAndVersionsList(
@@ -129,7 +128,7 @@ class WebIDBGetDBNamesCallbacksImpl : public WebIDBCallbacks {
ExecutionContext::From(promise_resolver_->GetScriptState()),
&async_task_id_, "success");
promise_resolver_->Resolve(name_and_version_list);
- promise_resolver_.Clear();
+ // Note: Resolve may cause |this| to be deleted.
}

void SuccessStringList(const Vector<String>&) override { NOTREACHED(); }
@@ -0,0 +1,62 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Adrienne Walker <enne@chromium.org>
Date: Wed, 5 Aug 2020 00:44:52 +0000
Subject: indexeddb: reset async tasks in ~WebIDBGetDBNamesCallbacksImpl

Since sometimes the WebIDBGetDBNamesCallbacksImpl can be destroyed when
the promise is resolved, make sure that no code that could reference it
is still around. Store the async task as an optional member so it can
be cleared during the destructor.

Followup to:
https://chromium-review.googlesource.com/c/chromium/src/+/2311620

(cherry picked from commit 4422ec665ddca3ac05ad90bac5d5ebee7cfc5536)

Bug: 1106682,1109467
Change-Id: Id6a0ff0a3703fab94e9684e41f16d5a1bac20468
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2321332
Reviewed-by: Daniel Murphy <dmurph@chromium.org>
Commit-Queue: enne <enne@chromium.org>
Auto-Submit: enne <enne@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#792121}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2337110
Reviewed-by: enne <enne@chromium.org>
Cr-Commit-Position: refs/branch-heads/4147@{#1029}
Cr-Branched-From: 16307825352720ae04d898f37efa5449ad68b606-refs/heads/master@{#768962}

diff --git a/third_party/blink/renderer/modules/indexeddb/idb_factory.cc b/third_party/blink/renderer/modules/indexeddb/idb_factory.cc
index 645e5cbf682c2a26f6a3e0742afb4e77c4388770..e2d0d49bed36e567a76c5610855a139774254b36 100644
--- a/third_party/blink/renderer/modules/indexeddb/idb_factory.cc
+++ b/third_party/blink/renderer/modules/indexeddb/idb_factory.cc
@@ -111,6 +111,7 @@ class WebIDBGetDBNamesCallbacksImpl : public WebIDBCallbacks {
Vector<mojom::blink::IDBNameAndVersionPtr> names_and_versions) override {
if (!promise_resolver_)
return;
+ DCHECK(!async_task_.has_value());

HeapVector<Member<IDBDatabaseInfo>> name_and_version_list;
name_and_version_list.ReserveInitialCapacity(name_and_version_list.size());
@@ -124,11 +125,12 @@ class WebIDBGetDBNamesCallbacksImpl : public WebIDBCallbacks {
name_and_version_list.push_back(idb_info);
}

- probe::AsyncTask async_task(
+ async_task_.emplace(
ExecutionContext::From(promise_resolver_->GetScriptState()),
&async_task_id_, "success");
promise_resolver_->Resolve(name_and_version_list);
- // Note: Resolve may cause |this| to be deleted.
+ // Note: Resolve may cause |this| to be deleted. async_task_ will be
+ // completed in the destructor.
}

void SuccessStringList(const Vector<String>&) override { NOTREACHED(); }
@@ -190,6 +192,7 @@ class WebIDBGetDBNamesCallbacksImpl : public WebIDBCallbacks {

private:
probe::AsyncTaskId async_task_id_;
+ base::Optional<probe::AsyncTask> async_task_;
Persistent<ScriptPromiseResolver> promise_resolver_;
};

0 comments on commit 4ad543c

Please sign in to comment.