From 3ab556a38fda71a0e63604ce3f249a21f985deed Mon Sep 17 00:00:00 2001 From: "viettrungluu@chromium.org" Date: Wed, 28 Sep 2011 18:07:43 +0000 Subject: 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 --- webkit/plugins/ppapi/ppb_video_capture_impl.cc | 44 +++++++++++++++++++++++--- webkit/plugins/ppapi/ppb_video_capture_impl.h | 7 ++++ 2 files changed, 47 insertions(+), 4 deletions(-) (limited to 'webkit') 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 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); }; -- cgit v1.1