Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
chore: cherry-pick 85f708fa7ab8 from chromium (#23048)
- Loading branch information
Showing
3 changed files
with
207 additions
and
0 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
125 changes: 125 additions & 0 deletions
125
patches/chromium/use_keepselfalive_on_audiocontext_to_keep_it_alive_until_rendering.patch
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,125 @@ | ||
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 | ||
From: Hongchan Choi <hongchan@chromium.org> | ||
Date: Tue, 18 Feb 2020 22:39:05 +0000 | ||
Subject: Use KeepSelfAlive on AudioContext to keep it alive until rendering | ||
stops | ||
|
||
When an ExecutionContext is abruptly/unexpectedly destroyed (e.g. | ||
shutting down of document or iframe), an AudioContext can also | ||
go away. This type of shutdown can be problematic because the render | ||
thread still might be touching resources in the AudioContext allocated | ||
by the main thread. | ||
|
||
This CL introduces a self-referencing pointer to the AudioContext, | ||
and it is cleared after the underlying render thread is stopped. In | ||
that way, the destruction of AudioContext can be done safely. | ||
|
||
Test: Locally confirmed the repro case doesn't crash (UAP) after 1hr. | ||
Bug: 1043446 | ||
Change-Id: I2e40b7d58ca9d647eed8a5971fc69dc87ee3d1fe | ||
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2049912 | ||
Reviewed-by: Raymond Toy <rtoy@chromium.org> | ||
Reviewed-by: Michael Lippautz <mlippautz@chromium.org> | ||
Commit-Queue: Hongchan Choi <hongchan@chromium.org> | ||
Cr-Commit-Position: refs/heads/master@{#742338} | ||
|
||
diff --git a/third_party/blink/renderer/modules/webaudio/audio_context.cc b/third_party/blink/renderer/modules/webaudio/audio_context.cc | ||
index 83a1ad5d891ebb5566b41068cc0dc18de3d40059..99dbfb89005270b1a13e988bbdea9b5bfedfe89a 100644 | ||
--- a/third_party/blink/renderer/modules/webaudio/audio_context.cc | ||
+++ b/third_party/blink/renderer/modules/webaudio/audio_context.cc | ||
@@ -132,7 +132,8 @@ AudioContext::AudioContext(Document& document, | ||
const WebAudioLatencyHint& latency_hint, | ||
base::Optional<float> sample_rate) | ||
: BaseAudioContext(&document, kRealtimeContext), | ||
- context_id_(g_context_id++) { | ||
+ context_id_(g_context_id++), | ||
+ keep_alive_(PERSISTENT_FROM_HERE, this) { | ||
destination_node_ = | ||
RealtimeAudioDestinationNode::Create(this, latency_hint, sample_rate); | ||
|
||
@@ -161,7 +162,7 @@ void AudioContext::Uninitialize() { | ||
DCHECK(IsMainThread()); | ||
DCHECK_NE(g_hardware_context_count, 0u); | ||
--g_hardware_context_count; | ||
- | ||
+ StopRendering(); | ||
DidClose(); | ||
RecordAutoplayMetrics(); | ||
BaseAudioContext::Uninitialize(); | ||
@@ -344,14 +345,26 @@ bool AudioContext::IsContextClosed() const { | ||
return close_resolver_ || BaseAudioContext::IsContextClosed(); | ||
} | ||
|
||
+void AudioContext::StartRendering() { | ||
+ DCHECK(IsMainThread()); | ||
+ | ||
+ if (!keep_alive_) | ||
+ keep_alive_ = this; | ||
+ BaseAudioContext::StartRendering(); | ||
+} | ||
+ | ||
void AudioContext::StopRendering() { | ||
DCHECK(IsMainThread()); | ||
DCHECK(destination()); | ||
|
||
- if (ContextState() == kRunning) { | ||
+ // It is okay to perform the following on a suspended AudioContext because | ||
+ // this method gets called from ExecutionContext::ContextDestroyed() meaning | ||
+ // the AudioContext is already unreachable from the user code. | ||
+ if (ContextState() != kClosed) { | ||
destination()->GetAudioDestinationHandler().StopRendering(); | ||
SetContextState(kClosed); | ||
GetDeferredTaskHandler().ClearHandlersToBeDeleted(); | ||
+ keep_alive_.Clear(); | ||
} | ||
} | ||
|
||
diff --git a/third_party/blink/renderer/modules/webaudio/audio_context.h b/third_party/blink/renderer/modules/webaudio/audio_context.h | ||
index 146e9e56086642edec99030a8a008accf0d408d9..31730c152a3b3f0e743cf7231a92a51a4d7eed69 100644 | ||
--- a/third_party/blink/renderer/modules/webaudio/audio_context.h | ||
+++ b/third_party/blink/renderer/modules/webaudio/audio_context.h | ||
@@ -13,6 +13,7 @@ | ||
#include "third_party/blink/renderer/modules/webaudio/audio_context_options.h" | ||
#include "third_party/blink/renderer/modules/webaudio/base_audio_context.h" | ||
#include "third_party/blink/renderer/platform/heap/handle.h" | ||
+#include "third_party/blink/renderer/platform/heap/self_keep_alive.h" | ||
|
||
namespace blink { | ||
|
||
@@ -133,8 +134,13 @@ class MODULES_EXPORT AudioContext : public BaseAudioContext { | ||
// Record the current autoplay metrics. | ||
void RecordAutoplayMetrics(); | ||
|
||
+ // Starts rendering via AudioDestinationNode. This sets the self-referencing | ||
+ // pointer to this object. | ||
+ void StartRendering() override; | ||
+ | ||
// Called when the context is being closed to stop rendering audio and clean | ||
- // up handlers. | ||
+ // up handlers. This clears the self-referencing pointer, making this object | ||
+ // available for the potential GC. | ||
void StopRendering(); | ||
|
||
// Called when suspending the context to stop reundering audio, but don't | ||
@@ -193,6 +199,8 @@ class MODULES_EXPORT AudioContext : public BaseAudioContext { | ||
// determine audibility on render quantum boundaries, so counting quanta is | ||
// all that's needed. | ||
size_t total_audible_renders_ = 0; | ||
+ | ||
+ SelfKeepAlive<AudioContext> keep_alive_; | ||
}; | ||
|
||
} // namespace blink | ||
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..bb0b2335d848c6b5f1b1cb9ce24ad5adb8b331c5 100644 | ||
--- a/third_party/blink/renderer/modules/webaudio/base_audio_context.h | ||
+++ b/third_party/blink/renderer/modules/webaudio/base_audio_context.h | ||
@@ -282,7 +282,7 @@ class MODULES_EXPORT BaseAudioContext | ||
|
||
DEFINE_ATTRIBUTE_EVENT_LISTENER(statechange, kStatechange) | ||
|
||
- void StartRendering(); | ||
+ virtual void StartRendering(); | ||
|
||
void NotifyStateChange(); | ||
|
80 changes: 80 additions & 0 deletions
80
patches/chromium/when_suspending_context_don_t_clear_handlers.patch
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,80 @@ | ||
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 | ||
From: Raymond Toy <rtoy@chromium.org> | ||
Date: Wed, 11 Dec 2019 00:33:00 +0000 | ||
Subject: When suspending context, don't clear handlers | ||
|
||
AudioContext.suspend() would call StopRendering(). This stops the audio | ||
thread from pulling the graph (eventually) but it also clears out any | ||
handlers, including those associated with automatic pull nodes for any | ||
AnalyserNode that isn't connected to the destination. When the context | ||
is resumed, the AnalyserNode isn't pulled anymore, so the output never | ||
changes. | ||
|
||
Add a SuspendRendering() method to handle AudioContext.suspend() which | ||
doesn't clear the handlers. Then when the context is resumed, | ||
AnalyserNodes will get pulled again. Then StopRendering() is used only | ||
for AudioContext.close() where it is ok to clear out the handlers since | ||
we can't resume a closed context. | ||
|
||
Bug: 1018499 | ||
Change-Id: I4b4ccf688b37e6b81d310d2596cfff9603048876 | ||
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1903894 | ||
Reviewed-by: Hongchan Choi <hongchan@chromium.org> | ||
Commit-Queue: Raymond Toy <rtoy@chromium.org> | ||
Cr-Commit-Position: refs/heads/master@{#723609} | ||
|
||
diff --git a/third_party/blink/renderer/modules/webaudio/audio_context.cc b/third_party/blink/renderer/modules/webaudio/audio_context.cc | ||
index 990fe75f1ddd4ebf9a526a9e416d2ee865afca86..83a1ad5d891ebb5566b41068cc0dc18de3d40059 100644 | ||
--- a/third_party/blink/renderer/modules/webaudio/audio_context.cc | ||
+++ b/third_party/blink/renderer/modules/webaudio/audio_context.cc | ||
@@ -200,7 +200,7 @@ ScriptPromise AudioContext::suspendContext(ScriptState* script_state) { | ||
|
||
// Stop rendering now. | ||
if (destination()) | ||
- StopRendering(); | ||
+ SuspendRendering(); | ||
|
||
// Since we don't have any way of knowing when the hardware actually stops, | ||
// we'll just resolve the promise now. | ||
@@ -350,11 +350,21 @@ void AudioContext::StopRendering() { | ||
|
||
if (ContextState() == kRunning) { | ||
destination()->GetAudioDestinationHandler().StopRendering(); | ||
- SetContextState(kSuspended); | ||
+ SetContextState(kClosed); | ||
GetDeferredTaskHandler().ClearHandlersToBeDeleted(); | ||
} | ||
} | ||
|
||
+void AudioContext::SuspendRendering() { | ||
+ DCHECK(IsMainThread()); | ||
+ DCHECK(destination()); | ||
+ | ||
+ if (ContextState() == kRunning) { | ||
+ destination()->GetAudioDestinationHandler().StopRendering(); | ||
+ SetContextState(kSuspended); | ||
+ } | ||
+} | ||
+ | ||
double AudioContext::baseLatency() const { | ||
DCHECK(IsMainThread()); | ||
DCHECK(destination()); | ||
diff --git a/third_party/blink/renderer/modules/webaudio/audio_context.h b/third_party/blink/renderer/modules/webaudio/audio_context.h | ||
index e0d4cf1aef21f06146b309ecd77b7e580aa8ae11..146e9e56086642edec99030a8a008accf0d408d9 100644 | ||
--- a/third_party/blink/renderer/modules/webaudio/audio_context.h | ||
+++ b/third_party/blink/renderer/modules/webaudio/audio_context.h | ||
@@ -133,8 +133,14 @@ class MODULES_EXPORT AudioContext : public BaseAudioContext { | ||
// Record the current autoplay metrics. | ||
void RecordAutoplayMetrics(); | ||
|
||
+ // Called when the context is being closed to stop rendering audio and clean | ||
+ // up handlers. | ||
void StopRendering(); | ||
|
||
+ // Called when suspending the context to stop reundering audio, but don't | ||
+ // clean up handlers because we expect to be resuming where we left off. | ||
+ void SuspendRendering(); | ||
+ | ||
void DidClose(); | ||
|
||
// Called by the audio thread to handle Promises for resume() and suspend(), |