Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: cherry-pick 4c57222340cf from chromium #23009

Merged
merged 6 commits into from Apr 9, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions patches/chromium/.patches
Expand Up @@ -100,3 +100,4 @@ move_readablestream_requests_onto_the_stack_before_iteration.patch
streams_convert_state_dchecks_to_checks.patch
audiocontext_haspendingactivity_unless_it_s_closed.patch
protect_automatic_pull_handlers_with_mutex.patch
cherry-pick-4c57222340cf.patch
77 changes: 77 additions & 0 deletions patches/chromium/cherry-pick-4c57222340cf.patch
@@ -0,0 +1,77 @@
From 4c57222340cfb78edadf08532f4468c676df5395 Mon Sep 17 00:00:00 2001
From: Raymond Toy <rtoy@chromium.org>
Date: Wed, 11 Mar 2020 20:23:49 +0000
Subject: [PATCH] Make finished_source_handlers_ hold scoped_refptrs

Previously, finished_source_handlers_ held raw pointers to
AudioHandlers and assumed that active_source_handlers_ also had a
copy. But when the context goes away, active_source_handlers_ would
be cleared, but not finished_source_handlers_, leaving pointers to
deleted objects.

So do two things:
1. Change finished_source_handlers_ to hold scoped_refptrs to manage
lifetime of the objects
2. Clear finished_source_handler_ in ClearHandlersToBeDeleted()

Either of these fix the repro case, but let's do both. Don't want to
leaving dangling objects.

Manually tested the repro case which no longer reproduces.

Bug: 1059686
Change-Id: I2f30c996e8589fa5c3890d32500c4bb4f3bc4286
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2098260
Reviewed-by: Hongchan Choi <hongchan@chromium.org>
Commit-Queue: Raymond Toy <rtoy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#749302}
---
.../blink/renderer/modules/webaudio/deferred_task_handler.cc | 5 ++---
.../blink/renderer/modules/webaudio/deferred_task_handler.h | 4 ++--
2 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/third_party/blink/renderer/modules/webaudio/deferred_task_handler.cc b/third_party/blink/renderer/modules/webaudio/deferred_task_handler.cc
index d0cbb762a8de9..d02a7f69fe191 100644
--- a/third_party/blink/renderer/modules/webaudio/deferred_task_handler.cc
+++ b/third_party/blink/renderer/modules/webaudio/deferred_task_handler.cc
@@ -77,9 +77,7 @@ void DeferredTaskHandler::BreakConnections() {
// connection.
wtf_size_t size = finished_source_handlers_.size();
if (size > 0) {
- for (auto* finished : finished_source_handlers_) {
- // Break connection first and then remove from the list because that can
- // cause the handler to be deleted.
+ for (auto finished : finished_source_handlers_) {
finished->BreakConnectionWithLock();
active_source_handlers_.erase(finished);
}
@@ -358,6 +356,7 @@ void DeferredTaskHandler::ClearHandlersToBeDeleted() {
deletable_orphan_handlers_.clear();
automatic_pull_handlers_.clear();
rendering_automatic_pull_handlers_.clear();
+ finished_source_handlers_.clear();
active_source_handlers_.clear();
}

diff --git a/third_party/blink/renderer/modules/webaudio/deferred_task_handler.h b/third_party/blink/renderer/modules/webaudio/deferred_task_handler.h
index 9d2ae46db53ff..3cf47ca77a8b1 100644
--- a/third_party/blink/renderer/modules/webaudio/deferred_task_handler.h
+++ b/third_party/blink/renderer/modules/webaudio/deferred_task_handler.h
@@ -190,7 +190,7 @@ class MODULES_EXPORT DeferredTaskHandler final
return &active_source_handlers_;
}

- Vector<AudioHandler*>* GetFinishedSourceHandlers() {
+ Vector<scoped_refptr<AudioHandler>>* GetFinishedSourceHandlers() {
return &finished_source_handlers_;
}

@@ -259,7 +259,7 @@ class MODULES_EXPORT DeferredTaskHandler final
// connection and elements here are removed from |active_source_handlers_|.
//
// This must be accessed only from the audio thread.
- Vector<AudioHandler*> finished_source_handlers_;
+ Vector<scoped_refptr<AudioHandler>> finished_source_handlers_;

scoped_refptr<base::SingleThreadTaskRunner> task_runner_;