I think this might have been started by https://chromium-review.googlesource.com/c/chromium/src/+/879466, which also got merged back to 64

Just to double-check: you're seeing window contents rather than just the current user's or another signed-in user's unblurred wallpaper, right? (See  issue 803762  and  issue 803761 .)

Since it might be relevant: was the device suspended due to inactivity or by closing the lid?

Is it possible to temporarily disable the Views-based lock screen in M64 and later until this is sorted out?

Is this related to note-taking app changes where we stop compositing to avoid flashing a non-black screen when launching the note-taking app?

I don't expect switching between views/webui will change behavior here. I'd also be very hesitant to switch to webui at this point since no one has been testing it for quite a while.

The views based lockscreen comes on faster than the WebUI; it seems very unlikely that the work for switching to views would cause this.

It may be possible that Toni's work may have accidentally caused it. In either case, we should definitely fix this in 65. If the fix is small enough, we might want to port to 64. This sounds like a fairly serious security issue since someone may be able to catch a glimpse of the screen on someone else's locked Chromebook.

It should not be related to note-taking app changes (that code should only kick in on stylus eject)

What changed between views and webui implementation is timing when OnLockAnimationComplete is called, and subsequently, when we call Compositor::SetVisible(false) on the root windows' compositors:
* for views, we call OnLockAninmationComplete immediately when the lock window is shown
* for webui, this is called after all animations in the lock screen UI finish, which adds extra delay (and thus give the compositor more time to pickup UI changes before it stops swapping frames)


btw. I managed to reproduce this on Tot caroline after closing the lid, and after using powerd_dbus_suspend from command line - and I can see the window contents when the issue reproduces

Hmm. When we're locking the screen in response to an imminent suspend (which sounds like it's the case here), ash::PowerEventObserver::SuspendImminent calls ash::LockStateController::LockWithoutAnimation. The Views-based lock screen should already be skipping animations in that case.

yeah, but it calls SessionControllerClient::Get()->NotifyChromeLockAnimationsComplete() as the lock window is shown (which ends up invoking PowerEventObserver::OnLockAnimationsComplete).

To comment 4, in my original case I did see the whole window and both times I had the lid open, let the machine blank screen and lock on its own and woken most likely by hitting shift.

The "lid open" part is pretty interesting. On battery, we usually lock about a minute before the system suspends (ten seconds after the screen is turned off due to inactivity), so even if the Views lock screen is announcing that it's ready before the compositor has put it on the screen, I'd expect the screen to be locked long before we suspend.

Here's the relevant portion from the powerd log in the feedback report:

