From 945fd0ca08cb1625c4e2af8a2b2745bd0e676a0a Mon Sep 17 00:00:00 2001 From: Jeremy Rose Date: Wed, 20 Jul 2022 09:32:18 -0700 Subject: [PATCH 1/2] chore: [18-x-y] cherry-pick 3cbd5973d704 from chromium --- patches/chromium/.patches | 1 + .../chromium/cherry-pick-3cbd5973d704.patch | 103 ++++++++++++++++++ 2 files changed, 104 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..574ef86d7437e --- /dev/null +++ b/patches/chromium/cherry-pick-3cbd5973d704.patch @@ -0,0 +1,103 @@ +From 3cbd5973d7049e630acdf28969c12c2dec82f6cf Mon Sep 17 00:00:00 2001 +From: Yoshisato Yanagisawa +Date: Mon, 18 Jul 2022 20:44:41 +0000 +Subject: [PATCH] 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 495b079..90facd12 100644 +--- a/content/browser/service_worker/service_worker_version.cc ++++ b/content/browser/service_worker/service_worker_version.cc +@@ -2012,18 +2012,17 @@ + + 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); +@@ -2050,7 +2049,6 @@ + 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(); +@@ -2059,17 +2057,22 @@ + + // 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 4d63846d..c7adbdce 100644 +--- a/content/browser/service_worker/service_worker_version.h ++++ b/content/browser/service_worker/service_worker_version.h +@@ -883,6 +883,8 @@ + 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); + From 120251b126f4ae663f94f2903c28099c223e31a6 Mon Sep 17 00:00:00 2001 From: PatchUp <73610968+patchup[bot]@users.noreply.github.com> Date: Wed, 20 Jul 2022 16:43:36 +0000 Subject: [PATCH 2/2] chore: update patches --- patches/chromium/cherry-pick-3cbd5973d704.patch | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/patches/chromium/cherry-pick-3cbd5973d704.patch b/patches/chromium/cherry-pick-3cbd5973d704.patch index 574ef86d7437e..caf5f852fe2a5 100644 --- a/patches/chromium/cherry-pick-3cbd5973d704.patch +++ b/patches/chromium/cherry-pick-3cbd5973d704.patch @@ -1,7 +1,7 @@ -From 3cbd5973d7049e630acdf28969c12c2dec82f6cf Mon Sep 17 00:00:00 2001 +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 From: Yoshisato Yanagisawa Date: Mon, 18 Jul 2022 20:44:41 +0000 -Subject: [PATCH] Keep refptr to ServiceWorkerVersion in MaybeTimeoutRequest +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. @@ -23,13 +23,12 @@ 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 495b079..90facd12 100644 +index 0b4b32d13db8958f579085a41bc5e20b26543f61..5797e3f954ae0769a91c8f259e19e50e2bf31a94 100644 --- a/content/browser/service_worker/service_worker_version.cc +++ b/content/browser/service_worker/service_worker_version.cc -@@ -2012,18 +2012,17 @@ +@@ -1993,18 +1993,17 @@ void ServiceWorkerVersion::OnTimeoutTimer() { MarkIfStale(); @@ -53,7 +52,7 @@ index 495b079..90facd12 100644 embedded_worker_->RemoveObserver(this); embedded_worker_->Detach(); embedded_worker_ = std::make_unique(this); -@@ -2050,7 +2049,6 @@ +@@ -2031,7 +2030,6 @@ void ServiceWorkerVersion::OnTimeoutTimer() { DCHECK(running_status() == EmbeddedWorkerStatus::STARTING || running_status() == EmbeddedWorkerStatus::STOPPING) << static_cast(running_status()); @@ -61,7 +60,7 @@ index 495b079..90facd12 100644 FinishStartWorker(blink::ServiceWorkerStatusCode::kErrorTimeout); if (running_status() == EmbeddedWorkerStatus::STARTING) embedded_worker_->Stop(); -@@ -2059,17 +2057,22 @@ +@@ -2040,17 +2038,22 @@ void ServiceWorkerVersion::OnTimeoutTimer() { // Requests have not finished before their expiration. bool stop_for_timeout = false; @@ -89,10 +88,10 @@ index 495b079..90facd12 100644 embedded_worker_->Stop(); diff --git a/content/browser/service_worker/service_worker_version.h b/content/browser/service_worker/service_worker_version.h -index 4d63846d..c7adbdce 100644 +index dcf0dfaf515f8058438d727b30fb4c508a7211a9..a88bc2513f988a00b538baa596a1e713c2600cf8 100644 --- a/content/browser/service_worker/service_worker_version.h +++ b/content/browser/service_worker/service_worker_version.h -@@ -883,6 +883,8 @@ +@@ -863,6 +863,8 @@ class CONTENT_EXPORT ServiceWorkerVersion bool is_browser_startup_complete, blink::ServiceWorkerStatusCode status);