Skip to content

Commit

Permalink
Revert "Reland "Take the playout AudioDevice from a MediaStreamTrack'…
Browse files Browse the repository at this point in the history
…s creation frame""

This reverts commit c10f7b2.

Reason for revert: https://crbug.com/1288159 (200+ failures on WebKit Linux Leak builder)

Original change's description:
> Reland "Take the playout AudioDevice from a MediaStreamTrack's creation frame"
>
> This is a reland of 6a17272
>
> Delta: Fix crash at the end of WPTs by checking the LocalDOMWindow's LocalFrame is not null before accessing it. When the frame is destroyed this is set to null.
>
> Original change's description:
> > Take the playout AudioDevice from a MediaStreamTrack's creation frame
> >
> > When playing a Remote Audio MediaStreamTrack (one received over a PeerConnection), use the WebRtcAudioDeviceImpl instance associated with the frame where the track was created.
> >
> > This fixes an issue where audio is not played if the track is attached to a media element inside a (same origin) iframe.
> >
> > Bug: 1239207
> > Change-Id: Ib97e43913fce75982b5e3f3b2c54a9d9a9a815a8
> > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3386796
> > Reviewed-by: Henrik Boström <hbos@chromium.org>
> > Commit-Queue: Tony Herre <toprice@chromium.org>
> > Cr-Commit-Position: refs/heads/main@{#959107}
>
> Bug: 1239207
> Change-Id: Ie572d38c3d2a06498cb52280bcd8e5efc9e3c755
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3394627
> Reviewed-by: Henrik Boström <hbos@chromium.org>
> Commit-Queue: Tony Herre <toprice@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#960125}

Bug: 1239207
Change-Id: I10c50b7f43187be4f103bbd26c71309fe081de26
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3396401
Auto-Submit: Chloe Pelling <cpelling@google.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Chloe Pelling <cpelling@google.com>
Owners-Override: Chloe Pelling <cpelling@google.com>
Cr-Commit-Position: refs/heads/main@{#960242}
  • Loading branch information
cpelling authored and Chromium LUCI CQ committed Jan 18, 2022
1 parent f28302b commit db9e9a7
Show file tree
Hide file tree
Showing 6 changed files with 17 additions and 71 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -139,17 +139,6 @@ MediaStreamRendererFactory::GetAudioRenderer(
std::move(on_render_error_callback));
}

// Get the AudioDevice associated with the frame where this track was created,
// in case the track has been moved to eg a same origin iframe. Without this,
// one can get into a situation where media is piped to a different audio
// device to that where control signals are sent, leading to no audio being
// played out - see crbug/1239207.
WebLocalFrame* track_creation_frame =
audio_components[0].Get()->CreationFrame();
if (track_creation_frame) {
frame = To<LocalFrame>(WebLocalFrame::ToCoreFrame(*track_creation_frame));
}

// This is a remote WebRTC media stream.
WebRtcAudioDeviceImpl* audio_device =
PeerConnectionDependencyFactory::From(*frame->DomWindow())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,8 @@ namespace blink {
RemoteVideoTrackAdapter::RemoteVideoTrackAdapter(
const scoped_refptr<base::SingleThreadTaskRunner>& main_thread,
webrtc::VideoTrackInterface* webrtc_track,
scoped_refptr<MetronomeProvider> metronome_provider,
ExecutionContext* track_execution_context)
: RemoteMediaStreamTrackAdapter(main_thread,
webrtc_track,
track_execution_context),
scoped_refptr<MetronomeProvider> metronome_provider)
: RemoteMediaStreamTrackAdapter(main_thread, webrtc_track),
metronome_provider_(std::move(metronome_provider)) {
std::unique_ptr<TrackObserver> observer(
new TrackObserver(main_thread, observed_track().get()));
Expand Down Expand Up @@ -68,11 +65,8 @@ void RemoteVideoTrackAdapter::InitializeWebVideoTrack(

RemoteAudioTrackAdapter::RemoteAudioTrackAdapter(
const scoped_refptr<base::SingleThreadTaskRunner>& main_thread,
webrtc::AudioTrackInterface* webrtc_track,
ExecutionContext* track_execution_context)
: RemoteMediaStreamTrackAdapter(main_thread,
webrtc_track,
track_execution_context),
webrtc::AudioTrackInterface* webrtc_track)
: RemoteMediaStreamTrackAdapter(main_thread, webrtc_track),
#if DCHECK_IS_ON()
unregistered_(false),
#endif
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,9 @@
#include "base/dcheck_is_on.h"
#include "base/memory/scoped_refptr.h"
#include "third_party/blink/public/platform/web_string.h"
#include "third_party/blink/renderer/core/frame/local_dom_window.h"
#include "third_party/blink/renderer/modules/modules_export.h"
#include "third_party/blink/renderer/modules/peerconnection/peer_connection_dependency_factory.h"
#include "third_party/blink/renderer/platform/mediastream/media_stream_component.h"
#include "third_party/blink/renderer/platform/mediastream/media_stream_source.h"
#include "third_party/blink/renderer/platform/wtf/casting.h"
#include "third_party/blink/renderer/platform/wtf/functional.h"
#include "third_party/blink/renderer/platform/wtf/thread_safe_ref_counted.h"
#include "third_party/webrtc/api/media_stream_interface.h"
Expand All @@ -38,11 +35,9 @@ class MODULES_EXPORT RemoteMediaStreamTrackAdapter
public:
RemoteMediaStreamTrackAdapter(
const scoped_refptr<base::SingleThreadTaskRunner>& main_thread,
WebRtcMediaStreamTrackType* webrtc_track,
ExecutionContext* track_execution_context)
WebRtcMediaStreamTrackType* webrtc_track)
: main_thread_(main_thread),
webrtc_track_(webrtc_track),
track_execution_context_(track_execution_context),
id_(String::FromUTF8(webrtc_track->id())) {}

RemoteMediaStreamTrackAdapter(const RemoteMediaStreamTrackAdapter&) = delete;
Expand Down Expand Up @@ -88,18 +83,6 @@ class MODULES_EXPORT RemoteMediaStreamTrackAdapter
auto* source = MakeGarbageCollected<MediaStreamSource>(id_, type, id_,
true /*remote*/);
component_ = MakeGarbageCollected<MediaStreamComponent>(id_, source);
// If we have a reference to a window frame where the track was created,
// store it on the component. This allows other code to use the correct
// per-frame object for the track, such as the audio device for playout.
if (track_execution_context_ && track_execution_context_->IsWindow() &&
To<LocalDOMWindow>(track_execution_context_.Get())->GetFrame()) {
// IsWindow() being true means that the ExecutionContext is a
// LocalDOMWindow, so these casts should be safe.
component_->SetCreationFrame(
WebFrame::FromCoreFrame(
To<LocalDOMWindow>(track_execution_context_.Get())->GetFrame())
->ToWebLocalFrame());
}
DCHECK(component_);
}

