Resolve the problems where we can leak the system tray UI

content/browser/renderer_host/media/media_stream_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/media_stream_manager.h"
 
 #include <list>
 
 #include "base/bind.h"
 #include "base/compiler_specific.h"
@@ skipping 160 matching lines @@
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
 
   // Create a new request based on options.
   DeviceRequest new_request(requester, options,
                             render_process_id,
                             render_view_id,
                             security_origin);
   StartEnumeration(&new_request, label);
 }
 
-void MediaStreamManager::CancelRequests(MediaStreamRequester* requester) {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
-  DeviceRequests::iterator it = requests_.begin();
-  while (it != requests_.end()) {
-    if (it->second.requester == requester && !RequestDone(it->second)) {
-      // The request isn't complete, but there might be some devices already
-      // opened -> close them.
-      DeviceRequest* request = &(it->second);
-      if (request->state[content::MEDIA_STREAM_DEVICE_TYPE_AUDIO_CAPTURE] ==
-          DeviceRequest::kOpening) {
-        for (StreamDeviceInfoArray::iterator it =
-             request->audio_devices.begin(); it != request->audio_devices.end();
-             ++it) {
-          audio_input_device_manager()->Close(it->session_id);
-        }
-      }
-      if (request->state[content::MEDIA_STREAM_DEVICE_TYPE_VIDEO_CAPTURE] ==
-          DeviceRequest::kOpening) {
-        for (StreamDeviceInfoArray::iterator it =
-             request->video_devices.begin(); it != request->video_devices.end();
-             ++it) {
-          video_capture_manager()->Close(it->session_id);
-        }
-      }
-      requests_.erase(it++);
-    } else {
-      ++it;
-    }
-  }
-}
-
 void MediaStreamManager::CancelGenerateStream(const std::string& label) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
 
   DeviceRequests::iterator it = requests_.find(label);
   if (it != requests_.end()) {
     // The request isn't complete.
     if (!RequestDone(it->second)) {
       DeviceRequest* request = &(it->second);
       if (request->state[content::MEDIA_STREAM_DEVICE_TYPE_AUDIO_CAPTURE] ==
           DeviceRequest::kOpening) {
@@ skipping 27 matching lines @@
     for (StreamDeviceInfoArray::iterator audio_it =
          it->second.audio_devices.begin();
          audio_it != it->second.audio_devices.end(); ++audio_it) {
       audio_input_device_manager()->Close(audio_it->session_id);
     }
     for (StreamDeviceInfoArray::iterator video_it =
          it->second.video_devices.begin();
          video_it != it->second.video_devices.end(); ++video_it) {
       video_capture_manager()->Close(video_it->session_id);
     }
-    if (it->second.type == DeviceRequest::kGenerateStream) {
+
+    if (it->second.type == DeviceRequest::kGenerateStream &&
+        RequestDone(it->second)) {
       NotifyObserverDevicesClosed(&(it->second));
     }
     requests_.erase(it);
     return;
   }
 }
 
 void MediaStreamManager::EnumerateDevices(
     MediaStreamRequester* requester,
     int render_process_id,
@@ skipping 71 matching lines @@
   // Create a label for this request and verify it is unique.
   std::string request_label;
   do {
     request_label = RandomLabel();
   } while (requests_.find(request_label) != requests_.end());
 
   requests_.insert(std::make_pair(request_label, *new_request));
 
   // Get user confirmation to use capture devices.
   // Need to make an asynchronous call to make sure the |requester| gets the
   // |label| before it would receive any event.

[wjia(left Chromium), 2012/08/10 01:23:13]

remove the comments about posting task.

[no longer working on chromium, 2012/08/10 11:50:56]

Done.

   if (new_request->type == DeviceRequest::kGenerateStream) {
-    BrowserThread::PostTask(BrowserThread::IO,
-        FROM_HERE,
-        base::Bind(&MediaStreamDeviceSettings::RequestCaptureDeviceUsage,
-                   base::Unretained(device_settings_.get()),
-                   request_label, new_request->render_process_id,
-                   new_request->render_view_id, new_request->options,
-                   new_request->security_origin));
+    device_settings_->RequestCaptureDeviceUsage(request_label,
+                                                new_request->render_process_id,
+                                                new_request->render_view_id,
+                                                new_request->options,
+                                                new_request->security_origin);
   }
 
   (*label) = request_label;
 }
 
 void MediaStreamManager::EnsureDeviceThreadAndListener() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
   if (device_thread_.get())
     return;
 
@@ skipping 117 matching lines @@
             request.state[device_it->stream_type] = DeviceRequest::kOpening;
             if (stream_type == content::MEDIA_STREAM_DEVICE_TYPE_AUDIO_CAPTURE)
               request.audio_devices.push_back(device);
             else
               request.video_devices.push_back(device);
             break;
           }
         }
         break;
       default:
