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, 7 insertions, 3 deletions

diff --git a/media/filters/video_renderer_base.h b/media/filters/video_renderer_base.h
index d18d6a1..e3af186 100644
--- a/media/filters/video_renderer_base.h
+++ b/media/filters/video_renderer_base.h
@@ -1,6 +1,6 @@
-// Copyright (c) 2009 The Chromium Authors. All rights reserved.  Use of this
-// source code is governed by a BSD-style license that can be found in the
-// LICENSE file.
+// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
 
 // VideoRendererBase creates its own thread for the sole purpose of timing frame
 // presentation.  It handles reading from the decoder and stores the results in
@@ -73,6 +73,10 @@ class VideoRendererBase : public VideoRenderer,
   // Implementors should avoid doing any sort of heavy work in this method and
   // instead post a task to a common/worker thread to handle rendering.  Slowing
   // down the video thread may result in losing synchronization with audio.
+  //
+  // IMPORTANT: This method is called on the video renderer thread, which is
+  // different from the thread OnInitialize(), OnStop(), and the rest of the
+  // class executes on.
   virtual void OnFrameAvailable() = 0;
 
  private: