Commit aaaae22e authored by xhwang's avatar xhwang Committed by Commit bot

Refactor MediaSourceDelegate destruction.

This CL introduces a waiter in ~WebMediaPlayerAndroid() and makes sure objects
are destructed after MediaSourceDelegate is completly stopped.

BUG=417864

Review URL: https://codereview.chromium.org/605013002

Cr-Commit-Position: refs/heads/master@{#297031}
parent 1ea0de3d
...@@ -81,13 +81,12 @@ MediaSourceDelegate::~MediaSourceDelegate() { ...@@ -81,13 +81,12 @@ MediaSourceDelegate::~MediaSourceDelegate() {
DCHECK(!video_stream_); DCHECK(!video_stream_);
} }
void MediaSourceDelegate::Destroy() { void MediaSourceDelegate::Stop(const base::Closure& stop_cb) {
DCHECK(main_task_runner_->BelongsToCurrentThread()); DCHECK(main_task_runner_->BelongsToCurrentThread());
DVLOG(1) << __FUNCTION__ << " : " << demuxer_client_id_; DVLOG(1) << __FUNCTION__ << " : " << demuxer_client_id_;
if (!chunk_demuxer_) { if (!chunk_demuxer_) {
DCHECK(!demuxer_client_); DCHECK(!demuxer_client_);
delete this;
return; return;
} }
...@@ -100,12 +99,11 @@ void MediaSourceDelegate::Destroy() { ...@@ -100,12 +99,11 @@ void MediaSourceDelegate::Destroy() {
chunk_demuxer_->Shutdown(); chunk_demuxer_->Shutdown();
// |this| will be transferred to the callback StopDemuxer() and // Continue to stop objects on the media thread.
// OnDemuxerStopDone(). They own |this| and OnDemuxerStopDone() will delete media_task_runner_->PostTask(
// it when called, hence using base::Unretained(this) is safe here. FROM_HERE,
media_task_runner_->PostTask(FROM_HERE, base::Bind(
base::Bind(&MediaSourceDelegate::StopDemuxer, &MediaSourceDelegate::StopDemuxer, base::Unretained(this), stop_cb));
base::Unretained(this)));
} }
bool MediaSourceDelegate::IsVideoEncrypted() { bool MediaSourceDelegate::IsVideoEncrypted() {
...@@ -122,7 +120,8 @@ base::Time MediaSourceDelegate::GetTimelineOffset() const { ...@@ -122,7 +120,8 @@ base::Time MediaSourceDelegate::GetTimelineOffset() const {
return chunk_demuxer_->GetTimelineOffset(); return chunk_demuxer_->GetTimelineOffset();
} }
void MediaSourceDelegate::StopDemuxer() { void MediaSourceDelegate::StopDemuxer(const base::Closure& stop_cb) {
DVLOG(2) << __FUNCTION__;
DCHECK(media_task_runner_->BelongsToCurrentThread()); DCHECK(media_task_runner_->BelongsToCurrentThread());
DCHECK(chunk_demuxer_); DCHECK(chunk_demuxer_);
...@@ -142,11 +141,9 @@ void MediaSourceDelegate::StopDemuxer() { ...@@ -142,11 +141,9 @@ void MediaSourceDelegate::StopDemuxer() {
chunk_demuxer_->Stop(); chunk_demuxer_->Stop();
chunk_demuxer_.reset(); chunk_demuxer_.reset();
// The callback DeleteSelf() owns |this| and will delete it when called. // |this| may be destroyed at this point in time as a result of running
// Hence using base::Unretained(this) is safe here. // |stop_cb|.
media_task_runner_->PostTask( stop_cb.Run();
FROM_HERE,
base::Bind(&MediaSourceDelegate::DeleteSelf, base::Unretained(this)));
} }
void MediaSourceDelegate::InitializeMediaSource( void MediaSourceDelegate::InitializeMediaSource(
...@@ -630,12 +627,6 @@ void MediaSourceDelegate::FinishResettingDecryptingDemuxerStreams() { ...@@ -630,12 +627,6 @@ void MediaSourceDelegate::FinishResettingDecryptingDemuxerStreams() {
demuxer_client_->DemuxerSeekDone(demuxer_client_id_, browser_seek_time_); demuxer_client_->DemuxerSeekDone(demuxer_client_id_, browser_seek_time_);
} }
void MediaSourceDelegate::DeleteSelf() {
DCHECK(main_task_runner_->BelongsToCurrentThread());
DVLOG(1) << __FUNCTION__ << " : " << demuxer_client_id_;
delete this;
}
void MediaSourceDelegate::NotifyDemuxerReady() { void MediaSourceDelegate::NotifyDemuxerReady() {
DCHECK(media_task_runner_->BelongsToCurrentThread()); DCHECK(media_task_runner_->BelongsToCurrentThread());
DVLOG(1) << __FUNCTION__ << " : " << demuxer_client_id_; DVLOG(1) << __FUNCTION__ << " : " << demuxer_client_id_;
......
...@@ -47,20 +47,12 @@ class MediaSourceDelegate : public media::DemuxerHost { ...@@ -47,20 +47,12 @@ class MediaSourceDelegate : public media::DemuxerHost {
UpdateNetworkStateCB; UpdateNetworkStateCB;
typedef base::Callback<void(const base::TimeDelta&)> DurationChangeCB; typedef base::Callback<void(const base::TimeDelta&)> DurationChangeCB;
// Helper class used by scoped_ptr to destroy an instance of
// MediaSourceDelegate.
class Destroyer {
public:
inline void operator()(void* media_source_delegate) const {
static_cast<MediaSourceDelegate*>(media_source_delegate)->Destroy();
}
};
MediaSourceDelegate( MediaSourceDelegate(
RendererDemuxerAndroid* demuxer_client, RendererDemuxerAndroid* demuxer_client,
int demuxer_client_id, int demuxer_client_id,
const scoped_refptr<base::SingleThreadTaskRunner>& task_runner, const scoped_refptr<base::SingleThreadTaskRunner>& task_runner,
const scoped_refptr<media::MediaLog> media_log); const scoped_refptr<media::MediaLog> media_log);
virtual ~MediaSourceDelegate();
// Initialize the MediaSourceDelegate. |media_source| will be owned by // Initialize the MediaSourceDelegate. |media_source| will be owned by
// this object after this call. // this object after this call.
...@@ -101,8 +93,8 @@ class MediaSourceDelegate : public media::DemuxerHost { ...@@ -101,8 +93,8 @@ class MediaSourceDelegate : public media::DemuxerHost {
// Called when DemuxerStreamPlayer needs to read data from ChunkDemuxer. // Called when DemuxerStreamPlayer needs to read data from ChunkDemuxer.
void OnReadFromDemuxer(media::DemuxerStream::Type type); void OnReadFromDemuxer(media::DemuxerStream::Type type);
// Called by the Destroyer to destroy an instance of this object. // Must be called explicitly before |this| can be destroyed.
void Destroy(); void Stop(const base::Closure& stop_cb);
// Called on the main thread to check whether the video stream is encrypted. // Called on the main thread to check whether the video stream is encrypted.
bool IsVideoEncrypted(); bool IsVideoEncrypted();
...@@ -111,9 +103,6 @@ class MediaSourceDelegate : public media::DemuxerHost { ...@@ -111,9 +103,6 @@ class MediaSourceDelegate : public media::DemuxerHost {
base::Time GetTimelineOffset() const; base::Time GetTimelineOffset() const;
private: private:
// This is private to enforce use of the Destroyer.
virtual ~MediaSourceDelegate();
// Methods inherited from DemuxerHost. // Methods inherited from DemuxerHost.
virtual void AddBufferedTimeRange(base::TimeDelta start, virtual void AddBufferedTimeRange(base::TimeDelta start,
base::TimeDelta end) OVERRIDE; base::TimeDelta end) OVERRIDE;
...@@ -146,15 +135,14 @@ class MediaSourceDelegate : public media::DemuxerHost { ...@@ -146,15 +135,14 @@ class MediaSourceDelegate : public media::DemuxerHost {
void ResetVideoDecryptingDemuxerStream(); void ResetVideoDecryptingDemuxerStream();
void FinishResettingDecryptingDemuxerStreams(); void FinishResettingDecryptingDemuxerStreams();
// Helper for deleting |this| on the main thread.
void DeleteSelf();
void OnDemuxerOpened(); void OnDemuxerOpened();
void OnNeedKey(const std::string& type, void OnNeedKey(const std::string& type,
const std::vector<uint8>& init_data); const std::vector<uint8>& init_data);
void NotifyDemuxerReady(); void NotifyDemuxerReady();
void StopDemuxer(); // Stops and clears objects on the media thread.
void StopDemuxer(const base::Closure& stop_cb);
void InitializeDemuxer(); void InitializeDemuxer();
void SeekInternal(const base::TimeDelta& seek_time); void SeekInternal(const base::TimeDelta& seek_time);
// Reads an access unit from the demuxer stream |stream| and stores it in // Reads an access unit from the demuxer stream |stream| and stores it in
......
...@@ -210,6 +210,16 @@ WebMediaPlayerAndroid::~WebMediaPlayerAndroid() { ...@@ -210,6 +210,16 @@ WebMediaPlayerAndroid::~WebMediaPlayerAndroid() {
delegate_->PlayerGone(this); delegate_->PlayerGone(this);
stream_texture_factory_->RemoveObserver(this); stream_texture_factory_->RemoveObserver(this);
if (media_source_delegate_) {
// Part of |media_source_delegate_| needs to be stopped on the media thread.
// Wait until |media_source_delegate_| is fully stopped before tearing
// down other objects.
base::WaitableEvent waiter(false, false);
media_source_delegate_->Stop(
base::Bind(&base::WaitableEvent::Signal, base::Unretained(&waiter)));
waiter.Wait();
}
} }
void WebMediaPlayerAndroid::load(LoadType load_type, void WebMediaPlayerAndroid::load(LoadType load_type,
......
...@@ -455,9 +455,6 @@ class WebMediaPlayerAndroid : public blink::WebMediaPlayer, ...@@ -455,9 +455,6 @@ class WebMediaPlayerAndroid : public blink::WebMediaPlayer,
bool force_use_overlay_embedded_video_; bool force_use_overlay_embedded_video_;
#endif // defined(VIDEO_HOLE) #endif // defined(VIDEO_HOLE)
scoped_ptr<MediaSourceDelegate,
MediaSourceDelegate::Destroyer> media_source_delegate_;
MediaPlayerHostMsg_Initialize_Type player_type_; MediaPlayerHostMsg_Initialize_Type player_type_;
// Whether the browser is currently connected to a remote media player. // Whether the browser is currently connected to a remote media player.
...@@ -503,6 +500,8 @@ class WebMediaPlayerAndroid : public blink::WebMediaPlayer, ...@@ -503,6 +500,8 @@ class WebMediaPlayerAndroid : public blink::WebMediaPlayer,
// as playback progresses. // as playback progresses.
media::TimeDeltaInterpolator interpolator_; media::TimeDeltaInterpolator interpolator_;
scoped_ptr<MediaSourceDelegate> media_source_delegate_;
// NOTE: Weak pointers must be invalidated before all other member variables. // NOTE: Weak pointers must be invalidated before all other member variables.
base::WeakPtrFactory<WebMediaPlayerAndroid> weak_factory_; base::WeakPtrFactory<WebMediaPlayerAndroid> weak_factory_;
......
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