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

content/browser/renderer_host/media/video_capture_device_impl.cc

 // Copyright 2013 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.
 
 #include "content/browser/renderer_host/media/video_capture_device_impl.h"
 
 #include "base/basictypes.h"
 #include "base/bind.h"
 #include "base/callback_forward.h"
 #include "base/callback_helpers.h"
@@ skipping 17 matching lines @@
 #include "ui/gfx/rect.h"
 
 namespace content {
 
 namespace {
 
 void DeleteCaptureMachineOnUIThread(
     scoped_ptr<VideoCaptureMachine> capture_machine) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   if (capture_machine) {
-    capture_machine->Stop();
     capture_machine.reset();
   }
 }
 
 }  // namespace
 
 ThreadSafeCaptureOracle::ThreadSafeCaptureOracle(
     scoped_ptr<media::VideoCaptureDevice::Client> client,
     scoped_ptr<VideoCaptureOracle> oracle,
     const media::VideoCaptureParams& params)
@@ skipping 191 matching lines @@
 
   oracle_proxy_->Stop();
   oracle_proxy_ = NULL;
 
   TransitionStateTo(kIdle);
 
   // Stops the capture machine asynchronously.
   BrowserThread::PostTask(
       BrowserThread::UI, FROM_HERE, base::Bind(
           &VideoCaptureMachine::Stop,
-          base::Unretained(capture_machine_.get())));
+          base::Unretained(capture_machine_.get()),
+          base::Bind(&base::DoNothing)));
 }
 
 void VideoCaptureDeviceImpl::CaptureStarted(bool success) {
   DCHECK(thread_checker_.CalledOnValidThread());
   if (!success) {
     DVLOG(1) << "Failed to start capture machine.";
     Error();
   }
 }
 
 VideoCaptureDeviceImpl::VideoCaptureDeviceImpl(
     scoped_ptr<VideoCaptureMachine> capture_machine)
     : state_(kIdle),
       capture_machine_(capture_machine.Pass()) {}
 
 VideoCaptureDeviceImpl::~VideoCaptureDeviceImpl() {
   // If capture_machine is not NULL, then we need to return to the UI thread to
   // safely stop the capture machine.
   if (capture_machine_) {
     BrowserThread::PostTask(
-        BrowserThread::UI, FROM_HERE, base::Bind(
-            &DeleteCaptureMachineOnUIThread, base::Passed(&capture_machine_)));
+        BrowserThread::UI, FROM_HERE,
+        base::Bind(&VideoCaptureMachine::Stop,
+                   base::Unretained(capture_machine_.get()),
+                   base::Bind(&DeleteCaptureMachineOnUIThread,
+                              base::Passed(&capture_machine_))));

[imcheng, 2014/01/09 01:15:17]

Not sure if this is problematic here - will VideoCaptureMachine::Stop be holding
a valid reference to the capture machine?

[Alpha Left Google, 2014/01/09 01:37:07]

when you call base::Bind(&DeleteCaptureMachineOnUIThread,
base::Passed(&capture_machine_)) then the ownership of CaptureMachine is held by
the callback created.

The problem here is the mixed use of capture_machine_.get() and
base::Passed(&capture_machine_). The latter clears the capture_machine_. The
consequence is you'll get undefined behavior.

What you can do is like this:
VideoCaptureMachine* capture_machine_ptr = capture_machine_.get();

and then
base::Unretained(capture_machine_ptr) and
base::Passed(&capture_machine_)

[imcheng, 2014/01/09 23:31:09]

Thanks for the explanation. I changed the code to get the raw pointer first.

On 2014/01/09 01:37:07, Alpha wrote:

   }
   DVLOG(1) << "VideoCaptureDeviceImpl@" << this << " destroying.";
 }
 
 void VideoCaptureDeviceImpl::TransitionStateTo(State next_state) {
   DCHECK(thread_checker_.CalledOnValidThread());
 
 #ifndef NDEBUG
   static const char* kStateNames[] = {
     "Idle", "Allocated", "Capturing", "Error"
@@ skipping 12 matching lines @@
     return;
 
   if (oracle_proxy_)
     oracle_proxy_->ReportError();
 
   StopAndDeAllocate();
   TransitionStateTo(kError);
 }
 
 }  // namespace content