From 50595024568a50b1f860f14d8c23414a09c1b990 Mon Sep 17 00:00:00 2001 From: Jeremy Rose Date: Thu, 21 Jul 2022 00:37:35 -0700 Subject: [PATCH] chore: cherry-pick 3cbd5973d704 from chromium (#35002) * chore: [18-x-y] cherry-pick 3cbd5973d704 from chromium * chore: update patches Co-authored-by: PatchUp <73610968+patchup[bot]@users.noreply.github.com> --- patches/chromium/.patches | 1 + .../chromium/cherry-pick-3cbd5973d704.patch | 102 ++++++++++++++++++ 2 files changed, 103 insertions(+) create mode 100644 patches/chromium/cherry-pick-3cbd5973d704.patch diff --git a/patches/chromium/.patches b/patches/chromium/.patches index 0a9790f352a75..ffdbef87cb5b4 100644 --- a/patches/chromium/.patches +++ b/patches/chromium/.patches @@ -132,3 +132,4 @@ posix_replace_doubleforkandexec_with_forkandspawn.patch cherry-pick-f427936d32db.patch cherry-pick-22c61cfae5d1.patch remove_default_window_title.patch +cherry-pick-3cbd5973d704.patch diff --git a/patches/chromium/cherry-pick-3cbd5973d704.patch b/patches/chromium/cherry-pick-3cbd5973d704.patch new file mode 100644 index 0000000000000..caf5f852fe2a5 --- /dev/null +++ b/patches/chromium/cherry-pick-3cbd5973d704.patch @@ -0,0 +1,102 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Yoshisato Yanagisawa +Date: Mon, 18 Jul 2022 20:44:41 +0000 +Subject: Keep refptr to ServiceWorkerVersion in MaybeTimeoutRequest + +The callback in ServiceWorkerVersion::MaybeTimeoutRequest may reduce the +reference to the version object, which can be the last reference to it. +In thet case, the object can be destroyed and the `inflight_requests_` +field access after the callback become invalid. +This CL keeps this to avoid the object destruction. + +(cherry picked from commit 5926fa916d9ad53c77e31ee757e1979275d7466c) + +Bug: 1339844 +Change-Id: I6564627bad0527dea007ca73261c5636dab56755 +Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3739330 +Commit-Queue: Yoshisato Yanagisawa +Reviewed-by: Hiroki Nakagawa +Reviewed-by: Sergei Glazunov +Cr-Original-Commit-Position: refs/heads/main@{#1023475} +Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3766241 +Bot-Commit: Rubber Stamper +Owners-Override: Srinivas Sista +Cr-Commit-Position: refs/branch-heads/5005@{#1263} +Cr-Branched-From: 5b4d9450fee01f821b6400e947b3839727643a71-refs/heads/main@{#992738} + +diff --git a/content/browser/service_worker/service_worker_version.cc b/content/browser/service_worker/service_worker_version.cc +index 0b4b32d13db8958f579085a41bc5e20b26543f61..5797e3f954ae0769a91c8f259e19e50e2bf31a94 100644 +--- a/content/browser/service_worker/service_worker_version.cc ++++ b/content/browser/service_worker/service_worker_version.cc +@@ -1993,18 +1993,17 @@ void ServiceWorkerVersion::OnTimeoutTimer() { + + MarkIfStale(); + ++ // Global `this` protecter. ++ // callbacks initiated by this function sometimes reduce refcnt to 0 ++ // to make this instance freed. ++ scoped_refptr protect_this(this); ++ + // Stopping the worker hasn't finished within a certain period. + if (GetTickDuration(stop_time_) > kStopWorkerTimeout) { + DCHECK_EQ(EmbeddedWorkerStatus::STOPPING, running_status()); + ReportError(blink::ServiceWorkerStatusCode::kErrorTimeout, + "DETACH_STALLED_IN_STOPPING"); + +- // Detach the worker. Remove |this| as a listener first; otherwise +- // OnStoppedInternal might try to restart before the new worker +- // is created. Also, protect |this|, since swapping out the +- // EmbeddedWorkerInstance could destroy our ServiceWorkerHost which could in +- // turn destroy |this|. +- scoped_refptr protect_this(this); + embedded_worker_->RemoveObserver(this); + embedded_worker_->Detach(); + embedded_worker_ = std::make_unique(this); +@@ -2031,7 +2030,6 @@ void ServiceWorkerVersion::OnTimeoutTimer() { + DCHECK(running_status() == EmbeddedWorkerStatus::STARTING || + running_status() == EmbeddedWorkerStatus::STOPPING) + << static_cast(running_status()); +- scoped_refptr protect(this); + FinishStartWorker(blink::ServiceWorkerStatusCode::kErrorTimeout); + if (running_status() == EmbeddedWorkerStatus::STARTING) + embedded_worker_->Stop(); +@@ -2040,17 +2038,22 @@ void ServiceWorkerVersion::OnTimeoutTimer() { + + // Requests have not finished before their expiration. + bool stop_for_timeout = false; +- auto timeout_iter = request_timeouts_.begin(); +- while (timeout_iter != request_timeouts_.end()) { ++ std::set request_timeouts; ++ request_timeouts.swap(request_timeouts_); ++ auto timeout_iter = request_timeouts.begin(); ++ while (timeout_iter != request_timeouts.end()) { + const InflightRequestTimeoutInfo& info = *timeout_iter; +- if (!RequestExpired(info.expiration)) ++ if (!RequestExpired(info.expiration)) { + break; ++ } + if (MaybeTimeoutRequest(info)) { + stop_for_timeout = + stop_for_timeout || info.timeout_behavior == KILL_ON_TIMEOUT; + } +- timeout_iter = request_timeouts_.erase(timeout_iter); ++ timeout_iter = request_timeouts.erase(timeout_iter); + } ++ DCHECK(request_timeouts_.empty()); ++ request_timeouts_.swap(request_timeouts); + if (stop_for_timeout && running_status() != EmbeddedWorkerStatus::STOPPING) + embedded_worker_->Stop(); + +diff --git a/content/browser/service_worker/service_worker_version.h b/content/browser/service_worker/service_worker_version.h +index dcf0dfaf515f8058438d727b30fb4c508a7211a9..a88bc2513f988a00b538baa596a1e713c2600cf8 100644 +--- a/content/browser/service_worker/service_worker_version.h ++++ b/content/browser/service_worker/service_worker_version.h +@@ -863,6 +863,8 @@ class CONTENT_EXPORT ServiceWorkerVersion + bool is_browser_startup_complete, + blink::ServiceWorkerStatusCode status); + ++ // The caller of MaybeTimeoutRequest must increase reference count of |this| ++ // to avoid it deleted during the execution. + bool MaybeTimeoutRequest(const InflightRequestTimeoutInfo& info); + void SetAllRequestExpirations(const base::TimeTicks& expiration); +