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

Issue 839522 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: May 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug-Regression

Blocking:
issue 817554



Sign in to add a comment

Instant Tethering should not start up a host scan on lock screen

Project Member Reported by khorimoto@chromium.org, May 3 2018

Issue description

Repro:
(0) Make sure you have a lock screen enabled.
(1) Suspend device (i.e., close the laptop lid).
(2) Resume device (i.e., open the laptop lid). Lock screen appears.

Expected:
Instant Tethering scan does not start up for now, since its Bluetooth connection could conflict with the ongoing EasyUnlock Bluetooth connection.

Actual:
Instant Tethering scan does start up. Bluetooth connections can step on each other's toes.

Note: This also needs to be removed after the SecureChannel API; see issue 817554.
 
Components: UI>Shell>Networking>Tethering
Issue 839301 has been merged into this issue.
 Issue 817726  has been merged into this issue.
Project Member

Comment 4 by bugdroid1@chromium.org, May 5 2018

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

commit e602d3a83ceaaa2d8e66e8282fc78fb7f9474df6
Author: Kyle Horimoto <khorimoto@google.com>
Date: Sat May 05 01:11:07 2018

[CrOS Tether] Do not perform host scans while the device is locked.

EasyUnlock tries to create Bluetooth connections at the lock screen, and
when both Instant Tethering and EasyUnlock try to create a Bluetooth
connection to the same device at the same time, they can cause
instability. By eliminating the possibility of this conflict, we work
around this issue.

Note that a full-fledged fix for this problem is in the works; see
 https://crbug.com/752273 .

Bug:  839522 
Change-Id: I468515579ca46cabf279e11531ca489a71eb3255
Reviewed-on: https://chromium-review.googlesource.com/1045254
Commit-Queue: Kyle Horimoto <khorimoto@chromium.org>
Reviewed-by: James Hawkins <jhawkins@chromium.org>
Cr-Commit-Position: refs/heads/master@{#556282}
[modify] https://crrev.com/e602d3a83ceaaa2d8e66e8282fc78fb7f9474df6/chromeos/components/tether/host_scan_scheduler_impl.cc
[modify] https://crrev.com/e602d3a83ceaaa2d8e66e8282fc78fb7f9474df6/chromeos/components/tether/host_scan_scheduler_impl.h
[modify] https://crrev.com/e602d3a83ceaaa2d8e66e8282fc78fb7f9474df6/chromeos/components/tether/host_scan_scheduler_impl_unittest.cc

Blocking: 817554
Status: Fixed (was: Assigned)
Labels: Merge-Request-67
Status: Available (was: Fixed)
This improves both latency and stability by a very significant amount.  We should get this into 67.
Project Member

Comment 7 by sheriffbot@chromium.org, May 7 2018

Labels: -Merge-Request-67 Merge-Review-67 Hotlist-Merge-Review
This bug requires manual review: M67 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), kbleicher@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
 Issue 817502  has been merged into this issue.
Labels: -Type-Bug Type-Bug-Regression
Status: Started (was: Available)
kbleicher@ requested extra information regarding the issue and fix.

User-visible issue: EasyUnlock failed approximately 95% of the time when resuming from sleep. EasyUnlock only succeeded after a user manually locked the screen, but not when the signin screen was visible after device suspension.

Testing: Unit tests are included in the CL above. The issue has been thoroughly tested manually by our entire team. There is minimal risk with this patch; the only logical change is an early return from a function when the screen is locked.

Category: This fixes a regression in functionality.
Labels: -Merge-Review-67 Merge-Approved-67
I reviewed over IM with khorimoto@; we typically limit late approvals to same-milestone regression, but this is a small and well-tested change resolving an impacting bug.

Thus: Approving merge to M67 Chrome OS.

Status: Fixed (was: Started)
Project Member

Comment 12 by bugdroid1@chromium.org, May 11 2018

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

commit c717412da0050ba41cbdcac8158cb9615934746a
Author: Kyle Horimoto <khorimoto@google.com>
Date: Fri May 11 22:27:35 2018

[CrOS Tether] Do not perform host scans while the device is locked.

EasyUnlock tries to create Bluetooth connections at the lock screen, and
when both Instant Tethering and EasyUnlock try to create a Bluetooth
connection to the same device at the same time, they can cause
instability. By eliminating the possibility of this conflict, we work
around this issue.

Note that a full-fledged fix for this problem is in the works; see
 https://crbug.com/752273 .

TBR=khorimoto@google.com

(cherry picked from commit e602d3a83ceaaa2d8e66e8282fc78fb7f9474df6)

Bug:  839522 
Change-Id: I468515579ca46cabf279e11531ca489a71eb3255
Reviewed-on: https://chromium-review.googlesource.com/1045254
Commit-Queue: Kyle Horimoto <khorimoto@chromium.org>
Reviewed-by: James Hawkins <jhawkins@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#556282}
Reviewed-on: https://chromium-review.googlesource.com/1056204
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Cr-Commit-Position: refs/branch-heads/3396@{#576}
Cr-Branched-From: 9ef2aa869bc7bc0c089e255d698cca6e47d6b038-refs/heads/master@{#550428}
[modify] https://crrev.com/c717412da0050ba41cbdcac8158cb9615934746a/chromeos/components/tether/host_scan_scheduler_impl.cc
[modify] https://crrev.com/c717412da0050ba41cbdcac8158cb9615934746a/chromeos/components/tether/host_scan_scheduler_impl.h
[modify] https://crrev.com/c717412da0050ba41cbdcac8158cb9615934746a/chromeos/components/tether/host_scan_scheduler_impl_unittest.cc

Sign in to add a comment