diff options
author | viettrungluu@chromium.org <viettrungluu@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2011-09-28 18:07:43 +0000 |
---|---|---|
committer | viettrungluu@chromium.org <viettrungluu@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2011-09-28 18:07:43 +0000 |
commit | 3ab556a38fda71a0e63604ce3f249a21f985deed (patch) | |
tree | ed73cb68eddf34ff716f7b96a71891a0feeadd6c | |
parent | f491617687763f9f6b8d8d1d44d5aa13ef6d730e (diff) | |
download | chromium_src-3ab556a38fda71a0e63604ce3f249a21f985deed.zip chromium_src-3ab556a38fda71a0e63604ce3f249a21f985deed.tar.gz chromium_src-3ab556a38fda71a0e63604ce3f249a21f985deed.tar.bz2 |
Properly scope the lifetime of the |PPB_VideoCapture_Impl|.
Since it/its proxy are |media::VideoCapture::EventHandler|s, they must remain
alive from the |StartCapture()| call until |OnRemoved()| is received.
(Precisely, the |PPB_VideoCapture_Impl| owns a |PlatformVideoCapture(Impl)|,
which owns a |VideoCaptureHandlerProxy|. Keeping the |PPB_VideoCapture_Impl|
alive keeps all these things alive.)
BUG=none
TEST=Closing a page with a PPAPI plugin using the video capture interface
doesn't cause the renderer to crash. Other similar things should also work
more or less properly (and at least not crash).
Review URL: http://codereview.chromium.org/8052023
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@103138 0039d316-1c4b-4281-b951-d872f2087c98
-rw-r--r-- | media/video/capture/video_capture_proxy.cc | 13 | ||||
-rw-r--r-- | media/video/capture/video_capture_proxy.h | 3 | ||||
-rw-r--r-- | webkit/plugins/ppapi/ppb_video_capture_impl.cc | 44 | ||||
-rw-r--r-- | webkit/plugins/ppapi/ppb_video_capture_impl.h | 7 |
4 files changed, 62 insertions, 5 deletions
diff --git a/media/video/capture/video_capture_proxy.cc b/media/video/capture/video_capture_proxy.cc index a8d00cb..221f2c6 100644 --- a/media/video/capture/video_capture_proxy.cc +++ b/media/video/capture/video_capture_proxy.cc @@ -69,7 +69,11 @@ void VideoCaptureHandlerProxy::OnError(VideoCapture* capture, int error_code) { } void VideoCaptureHandlerProxy::OnRemoved(VideoCapture* capture) { - // TODO(vtl): add logic when this event handler is removed. + main_message_loop_->PostTask(FROM_HERE, NewRunnableMethod( + this, + &VideoCaptureHandlerProxy::OnRemovedOnMainThread, + capture, + GetState(capture))); } void VideoCaptureHandlerProxy::OnBufferReady( @@ -123,6 +127,13 @@ void VideoCaptureHandlerProxy::OnErrorOnMainThread( proxied_->OnError(capture, error_code); } +void VideoCaptureHandlerProxy::OnRemovedOnMainThread( + VideoCapture* capture, + const VideoCaptureState& state) { + state_ = state; + proxied_->OnRemoved(capture); +} + void VideoCaptureHandlerProxy::OnBufferReadyOnMainThread( VideoCapture* capture, const VideoCaptureState& state, diff --git a/media/video/capture/video_capture_proxy.h b/media/video/capture/video_capture_proxy.h index 0594379..54fa666 100644 --- a/media/video/capture/video_capture_proxy.h +++ b/media/video/capture/video_capture_proxy.h @@ -74,6 +74,9 @@ class MEDIA_EXPORT VideoCaptureHandlerProxy VideoCapture* capture, const VideoCaptureState& state, int error_code); + void OnRemovedOnMainThread( + VideoCapture* capture, + const VideoCaptureState& state); void OnBufferReadyOnMainThread( VideoCapture* capture, const VideoCaptureState& state, diff --git a/webkit/plugins/ppapi/ppb_video_capture_impl.cc b/webkit/plugins/ppapi/ppb_video_capture_impl.cc index a8bc2b3..8d3da53 100644 --- a/webkit/plugins/ppapi/ppb_video_capture_impl.cc +++ b/webkit/plugins/ppapi/ppb_video_capture_impl.cc @@ -31,15 +31,15 @@ PPB_VideoCapture_Impl::PPB_VideoCapture_Impl(PP_Instance instance) : Resource(instance), buffer_count_hint_(0), ppp_videocapture_(NULL), - status_(PP_VIDEO_CAPTURE_STATUS_STOPPED) { + status_(PP_VIDEO_CAPTURE_STATUS_STOPPED), + is_dead_(false) { } PPB_VideoCapture_Impl::~PPB_VideoCapture_Impl() { - if (platform_video_capture_.get()) - StopCapture(); } bool PPB_VideoCapture_Impl::Init() { + DCHECK(!is_dead_); PluginInstance* instance = ResourceHelper::GetPluginInstance(this); if (!instance) return false; @@ -57,9 +57,20 @@ PPB_VideoCapture_API* PPB_VideoCapture_Impl::AsPPB_VideoCapture_API() { return this; } +void PPB_VideoCapture_Impl::LastPluginRefWasDeleted() { + if (platform_video_capture_.get()) + StopCapture(); + DCHECK(buffers_.empty()); + ppp_videocapture_ = NULL; + is_dead_ = true; + + ::ppapi::Resource::LastPluginRefWasDeleted(); +} + int32_t PPB_VideoCapture_Impl::StartCapture( const PP_VideoCaptureDeviceInfo_Dev& requested_info, uint32_t buffer_count) { + DCHECK(!is_dead_); switch (status_) { case PP_VIDEO_CAPTURE_STATUS_STARTING: case PP_VIDEO_CAPTURE_STATUS_STARTED: @@ -83,11 +94,13 @@ int32_t PPB_VideoCapture_Impl::StartCapture( false // resolution_fixed }; status_ = PP_VIDEO_CAPTURE_STATUS_STARTING; + AddRef(); // Balanced in |OnRemoved()|. platform_video_capture_->StartCapture(this, capability); return PP_OK; } int32_t PPB_VideoCapture_Impl::ReuseBuffer(uint32_t buffer) { + DCHECK(!is_dead_); if (buffer >= buffers_.size() || !buffers_[buffer].in_use) return PP_ERROR_BADARGUMENT; buffers_[buffer].in_use = false; @@ -95,6 +108,7 @@ int32_t PPB_VideoCapture_Impl::ReuseBuffer(uint32_t buffer) { } int32_t PPB_VideoCapture_Impl::StopCapture() { + DCHECK(!is_dead_); switch (status_) { case PP_VIDEO_CAPTURE_STATUS_STOPPED: case PP_VIDEO_CAPTURE_STATUS_STOPPING: @@ -112,6 +126,9 @@ int32_t PPB_VideoCapture_Impl::StopCapture() { } void PPB_VideoCapture_Impl::OnStarted(media::VideoCapture* capture) { + if (is_dead_) + return; + switch (status_) { case PP_VIDEO_CAPTURE_STATUS_STARTING: case PP_VIDEO_CAPTURE_STATUS_PAUSED: @@ -127,6 +144,9 @@ void PPB_VideoCapture_Impl::OnStarted(media::VideoCapture* capture) { } void PPB_VideoCapture_Impl::OnStopped(media::VideoCapture* capture) { + if (is_dead_) + return; + switch (status_) { case PP_VIDEO_CAPTURE_STATUS_STOPPING: break; @@ -142,6 +162,9 @@ void PPB_VideoCapture_Impl::OnStopped(media::VideoCapture* capture) { } void PPB_VideoCapture_Impl::OnPaused(media::VideoCapture* capture) { + if (is_dead_) + return; + switch (status_) { case PP_VIDEO_CAPTURE_STATUS_STARTING: case PP_VIDEO_CAPTURE_STATUS_STARTED: @@ -158,6 +181,9 @@ void PPB_VideoCapture_Impl::OnPaused(media::VideoCapture* capture) { void PPB_VideoCapture_Impl::OnError(media::VideoCapture* capture, int error_code) { + if (is_dead_) + return; + // Today, the media layer only sends "1" as an error. DCHECK(error_code == 1); // It either comes because some error was detected while starting (e.g. 2 @@ -168,12 +194,15 @@ void PPB_VideoCapture_Impl::OnError(media::VideoCapture* capture, } void PPB_VideoCapture_Impl::OnRemoved(media::VideoCapture* capture) { - // TODO(vtl): add logic when this event handler is removed. + Release(); } void PPB_VideoCapture_Impl::OnBufferReady( media::VideoCapture* capture, scoped_refptr<media::VideoCapture::VideoFrameBuffer> buffer) { + if (is_dead_) + return; + DCHECK(buffer.get()); for (uint32_t i = 0; i < buffers_.size(); ++i) { if (!buffers_[i].in_use) { @@ -194,6 +223,11 @@ void PPB_VideoCapture_Impl::OnBufferReady( void PPB_VideoCapture_Impl::OnDeviceInfoReceived( media::VideoCapture* capture, const media::VideoCaptureParams& device_info) { + // No need to call |ReleaseBuffers()|: if we're dead, |StopCapture()| should + // already have been called. + if (is_dead_) + return; + PP_VideoCaptureDeviceInfo_Dev info = { device_info.width, device_info.height, @@ -242,6 +276,7 @@ void PPB_VideoCapture_Impl::OnDeviceInfoReceived( } void PPB_VideoCapture_Impl::ReleaseBuffers() { + DCHECK(!is_dead_); ResourceTracker *tracker = ResourceTracker::Get(); for (size_t i = 0; i < buffers_.size(); ++i) { buffers_[i].buffer->Unmap(); @@ -251,6 +286,7 @@ void PPB_VideoCapture_Impl::ReleaseBuffers() { } void PPB_VideoCapture_Impl::SendStatus() { + DCHECK(!is_dead_); ppp_videocapture_->OnStatus(pp_instance(), pp_resource(), status_); } diff --git a/webkit/plugins/ppapi/ppb_video_capture_impl.h b/webkit/plugins/ppapi/ppb_video_capture_impl.h index 1fab94f..2c314fe 100644 --- a/webkit/plugins/ppapi/ppb_video_capture_impl.h +++ b/webkit/plugins/ppapi/ppb_video_capture_impl.h @@ -35,6 +35,7 @@ class PPB_VideoCapture_Impl : public ::ppapi::Resource, // Resource overrides. virtual PPB_VideoCapture_API* AsPPB_VideoCapture_API() OVERRIDE; + virtual void LastPluginRefWasDeleted() OVERRIDE; // PPB_VideoCapture implementation. virtual int32_t StartCapture( @@ -76,6 +77,12 @@ class PPB_VideoCapture_Impl : public ::ppapi::Resource, const PPP_VideoCapture_Dev* ppp_videocapture_; PP_VideoCaptureStatus_Dev status_; + // Signifies that the plugin has given up all its refs, but the object is + // still alive, possibly because the backend hasn't released the object as + // |EventHandler| yet. It can be removed if/when |EventHandler| is made to be + // refcounted (and made into a "member" of this object instead). + bool is_dead_; + DISALLOW_COPY_AND_ASSIGN(PPB_VideoCapture_Impl); }; |