From 8abb0e8ca0892b54f53fb5590d459100eda0f11a Mon Sep 17 00:00:00 2001 From: Jeremy Rose Date: Tue, 21 Jun 2022 13:56:50 -0700 Subject: [PATCH 1/3] chore: [18-x-y] cherry-pick ecad352cd614 from chromium --- patches/chromium/.patches | 1 + .../chromium/cherry-pick-ecad352cd614.patch | 106 ++++++++++++++++++ 2 files changed, 107 insertions(+) create mode 100644 patches/chromium/cherry-pick-ecad352cd614.patch diff --git a/patches/chromium/.patches b/patches/chromium/.patches index e8f586de2908d..c5228f99fa6d1 100644 --- a/patches/chromium/.patches +++ b/patches/chromium/.patches @@ -129,3 +129,4 @@ cherry-pick-919b1ffe1fe7.patch cherry-pick-f1dd785e021e.patch cherry-pick-b03797bdb1df.patch posix_replace_doubleforkandexec_with_forkandspawn.patch +cherry-pick-ecad352cd614.patch diff --git a/patches/chromium/cherry-pick-ecad352cd614.patch b/patches/chromium/cherry-pick-ecad352cd614.patch new file mode 100644 index 0000000000000..df8be74b8c150 --- /dev/null +++ b/patches/chromium/cherry-pick-ecad352cd614.patch @@ -0,0 +1,106 @@ +From ecad352cd61420d6bf8c9c39041b5369372ecf94 Mon Sep 17 00:00:00 2001 +From: Ted Meyer +Date: Wed, 08 Jun 2022 04:33:20 +0000 +Subject: [PATCH] Add Stop method to BatchingMediaLog + +Now that ~MediaLog is posted for a later destruction due to garbage +collector ownership of CodecLogger, it's possible for the +SendQueuedMediaEvents call from ~BatchingMediaLog to reference +InspectorMediaEventHandler::inspector_context_ after it has been freed. + +This fix forces BatchingMediaLog to shut down it's logging capabilities +when the destruction call is caused by the garbage collector deletion +phase + +R=​liberato + +(cherry picked from commit 1bbfaf23cd8a1e977cb445a82a4caae107632a59) + +Bug: 1333333 +Change-Id: I0bdca72a71177c4c5a6a9dc692aad3de4c25f4e2 +Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3689639 +Commit-Queue: Ted (Chromium) Meyer +Reviewed-by: Eugene Zemtsov +Cr-Original-Commit-Position: refs/heads/main@{#1011247} +Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3694435 +Auto-Submit: Ted (Chromium) Meyer +Reviewed-by: Eugene Zemtsov +Commit-Queue: Eugene Zemtsov +Cr-Commit-Position: refs/branch-heads/5060@{#672} +Cr-Branched-From: b83393d0f4038aeaf67f970a024d8101df7348d1-refs/heads/main@{#1002911} +--- + +diff --git a/content/renderer/media/batching_media_log.cc b/content/renderer/media/batching_media_log.cc +index 42b6fb6..efd56d9 100644 +--- a/content/renderer/media/batching_media_log.cc ++++ b/content/renderer/media/batching_media_log.cc +@@ -76,6 +76,11 @@ + SendQueuedMediaEvents(); + } + ++void BatchingMediaLog::Stop() { ++ base::AutoLock lock(lock_); ++ event_handlers_.clear(); ++} ++ + void BatchingMediaLog::OnWebMediaPlayerDestroyedLocked() { + base::AutoLock lock(lock_); + for (const auto& handler : event_handlers_) +diff --git a/content/renderer/media/batching_media_log.h b/content/renderer/media/batching_media_log.h +index eb757a4..6c2df68 100644 +--- a/content/renderer/media/batching_media_log.h ++++ b/content/renderer/media/batching_media_log.h +@@ -45,6 +45,8 @@ + + ~BatchingMediaLog() override; + ++ void Stop() override; ++ + // Will reset |last_ipc_send_time_| with the value of NowTicks(). + void SetTickClockForTesting(const base::TickClock* tick_clock); + +diff --git a/media/base/media_log.cc b/media/base/media_log.cc +index 3728433..46887da3 100644 +--- a/media/base/media_log.cc ++++ b/media/base/media_log.cc +@@ -49,6 +49,9 @@ + return ""; + } + ++// Default implementation. ++void MediaLog::Stop() {} ++ + void MediaLog::AddMessage(MediaLogMessageLevel level, std::string message) { + std::unique_ptr record( + CreateRecord(MediaLogRecord::Type::kMessage)); +diff --git a/media/base/media_log.h b/media/base/media_log.h +index e70a95b..cce6a266 100644 +--- a/media/base/media_log.h ++++ b/media/base/media_log.h +@@ -131,6 +131,10 @@ + // even if this occurs, in the "won't crash" sense. + virtual std::unique_ptr Clone(); + ++ // Can be used for stopping a MediaLog during a garbage-collected destruction ++ // sequence. ++ virtual void Stop(); ++ + protected: + // Ensures only subclasses and factories (e.g. Clone()) can create MediaLog. + MediaLog(); +diff --git a/third_party/blink/renderer/modules/webcodecs/codec_logger.h b/third_party/blink/renderer/modules/webcodecs/codec_logger.h +index 034c448..4dc058c 100644 +--- a/third_party/blink/renderer/modules/webcodecs/codec_logger.h ++++ b/third_party/blink/renderer/modules/webcodecs/codec_logger.h +@@ -81,7 +81,10 @@ + // media logs must be posted for destruction, since they can cause the + // garbage collector to trigger an immediate cleanup and delete the owning + // instance of |CodecLogger|. +- task_runner_->DeleteSoon(FROM_HERE, std::move(parent_media_log_)); ++ if (parent_media_log_) { ++ parent_media_log_->Stop(); ++ task_runner_->DeleteSoon(FROM_HERE, std::move(parent_media_log_)); ++ } + } + + void SendPlayerNameInformation(const ExecutionContext& context, From f1cd5183c4b39848b367bd7739efbcccde38136e Mon Sep 17 00:00:00 2001 From: Jeremy Rose Date: Tue, 28 Jun 2022 15:01:18 -0700 Subject: [PATCH 2/3] fix patch --- .../chromium/cherry-pick-ecad352cd614.patch | 46 +++++++++++-------- 1 file changed, 27 insertions(+), 19 deletions(-) diff --git a/patches/chromium/cherry-pick-ecad352cd614.patch b/patches/chromium/cherry-pick-ecad352cd614.patch index df8be74b8c150..6a5c084ca7a6f 100644 --- a/patches/chromium/cherry-pick-ecad352cd614.patch +++ b/patches/chromium/cherry-pick-ecad352cd614.patch @@ -1,7 +1,10 @@ -From ecad352cd61420d6bf8c9c39041b5369372ecf94 Mon Sep 17 00:00:00 2001 +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 From: Ted Meyer -Date: Wed, 08 Jun 2022 04:33:20 +0000 -Subject: [PATCH] Add Stop method to BatchingMediaLog +Date: Wed, 8 Jun 2022 04:33:20 +0000 +Subject: Add Stop method to BatchingMediaLog +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit Now that ~MediaLog is posted for a later destruction due to garbage collector ownership of CodecLogger, it's possible for the @@ -28,13 +31,12 @@ Reviewed-by: Eugene Zemtsov Commit-Queue: Eugene Zemtsov Cr-Commit-Position: refs/branch-heads/5060@{#672} Cr-Branched-From: b83393d0f4038aeaf67f970a024d8101df7348d1-refs/heads/main@{#1002911} ---- diff --git a/content/renderer/media/batching_media_log.cc b/content/renderer/media/batching_media_log.cc -index 42b6fb6..efd56d9 100644 +index c7263471a3b352fe329c26dc258ccf7af2049820..fbd8f5c7720d3636870d63bfcefbdb55d862fef7 100644 --- a/content/renderer/media/batching_media_log.cc +++ b/content/renderer/media/batching_media_log.cc -@@ -76,6 +76,11 @@ +@@ -75,6 +75,11 @@ BatchingMediaLog::~BatchingMediaLog() { SendQueuedMediaEvents(); } @@ -47,10 +49,10 @@ index 42b6fb6..efd56d9 100644 base::AutoLock lock(lock_); for (const auto& handler : event_handlers_) diff --git a/content/renderer/media/batching_media_log.h b/content/renderer/media/batching_media_log.h -index eb757a4..6c2df68 100644 +index eb757a4ca922bd5456d0e436d10cc540ee9134ae..6c2df688caee4d2fc17e973cdad12f20342e56fe 100644 --- a/content/renderer/media/batching_media_log.h +++ b/content/renderer/media/batching_media_log.h -@@ -45,6 +45,8 @@ +@@ -45,6 +45,8 @@ class CONTENT_EXPORT BatchingMediaLog : public media::MediaLog { ~BatchingMediaLog() override; @@ -60,10 +62,10 @@ index eb757a4..6c2df68 100644 void SetTickClockForTesting(const base::TickClock* tick_clock); diff --git a/media/base/media_log.cc b/media/base/media_log.cc -index 3728433..46887da3 100644 +index 3728433c5eb473cc9c82b6cf33b22bbf89471eea..46887da3dc21c504b26d6ff174eb14d68298c6b1 100644 --- a/media/base/media_log.cc +++ b/media/base/media_log.cc -@@ -49,6 +49,9 @@ +@@ -49,6 +49,9 @@ std::string MediaLog::GetErrorMessageLocked() { return ""; } @@ -74,10 +76,10 @@ index 3728433..46887da3 100644 std::unique_ptr record( CreateRecord(MediaLogRecord::Type::kMessage)); diff --git a/media/base/media_log.h b/media/base/media_log.h -index e70a95b..cce6a266 100644 +index e70a95b72f75f9bbfb114a94396c048bce2a3647..cce6a266327b43837d7a6ccaca7f2969c06fef4b 100644 --- a/media/base/media_log.h +++ b/media/base/media_log.h -@@ -131,6 +131,10 @@ +@@ -131,6 +131,10 @@ class MEDIA_EXPORT MediaLog { // even if this occurs, in the "won't crash" sense. virtual std::unique_ptr Clone(); @@ -89,18 +91,24 @@ index e70a95b..cce6a266 100644 // Ensures only subclasses and factories (e.g. Clone()) can create MediaLog. MediaLog(); diff --git a/third_party/blink/renderer/modules/webcodecs/codec_logger.h b/third_party/blink/renderer/modules/webcodecs/codec_logger.h -index 034c448..4dc058c 100644 +index 2d389c1835365e423d6289fb883cb6cccd4d8e84..50abcbbdcfe434c53476b14d8eb929a6f38c892d 100644 --- a/third_party/blink/renderer/modules/webcodecs/codec_logger.h +++ b/third_party/blink/renderer/modules/webcodecs/codec_logger.h -@@ -81,7 +81,10 @@ - // media logs must be posted for destruction, since they can cause the - // garbage collector to trigger an immediate cleanup and delete the owning - // instance of |CodecLogger|. -- task_runner_->DeleteSoon(FROM_HERE, std::move(parent_media_log_)); +@@ -71,7 +71,16 @@ class MODULES_EXPORT CodecLogger final { + media_log_ = parent_media_log_->Clone(); + } + +- ~CodecLogger() { DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); } ++ ~CodecLogger() { ++ DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); ++ // media logs must be posted for destruction, since they can cause the ++ // garbage collector to trigger an immediate cleanup and delete the owning ++ // instance of |CodecLogger|. + if (parent_media_log_) { + parent_media_log_->Stop(); + task_runner_->DeleteSoon(FROM_HERE, std::move(parent_media_log_)); + } - } ++ } void SendPlayerNameInformation(const ExecutionContext& context, + std::string loadedAs) { From 9ffffc02c494b34546a502baf6f02f48dce76761 Mon Sep 17 00:00:00 2001 From: Jeremy Rose Date: Thu, 21 Jul 2022 15:37:39 -0700 Subject: [PATCH 3/3] fix patch more? --- .../chromium/cherry-pick-ecad352cd614.patch | 29 +++++++++++++++++-- 1 file changed, 27 insertions(+), 2 deletions(-) diff --git a/patches/chromium/cherry-pick-ecad352cd614.patch b/patches/chromium/cherry-pick-ecad352cd614.patch index 6a5c084ca7a6f..d698f91ea0556 100644 --- a/patches/chromium/cherry-pick-ecad352cd614.patch +++ b/patches/chromium/cherry-pick-ecad352cd614.patch @@ -91,11 +91,26 @@ index e70a95b72f75f9bbfb114a94396c048bce2a3647..cce6a266327b43837d7a6ccaca7f2969 // Ensures only subclasses and factories (e.g. Clone()) can create MediaLog. MediaLog(); diff --git a/third_party/blink/renderer/modules/webcodecs/codec_logger.h b/third_party/blink/renderer/modules/webcodecs/codec_logger.h -index 2d389c1835365e423d6289fb883cb6cccd4d8e84..50abcbbdcfe434c53476b14d8eb929a6f38c892d 100644 +index 2d389c1835365e423d6289fb883cb6cccd4d8e84..fae0d8a4fb41b61316c5a61a1ba3d777562d206d 100644 --- a/third_party/blink/renderer/modules/webcodecs/codec_logger.h +++ b/third_party/blink/renderer/modules/webcodecs/codec_logger.h -@@ -71,7 +71,16 @@ class MODULES_EXPORT CodecLogger final { +@@ -7,6 +7,10 @@ + + #include + ++#include "base/check.h" ++#include "base/location.h" ++#include "base/memory/scoped_refptr.h" ++#include "base/sequence_checker.h" + #include "media/base/media_log.h" + #include "media/base/media_util.h" + #include "media/base/status.h" +@@ -69,9 +73,20 @@ class MODULES_EXPORT CodecLogger final { + // This allows us to destroy |parent_media_log_| and stop logging, + // without causing problems to |media_log_| users. media_log_ = parent_media_log_->Clone(); ++ ++ task_runner_ = task_runner; } - ~CodecLogger() { DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); } @@ -112,3 +127,13 @@ index 2d389c1835365e423d6289fb883cb6cccd4d8e84..50abcbbdcfe434c53476b14d8eb929a6 void SendPlayerNameInformation(const ExecutionContext& context, std::string loadedAs) { +@@ -132,6 +147,9 @@ class MODULES_EXPORT CodecLogger final { + // can be safely accessed, and whose raw pointer can be given callbacks. + std::unique_ptr media_log_; + ++ // Keep task runner around for posting the media log to upon destruction. ++ scoped_refptr task_runner_; ++ + SEQUENCE_CHECKER(sequence_checker_); + }; +