Expand All @@ -113,7 +96,6 @@ class MODULES_EXPORT RemoteMediaStreamTrackAdapter
private:
const scoped_refptr<WebRtcMediaStreamTrackType> webrtc_track_;
CrossThreadPersistent<MediaStreamComponent> component_;
CrossThreadPersistent<ExecutionContext> track_execution_context_;
// const copy of the webrtc track id that allows us to check it from both the
// main and signaling threads without incurring a synchronous thread hop.
const String id_;
Expand All @@ -126,8 +108,7 @@ class MODULES_EXPORT RemoteVideoTrackAdapter
RemoteVideoTrackAdapter(
const scoped_refptr<base::SingleThreadTaskRunner>& main_thread,
webrtc::VideoTrackInterface* webrtc_track,
scoped_refptr<MetronomeProvider> metronome_provider,
ExecutionContext* execution_context);
scoped_refptr<MetronomeProvider> metronome_provider);

protected:
~RemoteVideoTrackAdapter() override;
Expand All @@ -149,8 +130,7 @@ class MODULES_EXPORT RemoteAudioTrackAdapter
// Called on the signaling thread.
RemoteAudioTrackAdapter(
const scoped_refptr<base::SingleThreadTaskRunner>& main_thread,
webrtc::AudioTrackInterface* webrtc_track,
ExecutionContext* execution_context);
webrtc::AudioTrackInterface* webrtc_track);

RemoteAudioTrackAdapter(const RemoteAudioTrackAdapter&) = delete;
RemoteAudioTrackAdapter& operator=(const RemoteAudioTrackAdapter&) = delete;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,17 +55,13 @@ WebRtcMediaStreamTrackAdapter::CreateRemoteTrackAdapter(
scoped_refptr<WebRtcMediaStreamTrackAdapter> remote_track_adapter(
base::AdoptRef(new WebRtcMediaStreamTrackAdapter(factory, main_thread)));
if (webrtc_track->kind() == webrtc::MediaStreamTrackInterface::kAudioKind) {
remote_track_adapter->InitializeRemoteAudioTrack(
base::WrapRefCounted(
static_cast<webrtc::AudioTrackInterface*>(webrtc_track.get())),
factory->GetSupplementable());
remote_track_adapter->InitializeRemoteAudioTrack(base::WrapRefCounted(
static_cast<webrtc::AudioTrackInterface*>(webrtc_track.get())));
} else {
DCHECK_EQ(webrtc_track->kind(),
webrtc::MediaStreamTrackInterface::kVideoKind);
remote_track_adapter->InitializeRemoteVideoTrack(
base::WrapRefCounted(
static_cast<webrtc::VideoTrackInterface*>(webrtc_track.get())),
factory->GetSupplementable());
remote_track_adapter->InitializeRemoteVideoTrack(base::WrapRefCounted(
static_cast<webrtc::VideoTrackInterface*>(webrtc_track.get())));
}
return remote_track_adapter;
}
Expand Down Expand Up @@ -227,8 +223,7 @@ void WebRtcMediaStreamTrackAdapter::InitializeLocalVideoTrack(
}

