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

Issue 889641 link

Starred by 1 user

Issue metadata

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


Participants' hotlists:
Better-Together-Launch-Blockers


Sign in to add a comment

SmartLock "Unable to Unlock" when closing and opening lid

Project Member Reported by jlklein@chromium.org, Sep 26

Issue description

Eve on 71.0.3558.0 (newest dev channel build

What steps will reproduce the problem?
(1) Set up Better Together
(2) Physically close laptop and reopen it. Note that I haven't reproduced this issue using search+L to lock the screen.
(3) On the lock screen, SmartLock looks like it's trying to connect for a fraction of a second and then quickly switches to the "unable to unlock" error.

This reproduces 100% of the time for me.
 
proximity_auth_logs_2018-09-26T05_02_27.529Z.txt
94.2 KB View Download
Cc: r...@chromium.org josephsih@chromium.org


I should also mention that magictether works fine for me. I suspect that there's some new race condition with the Bluetooth chip coming back up after sleep. The most damning logs are these:

22:00:07.832
unlock_manager_impl.cc:457
Updating screenlock state from [bluetooth connecting] to [no bluetooth]
22:00:07.926
secure_channel_impl.cc:283
SecureChannelImpl::ListenForConnectionFromDevice(): Rejecting request ID: (38AE71B705110FFB7143D263007BB893) for reason: ConnectionAttemptFailureReason::ADAPTER_NOT_PRESENT
22:00:07.929
secure_channel_impl.cc:283
SecureChannelImpl::ListenForConnectionFromDevice(): Rejecting request ID: (3584DA485F8A7C0DE73A987DEA3EDD9E) for reason: ConnectionAttemptFailureReason::ADAPTER_NOT_PRESENT
22:00:07.997
remote_device_life_cycle_impl.cc:206
Failed to create connection to remote device: CAESR...hxXMy, for reason: ConnectionAttemptFailureReason::ADAPTER_NOT_PRESENT. Giving up.


This looks like two issues:

(1) When the device is resumed from sleep, there is a brief period at which Chrome does not realize a Bluetooth adapter exists. This should be fixed at the Bluetooth level.

(2) SmartLock should not try to make a Bluetooth connection if there is no Bluetooth adapter present. If it discovers that it is in this state, it should observe the adapter and wait for the callback indicating that the Bluetooth adapter is both present and enabled. This should be fixed at the SmartLock level.
Status: Started (was: Assigned)
Investigating.
Recently, we saw users complained about bluetooth malfunction after suspend-resume on Nocturne. The root cause was that Intel controller with fw 0x37 sometimes failed to reset. We have brought this issue to Intel's attention.

In the log snippet in C#1, it showed "ADAPTER_NOT_PRESENT". This sounds like the same issue.

Next time when you saw the problem, could you attach the /var/log/messages so that we could confirm if this is the same issue? Thanks!
Cannot repro at all on ToT on platform 11065.0.0. Inferring from Jeremy's reported chrome version, he is probably on platform 11099.0.0, so I'm going to flash my test device to that image, and try to repro again.
Yeah, it wouldn't surprise me at all if this was platform version dependent. I suspect something changed with the timing of the adapters starting/stopping when the device goes to sleep.
I can 100% repro this if the Pixelbook is not plugged into power.

When examining logs after reproducing the issue, I notice that Tether has no issue using Bluetooth after unlocking the screen. So, although this issue may been exposed by a change to how Bluetooth behaves, it really seems like this is an issue with how Smart Lock listens to Bluetooth events, instead of an issue with Bluetooth itself.

Additionally, I'm certain that this issue has also been exposed in part due to recent changes in the SecureChannel API which makes SecureChannel give up on connections if the Bluetooth adapter is not powered or present -- this makes RemoteDeviceLifeCycle believe not only that a connection failed, but that authentication failed, which makes it give up entirely [1]. (this log is seen in comment #1)

Plan of attack on this issue: Smart Lock needs to give up on trying to create a connection to the host device when Bluetooth is disabled, but know to retry that connection once Bluetooth is enabled again. I'm investigating now the best way to do that.

1) https://cs.chromium.org/chromium/src/chromeos/components/proximity_auth/remote_device_life_cycle_impl.cc?sq=package:chromium&dr=CSs&g=0&l=210

You should observe the Bluetooth adapter by overriding the AdapterPresentChanged() and AdapterPoweredChanged() callbacks. Only request a connection if it is present and powered.
Yes, the difficulty is that UnlockManagerImpl already listens on those methods, but doesn't correctly coordinate with RemoteDeviceLifeCycle. I'm trying to figure out the best way to coordinate the two.
Recall that when you originally integrated SecureChannel with EasyUnlock, we determined that RemoteDeviceLifeCycle was no longer necessary but that for the sake of doing the fastest refactor as possible, we would leave it there for now and remove it post-launch.

If your potential fix is difficult, this could be a good time to get rid of RemoteDeviceLifeCycle entirely when the flag is enabled.
Project Member

Comment 11 by bugdroid1@chromium.org, Sep 28

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

commit c5aaf4437cb5055dacced526335af4c85d1ba1cf
Author: Ryan Hansberry <hansberry@chromium.org>
Date: Fri Sep 28 22:47:56 2018

Smart Lock: Dynamically stop and start according to Bluetooth power changes.

Though UnlockManager listens to Bluetooth power events, it previously did not
communicate those events to lower objects, such as RemoteDeviceLifeCycle. This
caused  crbug.com/889641 : RemoteDeviceLifeCycle attempted to begin a connection
to the host device without first checking if Bluetooth was present or powered,
and because Bluetooth is briefly not present on wake after the laptop lid is
closed, RemoteDeviceLifeCycle would always fail.

An alternate approach to this issue might have been to place Bluetooth
listening logic inside RemoteDeviceLifeCycle, but UnlockManager is already
a complex state machine that handles several events including Bluetooth
power -- it's cleaner and easier to follow if state changes remain in this
one class.

Bug:  889641 
Change-Id: I2c80da4c4e965b6fc11a0c70230705824dc34dbe
Reviewed-on: https://chromium-review.googlesource.com/1252369
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Commit-Queue: Ryan Hansberry <hansberry@chromium.org>
Cr-Commit-Position: refs/heads/master@{#595241}
[modify] https://crrev.com/c5aaf4437cb5055dacced526335af4c85d1ba1cf/chromeos/components/proximity_auth/proximity_auth_system.cc
[modify] https://crrev.com/c5aaf4437cb5055dacced526335af4c85d1ba1cf/chromeos/components/proximity_auth/proximity_auth_system_unittest.cc
[modify] https://crrev.com/c5aaf4437cb5055dacced526335af4c85d1ba1cf/chromeos/components/proximity_auth/remote_device_life_cycle_impl.cc
[modify] https://crrev.com/c5aaf4437cb5055dacced526335af4c85d1ba1cf/chromeos/components/proximity_auth/remote_device_life_cycle_impl_unittest.cc
[modify] https://crrev.com/c5aaf4437cb5055dacced526335af4c85d1ba1cf/chromeos/components/proximity_auth/unlock_manager_impl.cc
[modify] https://crrev.com/c5aaf4437cb5055dacced526335af4c85d1ba1cf/chromeos/components/proximity_auth/unlock_manager_impl.h
[modify] https://crrev.com/c5aaf4437cb5055dacced526335af4c85d1ba1cf/chromeos/components/proximity_auth/unlock_manager_impl_unittest.cc

Status: Fixed (was: Started)

Sign in to add a comment