New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 763961 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Sep 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug



Sign in to add a comment

Fullscreen applist broken in mash.

Project Member Reported by msw@chromium.org, Sep 11 2017

Issue description

Chrome Version: 63.0.3207.0
(1) run chrome --mash
(2) click the app list button on the shelf
Expected: The app list window shows.
Actual: Graphical glitch shows, or nothing happens.

See the screenshot from 63.0.3207.0, jamescook@ sees nothing on device.
Alex, PTAL or pass back to me if you're not up for it, thanks.
 
mash_app_list.png
21.6 KB View Download
Labels: Touch-Friendly-Launcher
Status: Started (was: Assigned)
I'll take a look!
Getting this error on showing the launcher:

[195452:195452:0911/153405.263384:ERROR:layer_tree_host_impl.cc(2408)] Forcing zero-copy tile initialization as worker context is missing

Comment 3 by msw@chromium.org, Sep 11 2017

That may not be related to this defect, it's also output in many other cases:
- Showing a new Chrome window via Ctrl+N
- Showing the Files app
- Showing a context menu on a tab or bookmark[bar]
- Showing the chrome app menu
Cc: fsam...@chromium.org
Fady, could this be a compositor problem?

Alex, does the app list do anything fancy with the background, like blur? On my beta channel chromebook it looks like just transparency, nothing tricky.

