summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorthakis@chromium.org <thakis@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2010-12-01 15:48:17 +0000
committerthakis@chromium.org <thakis@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2010-12-01 15:48:17 +0000
commit7727ae0dd82367997f3510726d63855ff5a5a1f0 (patch)
tree091555f090c3620588535b556b320a394ff76ca8
parent292e8b77def5fc2be164c5882ae7351ec0b11642 (diff)
downloadchromium_src-7727ae0dd82367997f3510726d63855ff5a5a1f0.zip
chromium_src-7727ae0dd82367997f3510726d63855ff5a5a1f0.tar.gz
chromium_src-7727ae0dd82367997f3510726d63855ff5a5a1f0.tar.bz2
Merge 67824 - Stopgap fix for crash in issue 53867 comment 15
gwtquake lets the renderer create > 300 threads (one per audio element?), and eventually thread creation fails. This CL makes the media code more robust against thread creation failure (currently, it just crashes the renderer). The Real Fix probably is to have a thread pool for media stuff instead of one thread per media object. And maybe threads just leak under some circumstances. I will file a follow-up bug for that case, hopefully with a reduced test case. BUG=53867,61293 TEST=Completing the first level in gwtquake shouldn't crash the renderer. Review URL: http://codereview.chromium.org/5362003 TBR=thakis@chromium.org Review URL: http://codereview.chromium.org/5469001 git-svn-id: svn://svn.chromium.org/chrome/branches/597/src@67853 0039d316-1c4b-4281-b951-d872f2087c98
-rw-r--r--chrome/renderer/render_view.cc13
-rw-r--r--media/base/pipeline_impl.cc122
-rw-r--r--media/base/pipeline_impl.h2
-rw-r--r--webkit/glue/webmediaplayer_impl.cc28
-rw-r--r--webkit/glue/webmediaplayer_impl.h14
-rw-r--r--webkit/support/webkit_support.cc12
-rw-r--r--webkit/tools/test_shell/test_webview_delegate.cc12
7 files changed, 123 insertions, 80 deletions
diff --git a/chrome/renderer/render_view.cc b/chrome/renderer/render_view.cc
index bc99517d..70f2f67 100644
--- a/chrome/renderer/render_view.cc
+++ b/chrome/renderer/render_view.cc
@@ -2847,10 +2847,15 @@ WebMediaPlayer* RenderView::createMediaPlayer(
collection->AddVideoRenderer(renderer);
video_renderer = renderer;
- return new webkit_glue::WebMediaPlayerImpl(
- client, collection.release(), bridge_factory_simple,
- bridge_factory_buffered,
- cmd_line->HasSwitch(switches::kSimpleDataSource),video_renderer);
+ scoped_ptr<webkit_glue::WebMediaPlayerImpl> result(
+ new webkit_glue::WebMediaPlayerImpl(client, collection.release()));
+ if (!result->Initialize(bridge_factory_simple,
+ bridge_factory_buffered,
+ cmd_line->HasSwitch(switches::kSimpleDataSource),
+ video_renderer)) {
+ return NULL;
+ }
+ return result.release();
}
WebApplicationCacheHost* RenderView::createApplicationCacheHost(
diff --git a/media/base/pipeline_impl.cc b/media/base/pipeline_impl.cc
index c53044f..049ab00 100644
--- a/media/base/pipeline_impl.cc
+++ b/media/base/pipeline_impl.cc
@@ -937,7 +937,7 @@ void PipelineImpl::FinishDestroyingFiltersTask() {
}
}
-void PipelineImpl::PrepareFilter(scoped_refptr<MediaFilter> filter) {
+bool PipelineImpl::PrepareFilter(scoped_refptr<MediaFilter> filter) {
DCHECK_EQ(MessageLoop::current(), message_loop_);
DCHECK(IsPipelineOk());
@@ -948,7 +948,7 @@ void PipelineImpl::PrepareFilter(scoped_refptr<MediaFilter> filter) {
if (!thread.get() || !thread->Start()) {
NOTREACHED() << "Could not start filter thread";
SetError(PIPELINE_ERROR_INITIALIZATION_FAILED);
- return;
+ return false;
}
filter->set_message_loop(thread->message_loop());
@@ -959,6 +959,7 @@ void PipelineImpl::PrepareFilter(scoped_refptr<MediaFilter> filter) {
DCHECK(IsPipelineOk());
filter->set_host(this);
filters_.push_back(make_scoped_refptr(filter.get()));
+ return true;
}
void PipelineImpl::InitializeDataSource() {
@@ -977,7 +978,9 @@ void PipelineImpl::InitializeDataSource() {
return;
}
- PrepareFilter(data_source);
+ if (!PrepareFilter(data_source))
+ return;
+
pipeline_init_state_->data_source_ = data_source;
data_source->Initialize(
url_, NewCallback(this, &PipelineImpl::OnFilterInitialize));
@@ -998,7 +1001,9 @@ void PipelineImpl::InitializeDemuxer(
return;
}
- PrepareFilter(demuxer);
+ if (!PrepareFilter(demuxer))
+ return;
+
pipeline_init_state_->demuxer_ = demuxer;
demuxer->Initialize(data_source,
NewCallback(this, &PipelineImpl::OnFilterInitialize));
@@ -1012,23 +1017,25 @@ bool PipelineImpl::InitializeAudioDecoder(
scoped_refptr<DemuxerStream> stream =
FindDemuxerStream(demuxer, mime_type::kMajorTypeAudio);
- if (stream) {
- scoped_refptr<AudioDecoder> audio_decoder;
- filter_collection_->SelectAudioDecoder(&audio_decoder);
+ if (!stream)
+ return false;
- if (audio_decoder) {
- PrepareFilter(audio_decoder);
- pipeline_init_state_->audio_decoder_ = audio_decoder;
- audio_decoder->Initialize(
- stream,
- NewCallback(this, &PipelineImpl::OnFilterInitialize));
- return true;
- } else {
- SetError(PIPELINE_ERROR_REQUIRED_FILTER_MISSING);
- }
+ scoped_refptr<AudioDecoder> audio_decoder;
+ filter_collection_->SelectAudioDecoder(&audio_decoder);
+
+ if (!audio_decoder) {
+ SetError(PIPELINE_ERROR_REQUIRED_FILTER_MISSING);
+ return false;
}
- return false;
+ if (!PrepareFilter(audio_decoder))
+ return false;
+
+ pipeline_init_state_->audio_decoder_ = audio_decoder;
+ audio_decoder->Initialize(
+ stream,
+ NewCallback(this, &PipelineImpl::OnFilterInitialize));
+ return true;
}
bool PipelineImpl::InitializeVideoDecoder(
@@ -1039,22 +1046,25 @@ bool PipelineImpl::InitializeVideoDecoder(
scoped_refptr<DemuxerStream> stream =
FindDemuxerStream(demuxer, mime_type::kMajorTypeVideo);
- if (stream) {
- scoped_refptr<VideoDecoder> video_decoder;
- filter_collection_->SelectVideoDecoder(&video_decoder);
+ if (!stream)
+ return false;
+
+ scoped_refptr<VideoDecoder> video_decoder;
+ filter_collection_->SelectVideoDecoder(&video_decoder);
- if (video_decoder) {
- PrepareFilter(video_decoder);
- pipeline_init_state_->video_decoder_ = video_decoder;
- video_decoder->Initialize(
- stream,
- NewCallback(this, &PipelineImpl::OnFilterInitialize));
- return true;
- } else {
- SetError(PIPELINE_ERROR_REQUIRED_FILTER_MISSING);
- }
+ if (!video_decoder) {
+ SetError(PIPELINE_ERROR_REQUIRED_FILTER_MISSING);
+ return false;
}
- return false;
+
+ if (!PrepareFilter(video_decoder))
+ return false;
+
+ pipeline_init_state_->video_decoder_ = video_decoder;
+ video_decoder->Initialize(
+ stream,
+ NewCallback(this, &PipelineImpl::OnFilterInitialize));
+ return true;
}
bool PipelineImpl::InitializeAudioRenderer(
@@ -1062,19 +1072,21 @@ bool PipelineImpl::InitializeAudioRenderer(
DCHECK_EQ(MessageLoop::current(), message_loop_);
DCHECK(IsPipelineOk());
- if (decoder) {
- filter_collection_->SelectAudioRenderer(&audio_renderer_);
+ if (!decoder)
+ return false;
- if (audio_renderer_) {
- PrepareFilter(audio_renderer_);
- audio_renderer_->Initialize(
- decoder, NewCallback(this, &PipelineImpl::OnFilterInitialize));
- return true;
- } else {
- SetError(PIPELINE_ERROR_REQUIRED_FILTER_MISSING);
- }
+ filter_collection_->SelectAudioRenderer(&audio_renderer_);
+ if (!audio_renderer_) {
+ SetError(PIPELINE_ERROR_REQUIRED_FILTER_MISSING);
+ return false;
}
- return false;
+
+ if (!PrepareFilter(audio_renderer_))
+ return false;
+
+ audio_renderer_->Initialize(
+ decoder, NewCallback(this, &PipelineImpl::OnFilterInitialize));
+ return true;
}
bool PipelineImpl::InitializeVideoRenderer(
@@ -1082,19 +1094,21 @@ bool PipelineImpl::InitializeVideoRenderer(
DCHECK_EQ(MessageLoop::current(), message_loop_);
DCHECK(IsPipelineOk());
- if (decoder) {
- filter_collection_->SelectVideoRenderer(&video_renderer_);
+ if (!decoder)
+ return false;
- if (video_renderer_) {
- PrepareFilter(video_renderer_);
- video_renderer_->Initialize(
- decoder, NewCallback(this, &PipelineImpl::OnFilterInitialize));
- return true;
- } else {
- SetError(PIPELINE_ERROR_REQUIRED_FILTER_MISSING);
- }
+ filter_collection_->SelectVideoRenderer(&video_renderer_);
+ if (!video_renderer_) {
+ SetError(PIPELINE_ERROR_REQUIRED_FILTER_MISSING);
+ return false;
}
- return false;
+
+ if (!PrepareFilter(video_renderer_))
+ return false;
+
+ video_renderer_->Initialize(
+ decoder, NewCallback(this, &PipelineImpl::OnFilterInitialize));
+ return true;
}
scoped_refptr<DemuxerStream> PipelineImpl::FindDemuxerStream(
diff --git a/media/base/pipeline_impl.h b/media/base/pipeline_impl.h
index 86961cf..603194a 100644
--- a/media/base/pipeline_impl.h
+++ b/media/base/pipeline_impl.h
@@ -234,7 +234,7 @@ class PipelineImpl : public Pipeline, public FilterHost {
// PrepareFilter() creates the filter's thread and injects a FilterHost and
// MessageLoop.
- void PrepareFilter(scoped_refptr<MediaFilter> filter);
+ bool PrepareFilter(scoped_refptr<MediaFilter> filter);
// The following initialize methods are used to select a specific type of
// MediaFilter object from MediaFilterCollection and initialize it
diff --git a/webkit/glue/webmediaplayer_impl.cc b/webkit/glue/webmediaplayer_impl.cc
index bbf6172..58489cc 100644
--- a/webkit/glue/webmediaplayer_impl.cc
+++ b/webkit/glue/webmediaplayer_impl.cc
@@ -225,28 +225,32 @@ void WebMediaPlayerImpl::Proxy::PutCurrentFrame(
WebMediaPlayerImpl::WebMediaPlayerImpl(
WebKit::WebMediaPlayerClient* client,
- media::MediaFilterCollection* collection,
- MediaResourceLoaderBridgeFactory* bridge_factory_simple,
- MediaResourceLoaderBridgeFactory* bridge_factory_buffered,
- bool use_simple_data_source,
- scoped_refptr<WebVideoRenderer> web_video_renderer)
+ media::MediaFilterCollection* collection)
: network_state_(WebKit::WebMediaPlayer::Empty),
ready_state_(WebKit::WebMediaPlayer::HaveNothing),
main_loop_(NULL),
filter_collection_(collection),
+ pipeline_(NULL),
pipeline_thread_("PipelineThread"),
paused_(true),
playback_rate_(0.0f),
client_(client),
+ proxy_(NULL),
pipeline_stopped_(false, false) {
// Saves the current message loop.
DCHECK(!main_loop_);
main_loop_ = MessageLoop::current();
+}
+bool WebMediaPlayerImpl::Initialize(
+ MediaResourceLoaderBridgeFactory* bridge_factory_simple,
+ MediaResourceLoaderBridgeFactory* bridge_factory_buffered,
+ bool use_simple_data_source,
+ scoped_refptr<WebVideoRenderer> web_video_renderer) {
// Create the pipeline and its thread.
if (!pipeline_thread_.Start()) {
NOTREACHED() << "Could not start PipelineThread";
- return;
+ return false;
}
pipeline_ = new media::PipelineImpl(pipeline_thread_.message_loop());
@@ -290,6 +294,8 @@ WebMediaPlayerImpl::WebMediaPlayerImpl(
filter_collection_->AddAudioDecoder(new media::FFmpegAudioDecoder());
filter_collection_->AddVideoDecoder(new media::FFmpegVideoDecoder(NULL));
filter_collection_->AddAudioRenderer(new media::NullAudioRenderer());
+
+ return true;
}
WebMediaPlayerImpl::~WebMediaPlayerImpl() {
@@ -776,10 +782,12 @@ void WebMediaPlayerImpl::Destroy() {
// Make sure to kill the pipeline so there's no more media threads running.
// Note: stopping the pipeline might block for a long time.
- pipeline_->Stop(NewCallback(this,
- &WebMediaPlayerImpl::PipelineStoppedCallback));
- pipeline_stopped_.Wait();
- pipeline_thread_.Stop();
+ if (pipeline_) {
+ pipeline_->Stop(NewCallback(this,
+ &WebMediaPlayerImpl::PipelineStoppedCallback));
+ pipeline_stopped_.Wait();
+ pipeline_thread_.Stop();
+ }
// And then detach the proxy, it may live on the render thread for a little
// longer until all the tasks are finished.
diff --git a/webkit/glue/webmediaplayer_impl.h b/webkit/glue/webmediaplayer_impl.h
index 9361020..26825da 100644
--- a/webkit/glue/webmediaplayer_impl.h
+++ b/webkit/glue/webmediaplayer_impl.h
@@ -169,14 +169,18 @@ class WebMediaPlayerImpl : public WebKit::WebMediaPlayer,
// |collection| can override the default filters by adding extra filters to
// |collection| before calling this method.
//
+ // Callers must call |Initialize()| before they can use the object.
WebMediaPlayerImpl(WebKit::WebMediaPlayerClient* client,
- media::MediaFilterCollection* collection,
- MediaResourceLoaderBridgeFactory* bridge_factory_simple,
- MediaResourceLoaderBridgeFactory* bridge_factory_buffered,
- bool use_simple_data_source,
- scoped_refptr<WebVideoRenderer> web_video_renderer);
+ media::MediaFilterCollection* collection);
virtual ~WebMediaPlayerImpl();
+ // Finalizes initialization of the object.
+ bool Initialize(
+ MediaResourceLoaderBridgeFactory* bridge_factory_simple,
+ MediaResourceLoaderBridgeFactory* bridge_factory_buffered,
+ bool use_simple_data_source,
+ scoped_refptr<WebVideoRenderer> web_video_renderer);
+
virtual void load(const WebKit::WebURL& url);
virtual void cancelLoad();
diff --git a/webkit/support/webkit_support.cc b/webkit/support/webkit_support.cc
index 55550de..090c316 100644
--- a/webkit/support/webkit_support.cc
+++ b/webkit/support/webkit_support.cc
@@ -295,9 +295,15 @@ WebKit::WebMediaPlayer* CreateMediaPlayer(WebFrame* frame,
new webkit_glue::VideoRendererImpl(false));
collection->AddVideoRenderer(video_renderer);
- return new webkit_glue::WebMediaPlayerImpl(
- client, collection.release(), bridge_factory_simple,
- bridge_factory_buffered, false, video_renderer);
+ scoped_ptr<webkit_glue::WebMediaPlayerImpl> result(
+ new webkit_glue::WebMediaPlayerImpl(client, collection.release()));
+ if (!result->Initialize(bridge_factory_simple,
+ bridge_factory_buffered,
+ false,
+ video_renderer)) {
+ return NULL;
+ }
+ return result.release();
}
WebKit::WebApplicationCacheHost* CreateApplicationCacheHost(
diff --git a/webkit/tools/test_shell/test_webview_delegate.cc b/webkit/tools/test_shell/test_webview_delegate.cc
index 4ad7349..cca5abbea 100644
--- a/webkit/tools/test_shell/test_webview_delegate.cc
+++ b/webkit/tools/test_shell/test_webview_delegate.cc
@@ -754,9 +754,15 @@ WebMediaPlayer* TestWebViewDelegate::createMediaPlayer(
new webkit_glue::VideoRendererImpl(false));
collection->AddVideoRenderer(video_renderer);
- return new webkit_glue::WebMediaPlayerImpl(
- client, collection.release(), bridge_factory_simple,
- bridge_factory_buffered, false, video_renderer);
+ scoped_ptr<webkit_glue::WebMediaPlayerImpl> result(
+ new webkit_glue::WebMediaPlayerImpl(client, collection.release()));
+ if (!result->Initialize(bridge_factory_simple,
+ bridge_factory_buffered,
+ false,
+ video_renderer)) {
+ return NULL;
+ }
+ return result.release();
}
WebApplicationCacheHost* TestWebViewDelegate::createApplicationCacheHost(