New issue
Advanced search Search tips

Issue 729425 link

Starred by 2 users

Issue metadata

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



Sign in to add a comment

mash_browser_tests fails ExperimentalAppWindowApiTest.SetIcon after my cl

Project Member Reported by msw@chromium.org, Jun 5 2017

Issue description

mash_browser_tests fails ExperimentalAppWindowApiTest.SetIcon after my cl

Here's the CL that causes the failure, the cause isn't obvious:
  https://codereview.chromium.org/2918223002/
I haven't been able to reproduce the same failure locally.
I'm going to un-whitelist the test for now and investigate soon.

Here's the trybot failure log link and relevant output:
https://luci-logdog.appspot.com/v/?s=chromium%2Fbb%2Ftryserver.chromium.linux%2Flinux_chromium_chromeos_ozone_rel_ng%2F399593%2F%2B%2Frecipes%2Fsteps%2Fmash_browser_tests__with_patch_%2F0%2Flogs%2FExperimentalAppWindowApiTest.SetIcon%2F0

[25754:25754:0604/152348.156365:FATAL:shelf_model.cc(120)] Check failed: ItemIndexByID(item.id) == -1 (2 vs. -1) The id is not unique: app_id:nmgadokdjbfnhdjdmaihehhdigpdhpoi, launch_id:
#0 0x000003787eec base::debug::StackTrace::StackTrace()
#1 0x0000037a0a61 logging::LogMessage::~LogMessage()
#2 0x000006368a90 ash::ShelfModel::AddAt()
#3 0x000006522200 ChromeLauncherController::OnShelfItemAdded()
#4 0x000002a25994 ash::mojom::ShelfObserverStubDispatch::Accept()
 

 

Comment 1 by msw@chromium.org, Jun 5 2017

Components: UI>Shell>Shelf
Project Member

Comment 2 by bugdroid1@chromium.org, Jun 5 2017

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

commit 70ac45fc6ef3bc0acb2260a5aba251a13afd6f15
Author: msw <msw@chromium.org>
Date: Mon Jun 05 02:02:45 2017

mash: Make ShelfWindowWatcher items for unknown windows.

Restore some ShelfWindowWatcher functionality for Mash:
Assign default shelf ids and item types to windows.
(respect the WindowState::ignored_by_shelf flag)
(use TYPE_DIALOG to avoid Chrome setting an icon, etc.)
(ignore windows with transient parents)
Update the items if an app (ie. Chrome) claims the windows.

Inline GetShelfItemIndexForWindow helper function.
Improve ShelfController and CLC logging.
Add transient, ignored_by_shelf, and mash unit tests.