jamescook@ we use blur, but I tested this with and without blur, and in both configurations it is broken.(you can turn blur off by going to app_list_features.cc and setting the IsBackgroundBlurEnabled flag, or chrome://flags and search for "background blur".

I think the problem is that we don't properly set the AppList's ParentWindow.

I see this difference between Ash[1] and Mash[2] launchers.

This is because we pass nullptr to AppListView's initialize function[3]

Could this be caused because we don't ever set the parent window in AppListView?

Is there a proper way in Mash to get the AppListContainer(or the proper root window in this case)? I tried doing it the "normal" way, and ended up learning a lot about mash!
 

[1]
In Ash:
RootWindow-0 (0x3f1c234d27a0) type=0  visible 0,0 1366x768, subpixel offset=[0.000000 0.000000]
   ScreenRotationContainer (0x3f1c231b14a0) type=0  visible 0,0 1366x768, subpixel offset=[0.000000 0.000000]
      WallpaperContainer (0x3f1c234191a0) type=0  visible 0,0 1366x768, subpixel offset=[0.000000 0.000000]
         WallpaperView (0x3f1c25d61c20) type=2  visible 0,0 1366x768, subpixel offset=[0.000000 0.000000]
      NonLockScreenContainersContainer (0x3f1c234d2da0) type=0  visible 0,0 1366x768, subpixel offset=[0.000000 0.000000]
         UnparentedControlContainer (0x3f1c23419aa0) type=0  0,0 1366x768, subpixel offset=[0.000000 0.000000]
         DefaultContainer (0x3f1c234d24a0) type=0  visible 0,0 1366x768 [snapped] , subpixel offset=[0.000000 0.000000]
            "" (0x3f1c2527a1a0) type=1  visible 0,0 1366x720 [snapped] , subpixel offset=[0.000000 0.000000]
               NativeViewHostAuraClip (0x3f1c25763048) type=3  visible 0,105 1366x615, subpixel offset=[0.000000 0.000000]
                  WebContentsViewAura (0x3f1c253464a0) type=3  visible 0,0 1366x615, subpixel offset=[0.000000 0.000000]
                     RenderWidgetHostViewAura (0x3f1c257ad4a0) type=3  visible 0,0 1366x615, subpixel offset=[0.000000 0.000000]
            StatusBubble (0x3f1c25f4c620) type=2  -1,699 455x22 [snapped] , subpixel offset=[0.000000 0.000000]
         AlwaysOnTopContainer (0x3f1c23419320) type=0  visible 0,0 1366x768 [snapped] , subpixel offset=[0.000000 0.000000]
         AppListContainer (0x3f1c231b17a0) type=0  visible 0,0 1366x768 [snapped] , subpixel offset=[0.000000 0.000000]
            AppList (0x3f1c25e887a0) type=2 [active]  visible 0,448 1366x768 [snapped] , subpixel offset=[0.000000 0.000000]
               SearchBoxView (0x3f1c266ffc20) type=3  visible 407,20 552x56, subpixel offset=[0.000000 0.000000]

[2]
In Mash: 
RootWindow-0 (0x13e61758ac20) type=0  visible 0,0 1024x768, subpixel offset=[0.000000 0.000000]
   ScreenRotationContainer (0x13e61758a320) type=0  visible 0,0 1024x768, subpixel offset=[0.000000 0.000000]
      WallpaperContainer (0x13e617a4a4a0) type=0  visible 0,0 1024x768, subpixel offset=[0.000000 0.000000]
         WallpaperView (0x13e617a3a4a0) type=2  visible 0,0 1024x768, subpixel offset=[0.000000 0.000000]
      NonLockScreenContainersContainer (0x13e617a4a7a0) type=0  visible 0,0 1024x768, subpixel offset=[0.000000 0.000000]
         UnparentedControlContainer (0x13e617ac11a0) type=0  0,0 1024x768, subpixel offset=[0.000000 0.000000]
         DefaultContainer (0x13e617ac14a0) type=0  visible 0,0 1024x768 [snapped] , subpixel offset=[0.000000 0.000000]
            QuickLaunch (0x13e617b19aa0) type=1  visible 10,640 508x66 [snapped] , subpixel offset=[0.000000 0.000000]
               DesktopNativeWidgetAura - content window (0x13e617b3ac20) type=0  visible 0,0 508x66, subpixel offset=[0.000000 0.000000]
            BrowserFrame (0x13e617dcc320) type=1  visible 0,0 1024x720 [snapped] , subpixel offset=[0.000000 0.000000]
               DesktopNativeWidgetAura - content window (0x13e617dccda0) type=0  visible 0,0 1024x720, subpixel offset=[0.000000 0.000000]
                  NativeViewHostAuraClip (0x13e617b3a920) type=0  visible 0,105 1024x615, subpixel offset=[0.000000 0.000000]
                     WebContentsViewAura (0x13e617dcc7a0) type=0  visible 0,0 1024x615, subpixel offset=[0.000000 0.000000]
                        RenderWidgetHostViewAura (0x13e617dccaa0) type=0  visible 0,0 1024x615, subpixel offset=[0.000000 0.000000]
            StatusBubble (0x13e617a3a320) type=2  -1,699 341x22 [snapped] , subpixel offset=[0.000000 0.000000]
               DesktopNativeWidgetAura - content window (0x13e617a3a020) type=0  0,0 341x22, subpixel offset=[0.000000 0.000000]
            AppList (0x13e617d68020) type=2 [active]  visible 348,0 328x720 [snapped] , subpixel offset=[0.000000 0.000000]
               DesktopNativeWidgetAura - content window (0x13e617fb4aa0) type=0  visible -348,448 1024x768, subpixel offset=[0.000000 0.000000]
                  SearchBoxView (0x13e618040320) type=0  visible -112,20 552x56, subpixel offset=[0.000000 0.000000]
         AlwaysOnTopContainer (0x13e617ac17a0) type=0  visible 0,0 1024x768 [snapped] , subpixel offset=[0.000000 0.000000]
         AppListContainer (0x13e617ac1aa0) type=0  visible 0,0 1024x768 [snapped] , subpixel offset=[0.000000 0.000000]

[3]
https://cs.chromium.org/chromium/src/chrome/browser/ui/ash/app_list/app_list_presenter_delegate_mus.cc?type=cs&q=app_list_presenter_delegate_mus&sq=package:chromium&l=70


Diff between [1] and [2] is that AppList and it's children are [1] under AppListContainer, and [2] under DefaultContainer (while AppListContainer is empty).
Yeah, that seems like a problem.

For now to parent windows in mash, search for this https://cs.chromium.org/search/?q=kContainerId_InitProperty&type=cs

It'll show some examples of classic ash vs. mash window parenting. (The issue is that chrome browser, as "just an app", has no knowledge of the window manager's complete window hierarchy. So it can't just grab an arbitrary aura::Window* to use as a parent -- some are owned by the ash process.)

This is only a problem because the app list is created by the browser process. Once it moves into ash process then the app list will be able to grab any aura::Window* it wants.

I'm going to have to postpone working on this because M61/M62 issues have come up. I'll comment here again when I pick it back up.
Cc: -jamescook@chromium.org newcomer@chromium.org
Owner: jamescook@chromium.org
I'll take a look.

Project Member

Comment 10 by bugdroid1@chromium.org, Sep 26 2017

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

commit ebb057907a7021a089e42a5d51d77a16558c4f24
Author: James Cook <jamescook@chromium.org>
Date: Tue Sep 26 00:10:07 2017

cros: Fix fullscreen app list under mash

* Place app list widget in the ash app list window container, instead of
  in the container for browser windows.
* Perform widget bounds computations in local coordinates. This avoids
  problems with screen-to-local computations under mash where the native
  window's parent is the desktop widget, not the window container as in
  classic ash.
* Use side shelf mode in mash, which skips some problematic opening
  animations. See  issue 768437 .
* Introduce AppListView::InitParams so we don't have 2 ints and 2 bools
  as adjacent parameters.
* Eliminate some unused Init() method params.

Bug:  763961 ,  768437 
Test: app_list_unittests, manual testing of fullscreen app list with multiple monitors (stacked vertically) and/or side shelf mode.

Change-Id: I933f5130feab3d8b4edfc2b573f52748e9c82539
Reviewed-on: https://chromium-review.googlesource.com/682343
Commit-Queue: James Cook <jamescook@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#504226}
[modify] https://crrev.com/ebb057907a7021a089e42a5d51d77a16558c4f24/ash/app_list/app_list_presenter_delegate.cc
[modify] https://crrev.com/ebb057907a7021a089e42a5d51d77a16558c4f24/chrome/browser/ui/ash/app_list/app_list_presenter_delegate_mus.cc
[modify] https://crrev.com/ebb057907a7021a089e42a5d51d77a16558c4f24/ui/app_list/BUILD.gn
[modify] https://crrev.com/ebb057907a7021a089e42a5d51d77a16558c4f24/ui/app_list/DEPS
[modify] https://crrev.com/ebb057907a7021a089e42a5d51d77a16558c4f24/ui/app_list/presenter/app_list_presenter_impl_unittest.cc
[modify] https://crrev.com/ebb057907a7021a089e42a5d51d77a16558c4f24/ui/app_list/views/app_list_main_view.cc
[modify] https://crrev.com/ebb057907a7021a089e42a5d51d77a16558c4f24/ui/app_list/views/app_list_main_view.h
[modify] https://crrev.com/ebb057907a7021a089e42a5d51d77a16558c4f24/ui/app_list/views/app_list_main_view_unittest.cc
[modify] https://crrev.com/ebb057907a7021a089e42a5d51d77a16558c4f24/ui/app_list/views/app_list_view.cc
[modify] https://crrev.com/ebb057907a7021a089e42a5d51d77a16558c4f24/ui/app_list/views/app_list_view.h
[modify] https://crrev.com/ebb057907a7021a089e42a5d51d77a16558c4f24/ui/app_list/views/app_list_view_unittest.cc
[modify] https://crrev.com/ebb057907a7021a089e42a5d51d77a16558c4f24/ui/app_list/views/apps_grid_view_unittest.cc
[modify] https://crrev.com/ebb057907a7021a089e42a5d51d77a16558c4f24/ui/app_list/views/search_box_view_unittest.cc
[modify] https://crrev.com/ebb057907a7021a089e42a5d51d77a16558c4f24/ui/app_list/views/search_result_page_view_unittest.cc

Status: Fixed (was: Started)
It now works well enough for testing apps / shelf stuff. It currently skips the peeking and half states under mash -- fixing that is  issue 768437 .

Comment 12 by dchan@chromium.org, Jan 22 2018

Status: Archived (was: Fixed)

Comment 13 by dchan@chromium.org, Jan 23 2018

Status: Fixed (was: Archived)

Sign in to add a comment