Bug Fix: Desktop Capture Picker Not Modal

chrome/browser/extensions/api/desktop_capture/desktop_capture_base.cc

Index: chrome/browser/extensions/api/desktop_capture/desktop_capture_base.cc
diff --git a/chrome/browser/extensions/api/desktop_capture/desktop_capture_base.cc b/chrome/browser/extensions/api/desktop_capture/desktop_capture_base.cc
index 049a56b160d31a5e43666ae27df96f09b104d758..988b1dffee05ac846b90c7c78b0f45f253e1c311 100644
--- a/chrome/browser/extensions/api/desktop_capture/desktop_capture_base.cc
+++ b/chrome/browser/extensions/api/desktop_capture/desktop_capture_base.cc
@@ -17,6 +17,9 @@
 #include "chrome/browser/media/webrtc/media_capture_devices_dispatcher.h"
 #include "chrome/browser/media/webrtc/native_desktop_media_list.h"
 #include "chrome/browser/media/webrtc/tab_desktop_media_list.h"
+#include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/ui/browser_finder.h"
+#include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "components/version_info/version_info.h"
 #include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/render_process_host.h"
@@ -116,8 +119,16 @@ bool DesktopCaptureChooseDesktopMediaFunctionBase::Execute(
     return false;
   }
 
-  const gfx::NativeWindow parent_window =
-      web_contents->GetTopLevelNativeWindow();
+  gfx::NativeWindow parent_window = web_contents->GetTopLevelNativeWindow();
+  if (!parent_window) {
+    Browser* target_browser = chrome::FindAnyBrowser(

[Sergey Ulanov, 2016/10/24 21:37:31]

I'm not sure this is the right thing to do. This basically picks a random
window, which may be confusing in some cases, as the result may be a window that
is not the one the user currently interracts with (e.g. you click extension
button in one browser, dialog pops up in another one). I think it's better to
use the last active tabbed window for that profile.

The video in the bug shows extension button clicked. Why
GetTopLevelNativeWindow() doesn't return correct window in that case?

[qiangchen, 2016/10/24 22:18:39]

The chooseDesktopMedia is from the background Javascript of the extension, and
logically, the extension is not part of the tab it is interacting with, and
further the extension seems not to belong to any browser window. So
|web_contents->getTopLevelWindow| return null.

+        Profile::FromBrowserContext(web_contents->GetBrowserContext()), false);
+
+    content::WebContents* target_contents =

[Sergey Ulanov, 2016/10/24 21:37:31]

I don't think you need to get target content. Instead you can use 
target_browser->window()->GetNativeWindow() to get the NativeWindow for the
browser.

[qiangchen, 2016/10/24 22:18:39]

Done.

+        target_browser->tab_strip_model()->GetActiveWebContents();

[Sergey Ulanov, 2016/10/24 21:37:31]

Need to check that target_browser isn't nullptr.

[qiangchen, 2016/10/24 22:18:40]

Done.

+
+    parent_window = target_contents->GetTopLevelNativeWindow();
+  }
   std::unique_ptr<DesktopMediaList> screen_list;
   std::unique_ptr<DesktopMediaList> window_list;
   std::unique_ptr<DesktopMediaList> tab_list;