Fix DesktopMediaPickerViews to handle background parent_web_contents

chrome/browser/ui/views/desktop_media_picker_views.cc

Index: chrome/browser/ui/views/desktop_media_picker_views.cc
diff --git a/chrome/browser/ui/views/desktop_media_picker_views.cc b/chrome/browser/ui/views/desktop_media_picker_views.cc
index 2b16808a357909576408c927cd6fedbdd048d409..5d0214429cd6e3ccc8f43db8fc6ed22f039549c5 100644
--- a/chrome/browser/ui/views/desktop_media_picker_views.cc
+++ b/chrome/browser/ui/views/desktop_media_picker_views.cc
@@ -15,6 +15,7 @@
 #include "components/web_modal/popup_manager.h"
 #include "components/web_modal/web_contents_modal_dialog_manager.h"
 #include "content/public/browser/browser_thread.h"
+#include "content/public/browser/web_contents_delegate.h"
 #include "ui/aura/window_tree_host.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/events/event_constants.h"
@@ -407,26 +408,25 @@ DesktopMediaPickerDialogView::DesktopMediaPickerDialogView(
       GetMediaListViewHeightForRows(1), GetMediaListViewHeightForRows(2));
   AddChildView(scroll_view_);
 
-  // If |parent_web_contents| is set, the picker will be shown modal to the
-  // web contents. Otherwise, a new dialog widget inside |parent_window| will be
-  // created for the picker. Note that |parent_window| may also be NULL if
-  // parent web contents is not set. In this case the picker will be parented
-  // by a root window.
+  // If |parent_web_contents| is set and it's not a background page then the
+  // picker will be shown modal to the web contents. Otherwise the picker is
+  // shown in a separate window.
   views::Widget* widget = NULL;
-  if (parent_web_contents)
+  bool modal_dialog =
+      parent_web_contents &&
+      !parent_web_contents->GetDelegate()->IsNeverVisible(parent_web_contents);
+  if (modal_dialog) {
     widget = CreateWebModalDialogViews(this, parent_web_contents);
-  else
-    widget = DialogDelegate::CreateDialogWidget(this, context, parent_window);
+  } else {
+    widget = DialogDelegate::CreateDialogWidget(this, context, NULL);
+  }
 
-  // DesktopMediaList needs to know the ID of the picker window which
-  // matches the ID it gets from the OS. Depending on the OS and configuration
-  // we get this ID differently.
+  // DesktopMediaList needs to know the ID of the picker window so it can filter
+  // the picker window from the list of available sources. Filtering is not

[Wez, 2014/08/25 21:32:02]

nit: ID -> Id
nit: ... so it can be filtered from the list of available sources.

"it" refers to the picker window (or possibly its Id), not to the
DesktopMediaList.

[Sergey Ulanov, 2014/08/25 22:03:52]

Done.

+  // necessary when the picker is shown as a modal dialog, because in that case
+  // it's embedded in the parent window.

[Wez, 2014/08/25 21:32:02]

I think it'd be clearer to express *why* we need to filter here, first, then
explain the logic, e.g:

"If the picker is not modal to the calling web contents then it is displayed in
its own top-level window, so that window must be filtered out of the list of
top-level windows available for capture."

[Sergey Ulanov, 2014/08/25 22:03:52]

Done.

   DesktopMediaID::Id dialog_window_id = 0;
-
-  // If there is |parent_window| or |parent_web_contents|, the picker window
-  // is embedded in the parent and does not have its own native window id, so we
-  // do not filter in that case.
-  if (!parent_window && !parent_web_contents) {
+  if (!modal_dialog) {
 #if defined(USE_ASH)
     if (chrome::IsNativeWindowInAsh(widget->GetNativeWindow())) {
       dialog_window_id =
@@ -443,7 +443,7 @@ DesktopMediaPickerDialogView::DesktopMediaPickerDialogView(
 
   list_view_->StartUpdating(dialog_window_id);
 
-  if (parent_web_contents) {
+  if (modal_dialog) {
     web_modal::PopupManager* popup_manager =
         web_modal::PopupManager::FromWebContents(parent_web_contents);
     popup_manager->ShowModalDialog(GetWidget()->GetNativeView(),