Attempt to fixed crash on stop mirroring by stopping render thread on a worker thread.

content/browser/renderer_host/media/web_contents_video_capture_device.cc

 // Copyright (c) 2012 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.
 //
 // Implementation notes: This needs to work on a variety of hardware
 // configurations where the speed of the CPU and GPU greatly affect overall
 // performance. Spanning several threads, the process of capturing has been
 // split up into four conceptual stages:
 //
 //   1. Reserve Buffer: Before a frame can be captured, a slot in the client's
@@ skipping 90 matching lines @@
 
 // Wrapper function to invoke ThreadSafeCaptureOracle::CaptureFrameCallback, is
 // compatible with RenderWidgetHostViewFrameSubscriber::DeliverFrameCallback.
 void InvokeCaptureFrameCallback(
     const ThreadSafeCaptureOracle::CaptureFrameCallback& capture_frame_cb,
     base::TimeTicks timestamp,
     bool frame_captured) {
   capture_frame_cb.Run(timestamp, frame_captured);
 }
 
+void DeleteOnWorkerThread(scoped_ptr<base::Thread> render_thread,
+                          const base::Closure& callback) {
+  render_thread.reset();
+
+  // After thread join call the callback on UI thread.
+  BrowserThread::PostTask(BrowserThread::UI, FROM_HERE, callback);
+}
+
 // FrameSubscriber is a proxy to the ThreadSafeCaptureOracle that's compatible
 // with RenderWidgetHostViewFrameSubscriber. We create one per event type.
 class FrameSubscriber : public RenderWidgetHostViewFrameSubscriber {
  public:
   FrameSubscriber(VideoCaptureOracle::Event event_type,
                   const scoped_refptr<ThreadSafeCaptureOracle>& oracle)
       : event_type_(event_type),
         oracle_proxy_(oracle) {}
 
   virtual bool ShouldCaptureFrame(
@@ skipping 83 matching lines @@
     : public VideoCaptureMachine,
       public WebContentsObserver,
       public base::SupportsWeakPtr<WebContentsCaptureMachine> {
  public:
   WebContentsCaptureMachine(int render_process_id, int render_view_id);
   virtual ~WebContentsCaptureMachine();
 
   // VideoCaptureMachine overrides.
   virtual bool Start(
       const scoped_refptr<ThreadSafeCaptureOracle>& oracle_proxy) OVERRIDE;
-  virtual void Stop() OVERRIDE;
+  virtual void Stop(const base::Closure& callback) OVERRIDE;
 
   // Starts a copy from the backing store or the composited surface. Must be run
   // on the UI BrowserThread. |deliver_frame_cb| will be run when the operation
   // completes. The copy will occur to |target|.
   //
   // This may be used as a ContentCaptureSubscription::CaptureCallback.
   void Capture(const base::TimeTicks& start_time,
                const scoped_refptr<media::VideoFrame>& target,
                const RenderWidgetHostViewFrameSubscriber::DeliverFrameCallback&
                    deliver_frame_cb);
@@ skipping 53 matching lines @@
   // after any change to the WebContents that affects the RenderWidgetHost or
   // attached views.
   void RenewFrameSubscription();
 
   // Parameters saved in constructor.
   const int initial_render_process_id_;
   const int initial_render_view_id_;
 
   // A dedicated worker thread on which SkBitmap->VideoFrame conversion will
   // occur. Only used when this activity cannot be done on the GPU.
-  base::Thread render_thread_;
+  scoped_ptr<base::Thread> render_thread_;
 
   // Makes all the decisions about which frames to copy, and how.
   scoped_refptr<ThreadSafeCaptureOracle> oracle_proxy_;
 
   // Routing ID of any active fullscreen render widget or MSG_ROUTING_NONE
   // otherwise.
   int fullscreen_widget_id_;
 
   // Last known RenderView size.
   gfx::Size last_view_size_;
@@ skipping 245 matching lines @@
       count_frames_rendered_ = 0;
       last_frame_number_ = frame_number;
     }
   }
 }
 
 WebContentsCaptureMachine::WebContentsCaptureMachine(int render_process_id,
                                                      int render_view_id)
     : initial_render_process_id_(render_process_id),
       initial_render_view_id_(render_view_id),
-      render_thread_("WebContentsVideo_RenderThread"),
       fullscreen_widget_id_(MSG_ROUTING_NONE) {}
 
