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

Issue 766216 link

Starred by 2 users

Issue metadata

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



Sign in to add a comment

TetherAvailabilityRequests should be prioritized beneath all other message types

Project Member Reported by khorimoto@chromium.org, Sep 18 2017

Issue description

We should always prioritize Bluetooth traffic for active connection attempts over passive scans.

This prevents a situation where users with many potential tether hosts can get their messages stuck in a long queue, making the connection process appear very slow.
 
Also, if we have no other information to go off of, we should prioritize devices which have been updated most recently on CryptAuth:

https://cs.chromium.org/chromium/src/components/cryptauth/proto/cryptauth_api.proto?q=last_update_time_millis
Components: UI>Shell>Networking>Tethering
Labels: -M-63 M-64
Labels: -Pri-2 Pri-1
Cc: omrilio@chromium.org
Labels: -M-64 M-63
Owner: khorimoto@chromium.org
Status: Assigned (was: Available)
Chatted with Jeremy about this, and it seems that this bug can be pretty bad on accounts with many phones, often taking >30 seconds between clicking "connect" and the connect operation starting. We should fix this for M-63.

The "real" fix will be quite involved, so I plan to implement an easier workaround for M-63 and implement a more robust solution for M-64.
Issue 775176 has been merged into this issue.
Status: Started (was: Assigned)
Starting this now. Plan is to add 3 priorities for messages:
(1) High priority - connections which block the UI. These connections indicate actions which are displayed in the UI (e.g., connecting to a Tether network).
(2) Medium priority - connections which need to finish within a reasonable amount of time. These connections do not block the UI, but it is important to finish them within a timely manner (e.g., keep-alive tickles).
(3) Low priority - background connections. These connections are not triggered by a user and can take longer to complete (e.g., host scans).

I'll update BleAdvertisementDeviceQueue to prioritize connections using these priorities.
Labels: M-62
Labels: Merge-Request-63 Merge-Request-62
Project Member

Comment 9 by bugdroid1@chromium.org, Oct 25 2017

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

commit f7f462b7a9ab86e0bad1f8b695183761781aa81e
Author: Kyle Horimoto <khorimoto@google.com>
Date: Wed Oct 25 03:42:54 2017

[CrOS Tether] Add prioritization of connections.

Without this change, it is possible for high-priority connection
attempts to be stuck at the end of a queue. In most cases, this does not
have a drastic effect, but for users with many devices (e.g., Chrome OS
developers), it is possible that the device queue is so long that
connection attempts cannot complete.

This change assigns each connection attempt a priority and only attempts
connections to low-priority connections once high-priority connections
have completed.

