New issue
Advanced search Search tips

Issue 726005 link

Starred by 3 users

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 2
Type: Bug

Blocked on:
issue 852573

Blocking:
issue 630179
issue 730958



Sign in to add a comment

[MacViews] Wire up DesktopMediaPicker

Project Member Reported by ellyjo...@chromium.org, May 24 2017

Issue description

We need to use the Views media picker when --secondary-ui-md is supplied on Mac. This is a bit difficult since the current DesktopMediaPickerViews code assumes USE_AURA which is not true on Mac.
 
Summary: [MacViews] Wire up DesktopMediaPicker (was: Wire up DesktopMediaPicker)

Comment 2 by tapted@chromium.org, May 24 2017

Cc: qiangchen@chromium.org tapted@chromium.org
Blocking: 730958
I think we can punt these to a later phase. See  Issue 730958 .
Labels: M-X
Labels: -M-X -MacViews-Dialogs MacViews-Browser Target-68
Owner: robliao@chromium.org
Status: Assigned (was: Available)
MacViews triage: this is a MacViews-Browser bug now. I don't know whether it's better to keep using the Cocoa picker in the Views browser (surely not...?) or make the Views implementation not dependent on Aura.

Either way, assigning this to robliao@ - let's target M68 for this.

Comment 6 by gov...@chromium.org, Mar 27 2018

Labels: M-68

Comment 7 by gov...@chromium.org, Mar 29 2018

** Bulk Edit **

FYI: Starting 04/13 M68 will be in canary, M68 Dev promotion will be on 04/26.

Comment 8 by gov...@chromium.org, Apr 25 2018

Pls mark the bug as fixed if CL is landed in trunk and nothing else is pending. Thank you.
Labels: -Pri-2 -Target-68 Pri-1
Well, we missed M-68 for this and this would be a UI change, so making this Pri-1 to get this in before M69 feature freeze.
Status: Started (was: Assigned)
Blocking: 630179
Project Member

Comment 12 by bugdroid1@chromium.org, Jun 12 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ef538e3112d34f22bf36c50f71ec9c3108bda689

commit ef538e3112d34f22bf36c50f71ec9c3108bda689
Author: Robert Liao <robliao@chromium.org>
Date: Tue Jun 12 17:43:31 2018

Remove Unused aura/window.h Include in Desktop Media Picker Views Unittest

BUG=726005
TBR=ellyjones@chromium.org
Trivial include file removal.

Change-Id: I3e3e0238e77745343cb26ac9ea2761be7b6ccde0
Reviewed-on: https://chromium-review.googlesource.com/1097471
Reviewed-by: Robert Liao <robliao@chromium.org>
Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org>
Commit-Queue: Robert Liao <robliao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#566490}
[modify] https://crrev.com/ef538e3112d34f22bf36c50f71ec9c3108bda689/chrome/browser/ui/views/desktop_capture/desktop_media_picker_views_unittest.cc

Blockedon: 852573
Labels: Target-68
Project Member

Comment 16 by bugdroid1@chromium.org, Jun 14 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7830b6cc815ad9db04fb8493efe6abe31b94b23a

commit 7830b6cc815ad9db04fb8493efe6abe31b94b23a
Author: Robert Liao <robliao@chromium.org>
Date: Thu Jun 14 17:30:55 2018

Revert "Add Views DesktopMediaPicker into the Overall Views Build"

This reverts commit 170f8510730d63a686f53944c62d750eadc532f8.

Reason for revert: Causing some unit_test failures on Mac 10.10
https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Mac10.10%20Tests
starting around 33081


Original change's description:
> Add Views DesktopMediaPicker into the Overall Views Build
> 
> This moves the code into the toolkit_views source list and gates Aura
> specific code behind build flags.
> 
> BUG=726005,657883
> 
> Change-Id: I5e64ee0b16f2bfcb03da3bbc7c61486a786ee88d
> Reviewed-on: https://chromium-review.googlesource.com/1097843
> Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org>
> Commit-Queue: Robert Liao <robliao@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#566809}

TBR=ellyjones@chromium.org,robliao@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 726005, 657883
Change-Id: I4b8c6d38c1df4453ae1ee404f88a57046a908f35
Reviewed-on: https://chromium-review.googlesource.com/1101098
Reviewed-by: Robert Liao <robliao@chromium.org>
Commit-Queue: Robert Liao <robliao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#567327}
[modify] https://crrev.com/7830b6cc815ad9db04fb8493efe6abe31b94b23a/chrome/browser/ui/BUILD.gn
[modify] https://crrev.com/7830b6cc815ad9db04fb8493efe6abe31b94b23a/chrome/browser/ui/views/desktop_capture/desktop_media_list_view.cc
[modify] https://crrev.com/7830b6cc815ad9db04fb8493efe6abe31b94b23a/chrome/browser/ui/views/desktop_capture/desktop_media_picker_views.cc
[modify] https://crrev.com/7830b6cc815ad9db04fb8493efe6abe31b94b23a/chrome/browser/ui/views/desktop_capture/desktop_media_picker_views_unittest.cc
[modify] https://crrev.com/7830b6cc815ad9db04fb8493efe6abe31b94b23a/chrome/test/BUILD.gn

Labels: -M-68 -Target-68 Target-69 M-69
This is at risk for M69.
Project Member

Comment 18 by bugdroid1@chromium.org, Jun 20 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/59da623a9597b60252f1c657add234eecbb30918

commit 59da623a9597b60252f1c657add234eecbb30918
Author: Robert Liao <robliao@chromium.org>
Date: Wed Jun 20 23:55:26 2018

Add Views DesktopMediaPicker into the Overall Views Build

This moves the code into the toolkit_views source list and gates Aura
specific code behind build flags.

