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

Issue 803762 link

Starred by 1 user

Issue metadata

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



Sign in to add a comment

Non-corp account's wallpaper briefly visible on lock screen when resuming

Project Member Reported by derat@chromium.org, Jan 19 2018

Issue description

Google Chrome	63.0.3239.140 (Official Build) (64-bit)
Platform	10032.86.0 (Official Build) stable-channel eve

From http://listnr/product/5015361/report/84948821275:

i'm logged into both my corp and personal accounts. when i suspend the system by closing its lid (while on my personal account's desktop, i think) and then open the lid to resume, i sometimes see a flash of my personal account's wallpaper before my corp account's wallpaper and user pod is shown at the unlock screen. chrome should switch to the correct wallpaper when displaying the lock screen before suspending.

---

This may overlap with  issue 803761 . It doesn't seem to happen every time.
 

Comment 1 by wzang@chromium.org, Jan 19 2018

Status: WontFix (was: Assigned)
I can't repo in M64. On login/lock screens, which wallpaper to show completely depends on which user pod is being focused. In this case, after opening the lid, the web-ui lock screen takes some time to load, so before the lock screen is ready, the wallpaper controller has no information on which user pod the lock screen is going to show.

This should be fixed in M64 as the views-based lock is much faster, but please reopen if it's still visible (it doesn't happen every time even in M63 so I can't say for sure it's fixed.)

Comment 2 by derat@chromium.org, Jan 19 2018

Thanks for the details.

