New issue
Advanced search Search tips

Issue 657883 link

Starred by 4 users

Issue metadata

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

Blocking:
issue 671916


Show other hotlists

Hotlists containing this issue:
MacViews-Task-Queue


Sign in to add a comment

MacViews: Build more views-specific unit_tests (and make them pass)

Reported by mbl...@yandex-team.ru, Oct 20 2016

Issue description

UserAgent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_11_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/53.0.2785.101 YaBrowser/16.10.0.1732 (beta) Safari/537.36

Steps to reproduce the problem:
Currently there's a bunch of tests that are marked !is_mac, even though they mostly compile fine on mac_views_browser.

What is the expected behavior?

What went wrong?
We should properly separate Aura-dependent tests and make the rest compile on mac_views_browser.

Did this work before? N/A 

Chrome version: 53.0.2785.101  Channel: n/a
OS Version: OS X 10.11.6
Flash Version:
 

Comment 1 by meh...@chromium.org, Oct 20 2016

Components: Internals>Views

Comment 2 by tapted@chromium.org, Oct 21 2016

Labels: phase4 Proj-MacViews
Status: Available (was: Unconfirmed)
Summary: MacViews: Build more views-specific unit_tests (and make them pass) (was: MacViews: Build more views-specific unit_tests)
Following on from https://codereview.chromium.org/2440743002/, there will be some failures to fix:
 - ChooserDialogViewTest.*
 - ConfirmBubbleViewsTest.CreateAndClose
 - ToolbarActionsBarBubbleViewsTest.*
 - TranslateManagerRenderViewHostTest.*

Also some files that need to be added to the sources list
 - "../browser/ui/views/crypto_module_password_dialog_view_unittest.cc",
 - "../browser/ui/views/desktop_capture/desktop_media_picker_views_unittest.cc",
 - "../browser/ui/views/desktop_media_picker_views_deprecated_unittest.cc",
 - "../browser/ui/views/first_run_bubble_unittest.cc",

Comment 3 by tapted@chromium.org, Oct 21 2016

(some specific notes)

ChooserDialogViewTest does
    dialog_ = views::DialogDelegate::CreateDialogWidget(chooser_dialog_view.release(), GetContext(), nullptr);

That `nullptr` is passing a null parent window, but ChooserDialogView::GetModalType() says it is `ui::MODAL_TYPE_CHILD`, so a DCHECK is hit on Mac.

ToolbarActionsBarBubbleViewsTest has some close/lifetime problems.
Project Member

Comment 4 by bugdroid1@chromium.org, Oct 21 2016

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

commit ac6e6ef42555b47257d2e7bc9b7672794c011899
Author: mblsha <mblsha@yandex-team.ru>
Date: Fri Oct 21 12:40:28 2016

MacViews: Build more unit_tests.

I've enabled the tests that compile and link with mac_views_browser enabled.

BUG=657883

Review-Url: https://chromiumcodereview.appspot.com/2440743002
Cr-Commit-Position: refs/heads/master@{#426776}

[modify] https://crrev.com/ac6e6ef42555b47257d2e7bc9b7672794c011899/chrome/test/BUILD.gn

Comment 5 by tapted@chromium.org, Dec 12 2016

Blocking: 671916
Labels: MacViews-Cleanup
Project Member

Comment 7 by sheriffbot@chromium.org, Apr 12 2018

Labels: Hotlist-Recharge-Cold
Status: Untriaged (was: Available)
This issue has been Available for over a year. If it's no longer important or seems unlikely to be fixed, please consider closing it out. If it is important, please re-triage the issue.

Sorry for the inconvenience if the bug really should have been left as Available.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: M-69 Target-69
Owner: robliao@chromium.org
Status: Assigned (was: Untriaged)
Labels: -Pri-2 Pri-3
Triaging P3 for now. This is certainly a nice to have.
Project Member

Comment 11 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

Project Member

Comment 12 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

Project Member

Comment 13 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-Tests
Labels: M-69
Labels: -M-69 -Target-69 M-70 Target-70
Labels: Hotlist-DesktopUIChecked Hotlist-DesktopUIToolingRequired

Sign in to add a comment