Bug:  766216 , 672263
Change-Id: Ic94f59a86b97c0bfbda02e614d0a04c08394815b
Reviewed-on: https://chromium-review.googlesource.com/736686
Reviewed-by: Jeremy Klein <jlklein@chromium.org>
Cr-Commit-Position: refs/heads/master@{#511355}
[modify] https://crrev.com/f7f462b7a9ab86e0bad1f8b695183761781aa81e/chromeos/components/tether/BUILD.gn
[modify] https://crrev.com/f7f462b7a9ab86e0bad1f8b695183761781aa81e/chromeos/components/tether/ble_advertisement_device_queue.cc
[modify] https://crrev.com/f7f462b7a9ab86e0bad1f8b695183761781aa81e/chromeos/components/tether/ble_advertisement_device_queue.h
[modify] https://crrev.com/f7f462b7a9ab86e0bad1f8b695183761781aa81e/chromeos/components/tether/ble_advertisement_device_queue_unittest.cc
[modify] https://crrev.com/f7f462b7a9ab86e0bad1f8b695183761781aa81e/chromeos/components/tether/ble_connection_manager.cc
[modify] https://crrev.com/f7f462b7a9ab86e0bad1f8b695183761781aa81e/chromeos/components/tether/ble_connection_manager.h
[add] https://crrev.com/f7f462b7a9ab86e0bad1f8b695183761781aa81e/chromeos/components/tether/connection_priority.cc
[add] https://crrev.com/f7f462b7a9ab86e0bad1f8b695183761781aa81e/chromeos/components/tether/connection_priority.h

Labels: -Merge-Request-62 -Merge-Request-63 Merge-Approved-62 Merge-Approved-63
Project Member

Comment 11 by bugdroid1@chromium.org, Oct 25 2017

Labels: -merge-approved-63 merge-merged-3239
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/12c72d3b3e9c8708b48dcca34efad092f61996e8

commit 12c72d3b3e9c8708b48dcca34efad092f61996e8
Author: Kyle Horimoto <khorimoto@google.com>
Date: Wed Oct 25 03:46:11 2017

[CrOS Tether] Add prioritization of connections.

Without this change, it is possible for high-priority connection
attempts to be stuck at the end of a queue. In most cases, this does not
have a drastic effect, but for users with many devices (e.g., Chrome OS
developers), it is possible that the device queue is so long that
connection attempts cannot complete.

This change assigns each connection attempt a priority and only attempts
connections to low-priority connections once high-priority connections
have completed.

TBR=khorimoto@google.com

(cherry picked from commit f7f462b7a9ab86e0bad1f8b695183761781aa81e)

Bug:  766216 , 672263
Change-Id: Ic94f59a86b97c0bfbda02e614d0a04c08394815b
Reviewed-on: https://chromium-review.googlesource.com/736686
Reviewed-by: Jeremy Klein <jlklein@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#511355}
Reviewed-on: https://chromium-review.googlesource.com/737371
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Cr-Commit-Position: refs/branch-heads/3239@{#207}
Cr-Branched-From: adb61db19020ed8ecee5e91b1a0ea4c924ae2988-refs/heads/master@{#508578}
[modify] https://crrev.com/12c72d3b3e9c8708b48dcca34efad092f61996e8/chromeos/components/tether/BUILD.gn
[modify] https://crrev.com/12c72d3b3e9c8708b48dcca34efad092f61996e8/chromeos/components/tether/ble_advertisement_device_queue.cc
[modify] https://crrev.com/12c72d3b3e9c8708b48dcca34efad092f61996e8/chromeos/components/tether/ble_advertisement_device_queue.h
[modify] https://crrev.com/12c72d3b3e9c8708b48dcca34efad092f61996e8/chromeos/components/tether/ble_advertisement_device_queue_unittest.cc
[modify] https://crrev.com/12c72d3b3e9c8708b48dcca34efad092f61996e8/chromeos/components/tether/ble_connection_manager.cc
[modify] https://crrev.com/12c72d3b3e9c8708b48dcca34efad092f61996e8/chromeos/components/tether/ble_connection_manager.h
[add] https://crrev.com/12c72d3b3e9c8708b48dcca34efad092f61996e8/chromeos/components/tether/connection_priority.cc
[add] https://crrev.com/12c72d3b3e9c8708b48dcca34efad092f61996e8/chromeos/components/tether/connection_priority.h

Project Member

Comment 12 by bugdroid1@chromium.org, Oct 25 2017

Labels: -merge-approved-62 merge-merged-3202
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5a60e8fab2c2af284563f6b643bcd709a5bdd471

commit 5a60e8fab2c2af284563f6b643bcd709a5bdd471
Author: Kyle Horimoto <khorimoto@google.com>
Date: Wed Oct 25 03:50:05 2017

[CrOS Tether] Add prioritization of connections.

Without this change, it is possible for high-priority connection
attempts to be stuck at the end of a queue. In most cases, this does not
have a drastic effect, but for users with many devices (e.g., Chrome OS
developers), it is possible that the device queue is so long that
connection attempts cannot complete.

This change assigns each connection attempt a priority and only attempts
connections to low-priority connections once high-priority connections
have completed.

TBR=khorimoto@google.com

(cherry picked from commit f7f462b7a9ab86e0bad1f8b695183761781aa81e)

Bug:  766216 , 672263
Change-Id: Ic94f59a86b97c0bfbda02e614d0a04c08394815b
Reviewed-on: https://chromium-review.googlesource.com/736686
Reviewed-by: Jeremy Klein <jlklein@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#511355}
Reviewed-on: https://chromium-review.googlesource.com/737372
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Cr-Commit-Position: refs/branch-heads/3202@{#744}
Cr-Branched-From: fa6a5d87adff761bc16afc5498c3f5944c1daa68-refs/heads/master@{#499098}
[modify] https://crrev.com/5a60e8fab2c2af284563f6b643bcd709a5bdd471/chromeos/components/tether/BUILD.gn
[modify] https://crrev.com/5a60e8fab2c2af284563f6b643bcd709a5bdd471/chromeos/components/tether/ble_advertisement_device_queue.cc
[modify] https://crrev.com/5a60e8fab2c2af284563f6b643bcd709a5bdd471/chromeos/components/tether/ble_advertisement_device_queue.h
[modify] https://crrev.com/5a60e8fab2c2af284563f6b643bcd709a5bdd471/chromeos/components/tether/ble_advertisement_device_queue_unittest.cc
[modify] https://crrev.com/5a60e8fab2c2af284563f6b643bcd709a5bdd471/chromeos/components/tether/ble_connection_manager.cc
[modify] https://crrev.com/5a60e8fab2c2af284563f6b643bcd709a5bdd471/chromeos/components/tether/ble_connection_manager.h
[add] https://crrev.com/5a60e8fab2c2af284563f6b643bcd709a5bdd471/chromeos/components/tether/connection_priority.cc
[add] https://crrev.com/5a60e8fab2c2af284563f6b643bcd709a5bdd471/chromeos/components/tether/connection_priority.h

Status: Fixed (was: Started)

Sign in to add a comment