[0129/103819:INFO:state_controller.cc(437)] Scaling delays due to user activity while screen was dimmed or soon after it was turned off
...
[0129/132154:INFO:daemon.cc(1443)] Received updated external policy: ac_dim=7m ac_screen_off=7m30s ac_lock=7m40s ac_idle_warn=0s ac_idle=30m battery_dim=5m battery_screen_off=5m30s battery_lock
=5m40s battery_idle_warn=0s battery_idle=6m30s ac_idle=suspend battery_idle=suspend lid_closed=suspend use_audio=1 use_video=1 presentation_factor=2.0 user_activity_factor=2.0 wait_for_initial_
user_activity=0 force_nonzero_brightness_for_user_activity=1 (Prefs)
[0129/132154:INFO:state_controller.cc(855)] Updated settings: dim=10m screen_off=10m30s lock=10m40s idle_warn=0s idle=11m30s (suspend) lid_closed=suspend use_audio=1 use_video=1
...
[0129/134624:INFO:state_controller.cc(89)] Dimming screen after 10m
[0129/134624:INFO:internal_backlight_controller.cc(677)] Setting brightness to 6 (14.1949%) over 200 ms
[0129/134625:INFO:daemon.cc(782)] On battery at 91% (displayed as 93%), 5.684/6.271Ah at 0.435A, 11h25m until empty (11h22m until shutdown)
[0129/134654:INFO:state_controller.cc(89)] Turning screen off after 10m30s
[0129/134654:INFO:internal_backlight_controller.cc(677)] Setting brightness to 0 (0%) over 200 ms
[0129/134654:INFO:display_power_setter.cc(81)] Asking DisplayService to turn all displays off
[0129/134655:INFO:daemon.cc(1425)] Chrome is using normal display mode
[0129/134655:INFO:daemon.cc(782)] On battery at 91% (displayed as 93%), 5.681/6.271Ah at 0.446A, 11h30m20s until empty (11h27m20s until shutdown)
[0129/134704:INFO:state_controller.cc(89)] Locking screen after 10m40s
[0129/134704:INFO:daemon.cc(1443)] Received updated external policy: ac_dim=30s ac_screen_off=40s ac_lock=50s ac_idle_warn=0s ac_idle=30m battery_dim=30s battery_screen_off=40s battery_lock=50s battery_idle_warn=0s battery_idle=6m30s ac_idle=suspend battery_idle=suspend lid_closed=suspend use_audio=1 use_video=1 presentation_factor=2.0 user_activity_factor=2.0 wait_for_initial_user_activity=0 force_nonzero_brightness_for_user_activity=1 (Prefs)
[0129/134704:INFO:state_controller.cc(855)] Updated settings: dim=1m screen_off=1m10s lock=1m20s idle_warn=0s idle=7m (suspend) lid_closed=suspend use_audio=1 use_video=1
[0129/134704:INFO:state_controller.cc(996)] Ready to perform idle action (suspend) after 10m29s

