Skip to content

Commit

Permalink
chore: cherry-pick db71a0afc1d0 from chromium (#22943)
Browse files Browse the repository at this point in the history
  • Loading branch information
nornagon committed Apr 6, 2020
1 parent e7fce15 commit 7586295
Show file tree
Hide file tree
Showing 2 changed files with 115 additions and 0 deletions.
1 change: 1 addition & 0 deletions patches/chromium/.patches
Expand Up @@ -99,3 +99,4 @@ allow_restricted_clock_nanosleep_in_linux_sandbox.patch
move_readablestream_requests_onto_the_stack_before_iteration.patch
streams_convert_state_dchecks_to_checks.patch
cherry-pick-fd211b44535c.patch
cherry-pick-db71a0afc1d0.patch
114 changes: 114 additions & 0 deletions patches/chromium/cherry-pick-db71a0afc1d0.patch
@@ -0,0 +1,114 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Raymond Toy <rtoy@chromium.org>
Date: Thu, 19 Mar 2020 21:54:36 +0000
Subject: Clear context from orphan handlers when BaseAudioContext is going
away

When preparing to collect a BaseAudioContext, go through all the
rendering_orphan_handlers_ and deletable_orphan_handlers_ and remove
the context from the handler. This ensures that these handlers no
longer have references to the context when the BaseAudioContext is
destroyed because in some cases, these orphan handlers will get pulled
and access the context, which is already gone.

Clearing these in a prefinalizer ensures these orphan handlers don't
try to touch the context.

Manually verified that the repro case no longer reproduces.

Bug: 1062247
Change-Id: I50d083743903eb9544e09aa1ee912fc880331501
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2107806
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Hongchan Choi <hongchan@chromium.org>
Commit-Queue: Raymond Toy <rtoy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#751814}

diff --git a/third_party/blink/renderer/modules/webaudio/base_audio_context.cc b/third_party/blink/renderer/modules/webaudio/base_audio_context.cc
index 34adef56ced1effd3af5a50e091f58440a53e6a7..200776395f7d3c978d8ae76959ce472002f7a284 100644
--- a/third_party/blink/renderer/modules/webaudio/base_audio_context.cc
+++ b/third_party/blink/renderer/modules/webaudio/base_audio_context.cc
@@ -179,6 +179,12 @@ void BaseAudioContext::Uninitialize() {
DCHECK_EQ(resume_resolvers_.size(), 0u);
}

+void BaseAudioContext::Dispose() {
+ // BaseAudioContext is going away, so remove the context from the orphan
+ // handlers.
+ GetDeferredTaskHandler().ClearContextFromOrphanHandlers();
+}
+
void BaseAudioContext::ContextLifecycleStateChanged(
mojom::FrameLifecycleState state) {
// Don't need to do anything for an offline context.
diff --git a/third_party/blink/renderer/modules/webaudio/base_audio_context.h b/third_party/blink/renderer/modules/webaudio/base_audio_context.h
index d5aadf7e58ea8a67df7d0a83e5ed5b9b1d7772bb..0cd91cf7e7fad97f1af38c5eb63c8e6afa7d276a 100644
--- a/third_party/blink/renderer/modules/webaudio/base_audio_context.h
+++ b/third_party/blink/renderer/modules/webaudio/base_audio_context.h
@@ -96,6 +96,7 @@ class MODULES_EXPORT BaseAudioContext
public InspectorHelperMixin {
USING_GARBAGE_COLLECTED_MIXIN(BaseAudioContext);
DEFINE_WRAPPERTYPEINFO();
+ USING_PRE_FINALIZER(BaseAudioContext, Dispose);

public:
// The state of an audio context. On creation, the state is Suspended. The
@@ -115,6 +116,8 @@ class MODULES_EXPORT BaseAudioContext
return dest ? dest->GetAudioDestinationHandler().IsInitialized() : false;
}

+ void Dispose();
+
// Document notification
void ContextLifecycleStateChanged(mojom::FrameLifecycleState) override;
void ContextDestroyed(ExecutionContext*) override;
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 9fd38f0dde71b5c773a861992f0e989ba6a9d5e0..28d67536bb706552453effa0d9126ffa789c0504 100644
--- a/third_party/blink/renderer/modules/webaudio/deferred_task_handler.cc
+++ b/third_party/blink/renderer/modules/webaudio/deferred_task_handler.cc
@@ -293,10 +293,7 @@ void DeferredTaskHandler::HandleDeferredTasks() {
}

void DeferredTaskHandler::ContextWillBeDestroyed() {
- for (auto& handler : rendering_orphan_handlers_)
- handler->ClearContext();
- for (auto& handler : deletable_orphan_handlers_)
- handler->ClearContext();
+ ClearContextFromOrphanHandlers();
ClearHandlersToBeDeleted();
// Some handlers might live because of their cross thread tasks.
}
@@ -359,6 +356,19 @@ void DeferredTaskHandler::ClearHandlersToBeDeleted() {
active_source_handlers_.clear();
}

+void DeferredTaskHandler::ClearContextFromOrphanHandlers() {
+ DCHECK(IsMainThread());
+
+ // |rendering_orphan_handlers_| and |deletable_orphan_handlers_| can
+ // be modified on the audio thread.
+ GraphAutoLocker locker(*this);
+
+ for (auto& handler : rendering_orphan_handlers_)
+ handler->ClearContext();
+ for (auto& handler : deletable_orphan_handlers_)
+ handler->ClearContext();
+}
+
void DeferredTaskHandler::SetAudioThreadToCurrentThread() {
DCHECK(!IsMainThread());
audio_thread_.store(CurrentThread(), std::memory_order_relaxed);
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 0ede5f5b5dabeeef9decc94c94d91ddc7351c722..2900b0f7cf3c47c8e92cc3ad4dda665eabdc479f 100644
--- a/third_party/blink/renderer/modules/webaudio/deferred_task_handler.h
+++ b/third_party/blink/renderer/modules/webaudio/deferred_task_handler.h
@@ -109,6 +109,9 @@ class MODULES_EXPORT DeferredTaskHandler final
void RequestToDeleteHandlersOnMainThread();
void ClearHandlersToBeDeleted();

+ // Clear the context from the rendering and deletable orphan handlers.
+ void ClearContextFromOrphanHandlers();
+
bool AcceptsTailProcessing() const { return accepts_tail_processing_; }
void StopAcceptingTailProcessing() { accepts_tail_processing_ = false; }

0 comments on commit 7586295

Please sign in to comment.