Additionally, this links up DesktopMediaPicker::Create in a manner
similar to ScreenCaptureNotificationUI::Create[Cocoa].

BUG=726005,657883
NOAUTOREVERT=true
Need to investigate any failure on the Mac Bot. Not reproducible locally.

Change-Id: I9ece0b875268495d70154378c7608c3e1bae2938
Reviewed-on: https://chromium-review.googlesource.com/1102042
Commit-Queue: Robert Liao <robliao@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#569095}
[modify] https://crrev.com/59da623a9597b60252f1c657add234eecbb30918/chrome/browser/ui/BUILD.gn
[add] https://crrev.com/59da623a9597b60252f1c657add234eecbb30918/chrome/browser/ui/cocoa/media_picker/create_desktop_media_picker_cocoa.h
[add] https://crrev.com/59da623a9597b60252f1c657add234eecbb30918/chrome/browser/ui/cocoa/media_picker/create_desktop_media_picker_cocoa.mm
[modify] https://crrev.com/59da623a9597b60252f1c657add234eecbb30918/chrome/browser/ui/cocoa/media_picker/desktop_media_picker_cocoa.mm
[modify] https://crrev.com/59da623a9597b60252f1c657add234eecbb30918/chrome/browser/ui/views/desktop_capture/desktop_media_list_view.cc
[modify] https://crrev.com/59da623a9597b60252f1c657add234eecbb30918/chrome/browser/ui/views/desktop_capture/desktop_media_picker_views.cc
[modify] https://crrev.com/59da623a9597b60252f1c657add234eecbb30918/chrome/browser/ui/views/desktop_capture/desktop_media_picker_views_unittest.cc
[modify] https://crrev.com/59da623a9597b60252f1c657add234eecbb30918/chrome/test/BUILD.gn

CL landed at #18. Is there anything pending for M69? Also is it still at risk for M69 per comment #17?
Labels: -Pri-1 -Target-69 Target-70 Pri-3
Owner: ----
Status: Available (was: Started)
punting out of birthday
Project Member

Comment 21 by bugdroid1@chromium.org, Jun 21 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7f6ea60ef53c47383bd2954a8b908c8eecd25f5f

commit 7f6ea60ef53c47383bd2954a8b908c8eecd25f5f
Author: Robert Liao <robliao@chromium.org>
Date: Thu Jun 21 21:17:20 2018

Revert "Add Views DesktopMediaPicker into the Overall Views Build"

This reverts commit 59da623a9597b60252f1c657add234eecbb30918.

Reason for revert: This work was just deprioritized, so the investigation into the bot only failures is now on the backburner.

Original change's description:
> Add Views DesktopMediaPicker into the Overall Views Build
> 
> This moves the code into the toolkit_views source list and gates Aura
> specific code behind build flags.
> 
> Additionally, this links up DesktopMediaPicker::Create in a manner
> similar to ScreenCaptureNotificationUI::Create[Cocoa].
> 
> BUG=726005,657883
> NOAUTOREVERT=true
> Need to investigate any failure on the Mac Bot. Not reproducible locally.
> 
> Change-Id: I9ece0b875268495d70154378c7608c3e1bae2938
> Reviewed-on: https://chromium-review.googlesource.com/1102042
> Commit-Queue: Robert Liao <robliao@chromium.org>
> Reviewed-by: Scott Violet <sky@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#569095}

TBR=ellyjones@chromium.org,sky@chromium.org,robliao@chromium.org

Change-Id: Ice2800851d19895e687525bc63b8afa9d1746517
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 726005, 657883
Reviewed-on: https://chromium-review.googlesource.com/1110897
Reviewed-by: Robert Liao <robliao@chromium.org>
Commit-Queue: Robert Liao <robliao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#569377}
[modify] https://crrev.com/7f6ea60ef53c47383bd2954a8b908c8eecd25f5f/chrome/browser/ui/BUILD.gn
[delete] https://crrev.com/cb8b61b491aad9c3b36dbfd705d52a7bfc3c2dda/chrome/browser/ui/cocoa/media_picker/create_desktop_media_picker_cocoa.h
[delete] https://crrev.com/cb8b61b491aad9c3b36dbfd705d52a7bfc3c2dda/chrome/browser/ui/cocoa/media_picker/create_desktop_media_picker_cocoa.mm
[modify] https://crrev.com/7f6ea60ef53c47383bd2954a8b908c8eecd25f5f/chrome/browser/ui/cocoa/media_picker/desktop_media_picker_cocoa.mm
[modify] https://crrev.com/7f6ea60ef53c47383bd2954a8b908c8eecd25f5f/chrome/browser/ui/views/desktop_capture/desktop_media_list_view.cc
[modify] https://crrev.com/7f6ea60ef53c47383bd2954a8b908c8eecd25f5f/chrome/browser/ui/views/desktop_capture/desktop_media_picker_views.cc
[modify] https://crrev.com/7f6ea60ef53c47383bd2954a8b908c8eecd25f5f/chrome/browser/ui/views/desktop_capture/desktop_media_picker_views_unittest.cc
[modify] https://crrev.com/7f6ea60ef53c47383bd2954a8b908c8eecd25f5f/chrome/test/BUILD.gn

Labels: -M-69 Group-Architecture
Labels: M-69
Labels: -M-69 M-70
Labels: -Pri-3 -Target-70 -M-70 Target-73 M-73 Pri-2
Owner: a...@chromium.org
Status: Assigned (was: Available)
Mac triage: over to avi@ - it would be great to get rid of the Cocoa media picker.
Labels: Hotlist-DesktopUIChecked Hotlist-DesktopUIToolingRequired
*** UI Mass Triage***

Sign in to add a comment