powerd actually suspended immediately after the screen was locked, for a pretty subtle reason. The 5m/5m30s/5m40s/6m30s battery delays are getting extended to 10m/10m30s/10m40s/11m30s due to earlier user activity (see https://chromium.googlesource.com/chromiumos/platform2/+/master/power_manager/docs/inactivity_delays.md for more info). At 10m40s, we lock the screen as expected, which results in Chrome sending powerd an updated policy with battery delays of 30s/40s/50s/6m30s, again as expected. When powerd extends the delays this time, they just go to 1m/1m10s/1m20s/7m. We're past the seven-minute mark, so we start suspending immediately.

This is expected given the way that inactivity delays work, but the interaction between the shortened dim/off delays while the lock screen is shown and the delay-doubling logic is unexpected. I'll think about some way to make them coexist in a better way in powerd.

This powerd behavior has been present since M41 (early 2015, see  issue 190499 ), and it's not the cause of the regression, though -- note that Toni was able to repro seeing the desktop by closing and opening the lid, which is unrelated to what I described above.

So I'll look at improving this on the powerd side (probably just for M66 since this is tricky), but that'll just help for the suspend-due-to-idle case. Chrome should be fixed sooner.

Filed  issue 807861  against myself to track improving the powerd behavior described in #11.

Toni has a easy-to-merge potential workaround for the Chrome issue at https://crrev.com/c/896577. It'd be nice if it could be merged to M64 after it's in, but I don't know if that's still an option.

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

commit 524994174827569d34d8dc69090b358c4dd8dfb5
Author: Toni Barzic <tbarzic@google.com>
Date: Thu Feb 01 04:20:38 2018

Workaround for suspend happening to soon with views lock

Adds additional delay before views screen locker reports to
PowerEventObserver it's lock animations have finished. The goal is to
add more time between lock window UI being shown and device suspend
(more specifically stopping compositing on the root windows), and
thus prevent flash of non lock UI when showing lock screen after
device resume.

The issue surfaced by switching to views based lock UI - with webui
based lock screen, power event observer would wait for the animations
in web ui to finish before stopping the compositors, which added more
time (comared to views based solution) for the frame buffers to pick
up frame buffers from after lock window is shown.

BUG= 807511 

Change-Id: Iba75c03cf8058b19bed2dc6e791200a04252b603
Reviewed-on: https://chromium-review.googlesource.com/896577
Commit-Queue: Toni Barzic <tbarzic@chromium.org>
Reviewed-by: Dan Erat <derat@chromium.org>
Cr-Commit-Position: refs/heads/master@{#533553}
[modify] https://crrev.com/524994174827569d34d8dc69090b358c4dd8dfb5/chrome/browser/chromeos/login/lock/views_screen_locker.cc
[modify] https://crrev.com/524994174827569d34d8dc69090b358c4dd8dfb5/chrome/browser/chromeos/login/lock/views_screen_locker.h

Issue 808261 has been merged into this issue.

This bug requires manual review: Request affecting a post-stable build
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), kbleicher@(ChromeOS), abdulsyed@(Desktop)

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

This is a pretty serious security concern. Sufficient that I feel it justifies stopping the 64 rollout.

Approving Merge for M-64.   Please merge asap and update this bug when done.

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

commit b1555389e856b6cbc02dd82dea143d1871a309f6
Author: Toni Barzic <tbarzic@google.com>
Date: Fri Feb 02 01:51:08 2018

Workaround for suspend happening to soon with views lock

Adds additional delay before views screen locker reports to
PowerEventObserver it's lock animations have finished. The goal is to
add more time between lock window UI being shown and device suspend
(more specifically stopping compositing on the root windows), and
thus prevent flash of non lock UI when showing lock screen after
device resume.

The issue surfaced by switching to views based lock UI - with webui
based lock screen, power event observer would wait for the animations
in web ui to finish before stopping the compositors, which added more
time (comared to views based solution) for the frame buffers to pick
up frame buffers from after lock window is shown.

BUG= 807511 
TBR=tbarzic@google.com

(cherry picked from commit 524994174827569d34d8dc69090b358c4dd8dfb5)

Change-Id: Iba75c03cf8058b19bed2dc6e791200a04252b603
Reviewed-on: https://chromium-review.googlesource.com/896577
Commit-Queue: Toni Barzic <tbarzic@chromium.org>
Reviewed-by: Dan Erat <derat@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#533553}
Reviewed-on: https://chromium-review.googlesource.com/897738
Reviewed-by: Toni Barzic <tbarzic@chromium.org>
Cr-Commit-Position: refs/branch-heads/3282@{#636}
Cr-Branched-From: 5fdc0fab22ce7efd32532ee989b223fa12f8171e-refs/heads/master@{#520840}
[modify] https://crrev.com/b1555389e856b6cbc02dd82dea143d1871a309f6/chrome/browser/chromeos/login/lock/views_screen_locker.cc
[modify] https://crrev.com/b1555389e856b6cbc02dd82dea143d1871a309f6/chrome/browser/chromeos/login/lock/views_screen_locker.h

Your change meets the bar and is auto-approved for M65. Please go ahead and merge the CL to branch 3325 manually. Please contact milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), bhthompson@(ChromeOS), govind@(Desktop)

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

Adding Jordi from SST.

Can we remove the RVG bit on this? Or do we need to wait until a fix is pushed out?

Users are confused about why we've announced 64 stable, but they're not getting it.

Re: the push.  We only released to 1% non-ARC and the blog said it was being released over days.  I have a build in progress with the merged fix and hope to go back to 1% as soon as Monday.  I don't think we'd be served well by updating the blog.

I don't think we should update the blog.  I was just thinking that the TCs would find it helpful if they could see the bug and know what's going on.

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

commit 0afef99076d1136e0bc3f9b81f86564f53011ea2
Author: Toni Barzic <tbarzic@google.com>
Date: Fri Feb 02 19:03:02 2018

Workaround for suspend happening to soon with views lock

Adds additional delay before views screen locker reports to
PowerEventObserver it's lock animations have finished. The goal is to
add more time between lock window UI being shown and device suspend
(more specifically stopping compositing on the root windows), and
thus prevent flash of non lock UI when showing lock screen after
device resume.

The issue surfaced by switching to views based lock UI - with webui
based lock screen, power event observer would wait for the animations
in web ui to finish before stopping the compositors, which added more
time (comared to views based solution) for the frame buffers to pick
up frame buffers from after lock window is shown.

BUG= 807511 
TBR=tbarzic@google.com

(cherry picked from commit 524994174827569d34d8dc69090b358c4dd8dfb5)

Change-Id: Iba75c03cf8058b19bed2dc6e791200a04252b603
Reviewed-on: https://chromium-review.googlesource.com/896577
Commit-Queue: Toni Barzic <tbarzic@chromium.org>
Reviewed-by: Dan Erat <derat@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#533553}
Reviewed-on: https://chromium-review.googlesource.com/899775
Reviewed-by: Toni Barzic <tbarzic@chromium.org>
Cr-Commit-Position: refs/branch-heads/3325@{#262}
Cr-Branched-From: bc084a8b5afa3744a74927344e304c02ae54189f-refs/heads/master@{#530369}
[modify] https://crrev.com/0afef99076d1136e0bc3f9b81f86564f53011ea2/chrome/browser/chromeos/login/lock/views_screen_locker.cc
[modify] https://crrev.com/0afef99076d1136e0bc3f9b81f86564f53011ea2/chrome/browser/chromeos/login/lock/views_screen_locker.h

I see no reason for this change to have the Restrict-View-Google label.

#CBC-RS/TC-watchlist

 Issue 766799  has been merged into this issue.

Verified on Chrome OS 10176.68.0, 64.0.3282.144 Eve.

Steps:
1.Enable "screenlock when waking from sleep"
2.Close the lid
3.Open the lid 

Results.
Doesn't see desktop contents on screenlock.

I'm closing this, and will open another issue to improve the logic for determining when to stop compositing during suspend.

Not reproduced on Chrome OS 10176.68.0, 64.0.3282.144 
Testing covered on both x84 and ARM boards.

Devices: Peppy, Quawks, Minnie, Kevin, Parrot, Eve, Cave, Daisy, Samus and Cyan.

Uh, on R65 10323.17.0 I'm seeing that after login, if I close the lid and open it back after some time, it comes back up and the screen is unlocked. I verified this on Soraka and Samus. Also checkout following user feedback, looks like this is still not WAI?

User feedback report:
https://listnr.corp.google.com/report/85077485889

Product Version: 65.0.3325.65 dev
UI Language: en-US

Description:
ldap:joaodias
I opened the lid of the laptop after a day away, and it was unlocked. I expected to see the password screen...

Product Specific Data (whitelisted):
        CHROME VERSION: 65.0.3325.65 dev
        CHROMEOS_AUSERVER: <URL: 2>
        CHROMEOS_RELEASE_BOARD: soraka-signed-prempkeys
        CHROMEOS_RELEASE_DESCRIPTION: 10323.30.0 (Official Build) dev-channel soraka
        CHROMEOS_RELEASE_TRACK: dev-channel
        CHROMEOS_RELEASE_VERSION: 10323.30.0
        ENTERPRISE_ENROLLED: Managed
        cpu: Intel(R) Core(TM) m3-7Y30 CPU @ 1.00GHz

        expi:  3300108 3300134 3312643 3313324 3313551
        hardware_class: SORAKA B5B-A3D-C2A-38J-A8F
        routes: <IPv4: 19>/30 dev arcbr0 proto kernel scope link src <IPv4: 7>

From the description, that seems like a different issue - the issue you describe seems to be that the device does not lock at all, rather than contents from unlocked session flashing for a moment as the device resumes from suspend.

I suspect the problem described in comment #35 might be related to (duplicate of)  issue 811388 

Rajat, if you're still seeing this on R65, maybe file a feedback report and attach it to this bug for additional debugging?