New issue
Advanced search Search tips

Issue 811940 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Feb 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Bug



Sign in to add a comment

WidgetTest::GetAllWidgets() returns an empty set with mus

Project Member Reported by ellyjo...@chromium.org, Feb 13 2018

Issue description

WidgetTest::GetAllWidgets() seems to always return an empty set with Mus. For example, the new test added here: <https://chromium-review.googlesource.com/c/chromium/src/+/906890> gets an empty set of widgets back.
 

Comment 1 by sky@chromium.org, Feb 14 2018

Owner: msw@chromium.org
Project Member

Comment 2 by bugdroid1@chromium.org, Feb 16 2018

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

commit 4e890408553814249eb8788ff5bb5ef419d815f7
Author: Mike Wasserman <msw@chromium.org>
Date: Fri Feb 16 05:22:54 2018

Fix ContentSettingBubbleContentsBrowserTest to work on Chrome OS.

Add plumbing to check if a content setting bubble is showing.
Use that to test for bubble presence, not GetAllWidgets.
See http://crrev.com/c/922268 for GetAllWidgets changes.

Bug:  811940 
Change-Id: I5b8e36c5dcddb8dfd407ad0f889ad74fb9804273
Reviewed-on: https://chromium-review.googlesource.com/919922
Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org>
Commit-Queue: Michael Wasserman <msw@chromium.org>
Cr-Commit-Position: refs/heads/master@{#537212}
[modify] https://crrev.com/4e890408553814249eb8788ff5bb5ef419d815f7/chrome/browser/ui/cocoa/location_bar/content_setting_decoration.h
[modify] https://crrev.com/4e890408553814249eb8788ff5bb5ef419d815f7/chrome/browser/ui/cocoa/location_bar/content_setting_decoration.mm
[modify] https://crrev.com/4e890408553814249eb8788ff5bb5ef419d815f7/chrome/browser/ui/cocoa/location_bar/location_bar_view_mac.h
[modify] https://crrev.com/4e890408553814249eb8788ff5bb5ef419d815f7/chrome/browser/ui/cocoa/location_bar/location_bar_view_mac.mm
[modify] https://crrev.com/4e890408553814249eb8788ff5bb5ef419d815f7/chrome/browser/ui/location_bar/location_bar.h
[modify] https://crrev.com/4e890408553814249eb8788ff5bb5ef419d815f7/chrome/browser/ui/views/content_setting_bubble_contents_browsertest.cc
[modify] https://crrev.com/4e890408553814249eb8788ff5bb5ef419d815f7/chrome/browser/ui/views/location_bar/content_setting_image_view.h
[modify] https://crrev.com/4e890408553814249eb8788ff5bb5ef419d815f7/chrome/browser/ui/views/location_bar/location_bar_view.cc
[modify] https://crrev.com/4e890408553814249eb8788ff5bb5ef419d815f7/chrome/browser/ui/views/location_bar/location_bar_view.h

Project Member

Comment 3 by bugdroid1@chromium.org, Feb 16 2018

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

commit 8ce0e3727f0e58bc90a40d4710097d7099a3279a
Author: Mike Wasserman <msw@chromium.org>
Date: Fri Feb 16 17:09:14 2018

Chrome OS: Avoid GetAllWidgets in non-mash browser tests.

Avoid GetAllWidgets in classic ash and mus Chrome OS browser tests.
These configs cannot get a list of root windows at the Views layer.
Add a DCHECK to avoid using GetAllWidgets in those scenarios.

Win/Linux/Mac query their WMs directly for root windows; on Chrome OS:
1) Mash can use MusClient to get a list of root windows.
2) Unit tests can use AuraTestHelper to get the root window.
3) Non-mash browser tests must use ash::Shell to get root windows.

This should land after http://crrev.com/c/919922

Bug:  811940 
Change-Id: I609531529ff5c4379f6c1de47ca878b30e7dd614
Reviewed-on: https://chromium-review.googlesource.com/922268
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Michael Wasserman <msw@chromium.org>
Cr-Commit-Position: refs/heads/master@{#537331}
[modify] https://crrev.com/8ce0e3727f0e58bc90a40d4710097d7099a3279a/chrome/browser/ui/test/test_browser_dialog.cc
[modify] https://crrev.com/8ce0e3727f0e58bc90a40d4710097d7099a3279a/ui/views/test/widget_test_aura.cc

Comment 4 by msw@chromium.org, Feb 16 2018

Status: Fixed (was: Assigned)
Unfortunately, the answer for now is that WidgetTest::GetAllWidgets is not supported in Chrome OS non-mash browser tests. If we need similar functionality in multiple such browser tests, we can make a helper that uses ash::Shell::GetAllRootWindows for classic ash and mus Chrome OS configs, but hopefully that's not needed.
Components: -Internals>MUS Internals>Services>WindowService

Sign in to add a comment