TODO: Make ignored_by_shelf a window property to fix observation.
(WindowState changes don't notify; StatusBubble gets an item...)

BUG= 557406 , 729425 
TEST=chrome --mash shows a shelf item for QuickLaunch.
R=sky@chromium.org

Review-Url: https://codereview.chromium.org/2918223002
Cr-Commit-Position: refs/heads/master@{#476922}

[modify] https://crrev.com/70ac45fc6ef3bc0acb2260a5aba251a13afd6f15/ash/shelf/shelf_controller.cc
[modify] https://crrev.com/70ac45fc6ef3bc0acb2260a5aba251a13afd6f15/ash/shelf/shelf_window_watcher.cc
[modify] https://crrev.com/70ac45fc6ef3bc0acb2260a5aba251a13afd6f15/ash/shelf/shelf_window_watcher.h
[modify] https://crrev.com/70ac45fc6ef3bc0acb2260a5aba251a13afd6f15/ash/shelf/shelf_window_watcher_unittest.cc
[modify] https://crrev.com/70ac45fc6ef3bc0acb2260a5aba251a13afd6f15/chrome/browser/ui/ash/launcher/chrome_launcher_controller.cc
[modify] https://crrev.com/70ac45fc6ef3bc0acb2260a5aba251a13afd6f15/testing/buildbot/filters/mash.browser_tests.filter
[modify] https://crrev.com/70ac45fc6ef3bc0acb2260a5aba251a13afd6f15/testing/buildbot/filters/mojo.fyi.browser_tests.filter

Project Member

Comment 3 by bugdroid1@chromium.org, Jun 8 2017

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

commit feae432eb3cfeafbd4c30795171ac46462a23c80
Author: msw <msw@chromium.org>
Date: Thu Jun 08 04:24:22 2017

mash: Limit ShelfWindowWatcher to panels and dialogs.

Chrome now syncs its own ShelfModel with Ash in Mash.
Avoid ShelfWindowWatcher and ChromeLauncherController clashing.
(ie. don't create SWW items for windows that CLC will handle)

Set TYPE_APP for extension and arc app windows to avoid SWW items.
Set TYPE_APP_PANEL in extension code; simplify ChromeNativeAppWindowViewsAuraAsh.

Make SWW items for transient windows (per discussion w/oshima).
Only make SWW items for WINDOW_TYPE_NORMAL windows w/o a ShelfItemType.
(avoid making items for [transient] controls, popups, etc.)

Only make SWW items for visible windows (or minimized panels).
(clients set a ShelfItemType before Show() to avoid SWW items)
(fixes StatusAreaBubble, prior to WindowState::ignored_by_shelf)

Expand and refine ShelfWindowWatcher unit tests.
Fix simulated panel window creation in other unit tests.
Restore ExperimentalAppWindowApiTest.SetIcon to mash white lists.
Disable WindowSelectorTest.MultipleDisplays in mash for now.

TODO: Determine why OnAppWindowAdded is only called in Mash...

BUG= 557406 , 679087 , 695562 , 729425 , 730759 
TEST=Automated; chrome --mash doesn't crash opening the wallpaper picker.
R=sky@chromium.org
TBR=jamescook@chromium.org

Review-Url: https://codereview.chromium.org/2927693002
Cr-Commit-Position: refs/heads/master@{#477884}

[modify] https://crrev.com/feae432eb3cfeafbd4c30795171ac46462a23c80/ash/shelf/shelf_window_watcher.cc
[modify] https://crrev.com/feae432eb3cfeafbd4c30795171ac46462a23c80/ash/shelf/shelf_window_watcher_unittest.cc
[modify] https://crrev.com/feae432eb3cfeafbd4c30795171ac46462a23c80/ash/wm/overview/window_selector_unittest.cc
[modify] https://crrev.com/feae432eb3cfeafbd4c30795171ac46462a23c80/ash/wm/panels/panel_layout_manager_unittest.cc
[modify] https://crrev.com/feae432eb3cfeafbd4c30795171ac46462a23c80/ash/wm/panels/panel_window_resizer_unittest.cc
[modify] https://crrev.com/feae432eb3cfeafbd4c30795171ac46462a23c80/chrome/browser/ui/ash/launcher/arc_app_window_launcher_controller.cc
[modify] https://crrev.com/feae432eb3cfeafbd4c30795171ac46462a23c80/chrome/browser/ui/ash/launcher/extension_app_window_launcher_controller.cc
[modify] https://crrev.com/feae432eb3cfeafbd4c30795171ac46462a23c80/chrome/browser/ui/ash/launcher/extension_app_window_launcher_controller.h
[modify] https://crrev.com/feae432eb3cfeafbd4c30795171ac46462a23c80/chrome/browser/ui/views/apps/chrome_native_app_window_views_aura_ash.cc
[modify] https://crrev.com/feae432eb3cfeafbd4c30795171ac46462a23c80/testing/buildbot/filters/mash.browser_tests.filter
[modify] https://crrev.com/feae432eb3cfeafbd4c30795171ac46462a23c80/testing/buildbot/filters/mojo.fyi.browser_tests.filter

Comment 4 by vadimt@chromium.org, Nov 28 2017

Labels: Not-Touch-Friendly-Launcher

Comment 5 by msw@chromium.org, Feb 7 2018

Status: WontFix (was: Assigned)
This test appears to be running and passing on the Mojo_ChromiumOS FYI bot:
https://ci.chromium.org/buildbot/chromium.fyi/Mojo%20ChromiumOS/26729

Sign in to add a comment