Browse Source

chore: cherry-pick 0dc563cbbca5 from chromium (#25238)

Jeremy Rose 4 years ago
parent
commit
b032c415c1

+ 2 - 0
patches/chromium/.patches

@@ -126,4 +126,6 @@ cherry-pick-9d100199c92b.patch
 cherry-pick-bee371eeaf66.patch
 cherry-pick-9746a4cde14a.patch
 avoid_loading_dri_via_gbm_when_gpumemorybuffers_are_disabled.patch
+indexeddb_fix_crash_in_webidbgetdbnamescallbacksimpl.patch
+indexeddb_reset_async_tasks_in_webidbgetdbnamescallbacksimpl.patch
 reland_fix_uaf_in_selecttype.patch

+ 48 - 0
patches/chromium/indexeddb_fix_crash_in_webidbgetdbnamescallbacksimpl.patch

@@ -0,0 +1,48 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Adrienne Walker <[email protected]>
+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 <[email protected]>
+Reviewed-by: Daniel Murphy <[email protected]>
+Auto-Submit: enne <[email protected]>
+Cr-Original-Commit-Position: refs/heads/master@{#790857}
+Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2337096
+Reviewed-by: enne <[email protected]>
+Commit-Queue: enne <[email protected]>
+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 46c5b3432931f6caef98f05dbb6f3543b2711874..af1b4c4234c62671ed56b52d7bd13c3557d397e9 100644
+--- a/third_party/blink/renderer/modules/indexeddb/idb_factory.cc
++++ b/third_party/blink/renderer/modules/indexeddb/idb_factory.cc
+@@ -104,7 +104,6 @@ class WebIDBGetDBNamesCallbacksImpl : public WebIDBCallbacks {
+     promise_resolver_->Reject(MakeGarbageCollected<DOMException>(
+         DOMExceptionCode::kUnknownError,
+         "The databases() promise was rejected."));
+-    promise_resolver_.Clear();
+   }
+ 
+   void SuccessNamesAndVersionsList(
+@@ -128,7 +127,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(); }

+ 62 - 0
patches/chromium/indexeddb_reset_async_tasks_in_webidbgetdbnamescallbacksimpl.patch

@@ -0,0 +1,62 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Adrienne Walker <[email protected]>
+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 <[email protected]>
+Commit-Queue: enne <[email protected]>
+Auto-Submit: enne <[email protected]>
+Cr-Original-Commit-Position: refs/heads/master@{#792121}
+Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2337110
+Reviewed-by: enne <[email protected]>
+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 af1b4c4234c62671ed56b52d7bd13c3557d397e9..d0ea6c191643ad51ab9e1d97e7a81ca9a074092e 100644
+--- a/third_party/blink/renderer/modules/indexeddb/idb_factory.cc
++++ b/third_party/blink/renderer/modules/indexeddb/idb_factory.cc
+@@ -110,6 +110,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());
+@@ -123,11 +124,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(); }
+@@ -189,6 +191,7 @@ class WebIDBGetDBNamesCallbacksImpl : public WebIDBCallbacks {
+ 
+  private:
+   probe::AsyncTaskId async_task_id_;
++  base::Optional<probe::AsyncTask> async_task_;
+   Persistent<ScriptPromiseResolver> promise_resolver_;
+ };
+