Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(4030)

Unified Diff: chrome/browser/media/media_capture_devices_dispatcher.cc

Issue 98233009: Fix crash in MediaCaptureDevicesDispatcher::ProcessScreenCaptureAccessRequest. (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: Invalid the |web_contents| pointer just to be safe. Created 7 years ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: chrome/browser/media/media_capture_devices_dispatcher.cc
diff --git a/chrome/browser/media/media_capture_devices_dispatcher.cc b/chrome/browser/media/media_capture_devices_dispatcher.cc
index e1fc969d999e3b04771d10ced9767307f49a7b13..42806996b06cb650dd6a988f493af8e6ad6e07f4 100644
--- a/chrome/browser/media/media_capture_devices_dispatcher.cc
+++ b/chrome/browser/media/media_capture_devices_dispatcher.cc
@@ -111,6 +111,38 @@ string16 GetApplicationTitle(content::WebContents* web_contents,
return UTF8ToUTF16(title);
}
+// Helper to get list of media stream devices for desktop capture in |devices|.
+// Registers to display notification if |display_notification| is true.
+// Returns an instance of MediaStreamUI to be passed to content layer.
+scoped_ptr<content::MediaStreamUI> GetDevicesForDesktopCapture(
+ content::MediaStreamDevices& devices,
+ content::DesktopMediaID media_id,
+ bool capture_audio,
+ bool display_notification,
+ base::string16 application_title) {
+ DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+ scoped_ptr<content::MediaStreamUI> ui;
+
+ // Add selected desktop source to the list.
+ devices.push_back(content::MediaStreamDevice(
+ content::MEDIA_DESKTOP_VIDEO_CAPTURE, media_id.ToString(), "Screen"));
+ if (capture_audio) {
+ // Use the special loopback device ID for system audio capture.
+ devices.push_back(content::MediaStreamDevice(
+ content::MEDIA_LOOPBACK_AUDIO_CAPTURE,
+ media::AudioManagerBase::kLoopbackInputDeviceId, "System Audio"));
+ }
+
+ // If required, register to display the notification for stream capture.
+ if (display_notification) {
+ ui = ScreenCaptureNotificationUI::Create(l10n_util::GetStringFUTF16(
+ IDS_MEDIA_SCREEN_CAPTURE_NOTIFICATION_TEXT,
+ application_title));
+ }
+
+ return ui.Pass();
+}
+
} // namespace
MediaCaptureDevicesDispatcher::PendingAccessRequest::PendingAccessRequest(
@@ -251,11 +283,6 @@ void MediaCaptureDevicesDispatcher::ProcessDesktopCaptureAccessRequest(
return;
}
- // Add selected desktop source to the list.
- devices.push_back(content::MediaStreamDevice(
- content::MEDIA_DESKTOP_VIDEO_CAPTURE, media_id.ToString(),
- std::string()));
-
bool loopback_audio_supported = false;
#if defined(USE_CRAS) || defined(OS_WIN)
// Currently loopback audio capture is supported only on Windows and ChromeOS.
@@ -263,17 +290,14 @@ void MediaCaptureDevicesDispatcher::ProcessDesktopCaptureAccessRequest(
#endif
// Audio is only supported for screen capture streams.
- if (media_id.type == content::DesktopMediaID::TYPE_SCREEN &&
- request.audio_type == content::MEDIA_LOOPBACK_AUDIO_CAPTURE &&
- loopback_audio_supported) {
- devices.push_back(content::MediaStreamDevice(
- content::MEDIA_LOOPBACK_AUDIO_CAPTURE,
- media::AudioManagerBase::kLoopbackInputDeviceId, "System Audio"));
- }
+ bool capture_audio =
+ (media_id.type == content::DesktopMediaID::TYPE_SCREEN &&
+ request.audio_type == content::MEDIA_LOOPBACK_AUDIO_CAPTURE &&
+ loopback_audio_supported);
- ui = ScreenCaptureNotificationUI::Create(l10n_util::GetStringFUTF16(
- IDS_MEDIA_SCREEN_CAPTURE_NOTIFICATION_TEXT,
- GetApplicationTitle(web_contents, extension)));
+ ui = GetDevicesForDesktopCapture(
+ devices, media_id, capture_audio, true,
+ GetApplicationTitle(web_contents, extension));
callback.Run(devices, ui.Pass());
}
@@ -330,6 +354,14 @@ void MediaCaptureDevicesDispatcher::ProcessScreenCaptureAccessRequest(
// the given origin.
// 2. Request comes from a page with a secure origin or from an extension.
if (screen_capture_enabled && origin_is_secure) {
+ // Get title of the calling application prior to showing the message box.
+ // chrome::ShowMessageBox() starts a nested message loop which may allow
+ // |web_contents| to be destroyed on the UI thread before the message box
+ // is closed. See http://crbug.com/326690.
+ base::string16 application_title =
+ GetApplicationTitle(web_contents, extension);
+ web_contents = NULL;
+
// For component extensions, bypass message box.
bool user_approved = false;
if (!component_extension) {
@@ -350,24 +382,17 @@ void MediaCaptureDevicesDispatcher::ProcessScreenCaptureAccessRequest(
}
if (user_approved || component_extension) {
- devices.push_back(content::MediaStreamDevice(
- content::MEDIA_DESKTOP_VIDEO_CAPTURE, media_id.ToString(), "Screen"));
- if (request.audio_type == content::MEDIA_LOOPBACK_AUDIO_CAPTURE &&
- loopback_audio_supported) {
- // Use the special loopback device ID for system audio capture.
- devices.push_back(content::MediaStreamDevice(
- content::MEDIA_LOOPBACK_AUDIO_CAPTURE,
- media::AudioManagerBase::kLoopbackInputDeviceId, "System Audio"));
- }
- }
- }
+ bool capture_audio =
+ (request.audio_type == content::MEDIA_LOOPBACK_AUDIO_CAPTURE &&
+ loopback_audio_supported);
- // Unless we're being invoked from a component extension, register to display
- // the notification for stream capture.
- if (!devices.empty() && !component_extension) {
- ui = ScreenCaptureNotificationUI::Create(l10n_util::GetStringFUTF16(
- IDS_MEDIA_SCREEN_CAPTURE_NOTIFICATION_TEXT,
- GetApplicationTitle(web_contents, extension)));
+ // Unless we're being invoked from a component extension, register to
+ // display the notification for stream capture.
+ bool display_notification = !component_extension;
+
+ ui = GetDevicesForDesktopCapture(devices, media_id, capture_audio,
+ display_notification, application_title);
+ }
}
callback.Run(devices, ui.Pass());
« no previous file with comments | « no previous file | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698