Just to clarify, all of this should happening *before* the lid is opened, right? When the lid is closed, powerd waits to suspend until Chrome says that it's ready (which includes the lock screen being shown). We shouldn't be doing anything related to drawing the lock screen after the lid is opened and the system has resumed (except maybe compositing, I think, since it's historically been difficult to make sure that that finishes synchronously).

Comment 3 by osh...@chromium.org, Jan 19 2018

Status: Assigned (was: WontFix)
I'm also seeing this on 63.

#2 is right. The lock screen should be ready before suspend. In other words, it should be in locked state during suspend.

Can you confirm this is still the case?

It did looks to me (no proof) that the wallpaper layer is moved up (so real desktop isn't visible) but just no blur nor pod though.

Comment 4 by osh...@chromium.org, Jan 19 2018

I managed to capture the video. (it takes long because it's slowmo, although I think it's taking way too long, which could be another bug).

https://drive.google.com/corp/drive/u/0/folders/0Bx_xtqaB0-v0WDBUdmN6UkhmR3M

Looks like it's already lock but just no blur.

Comment 5 by wzang@chromium.org, Jan 19 2018

#2 is right. I'm investigating why this was happening, but we can mark it as won't fix since I still can't repo this in M64 with the new views-based lock. The issue shows up immediately after adding the 'show-webui-lock', suggesting the bug should be specific to web-ui lock.


Cc: wzang@chromium.org
Labels: -M-63 M-66
Owner: tbarzic@chromium.org
Working on a related issue, so I'll take this
Project Member

Comment 7 by bugdroid1@chromium.org, Feb 15 2018

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

commit 71239915f598bf55fde2b898c719ba27ba7d4755
Author: Toni Barzic <tbarzic@google.com>
Date: Thu Feb 15 17:38:40 2018

Cleanup wallpaper controller creation

Currently, setting a wallpaper logic is unnecessarily complex:
 * wallpaper widget is created by WallpaperController, and initially
   wrapped by WallpaperWidgetController, which is in turn wrapped by
   AnimatingWallpaperWidgetController, whose ownership is passed to
   RootWindowController (as
   RootWindowController::animating_wallpaper_widget_controller)
 * AnimatingWallpaperWidgetController observes the widgets show
   animation, and when it finishes, notifies RootWindowController
 * RootWindowController takes ownership of the wallpaper's
   WallpaperWidgetController and keeps it as
   RootWindowController::wallpaper_widget_controller()

This means that RootWindowController keeps track of two different
wallpaper widget controllers at the same time, and additionally changes
wallpaper widget controller each time wallpaper changes.
Each time wallpaper state has to change (e.g. reparenting) the calling
code has to update both animating_wallpaper_widget_controller and
wallpaper_widget_controller

This CL changes WallpaperWidget controller to track both current
wallpaper and pending "animating" wallpaper, and moves logic for
switching wallpapers to WallpaperWidgetController.
WallpaperWidgetController is owned by RootWindowController, initialized
with RootWindowController, and does not change during the session
(only widgets it manages are changed).
This simplifies code that uses WallpaperWidgetController, and makes it
easier to add observers that are interested in detecting wallpaper
changes (as the observers can be attached to a specific wallpaper
widget controller instance).
NOTE: this cl does not add observer functionality, but that will be
needed to determine when wallpaper is being shown while screen is
being locked, in order to defer suspend until wallpaper is ready
(see the linked issue).

BUG= 803762 

Change-Id: I63fc81d0913dc4cf551d91f65fd92a66fe6b73a0
Reviewed-on: https://chromium-review.googlesource.com/912130
Commit-Queue: Toni Barzic <tbarzic@chromium.org>
Reviewed-by: Wenzhao (Colin) Zang <wzang@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>
Cr-Commit-Position: refs/heads/master@{#537060}
[modify] https://crrev.com/71239915f598bf55fde2b898c719ba27ba7d4755/ash/login/ui/lock_screen.cc
[modify] https://crrev.com/71239915f598bf55fde2b898c719ba27ba7d4755/ash/root_window_controller.cc
[modify] https://crrev.com/71239915f598bf55fde2b898c719ba27ba7d4755/ash/root_window_controller.h
[modify] https://crrev.com/71239915f598bf55fde2b898c719ba27ba7d4755/ash/shell_unittest.cc
[modify] https://crrev.com/71239915f598bf55fde2b898c719ba27ba7d4755/ash/wallpaper/wallpaper_controller.cc
[modify] https://crrev.com/71239915f598bf55fde2b898c719ba27ba7d4755/ash/wallpaper/wallpaper_controller_unittest.cc
[modify] https://crrev.com/71239915f598bf55fde2b898c719ba27ba7d4755/ash/wallpaper/wallpaper_view.cc
[modify] https://crrev.com/71239915f598bf55fde2b898c719ba27ba7d4755/ash/wallpaper/wallpaper_view.h
[modify] https://crrev.com/71239915f598bf55fde2b898c719ba27ba7d4755/ash/wallpaper/wallpaper_widget_controller.cc
[modify] https://crrev.com/71239915f598bf55fde2b898c719ba27ba7d4755/ash/wallpaper/wallpaper_widget_controller.h

Project Member

Comment 8 by bugdroid1@chromium.org, Mar 13 2018

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

commit c7dbb870ac431aa1767e2e0c9a68106502f0571e
Author: Toni Barzic <tbarzic@chromium.org>
Date: Tue Mar 13 23:08:10 2018

Wait for animated wallpaper changes to finish before suspending displays

PowerEventObserver delays display suspend until lock screen UI is shown
and compositors submit enough frames to know the UI changes have reached
displays.
This does not account for wallpaper changes due to screen lock:
  * On screen lock, the active wallpaper will be changed with its blurred
    version.
  * If screen lock changes the active user (e.g. if the screen is locked
    from the secondary user), the wallpaper will change to the new
    active user's blurred wallpaper.
Given that PowerEventObserver does not ensure wallpaper switch is done
before it stops drawing to the display, the first frame shown on the
device resume might still contain the UI from before wallpaper change.

This changes PowerEventObserver to additionally wait for any active
wallpaper changes (which can be detected by checking
WallpaperWidgetController::IsAnimating - set while wallpaper widget is
being changed) to finish before it starts waiting for compositor frames
to get composited, thus ensuring the correct wallpaper is set when the
display/compositing is suspended.

Adds the following methods to WallpaperWidgetController:
  * AddPendingAnimationEndCallback - to provide a way for
    PowerEventObserver to get notified when the pending wallpaper change
    completes
  * EndPendingAnimation which can be used to force immediate wallpaper
    change when device starts suspending (and prevents suspend flow from
    being blocked on unnecessarily waiting for new wallpaper animation)

BUG= 820436 , 803762 

Change-Id: Iad2074280ebba740b95c50b9d60a028bf5a0d45c
Reviewed-on: https://chromium-review.googlesource.com/956757
Reviewed-by: Dan Erat <derat@chromium.org>
Reviewed-by: Wenzhao (Colin) Zang <wzang@chromium.org>
Commit-Queue: Toni Barzic <tbarzic@chromium.org>
Cr-Commit-Position: refs/heads/master@{#542945}
[modify] https://crrev.com/c7dbb870ac431aa1767e2e0c9a68106502f0571e/ash/system/power/power_event_observer.cc
[modify] https://crrev.com/c7dbb870ac431aa1767e2e0c9a68106502f0571e/ash/system/power/power_event_observer.h
[modify] https://crrev.com/c7dbb870ac431aa1767e2e0c9a68106502f0571e/ash/system/power/power_event_observer_unittest.cc
[modify] https://crrev.com/c7dbb870ac431aa1767e2e0c9a68106502f0571e/ash/wallpaper/wallpaper_widget_controller.cc
[modify] https://crrev.com/c7dbb870ac431aa1767e2e0c9a68106502f0571e/ash/wallpaper/wallpaper_widget_controller.h

Labels: Merge-Request-66
For c7dbb870ac431aa1767e2e0c9a68106502f0571e
Project Member

Comment 10 by sheriffbot@chromium.org, Mar 22 2018

Labels: -Merge-Request-66 Merge-Review-66 Hotlist-Merge-Review
This bug requires manual review: M66 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), josafat@(ChromeOS), abdulsyed@(Desktop)

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

Comment 11 by josa...@google.com, Mar 23 2018

Labels: -Merge-Review-66 Merge-Approved-66
Project Member

Comment 12 by bugdroid1@chromium.org, Mar 23 2018

Labels: -merge-approved-66 merge-merged-3359
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/524c3741778d2b9e77645de1605711efa68896b2

commit 524c3741778d2b9e77645de1605711efa68896b2
Author: Toni Barzic <tbarzic@chromium.org>
Date: Fri Mar 23 21:02:32 2018

Merge: Wait for animated wallpaper changes to finish before suspending displays

PowerEventObserver delays display suspend until lock screen UI is shown
and compositors submit enough frames to know the UI changes have reached
displays.
This does not account for wallpaper changes due to screen lock:
  * On screen lock, the active wallpaper will be changed with its blurred
    version.
  * If screen lock changes the active user (e.g. if the screen is locked
    from the secondary user), the wallpaper will change to the new
    active user's blurred wallpaper.
Given that PowerEventObserver does not ensure wallpaper switch is done
before it stops drawing to the display, the first frame shown on the
device resume might still contain the UI from before wallpaper change.

This changes PowerEventObserver to additionally wait for any active
wallpaper changes (which can be detected by checking
WallpaperWidgetController::IsAnimating - set while wallpaper widget is
being changed) to finish before it starts waiting for compositor frames
to get composited, thus ensuring the correct wallpaper is set when the
display/compositing is suspended.

Adds the following methods to WallpaperWidgetController:
  * AddPendingAnimationEndCallback - to provide a way for
    PowerEventObserver to get notified when the pending wallpaper change
    completes
  * EndPendingAnimation which can be used to force immediate wallpaper
    change when device starts suspending (and prevents suspend flow from
    being blocked on unnecessarily waiting for new wallpaper animation)

BUG= 820436 , 803762 
TBR=tbarzic@chromium.org

(cherry picked from commit c7dbb870ac431aa1767e2e0c9a68106502f0571e)

Change-Id: Iad2074280ebba740b95c50b9d60a028bf5a0d45c
Reviewed-on: https://chromium-review.googlesource.com/956757
Reviewed-by: Dan Erat <derat@chromium.org>
Reviewed-by: Wenzhao (Colin) Zang <wzang@chromium.org>
Commit-Queue: Toni Barzic <tbarzic@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#542945}
Reviewed-on: https://chromium-review.googlesource.com/978842
Reviewed-by: Toni Barzic <tbarzic@chromium.org>
Cr-Commit-Position: refs/branch-heads/3359@{#408}
Cr-Branched-From: 66afc5e5d10127546cc4b98b9117aff588b5e66b-refs/heads/master@{#540276}
[modify] https://crrev.com/524c3741778d2b9e77645de1605711efa68896b2/ash/system/power/power_event_observer.cc
[modify] https://crrev.com/524c3741778d2b9e77645de1605711efa68896b2/ash/system/power/power_event_observer.h
[modify] https://crrev.com/524c3741778d2b9e77645de1605711efa68896b2/ash/system/power/power_event_observer_unittest.cc
[modify] https://crrev.com/524c3741778d2b9e77645de1605711efa68896b2/ash/wallpaper/wallpaper_widget_controller.cc
[modify] https://crrev.com/524c3741778d2b9e77645de1605711efa68896b2/ash/wallpaper/wallpaper_widget_controller.h

Status: Fixed (was: Assigned)

Sign in to add a comment