New issue
Advanced search Search tips

Issue 824504 link

Starred by 3 users

Issue metadata

Status: Verified
Owner:
Closed: May 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug



Sign in to add a comment

On connecting external monitor, lockscreen UI gets corrupted on the device

Project Member Reported by mkarkada@chromium.org, Mar 21 2018

Issue description

Chrome OS version: 10452.19.0, 66.0.3359.43 dev channel eve

What steps will reproduce the problem?

Pre condition: Switch between displays from chrome://settings/display, such that on connecting external monitor, external monitor becomes the primary display and device becomes the extending display

Procedure:
1. Login to the device 
2. Lock the screen
3. Connect the device to external monitor.

What happens instead?
Lockscreen UI gets corrupted on the device.
Shelf and Uber tray remains on the device while the Userpod is shifted to external display.

Screenshots attached
 
Lockscreen UI on the device.png
388 KB View Download
Lockscreen UI on external monitor.png
482 KB View Download
Cc: r...@chromium.org
Owner: agawronska@chromium.org
Status: Assigned (was: Untriaged)
This is still reproducible. Tested on M67 build 10575.25.0, 67.0.3396.31 dev-channel eve
Labels: ReleaseBlock-Stable
Cc: wzang@chromium.org
I know Aga is swamped, rkc@ is someone else available to fix this?
I missed this, sorry. I will have a look at it today in the afternoon/evening.
I can repro.
Labels: -M-66 M-67
Moving to M67. We can evaluate for M66 if a safe fix is available in time 
Status: Started (was: Assigned)
Project Member

Comment 9 by bugdroid1@chromium.org, May 18 2018

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

commit 8af5e106aa826f15b44303af358312d3a7e35c96
Author: Aga Wronska <agawronska@chromium.org>
Date: Fri May 18 16:45:03 2018

Update shelf visibility when display configuration changes.

Shelf is supposed to be hidden on secondary diaplay on lock and login
screens. To reporduce the bug external display needs to be set as primary
screen and then attached while on lock screen. In that case external
monitor is first added as secondary and later set as primary. Shelf
visibility was not updated for the new secondary disply after the primary
display was changed.

Similar bug was also present for login screen. To fix it removing the early
return from ShelfLayoutManager::UpdateVisibilityState was needed. The issue
that required early return is already fixed.

Bug:  824504 
Change-Id: I2e3b8e9d6427e3496e72651ba4be1bab538fe25c
Reviewed-on: https://chromium-review.googlesource.com/1052488
Commit-Queue: Aga Wronska <agawronska@chromium.org>
Reviewed-by: Michael Wasserman <msw@chromium.org>
Cr-Commit-Position: refs/heads/master@{#559922}
[modify] https://crrev.com/8af5e106aa826f15b44303af358312d3a7e35c96/ash/shelf/shelf_controller.cc
[modify] https://crrev.com/8af5e106aa826f15b44303af358312d3a7e35c96/ash/shelf/shelf_layout_manager.cc
[modify] https://crrev.com/8af5e106aa826f15b44303af358312d3a7e35c96/ash/shelf/shelf_layout_manager.h
[modify] https://crrev.com/8af5e106aa826f15b44303af358312d3a7e35c96/ash/shelf/shelf_unittest.cc

Status: Fixed (was: Started)
Labels: Merge-TBD
[Auto-generated comment by a script] We noticed that this issue is targeted for M-67; it appears the fix may have landed after branch point, meaning a merge might be required. Please confirm if a merge is required here - if so add Merge-Request-67 label, otherwise remove Merge-TBD label. Thanks.
Cc: agawronska@chromium.org kaznacheev@chromium.org
 Issue 844186  has been merged into this issue.
Labels: Merge-Request-67
Project Member

Comment 14 by sheriffbot@chromium.org, May 18 2018

Labels: -Merge-Request-67 Merge-Review-67 Hotlist-Merge-Review
This bug requires manual review: We are only 10 days from stable.
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), kbleicher@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Hi, please provide testing details for merge review. Thanks.
Status: Verified (was: Fixed)
Issue not reproducible anymore. Verified on Chrome OS 10701.0.0, 68.0.3436.0. Fix can be merged to M67.
Labels: -Merge-Review-67 Merge-Approved-67
Merge approved for M67.
Project Member

Comment 18 by bugdroid1@chromium.org, May 21 2018

Labels: -merge-approved-67 merge-merged-3396
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/710eb36df4965c77e91fb5fbfef3a6db90de2359

commit 710eb36df4965c77e91fb5fbfef3a6db90de2359
Author: Aga Wronska <agawronska@chromium.org>
Date: Mon May 21 20:31:16 2018

Update shelf visibility when display configuration changes.

Shelf is supposed to be hidden on secondary diaplay on lock and login
screens. To reporduce the bug external display needs to be set as primary
screen and then attached while on lock screen. In that case external
monitor is first added as secondary and later set as primary. Shelf
visibility was not updated for the new secondary disply after the primary
display was changed.

Similar bug was also present for login screen. To fix it removing the early
return from ShelfLayoutManager::UpdateVisibilityState was needed. The issue
that required early return is already fixed.

TBR=agawronska@chromium.org

(cherry picked from commit 8af5e106aa826f15b44303af358312d3a7e35c96)

Bug:  824504 
Change-Id: I2e3b8e9d6427e3496e72651ba4be1bab538fe25c
Reviewed-on: https://chromium-review.googlesource.com/1052488
Commit-Queue: Aga Wronska <agawronska@chromium.org>
Reviewed-by: Michael Wasserman <msw@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#559922}
Reviewed-on: https://chromium-review.googlesource.com/1067492
Reviewed-by: Aga Wronska <agawronska@chromium.org>
Cr-Commit-Position: refs/branch-heads/3396@{#666}
Cr-Branched-From: 9ef2aa869bc7bc0c089e255d698cca6e47d6b038-refs/heads/master@{#550428}
[modify] https://crrev.com/710eb36df4965c77e91fb5fbfef3a6db90de2359/ash/shelf/shelf_controller.cc
[modify] https://crrev.com/710eb36df4965c77e91fb5fbfef3a6db90de2359/ash/shelf/shelf_layout_manager.cc
[modify] https://crrev.com/710eb36df4965c77e91fb5fbfef3a6db90de2359/ash/shelf/shelf_layout_manager.h
[modify] https://crrev.com/710eb36df4965c77e91fb5fbfef3a6db90de2359/ash/shelf/shelf_unittest.cc

Labels: -Hotlist-Merge-Review -Merge-TBD

Sign in to add a comment