-        BrowserThread::PostTask(BrowserThread::IO,
-            FROM_HERE,
-            base::Bind(&MediaStreamDeviceSettings::AvailableDevices,
-                       base::Unretained(device_settings_.get()),
-                       *it, stream_type, devices));
+        device_settings_->AvailableDevices(*it, stream_type, devices);
     }
   }
   label_list.clear();
   enumeration_in_progress_[stream_type] = false;
 }
 
 void MediaStreamManager::Error(MediaStreamType stream_type,
                                int capture_session_id,
                                MediaStreamProviderError error) {
   // Find the device for the error call.
@@ skipping 18 matching lines @@
           // 1. Already opened -> signal device failure and close device.
           //    Use device_idx to signal which of the devices encountered an
           //    error.
           if (stream_type == content::MEDIA_STREAM_DEVICE_TYPE_AUDIO_CAPTURE) {
             it->second.requester->AudioDeviceFailed(it->first, device_idx);
           } else if (stream_type ==
                      content::MEDIA_STREAM_DEVICE_TYPE_VIDEO_CAPTURE) {
             it->second.requester->VideoDeviceFailed(it->first, device_idx);
           }
           GetDeviceManager(stream_type)->Close(capture_session_id);
-          devices->erase(device_it);
-        } else if (it->second.audio_devices.size()
-            + it->second.video_devices.size() <= 1) {
-          // 2. Device not opened and no other devices for this request ->
-          //    signal stream error and remove the request.
-          it->second.requester->StreamGenerationFailed(it->first);
-          requests_.erase(it);
+          // We don't erase the devices here so that we can update the UI
+          // properly in StopGeneratedStream().
+          it->second.state[stream_type] = DeviceRequest::kError;
         } else {
-          // 3. Not opened but other devices exists for this request -> remove
-          //    device from list, but don't signal an error.
-          devices->erase(device_it);
+          // Request is not done, devices are not opened in this case.
+          if (it->second.audio_devices.size()
+              + it->second.video_devices.size() <= 1) {
+            // 2. Device not opened and no other devices for this request ->
+            //    signal stream error and remove the request.
+            it->second.requester->StreamGenerationFailed(it->first);
+            requests_.erase(it);
+          } else {
+            // 3. Not opened but other devices exists for this request -> remove
+            //    device from list, but don't signal an error.
+            devices->erase(device_it);
+          }

[wjia(left Chromium), 2012/08/10 01:23:13]

why this "else if" format change?

[no longer working on chromium, 2012/08/10 11:50:56]

It is more readable in this way, because:
The first "if" is checking the it->second.state[stream_type], and the second
"else if" is checking the sizes of the devices, they don't belong to the same
scope, so it should be better to separate them.

         }
         return;
       }
     }
   }
 }
 
 void MediaStreamManager::DevicesAccepted(const std::string& label,
                                          const StreamDeviceInfoArray& devices) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
@@ skipping 118 matching lines @@
         content::MediaStreamDevice(
             content::MEDIA_STREAM_DEVICE_TYPE_VIDEO_CAPTURE,
             it->device_id,
             it->name));
   }
 }
 
 bool MediaStreamManager::RequestDone(const DeviceRequest& request) const {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
   // Check if all devices are opened.
-  if (Requested(request.options,
-                content::MEDIA_STREAM_DEVICE_TYPE_AUDIO_CAPTURE)) {
+  MediaStreamType stream_type = content::MEDIA_STREAM_DEVICE_TYPE_AUDIO_CAPTURE;
+  if (Requested(request.options, stream_type)) {
+    if (request.state[stream_type] != DeviceRequest::kDone &&
+        request.state[stream_type] != DeviceRequest::kError)
+      return false;
+
     for (StreamDeviceInfoArray::const_iterator it =

[mflodman_chromium_OOO, 2012/08/10 07:15:44]

See comment in patch set #2.

[no longer working on chromium, 2012/08/10 11:50:56]

After discussing offline, we decided to leave it as it is now, and remove the
in_use flag on a separate CL.

          request.audio_devices.begin(); it != request.audio_devices.end();
          ++it) {
       if (it->in_use == false) {
         return false;
       }
     }
   }
-  if (Requested(request.options,
-                content::MEDIA_STREAM_DEVICE_TYPE_VIDEO_CAPTURE)) {
+
+  stream_type = content::MEDIA_STREAM_DEVICE_TYPE_VIDEO_CAPTURE;
+  if (Requested(request.options, stream_type)) {
+    if (request.state[stream_type] != DeviceRequest::kDone &&
+        request.state[stream_type] != DeviceRequest::kError)
+      return false;
+
     for (StreamDeviceInfoArray::const_iterator it =
          request.video_devices.begin(); it != request.video_devices.end();
          ++it) {
       if (it->in_use == false) {
         return false;
       }
     }
   }
+
   return true;
 }
 
 // Called to get media capture device manager of specified type.
 MediaStreamProvider* MediaStreamManager::GetDeviceManager(
     MediaStreamType stream_type) {
   if (stream_type == content::MEDIA_STREAM_DEVICE_TYPE_VIDEO_CAPTURE) {
     return video_capture_manager();
   } else if (stream_type == content::MEDIA_STREAM_DEVICE_TYPE_AUDIO_CAPTURE) {
     return audio_input_device_manager();
   }
   NOTREACHED();
   return NULL;
 }
 
 }  // namespace media_stream