This seems like it should be fairly trivial. The scan at login is done here:
https://cs.chromium.org/chromium/src/chromeos/components/tether/host_scan_scheduler_impl.cc?sq=package:chromium&dr=CSs&g=0&l=145

We'd either need to get rid of that block entirely or just gate it on some logic like this that checks if there's a connected or connecting network:
https://cs.chromium.org/chromium/src/chromeos/components/tether/host_scan_scheduler_impl.cc?sq=package:chromium&dr=CSs&g=0&l=98

Tackling.

Unfortunately this issue is a lot trickier than expected.

We still want explicit requests from UI (Settings and Quick Settings) to trigger scans, even if the Chromebook has an internet connection. This means that HostScanScheduler needs to always respect calls to its ScanRequested(). However, on session start, AutoConnectHandler also indirectly calls on this ScanRequested() method [1]. We currently have no way within HostScanScheduler to disambiguate if a call to ScanRequested() came from an allowed UI source, or from AutoConnectHandler.

The best way I see forward here is to add a NetworkTypePattern argument to the ScanRequested() method. This should allow HostScanScheduler to only start a Tether scan if the provided type is Cellular or Tether (AutoConnectHandler requests a scan type of Wifi). +stevenjb@, do you foresee any possible issues doing this?

1) https://cs.chromium.org/chromium/src/chromeos/network/auto_connect_handler.cc?q=autoconnecthandler&sq=package:chromium&dr=CSs&l=313

CCing Steven to get his input.

I just did some prototyping to debug this theory and it looks like there is some unexpected behavior preventing this from working cleanly:

1) After opening Quick Settings, HostScanScheduler::ScanRequested() receives a NetworkTypePattern of "wifi". (Shouldn't this be "Default"? It's scanning for Mobile, Ethernet, and Wifi).

2) After opening Mobile Data Settings page, HostScanScheduler::ScanRequested() receives a NetworkTypePattern of "PAtternDefault". Shouldn't this be Cellular or Mobile?

Either there are bugs in how these network types are being propagated throughout the stack, or I'm misunderstanding things. Curious to hear your thoughts here Steven.

HostScanScheduler is Tether specific. I don't recall the details of how that was implemented. I assume it responds to NEtworkStateHandler::NotifyScanRequested() and that you are adding the type to that observer method (which seems fine).

Most of the calls to NetworkStateHanlder::RequestScan() pass 'WiFi'. Scanning for 'Ethernet' is meaningless, and a 'Cellular' scan only scans for cellular providers, which is not useful from the system menu (aka 'quick settings').

The networkingPrivate interface defaults to 'All', which scans both WiFi and Cellular. IIRC this is by design since the JS Settings UI includes the detail subpage which allows changing the cellular provider.

If you want to include 'Tether' in the system menu scan, that seems reasonable to me.

If you want to pass the correct type parameter in the requestNetworkScan call from internet_subpage.js, that also seems fine.

Thanks for the valuable insight Steven! This was really helpful. I added you as reviewer on https://chromium-review.googlesource.com/c/chromium/src/+/1217707.

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

commit b8a3cffde9ef95e21bd72562a092dd18c0ccc595
Author: Ryan Hansberry <hansberry@chromium.org>
Date: Tue Sep 11 17:20:15 2018

Restrict conditions to perform Tether network scans.

Change HostScanScheduler's primary ScheduleScan() method to allow
clients to specify if a Tether host scan should only be performed if
the device is offline. TetherComponentImpl's call on this method has
been appropriately changed -- Tether now only kicks off a host scan
on component creation if the device is offline.

Additionally, add a NetworkTypePattern argument to the
NetworkStateHandlerObserver::ScanRequested() method that
HostScanSchedulerImpl overrides. This allows HostScanSchedulerImpl to
only scan if the request matches NetworkTypePattern::Tether(). Relevant
Settings and System Tray callsites have been tweaked to request the
network types they are actually trying to scan for.

