New issue
Advanced search Search tips

Issue 879591 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Sep 7
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug

Blocking:
issue 878599



Sign in to add a comment

AppListControllerImpl::OnVisibilityChanged never called laptop->tablet

Project Member Reported by manucornet@chromium.org, Aug 31

Issue description

Steps to repro:

* Open app list
* Switch to tablet mode

Note that the app list gets hidden but

    AppListControllerImpl::OnVisibilityChanged

never gets called, which causes some issues for other parts of the code that expect to be called back when the app list get hidden.
 
Blocking: 878599
Status: Started (was: Untriaged)
Project Member

Comment 3 by bugdroid1@chromium.org, Sep 6

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

commit b5ecbcc8e92fa86c4b62ec6d5f403555d15a141f
Author: Alex Newcomer <newcomer@chromium.org>
Date: Thu Sep 06 22:06:37 2018

cros: Update shelf background when going to tablet mode

The bug is that when transitioning to tablet mode with a window
(which will expand to fullscreen) open and a shown AppList results in
a stale shelf background.

This is a corner case, where the AppList does not change in visibility,
it remains visible and the shelf does not know to re-evaluate its
background.

Maybe the shelf should observe active windows instead of relying on
AppList visibility state changes?

For now, we fix this corner case by forcing the shelf to re-evaluate
it's background when the launcher stays open during the tablet mode
transition.

Test=automated and manual

Bug:  879591 
Change-Id: I71c8d97672824853b0edac77e1a91734ba148af0
Reviewed-on: https://chromium-review.googlesource.com/1200408
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Commit-Queue: Alex Newcomer <newcomer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#589329}
[modify] https://crrev.com/b5ecbcc8e92fa86c4b62ec6d5f403555d15a141f/ash/app_list/app_list_controller_impl.cc
[modify] https://crrev.com/b5ecbcc8e92fa86c4b62ec6d5f403555d15a141f/ash/shelf/shelf.cc
[modify] https://crrev.com/b5ecbcc8e92fa86c4b62ec6d5f403555d15a141f/ash/shelf/shelf.h
[modify] https://crrev.com/b5ecbcc8e92fa86c4b62ec6d5f403555d15a141f/ash/shelf/shelf_layout_manager.cc
[modify] https://crrev.com/b5ecbcc8e92fa86c4b62ec6d5f403555d15a141f/ash/shelf/shelf_layout_manager.h
[modify] https://crrev.com/b5ecbcc8e92fa86c4b62ec6d5f403555d15a141f/ash/shelf/shelf_layout_manager_unittest.cc

Labels: Merge-Request-70
Status: Fixed (was: Started)
Project Member

Comment 5 by sheriffbot@chromium.org, Sep 8

Labels: -Merge-Request-70 Hotlist-Merge-Approved Merge-Approved-70
Your change meets the bar and is auto-approved for M70. Please go ahead and merge the CL to branch 3538 manually. Please contact milestone owner if you have questions.
Owners: benmason@(Android), kariahda@(iOS), geohsu@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 6 by bugdroid1@chromium.org, Sep 11

Labels: -merge-approved-70 merge-merged-3538
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ab28209ef75facaad5967fe4290879096de0ccf9

commit ab28209ef75facaad5967fe4290879096de0ccf9
Author: Alex Newcomer <newcomer@chromium.org>
Date: Tue Sep 11 21:39:57 2018

cros: Update shelf background when going to tablet mode

The bug is that when transitioning to tablet mode with a window
(which will expand to fullscreen) open and a shown AppList results in
a stale shelf background.

This is a corner case, where the AppList does not change in visibility,
it remains visible and the shelf does not know to re-evaluate its
background.

Maybe the shelf should observe active windows instead of relying on
AppList visibility state changes?

For now, we fix this corner case by forcing the shelf to re-evaluate
it's background when the launcher stays open during the tablet mode
transition.

Test=automated and manual

TBR=newcomer@chromium.org

(cherry picked from commit b5ecbcc8e92fa86c4b62ec6d5f403555d15a141f)

Bug:  879591 
Change-Id: I71c8d97672824853b0edac77e1a91734ba148af0
Reviewed-on: https://chromium-review.googlesource.com/1200408
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Commit-Queue: Alex Newcomer <newcomer@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#589329}
Reviewed-on: https://chromium-review.googlesource.com/1220691
Reviewed-by: Alex Newcomer <newcomer@chromium.org>
Cr-Commit-Position: refs/branch-heads/3538@{#298}
Cr-Branched-From: 79f7c91a2b2a2932cd447fa6f865cb6662fa8fa6-refs/heads/master@{#587811}
[modify] https://crrev.com/ab28209ef75facaad5967fe4290879096de0ccf9/ash/app_list/app_list_controller_impl.cc
[modify] https://crrev.com/ab28209ef75facaad5967fe4290879096de0ccf9/ash/shelf/shelf.cc
[modify] https://crrev.com/ab28209ef75facaad5967fe4290879096de0ccf9/ash/shelf/shelf.h
[modify] https://crrev.com/ab28209ef75facaad5967fe4290879096de0ccf9/ash/shelf/shelf_layout_manager.cc
[modify] https://crrev.com/ab28209ef75facaad5967fe4290879096de0ccf9/ash/shelf/shelf_layout_manager.h
[modify] https://crrev.com/ab28209ef75facaad5967fe4290879096de0ccf9/ash/shelf/shelf_layout_manager_unittest.cc

Sign in to add a comment