Always scan for hosts if request come from UI (system tray or Settings network list is open) |
||||||||
Issue descriptionScans are currently gated on a 5 minute cadence when requested via NetworkStateHandlerObserver::ScanRequested (implemented by HostScanScheduler). This is fine for regular requests, but when the UI is open and the user is expecting all networks to be scanned for continuously, is inappropriate. Somehow pipe-through whether the system tray or Settings network list is currently open, and branch on that when deciding to honor a scan request. stevenjb@, can you please advise on the best way to determine in the system tray or Settings network list is currently open? Kyle and I figured we could emit events to observers when the views are displayed, e.g. here for system tray: https://cs.chromium.org/chromium/src/ash/system/network/tray_network.cc?sq=package:chromium&l=249
,
Jul 19 2017
Additionally, do not display scan notifications if either of these UI elements are open.
,
Jul 19 2017
We currently call NetworkStateHandlerObserver::ScanRequested (via chrome.networkingPrivate.requestNetworkScan) every 10 seconds when the UI is open to address this concern: https://cs.chromium.org/chromium/src/ui/webui/resources/cr_elements/chromeos/network/cr_network_select.js?q=requestNetworkScan&sq=package:chromium&dr=C&l=96 The SystemTray UI should be doing something similar.
,
Jul 19 2017
Hi Steven, We currently listen on NetworkStateHandlerObserver::ScanRequested to handle scan requests -- the issue is that NetworkStateHandler::RequestScan is not just called from system tray and Settings: https://cs.chromium.org/chromium/src/chromeos/network/network_state_handler.cc?type=cs&sq=package:chromium&l=858 (called from 8 places total). It's incorrect for Tether to always respect NetworkStateHandlerObserver::ScanRequested if we can't know that it came from a UI -- hence we need an additional signal to determine if UI is currently open. Steven, do you have any advice for how best to determine if the system tray or Settings network list is currently open? Or are you arguing to simply always respect NetworkStateHandlerObserver::ScanRequested calls?
,
Jul 19 2017
All of those cases should be UI initiated, or on initial login or wake from lock screen. Are there any exceptions there? I can't think of any reason why the scan rules for Tether should be different than WiFi. If any of the places it gets called do not appear to be user initiated, please let me know. We could add a parameter to the method if we need to.
,
Jul 19 2017
Looks like you're right Steven. I'll simply remove our 5 minute gating mechanism to resolve this issue.
,
Jul 20 2017
Blocking on crbug.com/746726 ; without it this creates a janky UI.
,
Jul 24 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/33966bd655ca99142ef91cb9ce854d17c215302c commit 33966bd655ca99142ef91cb9ce854d17c215302c Author: Ryan Hansberry <hansberry@chromium.org> Date: Mon Jul 24 20:55:08 2017 Tether: Always scan for hosts when requested by UI. Tether previously gated scans to once per 5 minutes. This CL removes that gating mechanism. Bug: 745160 , 672263 Change-Id: I254c2dda302f4caecdef68974d26cec3babe3fa5 Reviewed-on: https://chromium-review.googlesource.com/578255 Commit-Queue: Ryan Hansberry <hansberry@chromium.org> Reviewed-by: Kyle Horimoto <khorimoto@chromium.org> Cr-Commit-Position: refs/heads/master@{#489069} [modify] https://crrev.com/33966bd655ca99142ef91cb9ce854d17c215302c/chromeos/components/tether/host_scan_scheduler.cc [modify] https://crrev.com/33966bd655ca99142ef91cb9ce854d17c215302c/chromeos/components/tether/host_scan_scheduler_unittest.cc [modify] https://crrev.com/33966bd655ca99142ef91cb9ce854d17c215302c/chromeos/components/tether/host_scanner.cc [modify] https://crrev.com/33966bd655ca99142ef91cb9ce854d17c215302c/chromeos/components/tether/host_scanner.h [modify] https://crrev.com/33966bd655ca99142ef91cb9ce854d17c215302c/chromeos/components/tether/host_scanner_unittest.cc
,
Jul 24 2017
,
Jul 25 2017
Approving merge to M61.
,
Jul 25 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/94a8ccb5224a310331e344c56c8a78056cd2855b commit 94a8ccb5224a310331e344c56c8a78056cd2855b Author: Kyle Horimoto <khorimoto@google.com> Date: Tue Jul 25 22:08:14 2017 Tether: Always scan for hosts when requested by UI. Tether previously gated scans to once per 5 minutes. This CL removes that gating mechanism. TBR=hansberry@chromium.org (cherry picked from commit 33966bd655ca99142ef91cb9ce854d17c215302c) Bug: 745160 , 672263 Change-Id: I254c2dda302f4caecdef68974d26cec3babe3fa5 Reviewed-on: https://chromium-review.googlesource.com/578255 Commit-Queue: Ryan Hansberry <hansberry@chromium.org> Reviewed-by: Kyle Horimoto <khorimoto@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#489069} Reviewed-on: https://chromium-review.googlesource.com/584865 Cr-Commit-Position: refs/branch-heads/3163@{#41} Cr-Branched-From: ff259bab28b35d242e10186cd63af7ed404fae0d-refs/heads/master@{#488528} [modify] https://crrev.com/94a8ccb5224a310331e344c56c8a78056cd2855b/chromeos/components/tether/host_scan_scheduler.cc [modify] https://crrev.com/94a8ccb5224a310331e344c56c8a78056cd2855b/chromeos/components/tether/host_scan_scheduler_unittest.cc [modify] https://crrev.com/94a8ccb5224a310331e344c56c8a78056cd2855b/chromeos/components/tether/host_scanner.cc [modify] https://crrev.com/94a8ccb5224a310331e344c56c8a78056cd2855b/chromeos/components/tether/host_scanner.h [modify] https://crrev.com/94a8ccb5224a310331e344c56c8a78056cd2855b/chromeos/components/tether/host_scanner_unittest.cc
,
Jul 25 2017
,
Jan 22 2018
|
||||||||
►
Sign in to add a comment |
||||||||
Comment 1 by hansberry@chromium.org
, Jul 18 2017