-WebContentsCaptureMachine::~WebContentsCaptureMachine() {}
+WebContentsCaptureMachine::~WebContentsCaptureMachine() {
+  BrowserThread::PostBlockingPoolTask(
+      FROM_HERE,
+      base::Bind(&DeleteOnWorkerThread, base::Passed(&render_thread_),
+                 base::Bind(&base::DoNothing)));
+}
 
 bool WebContentsCaptureMachine::Start(
     const scoped_refptr<ThreadSafeCaptureOracle>& oracle_proxy) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   DCHECK(!started_);
 
   DCHECK(oracle_proxy.get());
   oracle_proxy_ = oracle_proxy;
 
-  if (!render_thread_.Start()) {
+  render_thread_.reset(new base::Thread("WebContentsVideo_RenderThread"));
+  if (!render_thread_->Start()) {
     DVLOG(1) << "Failed to spawn render thread.";
     return false;

[miu, 2014/01/13 23:25:10]

Add a render_thread_.reset() here before the return statement.

[imcheng, 2014/01/14 02:06:52]

Done. It seems it's also needed below in "if (!StartObservingWebContents()) {
... }" so I added it there as well. Let me know if it isn't the case.

   }
 
   if (!StartObservingWebContents())
     return false;
 
   started_ = true;
   return true;
 }
 
-void WebContentsCaptureMachine::Stop() {
+void WebContentsCaptureMachine::Stop(const base::Closure& callback) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   subscription_.reset();
   if (web_contents()) {
     web_contents()->DecrementCapturerCount();
     Observe(NULL);
   }
-  render_thread_.Stop();
+
+  // The render thread cannot be stopped on the UI thread, so post a message
+  // to the thread pool used for blocking operations.
+  BrowserThread::PostBlockingPoolTask(
+      FROM_HERE,
+      base::Bind(&DeleteOnWorkerThread, base::Passed(&render_thread_),
+                 callback));
+
   started_ = false;
 }
 
 void WebContentsCaptureMachine::Capture(
     const base::TimeTicks& start_time,
     const scoped_refptr<media::VideoFrame>& target,
     const RenderWidgetHostViewFrameSubscriber::DeliverFrameCallback&
         deliver_frame_cb) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
@@ skipping 102 matching lines @@
 void WebContentsCaptureMachine::DidCopyFromBackingStore(
     const base::TimeTicks& start_time,
     const scoped_refptr<media::VideoFrame>& target,
     const RenderWidgetHostViewFrameSubscriber::DeliverFrameCallback&
         deliver_frame_cb,
     bool success,
     const SkBitmap& bitmap) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
   base::TimeTicks now = base::TimeTicks::Now();
-  if (success) {
+  if (success && render_thread_ && render_thread_->message_loop_proxy()) {

[miu, 2014/01/13 23:22:14]

nit: This should be a DCHECK(render_thread_.get()) before the if-statement
instead, since CopyFromBackingStore() shouldn't be invoked after
subscription_.reset() is called in the WCCaptureMachine::Stop() method.

[imcheng, 2014/01/14 02:06:52]

Done.

     UMA_HISTOGRAM_TIMES("TabCapture.CopyTimeBitmap", now - start_time);
     TRACE_EVENT_ASYNC_STEP_INTO0("mirroring", "Capture", target.get(),
                                  "Render");
-    render_thread_.message_loop_proxy()->PostTask(FROM_HERE, base::Bind(
+    render_thread_->message_loop_proxy()->PostTask(FROM_HERE, base::Bind(
         &RenderVideoFrame, bitmap, target,
         base::Bind(deliver_frame_cb, start_time)));
   } else {
     // Capture can fail due to transient issues, so just skip this frame.
     DVLOG(1) << "CopyFromBackingStore failed; skipping frame.";
     deliver_frame_cb.Run(start_time, false);
   }
 }
 
 void WebContentsCaptureMachine::DidCopyFromCompositingSurfaceToVideoFrame(
@@ skipping 57 matching lines @@
     scoped_ptr<Client> client) {
   DVLOG(1) << "Allocating " << params.requested_format.frame_size.ToString();
   impl_->AllocateAndStart(params, client.Pass());
 }
 
 void WebContentsVideoCaptureDevice::StopAndDeAllocate() {
   impl_->StopAndDeAllocate();
 }
 
 }  // namespace content