Skip to content
This repository has been archived by the owner on Apr 3, 2020. It is now read-only.

Commit

Permalink
Fix DesktopMediaPickerViews to handle background parent_web_contents
Browse files Browse the repository at this point in the history
After r283702 DesktopMediaPickerViews is getting parent_web_contents even
when it corresponds to a background extension page.
DesktopMediaPickerViews was always trying to create a modal dialog when
web_contents is specified, and that doesn't work for background pages.
Now it explicitly checks if the web_contents is a background page and
in that case shows the picker in a separate window.

Also updated example for desktop capture API to make it easier to test
this scenario.

BUG=402579

Review URL: https://codereview.chromium.org/501713002

Cr-Commit-Position: refs/heads/master@{#292055}
(cherry picked from commit 71b32fc)

TBR=sergeyu@chromium.org

Review URL: https://codereview.chromium.org/522483007

Cr-Commit-Position: refs/branch-heads/2125@{#179}
Cr-Branched-From: b68026d-refs/heads/master@{#290040}
  • Loading branch information
SergeyUlanov committed Sep 2, 2014
1 parent a1cc1f3 commit 6debe58
Show file tree
Hide file tree
Showing 6 changed files with 38 additions and 23 deletions.
38 changes: 18 additions & 20 deletions chrome/browser/ui/views/desktop_media_picker_views.cc
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include "chrome/grit/generated_resources.h"
#include "components/web_modal/popup_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"
Expand Down Expand Up @@ -358,7 +359,6 @@ void DesktopMediaListView::OnSourceThumbnailChanged(int index) {
DesktopMediaPickerDialogView::DesktopMediaPickerDialogView(
content::WebContents* parent_web_contents,
gfx::NativeWindow context,
gfx::NativeWindow parent_window,
DesktopMediaPickerViews* parent,
const base::string16& app_name,
const base::string16& target_name,
Expand All @@ -384,26 +384,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.
// If the picker is not modal to the calling web contents then it is displayed
// in its own top-level window, so in that case it needs to be filtered out of
// the list of top-level windows available for capture, and to achieve that
// the Id is passed to DesktopMediaList.
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 =
Expand All @@ -420,7 +419,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(),
Expand Down Expand Up @@ -555,8 +554,7 @@ void DesktopMediaPickerViews::Show(content::WebContents* web_contents,
const DoneCallback& done_callback) {
callback_ = done_callback;
dialog_ = new DesktopMediaPickerDialogView(
web_contents, context, parent, this, app_name, target_name,
media_list.Pass());
web_contents, context, this, app_name, target_name, media_list.Pass());
}

void DesktopMediaPickerViews::NotifyDialogResult(DesktopMediaID source) {
Expand Down
1 change: 0 additions & 1 deletion chrome/browser/ui/views/desktop_media_picker_views.h
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,6 @@ class DesktopMediaPickerDialogView : public views::DialogDelegateView {
public:
DesktopMediaPickerDialogView(content::WebContents* parent_web_contents,
gfx::NativeWindow context,
gfx::NativeWindow parent_window,
DesktopMediaPickerViews* parent,
const base::string16& app_name,
const base::string16& target_name,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,8 @@ class DesktopMediaPickerViewsTest : public testing::Test {

picker_views_.reset(new DesktopMediaPickerViews());
picker_views_->Show(NULL,
NULL,
parent_widget_->GetNativeWindow(),
NULL,
app_name,
app_name,
media_list.PassAs<DesktopMediaList>(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,3 +38,9 @@ document.querySelector('#cancel').addEventListener('click', function(e) {
chrome.desktopCapture.cancelChooseDesktopMedia(pending_request_id);
}
});

document.querySelector('#startFromBackgroundPage')
.addEventListener('click', function(e) {
chrome.runtime.sendMessage(
{}, function(response) { console.log(response.farewell); });
});
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,11 @@ chrome.app.runtime.onLaunched.addListener(function() {
}
});
});

chrome.runtime.onMessage.addListener(function(message, sender, sendResponse) {
chrome.desktopCapture.chooseDesktopMedia(
["screen", "window"],
function(id) {
sendResponse({"id": id});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,11 @@
</head>
<body>
<video id="video" autoplay></video>
<p><button id="start">Start</button><button id="cancel">Cancel</button></p>
<p>
<button id="start">Start</button>
<button id="cancel">Cancel</button>
<button id="startFromBackgroundPage">Start from background page</button>
</p>
<script src="app.js"></script>
</body>
</html>

0 comments on commit 6debe58

Please sign in to comment.