void WebRtcMediaStreamTrackAdapter::InitializeRemoteAudioTrack(
const scoped_refptr<webrtc::AudioTrackInterface>& webrtc_audio_track,
ExecutionContext* track_execution_context) {
const scoped_refptr<webrtc::AudioTrackInterface>& webrtc_audio_track) {
DCHECK(!main_thread_->BelongsToCurrentThread());
DCHECK(!is_initialized_);
DCHECK(!remote_track_can_complete_initialization_.IsSignaled());
Expand All @@ -239,7 +234,7 @@ void WebRtcMediaStreamTrackAdapter::InitializeRemoteAudioTrack(
base::StringPrintf("InitializeRemoteAudioTrack([this=%p])", this));
remote_audio_track_adapter_ =
base::MakeRefCounted<blink::RemoteAudioTrackAdapter>(
main_thread_, webrtc_audio_track.get(), track_execution_context);
main_thread_, webrtc_audio_track.get());
webrtc_track_ = webrtc_audio_track;
// Set the initial volume to zero. When the track is put in an audio tag for
// playout, its volume is set to that of the tag. Without this, we could end
Expand All @@ -255,16 +250,15 @@ void WebRtcMediaStreamTrackAdapter::InitializeRemoteAudioTrack(
}

void WebRtcMediaStreamTrackAdapter::InitializeRemoteVideoTrack(
const scoped_refptr<webrtc::VideoTrackInterface>& webrtc_video_track,
ExecutionContext* track_execution_context) {
const scoped_refptr<webrtc::VideoTrackInterface>& webrtc_video_track) {
DCHECK(!main_thread_->BelongsToCurrentThread());
DCHECK(webrtc_video_track);
DCHECK_EQ(webrtc_video_track->kind(),
webrtc::MediaStreamTrackInterface::kVideoKind);
remote_video_track_adapter_ =
base::MakeRefCounted<blink::RemoteVideoTrackAdapter>(
main_thread_, webrtc_video_track.get(),
factory_->metronome_provider(), track_execution_context);
factory_->metronome_provider());
webrtc_track_ = webrtc_video_track;
remote_track_can_complete_initialization_.Signal();
PostCrossThreadTask(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,11 +99,9 @@ class MODULES_EXPORT WebRtcMediaStreamTrackAdapter
// Initialization of remote tracks starts on the webrtc signaling thread and
// finishes on the main thread.
void InitializeRemoteAudioTrack(
const scoped_refptr<webrtc::AudioTrackInterface>& webrtc_audio_track,
ExecutionContext* execution_context);
const scoped_refptr<webrtc::AudioTrackInterface>& webrtc_audio_track);
void InitializeRemoteVideoTrack(
const scoped_refptr<webrtc::VideoTrackInterface>& webrtc_video_track,
ExecutionContext* execution_context);
const scoped_refptr<webrtc::VideoTrackInterface>& webrtc_video_track);
void FinalizeRemoteTrackInitializationOnMainThread();
void EnsureTrackIsInitialized();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@

#include "third_party/blink/public/platform/modules/mediastream/web_media_stream_track.h"
#include "third_party/blink/public/platform/web_vector.h"
#include "third_party/blink/public/web/web_local_frame.h"
#include "third_party/blink/renderer/platform/audio/audio_source_provider.h"
#include "third_party/blink/renderer/platform/heap/collection_support/heap_vector.h"
#include "third_party/blink/renderer/platform/heap/handle.h"
Expand Down Expand Up @@ -107,11 +106,6 @@ class PLATFORM_EXPORT MediaStreamComponent final
void GetSettings(MediaStreamTrackPlatform::Settings&);
MediaStreamTrackPlatform::CaptureHandle GetCaptureHandle();

WebLocalFrame* CreationFrame() { return creation_frame_; }
void SetCreationFrame(WebLocalFrame* creation_frame) {
creation_frame_ = creation_frame;
}

String ToString() const;

void Trace(Visitor*) const;
Expand Down Expand Up @@ -144,7 +138,6 @@ class PLATFORM_EXPORT MediaStreamComponent final

AudioSourceProviderImpl source_provider_;
Member<MediaStreamSource> source_;

String id_;
int unique_id_;
bool enabled_ = true;
Expand All @@ -153,8 +146,6 @@ class PLATFORM_EXPORT MediaStreamComponent final
WebMediaStreamTrack::ContentHintType::kNone;
MediaConstraints constraints_;
std::unique_ptr<MediaStreamTrackPlatform> platform_track_;
// Frame where the referenced platform track was created, if applicable.
WebLocalFrame* creation_frame_;
};

typedef HeapVector<Member<MediaStreamComponent>> MediaStreamComponentVector;
Expand Down

0 comments on commit db9e9a7

Please sign in to comment.