Fix crash in MediaCaptureDevicesDispatcher::ProcessScreenCaptureAccessRequest.

chrome/browser/media/media_capture_devices_dispatcher.cc

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..0eaee418c7778a8db2cdf5fb537bfeb08585d82b 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,13 @@ 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);

[Ami GONE FROM CHROMIUM, 2013/12/10 20:18:28]

Please follow this with 
web_contents = NULL;
to be safe.

[hshi1, 2013/12/10 20:22:37]

Done.

+
     // For component extensions, bypass message box.
     bool user_approved = false;
     if (!component_extension) {
@@ -350,24 +381,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());