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

Issue 738108 link

Starred by 3 users

Issue metadata

Status: Archived
Owner:
Last visit > 30 days ago
Closed: Jul 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug



Sign in to add a comment

"Available host(s)" notification issues

Project Member Reported by hansberry@chromium.org, Jun 29 2017

Issue description

NotificationPresenter::RemovePotentialHotspotNotification() needs to be called before each host scan is called, and likely also when host scan results become stale.
 
Issue 741179 has been merged into this issue.
As jonmann@ said in bug 741179, we should also hide this notification once a connection attempt is started as well.
Owner: hansberry@chromium.org
Status: Started (was: Available)
Owner: khorimoto@chromium.org
Owner: ----
Status: Available (was: Started)
 Issue 731463  has been merged into this issue.
Summary: "Available host(s)" notification issues (was: 'available host(s)' notification should be dismissed when a new scan begins)
Merged with another related bug.

Issues to resolve:
(1) The notification is shown when users are already connected to the Internet. It should only be shown if there is no current connection.
(2) The notification should be removed when a connection attempt is started.
(3) The notification should be removed if the HostScanCache becomes empty.

Note: I don't think we should dismiss the notification when a new scan begins, as this bug originally said. When a new scan finishes and successfully finds devices, we can just update the existing notification. When a new scan finishes and does not find any devices, the HostScanCache will become empty, and the notification should be removed (see point (3) above).
I agree with your note -- only (1) and (3) should be addressed.
Oops! I misread (2) as "The notification should be removed when a host scan is started". Please disregard my past comment :)
One more issue:
(4) The notification should be removed if the user connects to the Internet via another connection type (e.g., Wi-Fi, Ethernet).
Owner: lesliewatkins@chromium.org
Status: Started (was: Available)
One more issue:
(5) The notification should be removed if the Tether TechnologyState becomes anything but ENABLED (e.g., if the user disabled tethering via the settings toggle).
Project Member

Comment 14 by bugdroid1@chromium.org, Jul 26 2017

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

commit a503adb07a399f623e6edb13d3ba85311639051c
Author: Leslie Watkins <lesliewatkins@chromium.org>
Date: Wed Jul 26 19:42:21 2017

Only show hotspot notification when the device is not connected to an existing network.

HostScanner now accepts NetworkStateHandler as an argument in the constructor.
HostScannerTest inherits from NetworkStateTest.