Bug:  881820 
Change-Id: I72833f821dcb4575e4b28c104fa1a731144fa1b0
Reviewed-on: https://chromium-review.googlesource.com/1217707
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Commit-Queue: Ryan Hansberry <hansberry@chromium.org>
Cr-Commit-Position: refs/heads/master@{#590367}
[modify] https://crrev.com/b8a3cffde9ef95e21bd72562a092dd18c0ccc595/ash/system/network/network_state_list_detailed_view.cc
[modify] https://crrev.com/b8a3cffde9ef95e21bd72562a092dd18c0ccc595/chrome/browser/resources/settings/internet_page/internet_subpage.js
[modify] https://crrev.com/b8a3cffde9ef95e21bd72562a092dd18c0ccc595/chromeos/components/tether/fake_host_scan_scheduler.cc
[modify] https://crrev.com/b8a3cffde9ef95e21bd72562a092dd18c0ccc595/chromeos/components/tether/fake_host_scan_scheduler.h
[modify] https://crrev.com/b8a3cffde9ef95e21bd72562a092dd18c0ccc595/chromeos/components/tether/host_scan_scheduler.h
[modify] https://crrev.com/b8a3cffde9ef95e21bd72562a092dd18c0ccc595/chromeos/components/tether/host_scan_scheduler_impl.cc
[modify] https://crrev.com/b8a3cffde9ef95e21bd72562a092dd18c0ccc595/chromeos/components/tether/host_scan_scheduler_impl.h
[modify] https://crrev.com/b8a3cffde9ef95e21bd72562a092dd18c0ccc595/chromeos/components/tether/host_scan_scheduler_impl_unittest.cc
[modify] https://crrev.com/b8a3cffde9ef95e21bd72562a092dd18c0ccc595/chromeos/components/tether/tether_component_impl.cc
[modify] https://crrev.com/b8a3cffde9ef95e21bd72562a092dd18c0ccc595/chromeos/network/network_state_handler.cc
[modify] https://crrev.com/b8a3cffde9ef95e21bd72562a092dd18c0ccc595/chromeos/network/network_state_handler.h
[modify] https://crrev.com/b8a3cffde9ef95e21bd72562a092dd18c0ccc595/chromeos/network/network_state_handler_observer.cc
[modify] https://crrev.com/b8a3cffde9ef95e21bd72562a092dd18c0ccc595/chromeos/network/network_state_handler_observer.h
[modify] https://crrev.com/b8a3cffde9ef95e21bd72562a092dd18c0ccc595/chromeos/network/network_state_handler_unittest.cc

Fixed. 

Note: There is no need to merge this into M70, so I'm moving the target to 71.

This needs to be merged to M70.

James is right; I misunderstood expectations around this issue.


Requesting merge into M70.

Your change meets the bar and is auto-approved for M70. Please go ahead and merge the CL to branch 3538 manually. Please contact milestone owner if you have questions.
Owners: benmason@(Android), kariahda@(iOS), geohsu@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Merge cherrypicked.

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

commit 97a8361fcc50dbdddc60e8f3c2eca2de6a1d6440
Author: Ryan Hansberry <hansberry@chromium.org>
Date: Wed Sep 12 17:53:01 2018

Restrict conditions to perform Tether network scans.

Change HostScanScheduler's primary ScheduleScan() method to allow
clients to specify if a Tether host scan should only be performed if
the device is offline. TetherComponentImpl's call on this method has
been appropriately changed -- Tether now only kicks off a host scan
on component creation if the device is offline.

Additionally, add a NetworkTypePattern argument to the
NetworkStateHandlerObserver::ScanRequested() method that
HostScanSchedulerImpl overrides. This allows HostScanSchedulerImpl to
only scan if the request matches NetworkTypePattern::Tether(). Relevant
Settings and System Tray callsites have been tweaked to request the
network types they are actually trying to scan for.

Bug:  881820 
Change-Id: I72833f821dcb4575e4b28c104fa1a731144fa1b0
Reviewed-on: https://chromium-review.googlesource.com/1217707
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Commit-Queue: Ryan Hansberry <hansberry@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#590367}(cherry picked from commit b8a3cffde9ef95e21bd72562a092dd18c0ccc595)
Reviewed-on: https://chromium-review.googlesource.com/1222289
Reviewed-by: Ryan Hansberry <hansberry@chromium.org>
Cr-Commit-Position: refs/branch-heads/3538@{#335}
Cr-Branched-From: 79f7c91a2b2a2932cd447fa6f865cb6662fa8fa6-refs/heads/master@{#587811}
[modify] https://crrev.com/97a8361fcc50dbdddc60e8f3c2eca2de6a1d6440/ash/system/network/network_state_list_detailed_view.cc
[modify] https://crrev.com/97a8361fcc50dbdddc60e8f3c2eca2de6a1d6440/chrome/browser/resources/settings/internet_page/internet_subpage.js
[modify] https://crrev.com/97a8361fcc50dbdddc60e8f3c2eca2de6a1d6440/chromeos/components/tether/fake_host_scan_scheduler.cc
[modify] https://crrev.com/97a8361fcc50dbdddc60e8f3c2eca2de6a1d6440/chromeos/components/tether/fake_host_scan_scheduler.h
[modify] https://crrev.com/97a8361fcc50dbdddc60e8f3c2eca2de6a1d6440/chromeos/components/tether/host_scan_scheduler.h
[modify] https://crrev.com/97a8361fcc50dbdddc60e8f3c2eca2de6a1d6440/chromeos/components/tether/host_scan_scheduler_impl.cc
[modify] https://crrev.com/97a8361fcc50dbdddc60e8f3c2eca2de6a1d6440/chromeos/components/tether/host_scan_scheduler_impl.h
[modify] https://crrev.com/97a8361fcc50dbdddc60e8f3c2eca2de6a1d6440/chromeos/components/tether/host_scan_scheduler_impl_unittest.cc
[modify] https://crrev.com/97a8361fcc50dbdddc60e8f3c2eca2de6a1d6440/chromeos/components/tether/tether_component_impl.cc
[modify] https://crrev.com/97a8361fcc50dbdddc60e8f3c2eca2de6a1d6440/chromeos/network/network_state_handler.cc
[modify] https://crrev.com/97a8361fcc50dbdddc60e8f3c2eca2de6a1d6440/chromeos/network/network_state_handler.h
[modify] https://crrev.com/97a8361fcc50dbdddc60e8f3c2eca2de6a1d6440/chromeos/network/network_state_handler_observer.cc
[modify] https://crrev.com/97a8361fcc50dbdddc60e8f3c2eca2de6a1d6440/chromeos/network/network_state_handler_observer.h
[modify] https://crrev.com/97a8361fcc50dbdddc60e8f3c2eca2de6a1d6440/chromeos/network/network_state_handler_unittest.cc

I chatted with Cindy about this earlier today, and she suggested it would be possible to merge this in to M69 in about a week, following the roll out of stable.

Thanks, Cindy!

Marking as open because we still need to merge this into 69.

Jesse or Cindy, can you clarify when we should expect merge approval into 69 for this issue?

I chatted with Cindy last week, and she suggested then that it would occur toward the end of this week.

M69 merge approved, per chat with Sameer.

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

commit fb39d4e126afa3d61d59be32e4ae2111c3aedaa2
Author: Ryan Hansberry <hansberry@chromium.org>
Date: Mon Oct 01 16:50:42 2018

Restrict conditions to perform Tether network scans.

Change HostScanScheduler's primary ScheduleScan() method to allow
clients to specify if a Tether host scan should only be performed if
the device is offline. TetherComponentImpl's call on this method has
been appropriately changed -- Tether now only kicks off a host scan
on component creation if the device is offline.

Additionally, add a NetworkTypePattern argument to the
NetworkStateHandlerObserver::ScanRequested() method that
HostScanSchedulerImpl overrides. This allows HostScanSchedulerImpl to
only scan if the request matches NetworkTypePattern::Tether(). Relevant
Settings and System Tray callsites have been tweaked to request the
network types they are actually trying to scan for.

Bug:  881820 
Change-Id: I72833f821dcb4575e4b28c104fa1a731144fa1b0
Reviewed-on: https://chromium-review.googlesource.com/1217707
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Commit-Queue: Ryan Hansberry <hansberry@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#590367}(cherry picked from commit b8a3cffde9ef95e21bd72562a092dd18c0ccc595)
Reviewed-on: https://chromium-review.googlesource.com/1254750
Reviewed-by: Ryan Hansberry <hansberry@chromium.org>
Cr-Commit-Position: refs/branch-heads/3497@{#976}
Cr-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753}
[modify] https://crrev.com/fb39d4e126afa3d61d59be32e4ae2111c3aedaa2/ash/system/network/network_state_list_detailed_view.cc
[modify] https://crrev.com/fb39d4e126afa3d61d59be32e4ae2111c3aedaa2/chrome/browser/resources/settings/internet_page/internet_subpage.js
[modify] https://crrev.com/fb39d4e126afa3d61d59be32e4ae2111c3aedaa2/chromeos/components/tether/fake_host_scan_scheduler.cc
[modify] https://crrev.com/fb39d4e126afa3d61d59be32e4ae2111c3aedaa2/chromeos/components/tether/fake_host_scan_scheduler.h
[modify] https://crrev.com/fb39d4e126afa3d61d59be32e4ae2111c3aedaa2/chromeos/components/tether/host_scan_scheduler.h
[modify] https://crrev.com/fb39d4e126afa3d61d59be32e4ae2111c3aedaa2/chromeos/components/tether/host_scan_scheduler_impl.cc
[modify] https://crrev.com/fb39d4e126afa3d61d59be32e4ae2111c3aedaa2/chromeos/components/tether/host_scan_scheduler_impl.h
[modify] https://crrev.com/fb39d4e126afa3d61d59be32e4ae2111c3aedaa2/chromeos/components/tether/host_scan_scheduler_impl_unittest.cc
[modify] https://crrev.com/fb39d4e126afa3d61d59be32e4ae2111c3aedaa2/chromeos/components/tether/tether_component_impl.cc
[modify] https://crrev.com/fb39d4e126afa3d61d59be32e4ae2111c3aedaa2/chromeos/network/network_state_handler.cc
[modify] https://crrev.com/fb39d4e126afa3d61d59be32e4ae2111c3aedaa2/chromeos/network/network_state_handler.h
[modify] https://crrev.com/fb39d4e126afa3d61d59be32e4ae2111c3aedaa2/chromeos/network/network_state_handler_observer.cc
[modify] https://crrev.com/fb39d4e126afa3d61d59be32e4ae2111c3aedaa2/chromeos/network/network_state_handler_observer.h
[modify] https://crrev.com/fb39d4e126afa3d61d59be32e4ae2111c3aedaa2/chromeos/network/network_state_handler_unittest.cc

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

Commit: fb39d4e126afa3d61d59be32e4ae2111c3aedaa2
Author: hansberry@chromium.org
Commiter: hansberry@chromium.org
Date: 2018-10-01 16:50:42 +0000 UTC

Restrict conditions to perform Tether network scans.

Change HostScanScheduler's primary ScheduleScan() method to allow
clients to specify if a Tether host scan should only be performed if
the device is offline. TetherComponentImpl's call on this method has
been appropriately changed -- Tether now only kicks off a host scan
on component creation if the device is offline.

Additionally, add a NetworkTypePattern argument to the
NetworkStateHandlerObserver::ScanRequested() method that
HostScanSchedulerImpl overrides. This allows HostScanSchedulerImpl to
only scan if the request matches NetworkTypePattern::Tether(). Relevant
Settings and System Tray callsites have been tweaked to request the
network types they are actually trying to scan for.

Bug:  881820 
Change-Id: I72833f821dcb4575e4b28c104fa1a731144fa1b0
Reviewed-on: https://chromium-review.googlesource.com/1217707
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Commit-Queue: Ryan Hansberry <hansberry@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#590367}(cherry picked from commit b8a3cffde9ef95e21bd72562a092dd18c0ccc595)
Reviewed-on: https://chromium-review.googlesource.com/1254750
Reviewed-by: Ryan Hansberry <hansberry@chromium.org>
Cr-Commit-Position: refs/branch-heads/3497@{#976}
Cr-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753}