Don't clear VideoRendererImpl's proxy_ variable during OnStop().

Nasty threading issue involving PipelineThread, VideoRendererThread and RenderThread:
  - VideoRendererThread continuously calls OnFrameAvailable(), as expected
  - RenderThread gets the signal to destroy a <video>
  - RenderThread blocks on PipelineThread to finish clean up
  - PipelineThread calls OnStop(), which sets proxy_ to NULL
  - Race condition occurs if VideoRendererThread was already on its way to call OnFrameAvailable() again

Now we could fix this by adding more locks (i.e., inside VideoRendererBase::ThreadMain() before we call OnFrameAvailable()), but that defeats the entire point of keeping VideoRendererThread as lock-free as possible.

We could also add an extra lock in VideoRendererImpl to synchronize access to proxy_, but that's unnecessary as proxy_ outlives VideoRendererImpl so there's no need to set it to NULL.  Furthermore it's OK to keep calling proxy_->Repaint() since that will post tasks to the RenderThread, which is conveniently blocked on PipelineThread to finish, who is blocked on VideoRendererThread to finish calling proxy_->Repaint()!  After that task is posted, everything unblocks and terminates and when RenderThread resumes it notifies proxy_ to cancel any pending tasks (such as that last repaint).

BUG=35858
TEST=layout tests should crash less frequently on shutdown

Review URL: http://codereview.chromium.org/1625003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@43766 0039d316-1c4b-4281-b951-d872f2087c98

1 files changed, 1 insertions, 5 deletions

diff --git a/webkit/glue/media/video_renderer_impl.cc b/webkit/glue/media/video_renderer_impl.cc
index 5ffd80e..9148baf 100644
--- a/webkit/glue/media/video_renderer_impl.cc
+++ b/webkit/glue/media/video_renderer_impl.cc
@@ -17,7 +17,7 @@ VideoRendererImpl::VideoRendererImpl(WebMediaPlayerImpl::Proxy* proxy,
       pts_logging_(pts_logging) {
   // TODO(hclam): decide whether to do the following line in this thread or
   // in the render thread.
-  proxy->SetVideoRenderer(this);
+  proxy_->SetVideoRenderer(this);
 }
 
 // static
@@ -55,13 +55,9 @@ bool VideoRendererImpl::OnInitialize(media::VideoDecoder* decoder) {
 }
 
 void VideoRendererImpl::OnStop() {
-  DCHECK(proxy_);
-  proxy_->SetVideoRenderer(NULL);
-  proxy_ = NULL;
 }
 
 void VideoRendererImpl::OnFrameAvailable() {
-  DCHECK(proxy_);
   proxy_->Repaint();
 }