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

Issue 759858 link

Starred by 0 users

Issue metadata

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



Sign in to add a comment

Tether networks alternate order in UI

Project Member Reported by khorimoto@chromium.org, Aug 28 2017

Issue description

Repro:
(0) Make sure you have at least 2 host devices on your account.
(1) Perform a host scan. Both devices appear in the list.
(2) Perform another host scan. It is likely that devices appear in another order. If this does not happen, keep scanning until it does.

Expected: Results stay constant in the UI.

Actual: Results switch order in the UI.

This is due to the sort order, which is determined by which device has last responded. I think we should probably sort by signal strength instead, so that devices with the highest signal strength are sorted near the top.
 
Owner: khorimoto@chromium.org
Status: Started (was: Available)
Project Member

Comment 2 by bugdroid1@chromium.org, Aug 31 2017

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

commit 1f1ed0c5e79c28ed3b0c70b5547be42f07f42cd1
Author: Kyle Horimoto <khorimoto@google.com>
Date: Thu Aug 31 19:30:34 2017

[CrOS Tether] Sort networks in UI according to network properties.

Previously, we would sort by which host has most recently responded.
This causes issues when a multiple scans occur because each new scan
causes devices to respond in a different order, which resulted in a
janky UI where devices would move up and down the list.

Instead, we now sort by network properties (e.g., signal strength). Now,
all devices will stay in the same sort order unless their properties
change.

Bug:  759858 , 672263
Change-Id: I10e14a6c01f20bfc54527c2d6a19fededb7b025d
Reviewed-on: https://chromium-review.googlesource.com/644558
Commit-Queue: Kyle Horimoto <khorimoto@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#498952}
[modify] https://crrev.com/1f1ed0c5e79c28ed3b0c70b5547be42f07f42cd1/chromeos/components/tether/BUILD.gn
[modify] https://crrev.com/1f1ed0c5e79c28ed3b0c70b5547be42f07f42cd1/chromeos/components/tether/host_scan_device_prioritizer_impl.cc
[modify] https://crrev.com/1f1ed0c5e79c28ed3b0c70b5547be42f07f42cd1/chromeos/components/tether/host_scan_device_prioritizer_impl.h
[modify] https://crrev.com/1f1ed0c5e79c28ed3b0c70b5547be42f07f42cd1/chromeos/components/tether/host_scan_device_prioritizer_impl_unittest.cc
[modify] https://crrev.com/1f1ed0c5e79c28ed3b0c70b5547be42f07f42cd1/chromeos/components/tether/initializer_impl.cc
[modify] https://crrev.com/1f1ed0c5e79c28ed3b0c70b5547be42f07f42cd1/chromeos/components/tether/initializer_impl.h
[add] https://crrev.com/1f1ed0c5e79c28ed3b0c70b5547be42f07f42cd1/chromeos/components/tether/network_list_sorter.cc
[add] https://crrev.com/1f1ed0c5e79c28ed3b0c70b5547be42f07f42cd1/chromeos/components/tether/network_list_sorter.h
[add] https://crrev.com/1f1ed0c5e79c28ed3b0c70b5547be42f07f42cd1/chromeos/components/tether/network_list_sorter_unittest.cc
[modify] https://crrev.com/1f1ed0c5e79c28ed3b0c70b5547be42f07f42cd1/chromeos/network/managed_state.h
[modify] https://crrev.com/1f1ed0c5e79c28ed3b0c70b5547be42f07f42cd1/chromeos/network/network_state.h

Labels: Merge-Request-61
Project Member

Comment 4 by sheriffbot@chromium.org, Aug 31 2017

Labels: -Merge-Request-61 Merge-Review-61 Hotlist-Merge-Review
This bug requires manual review: We are only 4 days from stable.
Please contact the milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), ketakid@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -Merge-Review-61 Merge-Approved-61
Approving merge to M61.
Project Member

Comment 6 by bugdroid1@chromium.org, Aug 31 2017

Labels: -merge-approved-61 merge-merged-3163
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/15dcee167c69b63cc5c4d5879e84c11054eed148

commit 15dcee167c69b63cc5c4d5879e84c11054eed148
Author: Kyle Horimoto <khorimoto@google.com>
Date: Thu Aug 31 19:51:52 2017

[CrOS Tether] Sort networks in UI according to network properties.

Previously, we would sort by which host has most recently responded.
This causes issues when a multiple scans occur because each new scan
causes devices to respond in a different order, which resulted in a
janky UI where devices would move up and down the list.

Instead, we now sort by network properties (e.g., signal strength). Now,
all devices will stay in the same sort order unless their properties
change.

TBR=khorimoto@google.com

(cherry picked from commit 1f1ed0c5e79c28ed3b0c70b5547be42f07f42cd1)

Bug:  759858 , 672263
Change-Id: I10e14a6c01f20bfc54527c2d6a19fededb7b025d
Reviewed-on: https://chromium-review.googlesource.com/644558
Commit-Queue: Kyle Horimoto <khorimoto@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#498952}
Reviewed-on: https://chromium-review.googlesource.com/646634
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Cr-Commit-Position: refs/branch-heads/3163@{#1040}
Cr-Branched-From: ff259bab28b35d242e10186cd63af7ed404fae0d-refs/heads/master@{#488528}
[modify] https://crrev.com/15dcee167c69b63cc5c4d5879e84c11054eed148/chromeos/components/tether/BUILD.gn
[modify] https://crrev.com/15dcee167c69b63cc5c4d5879e84c11054eed148/chromeos/components/tether/host_scan_device_prioritizer_impl.cc
[modify] https://crrev.com/15dcee167c69b63cc5c4d5879e84c11054eed148/chromeos/components/tether/host_scan_device_prioritizer_impl.h
[modify] https://crrev.com/15dcee167c69b63cc5c4d5879e84c11054eed148/chromeos/components/tether/host_scan_device_prioritizer_impl_unittest.cc
[modify] https://crrev.com/15dcee167c69b63cc5c4d5879e84c11054eed148/chromeos/components/tether/initializer_impl.cc
[modify] https://crrev.com/15dcee167c69b63cc5c4d5879e84c11054eed148/chromeos/components/tether/initializer_impl.h
[add] https://crrev.com/15dcee167c69b63cc5c4d5879e84c11054eed148/chromeos/components/tether/network_list_sorter.cc
[add] https://crrev.com/15dcee167c69b63cc5c4d5879e84c11054eed148/chromeos/components/tether/network_list_sorter.h
[add] https://crrev.com/15dcee167c69b63cc5c4d5879e84c11054eed148/chromeos/components/tether/network_list_sorter_unittest.cc
[modify] https://crrev.com/15dcee167c69b63cc5c4d5879e84c11054eed148/chromeos/network/managed_state.h
[modify] https://crrev.com/15dcee167c69b63cc5c4d5879e84c11054eed148/chromeos/network/network_state.h

Status: Fixed (was: Started)

Comment 8 by dchan@chromium.org, Jan 22 2018

Status: Archived (was: Fixed)

Comment 9 by dchan@chromium.org, Jan 23 2018

Status: Fixed (was: Archived)

Sign in to add a comment