Bug:  738108 
Change-Id: I2b29ee836745df28f5cfa717d2eec4058f9cc6ae
Reviewed-on: https://chromium-review.googlesource.com/585487
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Commit-Queue: Leslie Watkins <lesliewatkins@chromium.org>
Cr-Commit-Position: refs/heads/master@{#489727}
[modify] https://crrev.com/a503adb07a399f623e6edb13d3ba85311639051c/chromeos/components/tether/host_scan_scheduler_unittest.cc
[modify] https://crrev.com/a503adb07a399f623e6edb13d3ba85311639051c/chromeos/components/tether/host_scanner.cc
[modify] https://crrev.com/a503adb07a399f623e6edb13d3ba85311639051c/chromeos/components/tether/host_scanner.h
[modify] https://crrev.com/a503adb07a399f623e6edb13d3ba85311639051c/chromeos/components/tether/host_scanner_unittest.cc
[modify] https://crrev.com/a503adb07a399f623e6edb13d3ba85311639051c/chromeos/components/tether/initializer.cc

Project Member

Comment 15 by bugdroid1@chromium.org, Jul 28 2017

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

commit f64768f8eeda01ca16148258e804986b8129ce11
Author: Leslie Watkins <lesliewatkins@chromium.org>
Date: Fri Jul 28 02:10:04 2017

Added an Observer to HostScanCache that checks when the cache is empty.

Bug:  738108 
Change-Id: I6d6731c681f61b7867bda18548c254d165943f2e
Reviewed-on: https://chromium-review.googlesource.com/587909
Commit-Queue: Leslie Watkins <lesliewatkins@chromium.org>
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#490208}
[modify] https://crrev.com/f64768f8eeda01ca16148258e804986b8129ce11/chromeos/components/tether/BUILD.gn
[modify] https://crrev.com/f64768f8eeda01ca16148258e804986b8129ce11/chromeos/components/tether/fake_host_scan_cache.cc
[modify] https://crrev.com/f64768f8eeda01ca16148258e804986b8129ce11/chromeos/components/tether/fake_host_scan_cache.h
[add] https://crrev.com/f64768f8eeda01ca16148258e804986b8129ce11/chromeos/components/tether/host_scan_cache.cc
[modify] https://crrev.com/f64768f8eeda01ca16148258e804986b8129ce11/chromeos/components/tether/host_scan_cache.h
[add] https://crrev.com/f64768f8eeda01ca16148258e804986b8129ce11/chromeos/components/tether/host_scan_cache_unittest.cc
[modify] https://crrev.com/f64768f8eeda01ca16148258e804986b8129ce11/chromeos/components/tether/master_host_scan_cache.cc
[modify] https://crrev.com/f64768f8eeda01ca16148258e804986b8129ce11/chromeos/components/tether/master_host_scan_cache.h
[modify] https://crrev.com/f64768f8eeda01ca16148258e804986b8129ce11/chromeos/components/tether/network_host_scan_cache.cc
[modify] https://crrev.com/f64768f8eeda01ca16148258e804986b8129ce11/chromeos/components/tether/network_host_scan_cache.h
[modify] https://crrev.com/f64768f8eeda01ca16148258e804986b8129ce11/chromeos/components/tether/persistent_host_scan_cache_impl.cc
[modify] https://crrev.com/f64768f8eeda01ca16148258e804986b8129ce11/chromeos/components/tether/persistent_host_scan_cache_impl.h

Labels: Merge-Request-61
Labels: OS-Chrome
Project Member

Comment 19 by sheriffbot@chromium.org, Jul 30 2017

Labels: -Merge-Request-61 Hotlist-Merge-Approved Merge-Approved-61
Your change meets the bar and is auto-approved for M61. Please go ahead and merge the CL to branch 3163 manually. Please contact 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
Status: Fixed (was: Started)
Project Member

Comment 21 by bugdroid1@chromium.org, Jul 31 2017

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

commit ac64624587e42c3ee2ff4505016f9530fe7564d8
Author: Kyle Horimoto <khorimoto@google.com>
Date: Mon Jul 31 18:00:54 2017

Only show hotspot notification when the device is not connected to an existing network.

HostScanner now accepts NetworkStateHandler as an argument in the constructor.
HostScannerTest inherits from NetworkStateTest.

TBR=lesliewatkins@chromium.org

(cherry picked from commit a503adb07a399f623e6edb13d3ba85311639051c)

Bug:  738108 
Change-Id: I2b29ee836745df28f5cfa717d2eec4058f9cc6ae
Reviewed-on: https://chromium-review.googlesource.com/585487
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Commit-Queue: Leslie Watkins <lesliewatkins@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#489727}
Reviewed-on: https://chromium-review.googlesource.com/594362
Cr-Commit-Position: refs/branch-heads/3163@{#163}
Cr-Branched-From: ff259bab28b35d242e10186cd63af7ed404fae0d-refs/heads/master@{#488528}
[modify] https://crrev.com/ac64624587e42c3ee2ff4505016f9530fe7564d8/chromeos/components/tether/host_scan_scheduler_unittest.cc
[modify] https://crrev.com/ac64624587e42c3ee2ff4505016f9530fe7564d8/chromeos/components/tether/host_scanner.cc
[modify] https://crrev.com/ac64624587e42c3ee2ff4505016f9530fe7564d8/chromeos/components/tether/host_scanner.h
[modify] https://crrev.com/ac64624587e42c3ee2ff4505016f9530fe7564d8/chromeos/components/tether/host_scanner_unittest.cc
[modify] https://crrev.com/ac64624587e42c3ee2ff4505016f9530fe7564d8/chromeos/components/tether/initializer.cc

Project Member

Comment 22 by bugdroid1@chromium.org, Jul 31 2017

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

commit 73ed9375b06dd603272517a4e8e229ca7d8ae328
Author: Kyle Horimoto <khorimoto@google.com>
Date: Mon Jul 31 18:02:13 2017

Added an Observer to HostScanCache that checks when the cache is empty.

TBR=lesliewatkins@chromium.org

(cherry picked from commit f64768f8eeda01ca16148258e804986b8129ce11)

Bug:  738108 
Change-Id: I6d6731c681f61b7867bda18548c254d165943f2e
Reviewed-on: https://chromium-review.googlesource.com/587909
Commit-Queue: Leslie Watkins <lesliewatkins@chromium.org>
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#490208}
Reviewed-on: https://chromium-review.googlesource.com/594499
Cr-Commit-Position: refs/branch-heads/3163@{#164}
Cr-Branched-From: ff259bab28b35d242e10186cd63af7ed404fae0d-refs/heads/master@{#488528}
[modify] https://crrev.com/73ed9375b06dd603272517a4e8e229ca7d8ae328/chromeos/components/tether/BUILD.gn
[modify] https://crrev.com/73ed9375b06dd603272517a4e8e229ca7d8ae328/chromeos/components/tether/fake_host_scan_cache.cc
[modify] https://crrev.com/73ed9375b06dd603272517a4e8e229ca7d8ae328/chromeos/components/tether/fake_host_scan_cache.h
[add] https://crrev.com/73ed9375b06dd603272517a4e8e229ca7d8ae328/chromeos/components/tether/host_scan_cache.cc
[modify] https://crrev.com/73ed9375b06dd603272517a4e8e229ca7d8ae328/chromeos/components/tether/host_scan_cache.h
[add] https://crrev.com/73ed9375b06dd603272517a4e8e229ca7d8ae328/chromeos/components/tether/host_scan_cache_unittest.cc
[modify] https://crrev.com/73ed9375b06dd603272517a4e8e229ca7d8ae328/chromeos/components/tether/master_host_scan_cache.cc
[modify] https://crrev.com/73ed9375b06dd603272517a4e8e229ca7d8ae328/chromeos/components/tether/master_host_scan_cache.h
[modify] https://crrev.com/73ed9375b06dd603272517a4e8e229ca7d8ae328/chromeos/components/tether/network_host_scan_cache.cc
[modify] https://crrev.com/73ed9375b06dd603272517a4e8e229ca7d8ae328/chromeos/components/tether/network_host_scan_cache.h
[modify] https://crrev.com/73ed9375b06dd603272517a4e8e229ca7d8ae328/chromeos/components/tether/persistent_host_scan_cache_impl.cc
[modify] https://crrev.com/73ed9375b06dd603272517a4e8e229ca7d8ae328/chromeos/components/tether/persistent_host_scan_cache_impl.h

Project Member

Comment 23 by bugdroid1@chromium.org, Jul 31 2017

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

commit 8132d15cd8e059ebe441545e3a28f157eb9ea657
Author: Kyle Horimoto <khorimoto@google.com>
Date: Mon Jul 31 18:05:41 2017

Added NotificationRemover class.

This class listens for changes to default network and host scan cache, and removes Tether notifications when they are no longer relevant.

TBR=lesliewatkins@chromium.org

(cherry picked from commit 836e140b05b6b8b2c006d6c334abb8a7fae7c2c1)

Bug:  738108 , 672263
Change-Id: Iba45108e65e82ea60711e6666e89a5faedcd2b57
Reviewed-on: https://chromium-review.googlesource.com/591877
Commit-Queue: Kyle Horimoto <khorimoto@chromium.org>
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#490634}
Reviewed-on: https://chromium-review.googlesource.com/594364
Cr-Commit-Position: refs/branch-heads/3163@{#166}
Cr-Branched-From: ff259bab28b35d242e10186cd63af7ed404fae0d-refs/heads/master@{#488528}
[modify] https://crrev.com/8132d15cd8e059ebe441545e3a28f157eb9ea657/chromeos/components/tether/BUILD.gn
[modify] https://crrev.com/8132d15cd8e059ebe441545e3a28f157eb9ea657/chromeos/components/tether/initializer.cc
[modify] https://crrev.com/8132d15cd8e059ebe441545e3a28f157eb9ea657/chromeos/components/tether/initializer.h
[add] https://crrev.com/8132d15cd8e059ebe441545e3a28f157eb9ea657/chromeos/components/tether/notification_remover.cc
[add] https://crrev.com/8132d15cd8e059ebe441545e3a28f157eb9ea657/chromeos/components/tether/notification_remover.h
[add] https://crrev.com/8132d15cd8e059ebe441545e3a28f157eb9ea657/chromeos/components/tether/notification_remover_unittest.cc

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

Status: Archived (was: Fixed)

Sign in to add a comment