Commit 05198712 authored by Michael Lippautz's avatar Michael Lippautz Committed by Commit Bot

CrossThreadWeakPersistent: Further harden API

- Allow downcast construction;
- Disallow conversion to T* directly;
- Disallow Get();

This CL concludes hardening as all uses left go through an
intermediate CTP handle which retains the object as long
as the thread does not terminate.

Bug: 1125495
Change-Id: I4ab837127745378a57513d11ffbdea3fd961b20b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2396140
Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
Reviewed-by: default avatarKentaro Hara <haraken@chromium.org>
Reviewed-by: default avatarAnton Bikineev <bikineev@chromium.org>
Cr-Commit-Position: refs/heads/master@{#805306}
parent 27cb2b96
...@@ -60,8 +60,8 @@ void AudioWorkletObjectProxy::WillDestroyWorkerGlobalScope() { ...@@ -60,8 +60,8 @@ void AudioWorkletObjectProxy::WillDestroyWorkerGlobalScope() {
CrossThreadWeakPersistent<AudioWorkletMessagingProxy> CrossThreadWeakPersistent<AudioWorkletMessagingProxy>
AudioWorkletObjectProxy::GetAudioWorkletMessagingProxyWeakPtr() { AudioWorkletObjectProxy::GetAudioWorkletMessagingProxyWeakPtr() {
return WrapCrossThreadWeakPersistent( return CrossThreadWeakPersistent<AudioWorkletMessagingProxy>(
static_cast<AudioWorkletMessagingProxy*>(MessagingProxyWeakPtr().Get())); MessagingProxyWeakPtr());
} }
} // namespace blink } // namespace blink
...@@ -90,8 +90,9 @@ MediaElementAudioSourceHandler::~MediaElementAudioSourceHandler() { ...@@ -90,8 +90,9 @@ MediaElementAudioSourceHandler::~MediaElementAudioSourceHandler() {
Uninitialize(); Uninitialize();
} }
HTMLMediaElement* MediaElementAudioSourceHandler::MediaElement() const { CrossThreadPersistent<HTMLMediaElement>
return media_element_.Get(); MediaElementAudioSourceHandler::MediaElement() const {
return media_element_.Lock();
} }
void MediaElementAudioSourceHandler::Dispose() { void MediaElementAudioSourceHandler::Dispose() {
......
...@@ -49,7 +49,7 @@ class MediaElementAudioSourceHandler final : public AudioHandler { ...@@ -49,7 +49,7 @@ class MediaElementAudioSourceHandler final : public AudioHandler {
HTMLMediaElement&); HTMLMediaElement&);
~MediaElementAudioSourceHandler() override; ~MediaElementAudioSourceHandler() override;
HTMLMediaElement* MediaElement() const; CrossThreadPersistent<HTMLMediaElement> MediaElement() const;
// AudioHandler // AudioHandler
void Dispose() override; void Dispose() override;
......
...@@ -781,7 +781,7 @@ void OscillatorHandler::SetPeriodicWave(PeriodicWave* periodic_wave) { ...@@ -781,7 +781,7 @@ void OscillatorHandler::SetPeriodicWave(PeriodicWave* periodic_wave) {
} }
bool OscillatorHandler::PropagatesSilence() const { bool OscillatorHandler::PropagatesSilence() const {
return !IsPlayingOrScheduled() || HasFinished() || !periodic_wave_.Get(); return !IsPlayingOrScheduled() || HasFinished() || !periodic_wave_;
} }
void OscillatorHandler::HandleStoppableSourceNode() { void OscillatorHandler::HandleStoppableSourceNode() {
......
...@@ -181,7 +181,8 @@ void PannerHandler::Process(uint32_t frames_to_process) { ...@@ -181,7 +181,8 @@ void PannerHandler::Process(uint32_t frames_to_process) {
} }
// The audio thread can't block on this lock, so we call tryLock() instead. // The audio thread can't block on this lock, so we call tryLock() instead.
MutexTryLocker try_listener_locker(Listener()->ListenerLock()); auto listener = Listener();
MutexTryLocker try_listener_locker(listener->ListenerLock());
if (try_listener_locker.Locked()) { if (try_listener_locker.Locked()) {
if (!Context()->HasRealtimeConstraint() && if (!Context()->HasRealtimeConstraint() &&
...@@ -189,11 +190,11 @@ void PannerHandler::Process(uint32_t frames_to_process) { ...@@ -189,11 +190,11 @@ void PannerHandler::Process(uint32_t frames_to_process) {
// For an OfflineAudioContext, we need to make sure the HRTFDatabase // For an OfflineAudioContext, we need to make sure the HRTFDatabase
// is loaded before proceeding. For realtime contexts, we don't // is loaded before proceeding. For realtime contexts, we don't
// have to wait. The HRTF panner handles that case itself. // have to wait. The HRTF panner handles that case itself.
Listener()->WaitForHRTFDatabaseLoaderThreadCompletion(); listener->WaitForHRTFDatabaseLoaderThreadCompletion();
} }
if ((HasSampleAccurateValues() || Listener()->HasSampleAccurateValues()) && if ((HasSampleAccurateValues() || listener->HasSampleAccurateValues()) &&
(IsAudioRate() || Listener()->IsAudioRate())) { (IsAudioRate() || listener->IsAudioRate())) {
// It's tempting to skip sample-accurate processing if // It's tempting to skip sample-accurate processing if
// isAzimuthElevationDirty() and isDistanceConeGain() both return false. // isAzimuthElevationDirty() and isDistanceConeGain() both return false.
// But in general we can't because something may scheduled to start in the // But in general we can't because something may scheduled to start in the
...@@ -253,26 +254,27 @@ void PannerHandler::ProcessSampleAccurateValues(AudioBus* destination, ...@@ -253,26 +254,27 @@ void PannerHandler::ProcessSampleAccurateValues(AudioBus* destination,
frames_to_process); frames_to_process);
// Get the automation values from the listener. // Get the automation values from the listener.
auto listener = Listener();
const float* listener_x = const float* listener_x =
Listener()->GetPositionXValues(audio_utilities::kRenderQuantumFrames); listener->GetPositionXValues(audio_utilities::kRenderQuantumFrames);
const float* listener_y = const float* listener_y =
Listener()->GetPositionYValues(audio_utilities::kRenderQuantumFrames); listener->GetPositionYValues(audio_utilities::kRenderQuantumFrames);
const float* listener_z = const float* listener_z =
Listener()->GetPositionZValues(audio_utilities::kRenderQuantumFrames); listener->GetPositionZValues(audio_utilities::kRenderQuantumFrames);
const float* forward_x = const float* forward_x =
Listener()->GetForwardXValues(audio_utilities::kRenderQuantumFrames); listener->GetForwardXValues(audio_utilities::kRenderQuantumFrames);
const float* forward_y = const float* forward_y =
Listener()->GetForwardYValues(audio_utilities::kRenderQuantumFrames); listener->GetForwardYValues(audio_utilities::kRenderQuantumFrames);
const float* forward_z = const float* forward_z =
Listener()->GetForwardZValues(audio_utilities::kRenderQuantumFrames); listener->GetForwardZValues(audio_utilities::kRenderQuantumFrames);
const float* up_x = const float* up_x =
Listener()->GetUpXValues(audio_utilities::kRenderQuantumFrames); listener->GetUpXValues(audio_utilities::kRenderQuantumFrames);
const float* up_y = const float* up_y =
Listener()->GetUpYValues(audio_utilities::kRenderQuantumFrames); listener->GetUpYValues(audio_utilities::kRenderQuantumFrames);
const float* up_z = const float* up_z =
Listener()->GetUpZValues(audio_utilities::kRenderQuantumFrames); listener->GetUpZValues(audio_utilities::kRenderQuantumFrames);
// Compute the azimuth, elevation, and total gains for each position. // Compute the azimuth, elevation, and total gains for each position.
double azimuth[audio_utilities::kRenderQuantumFrames]; double azimuth[audio_utilities::kRenderQuantumFrames];
...@@ -327,9 +329,10 @@ void PannerHandler::Initialize() { ...@@ -327,9 +329,10 @@ void PannerHandler::Initialize() {
if (IsInitialized()) if (IsInitialized())
return; return;
auto listener = Listener();
panner_ = Panner::Create(panning_model_, Context()->sampleRate(), panner_ = Panner::Create(panning_model_, Context()->sampleRate(),
Listener()->HrtfDatabaseLoader()); listener->HrtfDatabaseLoader());
Listener()->AddPanner(*this); listener->AddPanner(*this);
// The panner is already marked as dirty, so |last_position_| and // The panner is already marked as dirty, so |last_position_| and
// |last_orientation_| will bet updated on first use. Don't need to // |last_orientation_| will bet updated on first use. Don't need to
...@@ -343,17 +346,18 @@ void PannerHandler::Uninitialize() { ...@@ -343,17 +346,18 @@ void PannerHandler::Uninitialize() {
return; return;
panner_.reset(); panner_.reset();
if (Listener()) { auto listener = Listener();
if (listener) {
// Listener may have gone in the same garbage collection cycle, which means // Listener may have gone in the same garbage collection cycle, which means
// that the panner does not need to be removed. // that the panner does not need to be removed.
Listener()->RemovePanner(*this); listener->RemovePanner(*this);
} }
AudioHandler::Uninitialize(); AudioHandler::Uninitialize();
} }
AudioListener* PannerHandler::Listener() { CrossThreadPersistent<AudioListener> PannerHandler::Listener() const {
return listener_; return listener_.Lock();
} }
String PannerHandler::PanningModel() const { String PannerHandler::PanningModel() const {
...@@ -625,13 +629,13 @@ void PannerHandler::AzimuthElevation(double* out_azimuth, ...@@ -625,13 +629,13 @@ void PannerHandler::AzimuthElevation(double* out_azimuth,
double* out_elevation) { double* out_elevation) {
DCHECK(Context()->IsAudioThread()); DCHECK(Context()->IsAudioThread());
auto listener = Listener();
// Calculate new azimuth and elevation if the panner or the listener changed // Calculate new azimuth and elevation if the panner or the listener changed
// position or orientation in any way. // position or orientation in any way.
if (IsAzimuthElevationDirty() || Listener()->IsListenerDirty()) { if (IsAzimuthElevationDirty() || listener->IsListenerDirty()) {
CalculateAzimuthElevation(&cached_azimuth_, &cached_elevation_, CalculateAzimuthElevation(&cached_azimuth_, &cached_elevation_,
GetPosition(), Listener()->GetPosition(), GetPosition(), listener->GetPosition(),
Listener()->Orientation(), listener->Orientation(), listener->UpVector());
Listener()->UpVector());
is_azimuth_elevation_dirty_ = false; is_azimuth_elevation_dirty_ = false;
} }
...@@ -642,11 +646,12 @@ void PannerHandler::AzimuthElevation(double* out_azimuth, ...@@ -642,11 +646,12 @@ void PannerHandler::AzimuthElevation(double* out_azimuth,
float PannerHandler::DistanceConeGain() { float PannerHandler::DistanceConeGain() {
DCHECK(Context()->IsAudioThread()); DCHECK(Context()->IsAudioThread());
auto listener = Listener();
// Calculate new distance and cone gain if the panner or the listener // Calculate new distance and cone gain if the panner or the listener
// changed position or orientation in any way. // changed position or orientation in any way.
if (IsDistanceConeGainDirty() || Listener()->IsListenerDirty()) { if (IsDistanceConeGainDirty() || listener->IsListenerDirty()) {
cached_distance_cone_gain_ = CalculateDistanceConeGain( cached_distance_cone_gain_ = CalculateDistanceConeGain(
GetPosition(), Orientation(), Listener()->GetPosition()); GetPosition(), Orientation(), listener->GetPosition());
is_distance_cone_gain_dirty_ = false; is_distance_cone_gain_dirty_ = false;
} }
......
...@@ -133,7 +133,8 @@ class PannerHandler final : public AudioHandler { ...@@ -133,7 +133,8 @@ class PannerHandler final : public AudioHandler {
AudioParamHandler& orientation_z); AudioParamHandler& orientation_z);
// BaseAudioContext's listener // BaseAudioContext's listener
AudioListener* Listener(); // AudioListener* Listener();
CrossThreadPersistent<AudioListener> Listener() const;
bool SetPanningModel(Panner::PanningModel); // Returns true on success. bool SetPanningModel(Panner::PanningModel); // Returns true on success.
bool SetDistanceModel(unsigned); // Returns true on success. bool SetDistanceModel(unsigned); // Returns true on success.
......
...@@ -267,7 +267,8 @@ class PersistentBase { ...@@ -267,7 +267,8 @@ class PersistentBase {
// node (if present) from |other|. // node (if present) from |other|.
persistent_node_.Uninitialize(); persistent_node_.Uninitialize();
} }
raw_ = other.raw_; // Explicit cast enabling downcasting.
raw_ = static_cast<T*>(other.raw_);
other.raw_ = nullptr; other.raw_ = nullptr;
// Efficiently move by just rewiring the node pointer. // Efficiently move by just rewiring the node pointer.
persistent_node_ = std::move(other.persistent_node_); persistent_node_ = std::move(other.persistent_node_);
...@@ -444,6 +445,11 @@ class PersistentBase { ...@@ -444,6 +445,11 @@ class PersistentBase {
#if DCHECK_IS_ON() #if DCHECK_IS_ON()
const ThreadState* creation_thread_state_; const ThreadState* creation_thread_state_;
#endif #endif
template <typename F,
WeaknessPersistentConfiguration,
CrossThreadnessPersistentConfiguration>
friend class PersistentBase;
}; };
// Persistent is a way to create a strong pointer from an off-heap object // Persistent is a way to create a strong pointer from an off-heap object
...@@ -765,9 +771,8 @@ class CrossThreadWeakPersistent ...@@ -765,9 +771,8 @@ class CrossThreadWeakPersistent
// the state of CTP manually before invoking any calls. // the state of CTP manually before invoking any calls.
T* operator->() const = delete; T* operator->() const = delete;
T& operator*() const = delete; T& operator*() const = delete;
// TODO(mlippautz): Also hide the following calls: operator T*() const = delete;
// - Parent::Get; T* Get() const = delete;
// - Parent::operator T*;
private: private:
template <typename U> template <typename U>
...@@ -779,7 +784,7 @@ template <typename U> ...@@ -779,7 +784,7 @@ template <typename U>
CrossThreadPersistent<T>& CrossThreadPersistent<T>::operator=( CrossThreadPersistent<T>& CrossThreadPersistent<T>::operator=(
const CrossThreadWeakPersistent<U>& other) { const CrossThreadWeakPersistent<U>& other) {
MutexLocker locker(ProcessHeap::CrossThreadPersistentMutex()); MutexLocker locker(ProcessHeap::CrossThreadPersistentMutex());
this->AssignUnsafe(other.Get()); this->AssignUnsafe(other.Parent::Get());
return *this; return *this;
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment