Fix race when setting Desktop window id.

content/browser/renderer_host/media/video_capture_manager.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.
 
 #include "content/browser/renderer_host/media/video_capture_manager.h"
 
 #include <algorithm>
 #include <set>
 
 #include "base/bind.h"
@@ skipping 262 matching lines @@
 void VideoCaptureManager::DoStopDevice(DeviceEntry* entry) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   DCHECK(std::find(devices_.begin(), devices_.end(), entry) != devices_.end());
 
   // Find the matching start request.
   for (DeviceStartQueue::reverse_iterator request =
            device_start_queue_.rbegin();
        request != device_start_queue_.rend(); ++request) {
     if (request->serial_id() == entry->serial_id) {
       request->set_abort_start();
+      // This is a CHECK to make sure the media::VideoCaptureDevice is not
+      // deleted on the wrong thread if |entry| is deleted.
+      CHECK(entry->video_capture_device() == NULL);

[Sergey Ulanov, 2015/02/05 18:52:06]

I think it's better to add a CHECK in ~DesktopCaptureDevice(), in case there are
other code paths that can delete it on a wrong thread.

[perkj_chrome, 2015/02/06 13:04:26]

How about in ~DeviceEntry to catch all types of capturers?

       DVLOG(3) << "DoStopDevice, aborting start request for device "
                << entry->id << " serial_id = " << entry->serial_id;
       return;
     }
   }
 
   DVLOG(3) << "DoStopDevice. Send stop request for device = " << entry->id
            << " serial_id = " << entry->serial_id << ".";
   if (entry->video_capture_device()) {
     // |entry->video_capture_device| can be null if creating the device fails.
@@ skipping 350 matching lines @@
     return;
   }
 
   // Post |existing_device->video_capture_device| to the VideoCaptureDevice to
   // the device_task_runner_. This is safe since the device is destroyed on the
   // device_task_runner_.
   device_task_runner_->PostTask(
       FROM_HERE,
       base::Bind(&VideoCaptureManager::SetDesktopCaptureWindowIdOnDeviceThread,
                  this,
                  existing_device->video_capture_device(),

[Sergey Ulanov, 2015/02/05 18:52:06]

can this be NULL?

[perkj_chrome, 2015/02/06 13:04:26]

Hmm, maybe that is the issue. 
video_capture_device is set on the device thread when the capturer is started.
But if this is called just after StartCaptureForClient, video_capturer_device()
will return null and this would fail.

It worked better before since DeviceEntry* was then posted to the device thread
and thus that would never happen.

                  window_id));
 }
 
 void VideoCaptureManager::DoStopDeviceOnDeviceThread(
     scoped_ptr<media::VideoCaptureDevice> device) {
   SCOPED_UMA_HISTOGRAM_TIMER("Media.VideoCaptureManager.StopDeviceTime");
   DCHECK(IsOnDeviceThread());
   device->StopAndDeAllocate();
   DVLOG(3) << "DoStopDeviceOnDeviceThread";
 }
@@ skipping 190 matching lines @@
     gfx::NativeViewId window_id) {
   DCHECK(IsOnDeviceThread());
   DCHECK(notification_window_ids_.find(session_id) ==
          notification_window_ids_.end());
   notification_window_ids_[session_id] = window_id;
   VLOG(2) << "Screen capture notification window saved for session "
           << session_id << " on device thread.";
 }
 
 }  // namespace content